diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 1e4fec9bd..49849a27d 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -33,6 +33,27 @@ import ( // execHook executes all of the hooks for the given hook event. 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 err + } + if err != nil { + if err := shutdown(); err != nil { + return err + } + return err + } + return shutdown() +} + +type ExecuteShutdownFunc = func() 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) (ExecuteShutdownFunc, error) { executingHooks := []*release.Hook{} for _, h := range rl.Hooks { @@ -51,12 +72,12 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, cfg.hookSetDeletePolicy(h) if err := cfg.deleteHookByPolicy(h, release.HookBeforeHookCreation, waitStrategy, timeout); err != nil { - return err + return shutdownNoOp, err } resources, err := cfg.KubeClient.Build(bytes.NewBufferString(h.Manifest), true) if err != nil { - return 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 @@ -77,12 +98,12 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, kube.ClientCreateOptionServerSideApply(serverSideApply, false)); err != nil { h.LastRun.CompletedAt = time.Now() h.LastRun.Phase = release.HookPhaseFailed - return 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 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) @@ -98,36 +119,38 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, } // If a hook is failed, check the annotation of the hook to determine whether the hook should be deleted // under failed condition. If so, then clear the corresponding resource object in the hook - if errDeleting := cfg.deleteHookByPolicy(h, release.HookFailed, waitStrategy, timeout); errDeleting != nil { - // We log the error here as we want to propagate the hook failure upwards to the release object. - log.Printf("error deleting the hook resource on hook failure: %v", errDeleting) - } - - // If a hook is failed, check the annotation of the previous successful hooks to determine whether the hooks - // should be deleted under succeeded condition. - if err := cfg.deleteHooksByPolicy(executingHooks[0:i], release.HookSucceeded, waitStrategy, timeout); err != nil { + return func() error { + if errDeleting := cfg.deleteHookByPolicy(h, release.HookFailed, waitStrategy, timeout); errDeleting != nil { + // We log the error here as we want to propagate the hook failure upwards to the release object. + log.Printf("error deleting the hook resource on hook failure: %v", errDeleting) + } + + // If a hook is failed, check the annotation of the previous successful hooks to determine whether the hooks + // should be deleted under succeeded condition. + if err := cfg.deleteHooksByPolicy(executingHooks[0:i], release.HookSucceeded, waitStrategy, timeout); err != nil { + return err + } return err - } - - return err + }, err } h.LastRun.Phase = release.HookPhaseSucceeded } - // If all hooks are successful, check the annotation of each hook to determine whether the hook should be deleted - // or output should be logged under succeeded condition. If so, then clear the corresponding resource object in each hook - for i := len(executingHooks) - 1; i >= 0; i-- { - h := executingHooks[i] - if err := cfg.outputLogsByPolicy(h, rl.Namespace, release.HookOutputOnSucceeded); err != nil { - // We log here as we still want to attempt hook resource deletion even if output logging fails. - log.Printf("error outputting logs for hook failure: %v", err) - } - if err := cfg.deleteHookByPolicy(h, release.HookSucceeded, waitStrategy, timeout); err != nil { - return err + return func() error { + // If all hooks are successful, check the annotation of each hook to determine whether the hook should be deleted + // or output should be logged under succeeded condition. If so, then clear the corresponding resource object in each hook + for i := len(executingHooks) - 1; i >= 0; i-- { + h := executingHooks[i] + if err := cfg.outputLogsByPolicy(h, rl.Namespace, release.HookOutputOnSucceeded); err != nil { + // We log here as we still want to attempt hook resource deletion even if output logging fails. + log.Printf("error outputting logs for hook failure: %v", err) + } + if err := cfg.deleteHookByPolicy(h, release.HookSucceeded, waitStrategy, timeout); err != nil { + return err + } } - } - - return nil + return nil + }, nil } // hookByWeight is a sorter for hooks diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go index b649579f4..992cdd701 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, error) { +func (r *ReleaseTesting) Run(name string) (ri.Releaser, ExecuteShutdownFunc, error) { if err := r.cfg.KubeClient.IsReachable(); err != nil { - return nil, err + return nil, shutdownNoOp, err } if err := chartutil.ValidateReleaseName(name); err != nil { - return nil, 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, err + return reli, shutdownNoOp, err } rel, err := releaserToV1Release(reli) if err != nil { - return rel, err + return reli, shutdownNoOp, err } skippedHooks := []*release.Hook{} @@ -102,14 +102,16 @@ func (r *ReleaseTesting) Run(name string) (ri.Releaser, error) { } serverSideApply := rel.ApplyMethod == string(release.ApplyMethodServerSideApply) - if err := r.cfg.execHook(rel, release.HookTest, kube.StatusWatcherStrategy, r.Timeout, serverSideApply); err != nil { + shutdown, err := r.cfg.execHookWithDelayedShutdown(rel, release.HookTest, kube.StatusWatcherStrategy, r.Timeout, serverSideApply) + + if err != nil { rel.Hooks = append(skippedHooks, rel.Hooks...) - r.cfg.Releases.Update(rel) - return rel, err + r.cfg.Releases.Update(reli) + return reli, shutdown, err } rel.Hooks = append(skippedHooks, rel.Hooks...) - return rel, r.cfg.Releases.Update(rel) + return reli, shutdown, r.cfg.Releases.Update(reli) } // GetPodLogs will write the logs for all test pods in the given release into diff --git a/pkg/cmd/release_testing.go b/pkg/cmd/release_testing.go index 88a6f351f..5a6159e7d 100644 --- a/pkg/cmd/release_testing.go +++ b/pkg/cmd/release_testing.go @@ -55,7 +55,7 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command } return compListReleases(toComplete, args, cfg) }, - RunE: func(_ *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, args []string) (returnError error) { client.Namespace = settings.Namespace() notName := regexp.MustCompile(`^!\s?name=`) for _, f := range filter { @@ -65,7 +65,16 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command client.Filters[action.ExcludeNameFilter] = append(client.Filters[action.ExcludeNameFilter], notName.ReplaceAllLiteralString(f, "")) } } - reli, runErr := client.Run(args[0]) + + reli, shutdown, runErr := client.Run(args[0]) + defer func() { + if shutdownErr := shutdown(); shutdownErr != nil { + if returnError == nil { + returnError = shutdownErr + } + } + }() + // We only return an error if we weren't even able to get the // release, otherwise we keep going so we can print status and logs // if requested