From d5080d71ef742eef0722416d257b9472deb1dc7b Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Thu, 18 Dec 2025 18:57:51 +0000 Subject: [PATCH] Handle deletion propagation defaults in uninstall action Signed-off-by: Evans Mungai --- pkg/action/uninstall.go | 12 +++++-- pkg/action/uninstall_test.go | 69 ++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index efbc72fef..212cccc43 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -276,12 +276,12 @@ func (u *Uninstall) deleteRelease(rel *release.Release) (kube.ResourceList, stri return nil, "", []error{fmt.Errorf("unable to build kubernetes objects for delete: %w", err)} } if len(resources) > 0 { - _, errs = u.cfg.KubeClient.Delete(resources, parseCascadingFlag(u.DeletionPropagation)) + _, errs = u.cfg.KubeClient.Delete(resources, u.parseCascadingFlag(u.DeletionPropagation)) } return resources, kept.String(), errs } -func parseCascadingFlag(cascadingFlag string) v1.DeletionPropagation { +func (u *Uninstall) parseCascadingFlag(cascadingFlag string) v1.DeletionPropagation { switch cascadingFlag { case "orphan": return v1.DeletePropagationOrphan @@ -290,6 +290,14 @@ func parseCascadingFlag(cascadingFlag string) v1.DeletionPropagation { case "background": return v1.DeletePropagationBackground default: + // When --wait is used with a non-hookOnly strategy (i.e. watcher or legacy), default to foreground + // deletion to ensure dependent resources are cleaned up before returning. + // This prevents the issue where helm uninstall --wait returns before + // all resources (including controller-created dependents) are fully deleted. + if u.WaitStrategy != kube.HookOnlyStrategy { + slog.Debug("uninstall: defaulting to foreground deletion due to wait strategy", "wait strategy", u.WaitStrategy, "cascading flag", cascadingFlag) + return v1.DeletePropagationForeground + } slog.Debug("uninstall: given cascade value, defaulting to delete propagation background", "value", cascadingFlag) return v1.DeletePropagationBackground } diff --git a/pkg/action/uninstall_test.go b/pkg/action/uninstall_test.go index fba1e391f..e2e895dbe 100644 --- a/pkg/action/uninstall_test.go +++ b/pkg/action/uninstall_test.go @@ -169,3 +169,72 @@ func TestUninstallRun_UnreachableKubeClient(t *testing.T) { assert.Nil(t, result) assert.ErrorContains(t, err, "connection refused") } + +func TestParseCascadingFlag(t *testing.T) { + tests := []struct { + name string + cascadingFlag string + waitStrategy kube.WaitStrategy + expectedResult string + }{ + { + name: "explicit orphan", + cascadingFlag: "orphan", + waitStrategy: kube.HookOnlyStrategy, + expectedResult: "Orphan", + }, + { + name: "explicit foreground", + cascadingFlag: "foreground", + waitStrategy: kube.HookOnlyStrategy, + expectedResult: "Foreground", + }, + { + name: "explicit background", + cascadingFlag: "background", + waitStrategy: kube.HookOnlyStrategy, + expectedResult: "Background", + }, + { + name: "default with hookOnly strategy returns background", + cascadingFlag: "", + waitStrategy: kube.HookOnlyStrategy, + expectedResult: "Background", + }, + { + name: "default with watcher strategy returns foreground", + cascadingFlag: "", + waitStrategy: kube.StatusWatcherStrategy, + expectedResult: "Foreground", + }, + { + name: "default with legacy strategy returns foreground", + cascadingFlag: "", + waitStrategy: kube.LegacyStrategy, + expectedResult: "Foreground", + }, + { + name: "unknown value with hookOnly strategy returns background", + cascadingFlag: "unknown", + waitStrategy: kube.HookOnlyStrategy, + expectedResult: "Background", + }, + { + name: "unknown value with watcher strategy returns foreground", + cascadingFlag: "unknown", + waitStrategy: kube.StatusWatcherStrategy, + expectedResult: "Foreground", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + unAction := uninstallAction(t) + unAction.WaitStrategy = tt.waitStrategy + + result := unAction.parseCascadingFlag(tt.cascadingFlag) + + assert.Equal(t, tt.expectedResult, string(result)) + }) + } +}