From 70fc5f97e2b42a2cf87ff8756d6baf4ab2d5a920 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 26 Nov 2025 23:29:57 +0100 Subject: [PATCH] Code review Signed-off-by: Benoit Tigeot --- pkg/action/hooks.go | 16 ++++++++-------- pkg/action/release_testing.go | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index c336e10fe..2ea851a3e 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -35,7 +35,7 @@ import ( func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, waitStrategy kube.WaitStrategy, timeout time.Duration, serverSideApply bool) error { shutdown, err := cfg.execHookWithDelayedShutdown(rl, hook, waitStrategy, timeout, serverSideApply) if shutdown == nil { - return nil + return err } if err != nil { if err := shutdown(); err != nil { @@ -46,14 +46,14 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, return shutdown() } -type ExecuteShutdownHooks = func() error +type executeShutdownFunc = func() error -func ShutdownNoOp() error { +func shutdownNoOp() error { return nil } // execHookWithDelayedShutdown executes all of the hooks for the given hook event and returns a shutdownHook function to trigger deletions after doing other things like e.g. retrieving logs. -func (cfg *Configuration) execHookWithDelayedShutdown(rl *release.Release, hook release.HookEvent, waitStrategy kube.WaitStrategy, timeout time.Duration, serverSideApply bool) (ExecuteShutdownHooks, error) { +func (cfg *Configuration) execHookWithDelayedShutdown(rl *release.Release, hook release.HookEvent, waitStrategy kube.WaitStrategy, timeout time.Duration, serverSideApply bool) (executeShutdownFunc, error) { executingHooks := []*release.Hook{} for _, h := range rl.Hooks { @@ -72,12 +72,12 @@ func (cfg *Configuration) execHookWithDelayedShutdown(rl *release.Release, hook cfg.hookSetDeletePolicy(h) if err := cfg.deleteHookByPolicy(h, release.HookBeforeHookCreation, waitStrategy, timeout); err != nil { - return ShutdownNoOp, err + return shutdownNoOp, err } resources, err := cfg.KubeClient.Build(bytes.NewBufferString(h.Manifest), true) if err != nil { - return ShutdownNoOp, fmt.Errorf("unable to build kubernetes object for %s hook %s: %w", hook, h.Path, err) + return shutdownNoOp, fmt.Errorf("unable to build kubernetes object for %s hook %s: %w", hook, h.Path, err) } // Record the time at which the hook was applied to the cluster @@ -98,12 +98,12 @@ func (cfg *Configuration) execHookWithDelayedShutdown(rl *release.Release, hook kube.ClientCreateOptionServerSideApply(serverSideApply, false)); err != nil { h.LastRun.CompletedAt = time.Now() h.LastRun.Phase = release.HookPhaseFailed - return ShutdownNoOp, fmt.Errorf("warning: Hook %s %s failed: %w", hook, h.Path, err) + return shutdownNoOp, fmt.Errorf("warning: Hook %s %s failed: %w", hook, h.Path, err) } waiter, err := cfg.KubeClient.GetWaiter(waitStrategy) if err != nil { - return ShutdownNoOp, fmt.Errorf("unable to get waiter: %w", err) + return shutdownNoOp, fmt.Errorf("unable to get waiter: %w", err) } // Watch hook resources until they have completed err = waiter.WatchUntilReady(resources, timeout) diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go index 2aa063515..673a678f5 100644 --- a/pkg/action/release_testing.go +++ b/pkg/action/release_testing.go @@ -57,24 +57,24 @@ func NewReleaseTesting(cfg *Configuration) *ReleaseTesting { } // Run executes 'helm test' against the given release. -func (r *ReleaseTesting) Run(name string) (ri.Releaser, ExecuteShutdownHooks, error) { +func (r *ReleaseTesting) Run(name string) (ri.Releaser, executeShutdownFunc, error) { if err := r.cfg.KubeClient.IsReachable(); err != nil { - return nil, ShutdownNoOp, err + return nil, shutdownNoOp, err } if err := chartutil.ValidateReleaseName(name); err != nil { - return nil, ShutdownNoOp, fmt.Errorf("releaseTest: Release name is invalid: %s", name) + return nil, shutdownNoOp, fmt.Errorf("releaseTest: Release name is invalid: %s", name) } // finds the non-deleted release with the given name reli, err := r.cfg.Releases.Last(name) if err != nil { - return reli, ShutdownNoOp, err + return reli, shutdownNoOp, err } rel, err := releaserToV1Release(reli) if err != nil { - return reli, ShutdownNoOp, err + return reli, shutdownNoOp, err } skippedHooks := []*release.Hook{}