From 8359e001a55f19bd4a4b2daf3f97f644659bde3c Mon Sep 17 00:00:00 2001 From: prafull01 Date: Tue, 29 Mar 2022 03:07:45 +0530 Subject: [PATCH] On hook failure, the cleanup mechanism should also run on all previous hooks Signed-off-by: prafull01 --- pkg/action/hooks.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 40c1ffdb6..92fc135a7 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -41,7 +41,7 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, // hooke are pre-ordered by kind, so keep order stable sort.Stable(hookByWeight(executingHooks)) - for _, h := range executingHooks { + for i, h := range executingHooks { // Set default delete policy to before-hook-creation if h.DeletePolicies == nil || len(h.DeletePolicies) == 0 { // TODO(jlegrone): Only apply before-hook-creation delete policy to run to completion @@ -91,6 +91,12 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, if err := cfg.deleteHookByPolicy(h, release.HookFailed); err != nil { return err } + + // If a hook is failed, check the annotation of the previous successful hooks to determine whether the hook + // should be deleted under succeeded condition. + if err := cfg.deleteHooksByPolicy(executingHooks[0:i], release.HookSucceeded); err != nil { + return err + } return err } h.LastRun.Phase = release.HookPhaseSucceeded @@ -98,10 +104,8 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, // 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 - } + if err := cfg.deleteHooksByPolicy(executingHooks, release.HookSucceeded); err != nil { + return err } return nil @@ -119,6 +123,17 @@ func (x hookByWeight) Less(i, j int) bool { return x[i].Weight < x[j].Weight } +// deleteHooksByPolicy deletes all hooks if the hook policy instructs it to +func (cfg *Configuration) deleteHooksByPolicy(hooks []*release.Hook, policy release.HookDeletePolicy) error { + for _, h := range hooks { + if err := cfg.deleteHookByPolicy(h, policy); err != nil { + return err + } + } + + return nil +} + // deleteHookByPolicy deletes a hook if the hook policy instructs it to func (cfg *Configuration) deleteHookByPolicy(h *release.Hook, policy release.HookDeletePolicy) error { // Never delete CustomResourceDefinitions; this could cause lots of