Include Jobs in Wait

When --wait-for-jobs was introduced it was added as a new flag
so the behavior on --wait didn't change in a minor version.
This happened in #8363. In a major version of Helm we can merge
these into a single behavior behind the --wait flag.

Signed-off-by: Matt Farina <matt.farina@suse.com>

pkg/action/install.go

@@ -85,7 +85,6 @@ type Install struct {
 	DisableHooks     bool
 	Replace          bool
 	WaitStrategy     kube.WaitStrategy
-	WaitForJobs      bool
 	Devel            bool
 	DependencyUpdate bool
 	Timeout          time.Duration
@@ -494,11 +493,7 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource
 		return rel, fmt.Errorf("failed to get waiter: %w", err)
 	}
 
-	if i.WaitForJobs {
-		err = waiter.WaitWithJobs(resources, i.Timeout)
-	} else {
-		err = waiter.Wait(resources, i.Timeout)
-	}
+	err = waiter.Wait(resources, i.Timeout)
 	if err != nil {
 		return rel, err
 	}

pkg/action/install_test.go

@@ -573,22 +573,6 @@ func TestInstallRelease_Wait_Interrupted(t *testing.T) {
 	time.Sleep(10 * time.Second)                   // wait for goroutine to finish
 	is.Equal(goroutines, runtime.NumGoroutine())
 }
-func TestInstallRelease_WaitForJobs(t *testing.T) {
-	is := assert.New(t)
-	instAction := installAction(t)
-	instAction.ReleaseName = "come-fail-away"
-	failer := instAction.cfg.KubeClient.(*kubefake.FailingKubeClient)
-	failer.WaitError = fmt.Errorf("I timed out")
-	instAction.cfg.KubeClient = failer
-	instAction.WaitStrategy = kube.StatusWatcherStrategy
-	instAction.WaitForJobs = true
-	vals := map[string]interface{}{}
-
-	res, err := instAction.Run(buildChart(), vals)
-	is.Error(err)
-	is.Contains(res.Info.Description, "I timed out")
-	is.Equal(res.Info.Status, release.StatusFailed)
-}
 
 func TestInstallRelease_RollbackOnFailure(t *testing.T) {
 	is := assert.New(t)

pkg/action/rollback.go

@@ -38,7 +38,6 @@ type Rollback struct {
 	Version      int
 	Timeout      time.Duration
 	WaitStrategy kube.WaitStrategy
-	WaitForJobs  bool
 	DisableHooks bool
 	DryRun       bool
 	// ForceReplace will, if set to `true`, ignore certain warnings and perform the rollback anyway.
@@ -221,20 +220,12 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas
 	if err != nil {
 		return nil, fmt.Errorf("unable to set metadata visitor from target release: %w", err)
 	}
-	if r.WaitForJobs {
-		if err := waiter.WaitWithJobs(target, r.Timeout); err != nil {
-			targetRelease.SetStatus(release.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error()))
-			r.cfg.recordRelease(currentRelease)
-			r.cfg.recordRelease(targetRelease)
-			return targetRelease, fmt.Errorf("release %s failed: %w", targetRelease.Name, err)
-		}
-	} else {
-		if err := waiter.Wait(target, r.Timeout); err != nil {
-			targetRelease.SetStatus(release.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error()))
-			r.cfg.recordRelease(currentRelease)
-			r.cfg.recordRelease(targetRelease)
-			return targetRelease, fmt.Errorf("release %s failed: %w", targetRelease.Name, err)
-		}
+
+	if err := waiter.Wait(target, r.Timeout); err != nil {
+		targetRelease.SetStatus(release.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error()))
+		r.cfg.recordRelease(currentRelease)
+		r.cfg.recordRelease(targetRelease)
+		return targetRelease, fmt.Errorf("release %s failed: %w", targetRelease.Name, err)
 	}
 
 	// post-rollback hooks

pkg/action/upgrade.go

@@ -66,8 +66,6 @@ type Upgrade struct {
 	Timeout time.Duration
 	// WaitStrategy determines what type of waiting should be done
 	WaitStrategy kube.WaitStrategy
-	// WaitForJobs determines whether the wait operation for the Jobs should be performed after the upgrade is requested.
-	WaitForJobs bool
 	// DisableHooks disables hook processing if set to true.
 	DisableHooks bool
 	// DryRun controls whether the operation is prepared, but not executed.
@@ -443,18 +441,10 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele
 		u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err)
 		return
 	}
