Enable --wait-for-jobs flag introduced in helm 3.5 (#1715)

Fixes #1650
Fixes #785

This change introduces:

- `--wait-for-jobs` CLI override option to helmfile binary
- `waitForJobs` helmDefaults parameter (default `false`)
- `waitForJobs` release parameter (default `false`)

Note that `--wait-for-jobs` was introduced in Helm 3.5 (https://github.com/helm/helm/pull/8363)

README.md

@@ -88,6 +88,8 @@ helmDefaults:
   verify: true
   # wait for k8s resources via --wait. (default false)
   wait: true
+  # 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 (default false, Implemented in Helm3.5)
+  waitForJobs: true
   # time in seconds to wait for any individual Kubernetes operation (like Jobs for hooks, and waits on pod/pvc/svc/deployment readiness) (default 300)
   timeout: 600
   # performs pods restart for the resource if applicable (default false)
@@ -169,9 +171,10 @@ releases:
     # will attempt to decrypt it using helm-secrets plugin
     secrets:
       - vault_secret.yaml
-    # Override helmDefaults options for verify, wait, timeout, recreatePods and force.
+    # Override helmDefaults options for verify, wait, waitForJobs, timeout, recreatePods and force.
     verify: true
     wait: true
+    waitForJobs: true
     timeout: 60
     recreatePods: true
     force: false

docs/writing-helmfile.md

@@ -91,7 +91,7 @@ releases:
 
 Release Templating supports the following parts of release definition:
 - basic fields: `name`, `namespace`, `chart`, `version`
-- boolean fields: `installed`, `wait`, `tillerless`, `verify` by the means of additional text
+- boolean fields: `installed`, `wait`, `waitForJobs`, `tillerless`, `verify` by the means of additional text
   fields designed for templating only: `installedTemplate`, `waitTemplate`, `tillerlessTemplate`, `verifyTemplate`
   ```yaml
   # ...

main.go

@@ -357,6 +357,10 @@ func main() {
 					Name:  "wait",
 					Usage: `Override helmDefaults.wait setting "helm upgrade --install --wait"`,
 				},
+				cli.BoolFlag{
+					Name:  "wait-for-jobs",
+					Usage: `Override helmDefaults.waitForJobs setting "helm upgrade --install --wait-for-jobs"`,
+				},
 			},
 			Action: action(func(run *app.App, c configImpl) error {
 				return run.Sync(c)
@@ -425,6 +429,10 @@ func main() {
 					Name:  "wait",
 					Usage: `Override helmDefaults.wait setting "helm upgrade --install --wait"`,
 				},
+				cli.BoolFlag{
+					Name:  "wait-for-jobs",
+					Usage: `Override helmDefaults.waitForJobs setting "helm upgrade --install --wait-for-jobs"`,
+				},
 			},
 			Action: action(func(run *app.App, c configImpl) error {
 				return run.Apply(c)
@@ -615,6 +623,10 @@ func (c configImpl) Wait() bool {
 	return c.c.Bool("wait")
 }
 
+func (c configImpl) WaitForJobs() bool {
+	return c.c.Bool("wait-for-jobs")
+}
+
 func (c configImpl) Values() []string {
 	return c.c.StringSlice("values")
 }

pkg/app/app.go

@@ -297,6 +297,7 @@ func (a *App) Sync(c SyncConfigProvider) error {
 			SkipRepos:   c.SkipDeps(),
 			SkipDeps:    c.SkipDeps(),
 			Wait:        c.Wait(),
+			WaitForJobs: c.WaitForJobs(),
 		}, func() {
 			ok, errs = a.sync(run, c)
 		})
@@ -323,6 +324,7 @@ func (a *App) Apply(c ApplyConfigProvider) error {
 			SkipRepos:   c.SkipDeps(),
 			SkipDeps:    c.SkipDeps(),
 			Wait:        c.Wait(),
+			WaitForJobs: c.WaitForJobs(),
 		}, func() {
 			matched, updated, es := a.apply(run, c)
 
@@ -1186,6 +1188,7 @@ Do you really want to apply?
 					Set:         c.Set(),
 					SkipCleanup: c.RetainValuesFiles() || c.SkipCleanup(),
 					Wait:        c.Wait(),
+					WaitForJobs: c.WaitForJobs(),
 				}
 				return subst.SyncReleases(&affectedReleases, helm, c.Values(), c.Concurrency(), &syncOpts)
 			}))
@@ -1402,6 +1405,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) {
 			opts := &state.SyncOpts{
 				Set:         c.Set(),
 				Wait:        c.Wait(),
+				WaitForJobs: c.WaitForJobs(),
 			}
 			return subst.SyncReleases(&affectedReleases, helm, c.Values(), c.Concurrency(), opts)
 		}))

pkg/app/app_test.go

@@ -215,6 +215,7 @@ func TestVisitDesiredStatesWithReleasesFiltered_Issue1008_MissingNonDefaultEnvIn
 		"/path/to/base.yaml": `
 helmDefaults:
   wait: true
+  waitForJobs: true
 `,
 		"/path/to/helmfile.yaml": `
 bases:
@@ -2316,6 +2317,7 @@ type applyConfig struct {
 	skipDiffOnInstall bool
 	logger            *zap.SugaredLogger
 	wait              bool
+	waitForJobs       bool
 }
 
 func (a applyConfig) Args() string {
@@ -2326,6 +2328,10 @@ func (a applyConfig) Wait() bool {
 	return a.wait
 }
 
+func (a applyConfig) WaitForJobs() bool {
+	return a.waitForJobs
+}
+
 func (a applyConfig) Values() []string {
 	return a.values
 }

pkg/app/config.go

@@ -40,6 +40,7 @@ type ApplyConfigProvider interface {
 	Set() []string
 	SkipDeps() bool
 	Wait() bool
+	WaitForJobs() bool
 
 	IncludeTests() bool
 
@@ -67,6 +68,7 @@ type SyncConfigProvider interface {
 	Set() []string
 	SkipDeps() bool
 	Wait() bool
+	WaitForJobs() bool
 
 	concurrencyConfig
 	loggingConfig

pkg/helmexec/exec_test.go

@@ -181,9 +181,9 @@ func Test_SyncRelease(t *testing.T) {
 	var buffer bytes.Buffer
 	logger := NewLogger(&buffer, "debug")
 	helm := MockExecer(logger, "dev")
-	helm.SyncRelease(HelmContext{}, "release", "chart", "--timeout 10", "--wait")
+	helm.SyncRelease(HelmContext{}, "release", "chart", "--timeout 10", "--wait", "--wait-for-jobs")
 	expected := `Upgrading release=release, chart=chart
-exec: helm --kube-context dev upgrade --install --reset-values release chart --timeout 10 --wait
+exec: helm --kube-context dev upgrade --install --reset-values release chart --timeout 10 --wait --wait-for-jobs
 `
 	if buffer.String() != expected {
 		t.Errorf("helmexec.SyncRelease()\nactual = %v\nexpect = %v", buffer.String(), expected)
@@ -204,9 +204,9 @@ func Test_SyncReleaseTillerless(t *testing.T) {
 	logger := NewLogger(&buffer, "debug")
 	helm := MockExecer(logger, "dev")
 	helm.SyncRelease(HelmContext{Tillerless: true, TillerNamespace: "foo"}, "release", "chart",
-		"--timeout 10", "--wait")
+		"--timeout 10", "--wait", "--wait-for-jobs")
 	expected := `Upgrading release=release, chart=chart
-exec: helm --kube-context dev tiller run foo -- helm upgrade --install --reset-values release chart --timeout 10 --wait
+exec: helm --kube-context dev tiller run foo -- helm upgrade --install --reset-values release chart --timeout 10 --wait --wait-for-jobs
 `
 	if buffer.String() != expected {
 		t.Errorf("helmexec.SyncRelease()\nactual = %v\nexpect = %v", buffer.String(), expected)
@@ -293,9 +293,9 @@ func Test_DiffRelease(t *testing.T) {
 	var buffer bytes.Buffer
 	logger := NewLogger(&buffer, "debug")
 	helm := MockExecer(logger, "dev")
-	helm.DiffRelease(HelmContext{}, "release", "chart", false, "--timeout 10", "--wait")
+	helm.DiffRelease(HelmContext{}, "release", "chart", false, "--timeout 10", "--wait", "--wait-for-jobs")
 	expected := `Comparing release=release, chart=chart
-exec: helm --kube-context dev diff upgrade --reset-values --allow-unreleased release chart --timeout 10 --wait
+exec: helm --kube-context dev diff upgrade --reset-values --allow-unreleased release chart --timeout 10 --wait --wait-for-jobs
 `
 	if buffer.String() != expected {
 		t.Errorf("helmexec.DiffRelease()\nactual = %v\nexpect = %v", buffer.String(), expected)
@@ -315,9 +315,9 @@ func Test_DiffReleaseTillerless(t *testing.T) {
 	var buffer bytes.Buffer
 	logger := NewLogger(&buffer, "debug")
 	helm := MockExecer(logger, "dev")
-	helm.DiffRelease(HelmContext{Tillerless: true}, "release", "chart", false, "--timeout 10", "--wait")
+	helm.DiffRelease(HelmContext{Tillerless: true}, "release", "chart", false, "--timeout 10", "--wait", "--wait-for-jobs")
 	expected := `Comparing release=release, chart=chart
-exec: helm --kube-context dev tiller run -- helm diff upgrade --reset-values --allow-unreleased release chart --timeout 10 --wait
+exec: helm --kube-context dev tiller run -- helm diff upgrade --reset-values --allow-unreleased release chart --timeout 10 --wait --wait-for-jobs
 `
 	if buffer.String() != expected {
 		t.Errorf("helmexec.DiffRelease()\nactual = %v\nexpect = %v", buffer.String(), expected)
@@ -407,8 +407,8 @@ func Test_exec(t *testing.T) {
 
 	buffer.Reset()
 	helm = MockExecer(logger, "dev")
-	helm.exec([]string{"diff", "release", "chart", "--timeout 10", "--wait"}, env)
+	helm.exec([]string{"diff", "release", "chart", "--timeout 10", "--wait", "--wait-for-jobs"}, env)
-	expected = `exec: helm --kube-context dev diff release chart --timeout 10 --wait
+	expected = `exec: helm --kube-context dev diff release chart --timeout 10 --wait --wait-for-jobs
 `
 	if buffer.String() != expected {
 		t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected)

pkg/state/state.go

@@ -130,6 +130,8 @@ type HelmSpec struct {
 	Devel bool `yaml:"devel"`
 	// Wait, if set to true, will wait until all Pods, PVCs, Services, and minimum number of Pods of a Deployment are in a ready state before marking the release as successful
 	Wait bool `yaml:"wait"`
+	// WaitForJobs, 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
+	WaitForJobs bool `yaml:"waitForJobs"`
 	// Timeout is the time in seconds to wait for any individual Kubernetes operation (like Jobs for hooks, and waits on pod/pvc/svc/deployment readiness) (default 300)
 	Timeout int `yaml:"timeout"`
 	// RecreatePods, when set to true, instruct helmfile to perform pods restart for the resource if applicable
@@ -186,6 +188,8 @@ type ReleaseSpec struct {
 	Devel *bool `yaml:"devel,omitempty"`
 	// Wait, if set to true, will wait until all Pods, PVCs, Services, and minimum number of Pods of a Deployment are in a ready state before marking the release as successful
 	Wait *bool `yaml:"wait,omitempty"`
+	// WaitForJobs, 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
+	WaitForJobs *bool `yaml:"waitForJobs,omitempty"`
 	// Timeout is the time in seconds to wait for any individual Kubernetes operation (like Jobs for hooks, and waits on pod/pvc/svc/deployment readiness) (default 300)
 	Timeout *int `yaml:"timeout,omitempty"`
 	// RecreatePods, when set to true, instruct helmfile to perform pods restart for the resource if applicable
@@ -512,6 +516,10 @@ func (st *HelmState) prepareSyncReleases(helm helmexec.Interface, additionalValu
 					flags = append(flags, "--wait")
 				}
 
+				if opts.WaitForJobs {
+					flags = append(flags, "--wait-for-jobs")
+				}
+
 				if len(errs) > 0 {
 					results <- syncPrepareResult{errors: errs, files: files}
 					continue
@@ -587,6 +595,7 @@ type SyncOpts struct {
 	Set         []string
 	SkipCleanup bool
 	Wait        bool
+	WaitForJobs bool
 }
 
 type SyncOpt interface{ Apply(*SyncOpts) }
@@ -886,6 +895,7 @@ type ChartPrepareOptions struct {
 	SkipDeps      bool
 	SkipResolve   bool
 	Wait          bool
+	WaitForJobs   bool
 }
 
 type chartPrepareResult struct {
@@ -2243,6 +2253,10 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp
 		flags = append(flags, "--wait")
 	}
 
+	if release.WaitForJobs != nil && *release.WaitForJobs || release.WaitForJobs == nil && st.HelmDefaults.WaitForJobs {
+		flags = append(flags, "--wait-for-jobs")
+	}
+
 	flags = append(flags, st.timeoutFlags(helm, release)...)
 
 	if release.Force != nil && *release.Force || release.Force == nil && st.HelmDefaults.Force {

pkg/state/state_test.go

@@ -313,6 +313,24 @@ func TestHelmState_flagsForUpgrade(t *testing.T) {
 				"--namespace", "test-namespace",
 			},
 		},
+		{
+			name: "wait-for-jobs",
+			defaults: HelmSpec{
+				WaitForJobs: false,
+			},
+			release: &ReleaseSpec{
+				Chart:       "test/chart",
+				Version:     "0.1",
+				WaitForJobs: &enable,
+				Name:        "test-charts",
+				Namespace:   "test-namespace",
+			},
+			want: []string{
+				"--version", "0.1",
+				"--wait-for-jobs",
+				"--namespace", "test-namespace",
+			},
+		},
 		{
 			name: "devel",
 			defaults: HelmSpec{

pkg/state/temp_test.go

@@ -37,39 +37,39 @@ func TestGenerateID(t *testing.T) {
 	run(testcase{
 		subject: "baseline",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
-		want:    "foo-values-67b55dc69b",
+		want:    "foo-values-779795898b",
 	})
 
 	run(testcase{
 		subject: "different bytes content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    []byte(`{"k":"v"}`),
-		want:    "foo-values-5988bf4947",
+		want:    "foo-values-6b7ffbccc",
 	})
 
 	run(testcase{
 		subject: "different map content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    map[string]interface{}{"k": "v"},
-		want:    "foo-values-5d6fb4db97",
+		want:    "foo-values-6d57bbfccf",
 	})
 
 	run(testcase{
 		subject: "different chart",
 		release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"},
-		want:    "foo-values-58db655b79",
+		want:    "foo-values-5c45b58947",
 	})
 
 	run(testcase{
 		subject: "different name",
 		release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"},
-		want:    "bar-values-797d6df4dc",
+		want:    "bar-values-99d5fdbcc",
 	})
 
 	run(testcase{
 		subject: "specific ns",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"},
-		want:    "myns-foo-values-5f867c6d49",
+		want:    "myns-foo-values-544cb97d7f",
 	})
 
 	for id, n := range ids {