Merge 5f449cbaed into f05c21b6b6

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 watcher or legacy strategy, default to foreground
+		// deletion to ensure dependent resources are cleaned up before returning.
+		// This prevents the issue where uninstallation with wait strategy returns before
+		// all resources (including controller-created dependents) are fully deleted.
+		if u.WaitStrategy == kube.StatusWatcherStrategy || u.WaitStrategy == kube.LegacyStrategy {
+			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
 	}

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))
+		})
+	}
+}

pkg/cmd/uninstall.go

@@ -25,6 +25,7 @@ import (
 
 	"helm.sh/helm/v4/pkg/action"
 	"helm.sh/helm/v4/pkg/cmd/require"
+	"helm.sh/helm/v4/pkg/kube"
 )
 
 const uninstallDesc = `
@@ -50,11 +51,20 @@ func newUninstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 		ValidArgsFunction: func(_ *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 			return compListReleases(toComplete, args, cfg)
 		},
-		RunE: func(_ *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			validationErr := validateCascadeFlag(client)
 			if validationErr != nil {
 				return validationErr
 			}
+
+			// When --wait is used with a non-hookOnly strategy, 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 !cmd.Flags().Changed("cascade") && client.WaitStrategy != kube.HookOnlyStrategy {
+				client.DeletionPropagation = "foreground"
+			}
+
 			for i := range args {
 
 				res, err := client.Run(args[i])
@@ -76,7 +86,7 @@ func newUninstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	f.BoolVar(&client.DisableHooks, "no-hooks", false, "prevent hooks from running during uninstallation")
 	f.BoolVar(&client.IgnoreNotFound, "ignore-not-found", false, `Treat "release not found" as a successful uninstall`)
 	f.BoolVar(&client.KeepHistory, "keep-history", false, "remove all associated resources and mark the release as deleted, but retain the release history")
-	f.StringVar(&client.DeletionPropagation, "cascade", "background", "Must be \"background\", \"orphan\", or \"foreground\". Selects the deletion cascading strategy for the dependents. Defaults to background.")
+	f.StringVar(&client.DeletionPropagation, "cascade", "background", "Must be \"background\", \"orphan\", or \"foreground\". Selects the deletion cascading strategy for the dependents. Defaults to background, or foreground if --wait is used.")
 	f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)")
 	f.StringVar(&client.Description, "description", "", "add a custom description")
 	AddWaitFlag(cmd, &client.WaitStrategy)

pkg/cmd/uninstall_test.go

@@ -17,10 +17,19 @@ limitations under the License.
 package cmd
 
 import (
+	"bytes"
+	"io"
 	"testing"
 
-	"helm.sh/helm/v4/pkg/release/common"
+	"github.com/stretchr/testify/assert"
+
+	"helm.sh/helm/v4/pkg/action"
+	"helm.sh/helm/v4/pkg/chart/common"
+	kubefake "helm.sh/helm/v4/pkg/kube/fake"
+	releasecommon "helm.sh/helm/v4/pkg/release/common"
 	release "helm.sh/helm/v4/pkg/release/v1"
+	"helm.sh/helm/v4/pkg/storage"
+	"helm.sh/helm/v4/pkg/storage/driver"
 )
 
 func TestUninstall(t *testing.T) {
@@ -63,8 +72,8 @@ func TestUninstall(t *testing.T) {
 			cmd:    "uninstall aeneas --keep-history",
 			golden: "output/uninstall-keep-history-earlier-deployed.txt",
 			rels: []*release.Release{
-				release.Mock(&release.MockReleaseOptions{Name: "aeneas", Version: 1, Status: common.StatusDeployed}),
-				release.Mock(&release.MockReleaseOptions{Name: "aeneas", Version: 2, Status: common.StatusFailed}),
+				release.Mock(&release.MockReleaseOptions{Name: "aeneas", Version: 1, Status: releasecommon.StatusDeployed}),
+				release.Mock(&release.MockReleaseOptions{Name: "aeneas", Version: 2, Status: releasecommon.StatusFailed}),
 			},
 		},
 		{
@@ -83,6 +92,111 @@ func TestUninstall(t *testing.T) {
 	runTestCmd(t, tests)
 }
 
+// TestUninstallCascadeDefaultWithWait tests that when --wait is used,
+// the default cascade behavior changes to foreground to ensure
+// dependent resources are cleaned up before returning.
+func TestUninstallCascadeDefaultWithWait(t *testing.T) {
+	tests := []struct {
+		name                        string
+		cmd                         string
+		expectedDeletionPropagation string
+	}{
+		{
+			name:                        "without --wait uses background cascade",
+			cmd:                         "uninstall test-release",
+			expectedDeletionPropagation: "Background",
+		},
+		{
+			name:                        "--wait without --cascade uses foreground cascade",
+			cmd:                         "uninstall test-release --wait",
+			expectedDeletionPropagation: "Foreground",
+		},
+		{
+			name:                        "--wait=watcher without --cascade uses foreground cascade",
+			cmd:                         "uninstall test-release --wait=watcher",
+			expectedDeletionPropagation: "Foreground",
+		},
+		{
+			name:                        "--wait=legacy without --cascade uses foreground cascade",
+			cmd:                         "uninstall test-release --wait=legacy",
+			expectedDeletionPropagation: "Foreground",
+		},
+		{
+			name:                        "--wait=hookOnly uses background cascade (default)",
+			cmd:                         "uninstall test-release --wait=hookOnly",
+			expectedDeletionPropagation: "Background",
+		},
+		{
+			name:                        "--wait with explicit --cascade=background respects user setting",
+			cmd:                         "uninstall test-release --wait --cascade=background",
+			expectedDeletionPropagation: "Background",
+		},
+		{
+			name:                        "--wait with explicit --cascade=orphan respects user setting",
+			cmd:                         "uninstall test-release --wait --cascade=orphan",
+			expectedDeletionPropagation: "Orphan",
+		},
+		{
+			name:                        "--wait with explicit --cascade=foreground respects user setting",
+			cmd:                         "uninstall test-release --wait --cascade=foreground",
+			expectedDeletionPropagation: "Foreground",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create a capturing KubeClient to verify the deletion propagation
+			capturedDeletionPropagation := ""
+			store := storage.Init(driver.NewMemory())
+			rel := release.Mock(&release.MockReleaseOptions{Name: "test-release"})
+			if err := store.Create(rel); err != nil {
+				t.Fatal(err)
+			}
+
+			capturingClient := &kubefake.CapturingKubeClient{
+				FailingKubeClient: kubefake.FailingKubeClient{
+					PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard},
+					BuildDummy:         true,
+				},
+				CapturedDeletionPropagation: &capturedDeletionPropagation,
+			}
+
+			actionConfig := &action.Configuration{
+				Releases:     store,
+				KubeClient:   capturingClient,
+				Capabilities: common.DefaultCapabilities,
+			}
+
+			buf := new(bytes.Buffer)
+			args := splitCmd(tt.cmd)
+
+			root, err := newRootCmdWithConfig(actionConfig, buf, args, SetupLogging)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			root.SetArgs(args)
+			root.SetOut(buf)
+			root.SetErr(buf)
+
+			_ = root.Execute()
+
+			// Verify the captured deletion propagation
+			assert.Equal(t, tt.expectedDeletionPropagation, capturedDeletionPropagation,
+				"Expected DeletionPropagation to be %q but got %q", tt.expectedDeletionPropagation, capturedDeletionPropagation)
+		})
+	}
+}
+
+func splitCmd(cmd string) []string {
+	// Simple split by space - doesn't handle quoted strings
+	result := []string{}
+	for _, part := range bytes.Fields([]byte(cmd)) {
+		result = append(result, string(part))
+	}
+	return result
+}
+
 func TestUninstallCompletion(t *testing.T) {
 	checkReleaseCompletion(t, "uninstall", true)
 }

pkg/kube/fake/capturing_kube_client.go

@@ -0,0 +1,39 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package fake
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"helm.sh/helm/v4/pkg/kube"
+)
+
+// CapturingKubeClient is a fake KubeClient that captures internal flags such as the
+// deletion propagation policy. It extends FailingKubeClient to provide dummy resources.
+type CapturingKubeClient struct {
+	FailingKubeClient
+	// CapturedDeletionPropagation stores the DeletionPropagation value passed to Delete
+	CapturedDeletionPropagation *string
+}
+
+// Delete captures the deletion propagation policy and returns success
+func (c *CapturingKubeClient) Delete(resources kube.ResourceList, policy metav1.DeletionPropagation) (*kube.Result, []error) {
+	if c.CapturedDeletionPropagation != nil {
+		*c.CapturedDeletionPropagation = string(policy)
+	}
+	return &kube.Result{Deleted: resources}, nil
+}