-	if u.WaitForJobs {
-		if err := waiter.WaitWithJobs(target, u.Timeout); err != nil {
-			u.cfg.recordRelease(originalRelease)
-			u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err)
-			return
-		}
-	} else {
-		if err := waiter.Wait(target, u.Timeout); err != nil {
-			u.cfg.recordRelease(originalRelease)
-			u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err)
-			return
-		}
+	if err := waiter.Wait(target, u.Timeout); err != nil {
+		u.cfg.recordRelease(originalRelease)
+		u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err)
+		return
 	}
 
 	// post-upgrade hooks
@@ -528,7 +518,6 @@ func (u *Upgrade) failRelease(rel *release.Release, created kube.ResourceList, e
 		if u.WaitStrategy == kube.HookOnlyStrategy {
 			rollin.WaitStrategy = kube.StatusWatcherStrategy
 		}
-		rollin.WaitForJobs = u.WaitForJobs
 		rollin.DisableHooks = u.DisableHooks
 		rollin.ForceReplace = u.ForceReplace
 		rollin.Timeout = u.Timeout

pkg/action/upgrade_test.go

@@ -93,29 +93,6 @@ func TestUpgradeRelease_Wait(t *testing.T) {
 	is.Equal(res.Info.Status, release.StatusFailed)
 }
 
-func TestUpgradeRelease_WaitForJobs(t *testing.T) {
-	is := assert.New(t)
-	req := require.New(t)
-
-	upAction := upgradeAction(t)
-	rel := releaseStub()
-	rel.Name = "come-fail-away"
-	rel.Info.Status = release.StatusDeployed
-	upAction.cfg.Releases.Create(rel)
-
-	failer := upAction.cfg.KubeClient.(*kubefake.FailingKubeClient)
-	failer.WaitError = fmt.Errorf("I timed out")
-	upAction.cfg.KubeClient = failer
-	upAction.WaitStrategy = kube.StatusWatcherStrategy
-	upAction.WaitForJobs = true
-	vals := map[string]interface{}{}
-
-	res, err := upAction.Run(rel.Name, buildChart(), vals)
-	req.Error(err)
-	is.Contains(res.Info.Description, "I timed out")
-	is.Equal(res.Info.Status, release.StatusFailed)
-}
-
 func TestUpgradeRelease_CleanupOnFail(t *testing.T) {
 	is := assert.New(t)
 	req := require.New(t)

pkg/cmd/install.go

@@ -199,7 +199,6 @@ func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Instal
 	f.BoolVar(&client.DisableHooks, "no-hooks", false, "prevent hooks from running during install")
 	f.BoolVar(&client.Replace, "replace", false, "reuse the given name, only if that name is a deleted release which remains in the history. This is unsafe in production")
 	f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)")
-	f.BoolVar(&client.WaitForJobs, "wait-for-jobs", false, "if set and --wait enabled, will wait until all Jobs have been completed before marking the release as successful. It will wait for as long as --timeout")
 	f.BoolVarP(&client.GenerateName, "generate-name", "g", false, "generate the name (and omit the NAME parameter)")
 	f.StringVar(&client.NameTemplate, "name-template", "", "specify template used to name the release")
 	f.StringVar(&client.Description, "description", "", "add a custom description")

pkg/cmd/install_test.go

@@ -114,12 +114,6 @@ func TestInstall(t *testing.T) {
 			cmd:    "install apollo testdata/testcharts/empty --wait",
 			golden: "output/install-with-wait.txt",
 		},
-		// Install, with wait-for-jobs
-		{
-			name:   "install with wait-for-jobs",
-			cmd:    "install apollo testdata/testcharts/empty --wait --wait-for-jobs",
-			golden: "output/install-with-wait-for-jobs.txt",
-		},
 		// Install, using the name-template
 		{
 			name:   "install with name-template",

pkg/cmd/rollback.go

@@ -82,7 +82,6 @@ func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	f.MarkDeprecated("force", "use --force-replace instead")
 	f.BoolVar(&client.DisableHooks, "no-hooks", false, "prevent hooks from running during rollback")
 	f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)")
-	f.BoolVar(&client.WaitForJobs, "wait-for-jobs", false, "if set and --wait enabled, will wait until all Jobs have been completed before marking the release as successful. It will wait for as long as --timeout")
 	f.BoolVar(&client.CleanupOnFail, "cleanup-on-fail", false, "allow deletion of new resources created in this rollback when rollback fails")
 	f.IntVar(&client.MaxHistory, "history-max", settings.MaxHistory, "limit the maximum number of revisions saved per release. Use 0 for no limit")
 	AddWaitFlag(cmd, &client.WaitStrategy)

pkg/cmd/rollback_test.go

@@ -56,11 +56,6 @@ func TestRollbackCmd(t *testing.T) {
 		cmd:    "rollback funny-honey 1 --wait",
 		golden: "output/rollback-wait.txt",
 		rels:   rels,
-	}, {
-		name:   "rollback a release with wait-for-jobs",
-		cmd:    "rollback funny-honey 1 --wait --wait-for-jobs",
-		golden: "output/rollback-wait-for-jobs.txt",
-		rels:   rels,
 	}, {
 		name:   "rollback a release without revision",
 		cmd:    "rollback funny-honey",

pkg/cmd/testdata/output/install-with-wait-for-jobs.txt

@@ -1,7 +0,0 @@
-NAME: apollo
-LAST DEPLOYED: Fri Sep  2 22:04:05 1977
-NAMESPACE: default
-STATUS: deployed
-REVISION: 1
-DESCRIPTION: Install complete
-TEST SUITE: None

pkg/cmd/testdata/output/rollback-wait-for-jobs.txt

@@ -1 +0,0 @@
-Rollback was a success! Happy Helming!

pkg/cmd/testdata/output/upgrade-with-wait-for-jobs.txt

@@ -1,8 +0,0 @@
-Release "crazy-bunny" has been upgraded. Happy Helming!
-NAME: crazy-bunny
-LAST DEPLOYED: Fri Sep  2 22:04:05 1977
-NAMESPACE: default
-STATUS: deployed
-REVISION: 3
-DESCRIPTION: Upgrade complete
-TEST SUITE: None

pkg/cmd/upgrade.go

@@ -137,7 +137,6 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 					instClient.SkipCRDs = client.SkipCRDs
 					instClient.Timeout = client.Timeout
 					instClient.WaitStrategy = client.WaitStrategy
-					instClient.WaitForJobs = client.WaitForJobs
 					instClient.Devel = client.Devel
 					instClient.Namespace = client.Namespace
 					instClient.RollbackOnFailure = client.RollbackOnFailure
@@ -281,7 +280,6 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	f.BoolVar(&client.ResetValues, "reset-values", false, "when upgrading, reset the values to the ones built into the chart")
 	f.BoolVar(&client.ReuseValues, "reuse-values", false, "when upgrading, reuse the last release's values and merge in any overrides from the command line via --set and -f. If '--reset-values' is specified, this is ignored")
 	f.BoolVar(&client.ResetThenReuseValues, "reset-then-reuse-values", false, "when upgrading, reset the values to the ones built into the chart, apply the last release's values and merge in any overrides from the command line via --set and -f. If '--reset-values' or '--reuse-values' is specified, this is ignored")
-	f.BoolVar(&client.WaitForJobs, "wait-for-jobs", false, "if set and --wait enabled, will wait until all Jobs have been completed before marking the release as successful. It will wait for as long as --timeout")
 	f.BoolVar(&client.RollbackOnFailure, "rollback-on-failure", false, "if set, Helm will rollback the upgrade to previous success release upon failure. The --wait flag will be defaulted to \"watcher\" if --rollback-on-failure is set")
 	f.BoolVar(&client.RollbackOnFailure, "atomic", false, "deprecated")
 	f.MarkDeprecated("atomic", "use --rollback-on-failure instead")

pkg/cmd/upgrade_test.go

@@ -138,12 +138,6 @@ func TestUpgradeCmd(t *testing.T) {
 			golden: "output/upgrade-with-wait.txt",
 			rels:   []*release.Release{relMock("crazy-bunny", 2, ch2)},
 		},
-		{
-			name:   "upgrade a release with wait-for-jobs",
-			cmd:    fmt.Sprintf("upgrade crazy-bunny --wait --wait-for-jobs '%s'", chartPath),
-			golden: "output/upgrade-with-wait-for-jobs.txt",
-			rels:   []*release.Release{relMock("crazy-bunny", 2, ch2)},
-		},
 		{
 			name:      "upgrade a release with missing dependencies",
 			cmd:       fmt.Sprintf("upgrade bonkers-bunny %s", missingDepsPath),

pkg/kube/client_test.go

@@ -753,7 +753,7 @@ func TestWaitJob(t *testing.T) {
 		t.Errorf("expected 1 resource created, got %d", len(result.Created))
 	}
 
-	if err := c.WaitWithJobs(resources, time.Second*30); err != nil {
+	if err := c.Wait(resources, time.Second*30); err != nil {
 		t.Errorf("expected wait without error, got %s", err)
 	}
 

pkg/kube/fake/fake.go

@@ -84,14 +84,6 @@ func (f *FailingKubeWaiter) Wait(resources kube.ResourceList, d time.Duration) e
 	return f.PrintingKubeWaiter.Wait(resources, d)
 }
 
-// WaitWithJobs returns the configured error if set or prints
-func (f *FailingKubeWaiter) WaitWithJobs(resources kube.ResourceList, d time.Duration) error {
-	if f.waitError != nil {
-		return f.waitError
-	}
-	return f.PrintingKubeWaiter.WaitWithJobs(resources, d)
-}
-
 // WaitForDelete returns the configured error if set or prints
 func (f *FailingKubeWaiter) WaitForDelete(resources kube.ResourceList, d time.Duration) error {
 	if f.waitForDeleteError != nil {

pkg/kube/fake/printer.go

@@ -70,11 +70,6 @@ func (p *PrintingKubeWaiter) Wait(resources kube.ResourceList, _ time.Duration)
 	return err
 }
 
-func (p *PrintingKubeWaiter) WaitWithJobs(resources kube.ResourceList, _ time.Duration) error {
-	_, err := io.Copy(p.Out, bufferize(resources))
-	return err
-}
-
 func (p *PrintingKubeWaiter) WaitForDelete(resources kube.ResourceList, _ time.Duration) error {
 	_, err := io.Copy(p.Out, bufferize(resources))
 	return err

pkg/kube/interface.go

@@ -58,9 +58,6 @@ type Waiter interface {
 	// Wait waits up to the given timeout for the specified resources to be ready.
 	Wait(resources ResourceList, timeout time.Duration) error
 
-	// WaitWithJobs wait up to the given timeout for the specified resources to be ready, including jobs.
-	WaitWithJobs(resources ResourceList, timeout time.Duration) error
-
 	// WaitForDelete wait up to the given timeout for the specified resources to be deleted.
 	WaitForDelete(resources ResourceList, timeout time.Duration) error
 

pkg/kube/statuswait.go

@@ -74,14 +74,6 @@ func (w *statusWaiter) WatchUntilReady(resourceList ResourceList, timeout time.D
 }
 
 func (w *statusWaiter) Wait(resourceList ResourceList, timeout time.Duration) error {
-	ctx, cancel := context.WithTimeout(context.TODO(), timeout)
-	defer cancel()
-	slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout)
-	sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper)
-	return w.wait(ctx, resourceList, sw)
-}
-
-func (w *statusWaiter) WaitWithJobs(resourceList ResourceList, timeout time.Duration) error {
 	ctx, cancel := context.WithTimeout(context.TODO(), timeout)
 	defer cancel()
 	slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout)
@@ -226,10 +218,6 @@ func (w *hookOnlyWaiter) Wait(_ ResourceList, _ time.Duration) error {
 	return nil
 }
 
-func (w *hookOnlyWaiter) WaitWithJobs(_ ResourceList, _ time.Duration) error {
-	return nil
-}
-
 func (w *hookOnlyWaiter) WaitForDelete(_ ResourceList, _ time.Duration) error {
 	return nil
 }

pkg/kube/statuswait_test.go

@@ -274,19 +274,11 @@ func TestStatusWait(t *testing.T) {
 		name         string
 		objManifests []string
 		expectErrs   []error
-		waitForJobs  bool
 	}{
 		{
 			name:         "Job is not complete",
 			objManifests: []string{jobNoStatusManifest},
 			expectErrs:   []error{errors.New("resource not ready, name: test, kind: Job, status: InProgress"), errors.New("context deadline exceeded")},
-			waitForJobs:  true,
-		},
-		{
-			name:         "Job is ready but not complete",
-			objManifests: []string{jobReadyManifest},
-			expectErrs:   nil,
-			waitForJobs:  false,
 		},
 		{
 			name:         "Pod is ready",
@@ -380,7 +372,7 @@ func TestWaitForJobComplete(t *testing.T) {
 				assert.NoError(t, err)
 			}
 			resourceList := getResourceListFromRuntimeObjs(t, c, objs)
-			err := statusWaiter.WaitWithJobs(resourceList, time.Second*3)
+			err := statusWaiter.Wait(resourceList, time.Second*3)
 			if tt.expectErrs != nil {
 				assert.EqualError(t, err, errors.Join(tt.expectErrs...).Error())
 				return

pkg/kube/wait.go

@@ -52,11 +52,6 @@ type legacyWaiter struct {
 }
 
 func (hw *legacyWaiter) Wait(resources ResourceList, timeout time.Duration) error {
-	hw.c = NewReadyChecker(hw.kubeClient, PausedAsReady(true))
-	return hw.waitForResources(resources, timeout)
-}
-
-func (hw *legacyWaiter) WaitWithJobs(resources ResourceList, timeout time.Duration) error {
 	hw.c = NewReadyChecker(hw.kubeClient, PausedAsReady(true), CheckJobs(true))
 	return hw.waitForResources(resources, timeout)
 }