fix: default to foreground cascade when --wait is used in uninstall

When `helm uninstall --wait` is called, the default deletion propagation
is now "foreground" instead of "background". This ensures dependent
resources (like Pods, ReplicaSets created by controllers) are fully
cleaned up before the command returns.

Previously, with background deletion, parent resources were deleted
immediately while garbage collection handled dependents asynchronously.
This caused subsequent namespace deletions to hang for 2+ minutes waiting
for those dependent resources to be cleaned up.

With foreground deletion, Kubernetes waits for all dependents to be
deleted before removing the parent resource, ensuring `--wait` truly
waits for all cleanup to complete.

Cascade behavior by command:

| Command                                      | Cascade    |
|----------------------------------------------|------------|
| `helm uninstall release`                     | background |
| `helm uninstall release --wait`              | foreground |
| `helm uninstall release --wait=hookOnly`     | background |
| `helm uninstall release --wait --cascade=background` | background |

Users can still explicitly set `--cascade=background` to restore the
previous behavior if needed.

Fixes #31651

Signed-off-by: Evans Mungai <mbuevans@gmail.com>
pull/31658/head
Evans Mungai 4 weeks ago
parent 4d1150d2af
commit 5895a7e8f9
No known key found for this signature in database
GPG Key ID: BBEB812143DD14E1

@ -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)

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

@ -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
}
Loading…
Cancel
Save