From d8953575af8587dfdc9ad75258c7198f1957a923 Mon Sep 17 00:00:00 2001 From: Michelle Fernandez Bieber Date: Sat, 22 Jan 2022 23:20:18 +0100 Subject: [PATCH 1/5] added shutdown hook that is executed after the logs have been retrieved Signed-off-by: Michelle Fernandez Bieber --- cmd/helm/release_testing.go | 3 ++- pkg/action/hooks.go | 50 +++++++++++++++++++++++++---------- pkg/action/release_testing.go | 16 ++++++----- 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/cmd/helm/release_testing.go b/cmd/helm/release_testing.go index 2637cbb9f..a2a5f1fc2 100644 --- a/cmd/helm/release_testing.go +++ b/cmd/helm/release_testing.go @@ -64,7 +64,8 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command client.Filters["!name"] = append(client.Filters["!name"], notName.ReplaceAllLiteralString(f, "")) } } - rel, runErr := client.Run(args[0]) + rel, shutdown, runErr := client.Run(args[0]) + defer shutdown() // 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 diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 40c1ffdb6..ebc0fa2f0 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -28,6 +28,24 @@ import ( // execHook executes all of the hooks for the given hook event. func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, timeout time.Duration) error { + shutdown, err := cfg.execHookWithDelayedShutdown(rl, hook, timeout) + if err != nil { + if err := shutdown(); err != nil { + return err + } + return err + } + return shutdown() +} + +type ExecuteShutdownHooks = func() error + +func ShutdownNoOp() error { + return nil +} + +// execHook executes all of the hooks for the given hook event and return 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, timeout time.Duration) (ExecuteShutdownHooks, error) { executingHooks := []*release.Hook{} for _, h := range rl.Hooks { @@ -52,12 +70,12 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, } if err := cfg.deleteHookByPolicy(h, release.HookBeforeHookCreation); err != nil { - return err + return ShutdownNoOp, err } resources, err := cfg.KubeClient.Build(bytes.NewBufferString(h.Manifest), true) if err != nil { - return errors.Wrapf(err, "unable to build kubernetes object for %s hook %s", hook, h.Path) + return ShutdownNoOp, errors.Wrapf(err, "unable to build kubernetes object for %s hook %s", hook, h.Path) } // Record the time at which the hook was applied to the cluster @@ -76,7 +94,7 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, if _, err := cfg.KubeClient.Create(resources); err != nil { h.LastRun.CompletedAt = helmtime.Now() h.LastRun.Phase = release.HookPhaseFailed - return errors.Wrapf(err, "warning: Hook %s %s failed", hook, h.Path) + return ShutdownNoOp, errors.Wrapf(err, "warning: Hook %s %s failed", hook, h.Path) } // Watch hook resources until they have completed @@ -88,23 +106,27 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, h.LastRun.Phase = release.HookPhaseFailed // 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 err := cfg.deleteHookByPolicy(h, release.HookFailed); err != nil { + return func() error { + if err := cfg.deleteHookByPolicy(h, release.HookFailed); 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 - // under succeeded condition. If so, then clear the corresponding resource object in each hook - for _, h := range executingHooks { - if err := cfg.deleteHookByPolicy(h, release.HookSucceeded); 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 + // under succeeded condition. If so, then clear the corresponding resource object in each hook + for _, h := range executingHooks { + if err := cfg.deleteHookByPolicy(h, release.HookSucceeded); 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 ecaeaf59f..5dbb0d3cb 100644 --- a/pkg/action/release_testing.go +++ b/pkg/action/release_testing.go @@ -49,19 +49,19 @@ func NewReleaseTesting(cfg *Configuration) *ReleaseTesting { } // Run executes 'helm test' against the given release. -func (r *ReleaseTesting) Run(name string) (*release.Release, error) { +func (r *ReleaseTesting) Run(name string) (*release.Release, ExecuteShutdownHooks, 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, errors.Errorf("releaseTest: Release name is invalid: %s", name) + return nil, ShutdownNoOp, errors.Errorf("releaseTest: Release name is invalid: %s", name) } // finds the non-deleted release with the given name rel, err := r.cfg.Releases.Last(name) if err != nil { - return rel, err + return rel, ShutdownNoOp, err } skippedHooks := []*release.Hook{} @@ -88,14 +88,16 @@ func (r *ReleaseTesting) Run(name string) (*release.Release, error) { rel.Hooks = executingHooks } - if err := r.cfg.execHook(rel, release.HookTest, r.Timeout); err != nil { + shutdown, err := r.cfg.execHookWithDelayedShutdown(rel, release.HookTest, r.Timeout) + + if err != nil { rel.Hooks = append(skippedHooks, rel.Hooks...) r.cfg.Releases.Update(rel) - return rel, err + return rel, shutdown, err } rel.Hooks = append(skippedHooks, rel.Hooks...) - return rel, r.cfg.Releases.Update(rel) + return rel, shutdown, r.cfg.Releases.Update(rel) } // GetPodLogs will write the logs for all test pods in the given release into From 49c3b897bc4f0579a3c3089132ec718baaf9d793 Mon Sep 17 00:00:00 2001 From: Michelle Fernandez Bieber Date: Sun, 23 Jan 2022 00:17:09 +0100 Subject: [PATCH 2/5] updated comment and made defer of shutdown function return errors as before and not the possible shutdown error Signed-off-by: Michelle Fernandez Bieber --- cmd/helm/release_testing.go | 12 ++++++++++-- pkg/action/hooks.go | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/cmd/helm/release_testing.go b/cmd/helm/release_testing.go index a2a5f1fc2..ba10fef30 100644 --- a/cmd/helm/release_testing.go +++ b/cmd/helm/release_testing.go @@ -54,7 +54,7 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command } return compListReleases(toComplete, args, cfg) }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) (returnError error) { client.Namespace = settings.Namespace() notName := regexp.MustCompile(`^!\s?name=`) for _, f := range filter { @@ -64,8 +64,16 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command client.Filters["!name"] = append(client.Filters["!name"], notName.ReplaceAllLiteralString(f, "")) } } + rel, shutdown, runErr := client.Run(args[0]) - defer shutdown() + 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 diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index ebc0fa2f0..5d735a69b 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -44,7 +44,7 @@ func ShutdownNoOp() error { return nil } -// execHook executes all of the hooks for the given hook event and return a shutdownHook function to trigger deletions after doing other things like e.g. retrieving logs. +// execHook 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, timeout time.Duration) (ExecuteShutdownHooks, error) { executingHooks := []*release.Hook{} From 77fdd197ef4cc7790dda42a1c24ad0ae6cefc750 Mon Sep 17 00:00:00 2001 From: Michelle Fernandez Bieber Date: Sun, 23 Jan 2022 00:25:32 +0100 Subject: [PATCH 3/5] cleaned up empty line Signed-off-by: Michelle Fernandez Bieber --- pkg/action/hooks.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 5d735a69b..0e4709051 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -112,7 +112,6 @@ func (cfg *Configuration) execHookWithDelayedShutdown(rl *release.Release, hook } return err }, err - } h.LastRun.Phase = release.HookPhaseSucceeded } From 508014a45192288c6f292f8d6db027fd483cd45a Mon Sep 17 00:00:00 2001 From: Michelle Fernandez Bieber Date: Fri, 22 Apr 2022 22:55:35 +0200 Subject: [PATCH 4/5] added check for nil shutdown Signed-off-by: Michelle Fernandez Bieber --- pkg/action/hooks.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 0e4709051..271ecd23a 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -29,6 +29,9 @@ import ( // execHook executes all of the hooks for the given hook event. func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, timeout time.Duration) error { shutdown, err := cfg.execHookWithDelayedShutdown(rl, hook, timeout) + if shutdown == nil { + return nil + } if err != nil { if err := shutdown(); err != nil { return err From 2139da7811e5479bfd6e945ab2a4c7bf26d06f1f Mon Sep 17 00:00:00 2001 From: Michelle Fernandez Bieber <37021266+mfbieber@users.noreply.github.com> Date: Sun, 23 Jul 2023 08:17:57 +0200 Subject: [PATCH 5/5] Update pkg/action/hooks.go Co-authored-by: Marco Lecheler Signed-off-by: Michelle Fernandez Bieber <37021266+mfbieber@users.noreply.github.com> --- pkg/action/hooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 271ecd23a..6eb31ce3f 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -47,7 +47,7 @@ func ShutdownNoOp() error { return nil } -// execHook 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. +// 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, timeout time.Duration) (ExecuteShutdownHooks, error) { executingHooks := []*release.Hook{}