Repository: roboll/helmfile
Pull request #1715 (2021-03-11 04:44:07)

Enable --wait-for-jobs flag introduced in helm3.5
Fixes https://github.com/roboll/helmfile/issues/1650, https://github.com/roboll/helmfile/issues/785

This PR adds:

- `--wait-for-jobs` CLI override option to helmfile binary
- `waitForJobs` helmDefaults parameter (default `false`)
- `waitForJobs` release parameter (default `false`)

`--wait-for-jobs` was introduced in Helm3.5 ( https://github.com/helm/helm/pull/8363 )


Patch:
diff --git a/README.md b/README.md
index cdbf9f1..20bc555 100644
--- a/README.md
+++ b/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
diff --git a/USERS.md b/USERS.md
index e6ad54c..6963279 100644
--- a/USERS.md
+++ b/USERS.md
@@ -28,4 +28,5 @@ information to this file.
 | [SettleMint](https://settlemint.com) | production | The SettleMint platform allows enterprises to spin up k8s clusters and deploy production grade blockchain networks and additional services. Helmfile is in charge of deploying these networks and services on demand out of the self service management ui. | Belgium, Singapore, UAE, India | October 2020 |
 | [AutoTrader (UK)](https://www.autotrader.co.uk) | production | We've used Helmfile for 2+ years to deploy 400+ services | UK | October 2020 |
 | [Trend Micro](https://www.trendmicro.com) | production | We manage 9 k8s clusters in a mono git repository with Helmfile | Taipei, Taiwan |  December 2019 |
+| [William Hill](https://www.williamhill.com) | production | We have standardised on Helmfile for managing both our application services and our Kubernetes platform components. | International |  March 2021 |
 <!-- TABLE_END -->
diff --git a/docs/writing-helmfile.md b/docs/writing-helmfile.md
index 3094f79..792be58 100644
--- a/docs/writing-helmfile.md
+++ b/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
   # ...
diff --git a/main.go b/main.go
index a978b89..2ea7c1f 100644
--- a/main.go
+++ b/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")
 }
diff --git a/pkg/app/app.go b/pkg/app/app.go
index b935b7d..8a78c9c 100644
--- a/pkg/app/app.go
+++ b/pkg/app/app.go
@@ -61,6 +61,8 @@ type HelmRelease struct {
 	Namespace string `json:"namespace"`
 	Enabled   bool   `json:"enabled"`
 	Labels    string `json:"labels"`
+	Chart     string `json:"chart"`
+	Version   string `json:"version"`
 }
 
 func New(conf ConfigProvider) *App {
@@ -292,9 +294,10 @@ func (a *App) Lint(c LintConfigProvider) error {
 func (a *App) Sync(c SyncConfigProvider) error {
 	return a.ForEachState(func(run *Run) (ok bool, errs []error) {
 		prepErr := run.withPreparedCharts("sync", state.ChartPrepareOptions{
-			SkipRepos: c.SkipDeps(),
-			SkipDeps:  c.SkipDeps(),
-			Wait:      c.Wait(),
+			SkipRepos:   c.SkipDeps(),
+			SkipDeps:    c.SkipDeps(),
+			Wait:        c.Wait(),
+			WaitForJobs: c.WaitForJobs(),
 		}, func() {
 			ok, errs = a.sync(run, c)
 		})
@@ -318,9 +321,10 @@ func (a *App) Apply(c ApplyConfigProvider) error {
 
 	err := a.ForEachState(func(run *Run) (ok bool, errs []error) {
 		prepErr := run.withPreparedCharts("apply", state.ChartPrepareOptions{
-			SkipRepos: c.SkipDeps(),
-			SkipDeps:  c.SkipDeps(),
-			Wait:      c.Wait(),
+			SkipRepos:   c.SkipDeps(),
+			SkipDeps:    c.SkipDeps(),
+			Wait:        c.Wait(),
+			WaitForJobs: c.WaitForJobs(),
 		}, func() {
 			matched, updated, es := a.apply(run, c)
 
@@ -509,6 +513,8 @@ func (a *App) ListReleases(c ListConfigProvider) error {
 					Namespace: r.Namespace,
 					Enabled:   installed,
 					Labels:    labels,
+					Chart:     r.Chart,
+					Version:   r.Version,
 				})
 			}
 		})
@@ -1027,7 +1033,7 @@ func (a *App) findDesiredStateFiles(specifiedPath string, opts LoadOpts) ([]stri
 		} else if defaultFile != "" {
 			return []string{defaultFile}, nil
 		} else {
-			return []string{}, fmt.Errorf("no state file found. It must be named %s/*.{yaml,yml}, %s, or %s, or otherwise specified with the --file flag", DefaultHelmfileDirectory, DefaultHelmfile, DeprecatedHelmfile)
+			return []string{}, fmt.Errorf("no state file found. It must be named %s/*.{yaml,yml} or %s, otherwise specified with the --file flag", DefaultHelmfileDirectory, DefaultHelmfile)
 		}
 	}
 
@@ -1182,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)
 			}))
@@ -1396,8 +1403,9 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) {
 			subst.Releases = rs
 
 			opts := &state.SyncOpts{
-				Set:  c.Set(),
-				Wait: c.Wait(),
+				Set:         c.Set(),
+				Wait:        c.Wait(),
+				WaitForJobs: c.WaitForJobs(),
 			}
 			return subst.SyncReleases(&affectedReleases, helm, c.Values(), c.Concurrency(), opts)
 		}))
diff --git a/pkg/app/config.go b/pkg/app/config.go
index 1e4d2ba..edfa461 100644
--- a/pkg/app/config.go
+++ b/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
diff --git a/pkg/app/formatters.go b/pkg/app/formatters.go
index c60c6c6..b285a1f 100644
--- a/pkg/app/formatters.go
+++ b/pkg/app/formatters.go
@@ -9,10 +9,10 @@ import (
 
 func FormatAsTable(releases []*HelmRelease) error {
 	table := uitable.New()
-	table.AddRow("NAME", "NAMESPACE", "ENABLED", "LABELS")
+	table.AddRow("NAME", "NAMESPACE", "ENABLED", "LABELS", "CHART", "VERSION")
 
 	for _, r := range releases {
-		table.AddRow(r.Name, r.Namespace, fmt.Sprintf("%t", r.Enabled), r.Labels)
+		table.AddRow(r.Name, r.Namespace, fmt.Sprintf("%t", r.Enabled), r.Labels, r.Chart, r.Version)
 	}
 
 	fmt.Println(table.String())
diff --git a/pkg/state/chart_dependency.go b/pkg/state/chart_dependency.go
index 95d3688..0c85be7 100644
--- a/pkg/state/chart_dependency.go
+++ b/pkg/state/chart_dependency.go
@@ -130,7 +130,7 @@ func (d *ResolvedDependencies) Get(chart, versionConstraint string) (string, err
 			}
 		}
 	}
-	return "", fmt.Errorf("no resolved dependency found for \"%s\"", chart)
+	return "", fmt.Errorf("no resolved dependency found for \"%s\", running \"helmfile deps\" may resolve the issue", chart)
 }
 
 func (st *HelmState) mergeLockedDependencies() (*HelmState, error) {
diff --git a/pkg/state/state.go b/pkg/state/state.go
index de690df..7ffe20c 100644
--- a/pkg/state/state.go
+++ b/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 {


Test patch:
diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go
index 110b9c2..b1c2e97 100644
--- a/pkg/app/app_test.go
+++ b/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
 }
@@ -4519,11 +4525,11 @@ releases:
 		assert.NilError(t, err)
 	})
 
-	expected := `NAME      	NAMESPACE	ENABLED	LABELS                    
-myrelease1	         	false  	common:label,id:myrelease1
-myrelease2	         	true   	common:label              
-myrelease3	         	true   	                          
-myrelease4	         	true   	id:myrelease1             
+	expected := `NAME      	NAMESPACE	ENABLED	LABELS                    	CHART   	VERSION
+myrelease1	         	false  	common:label,id:myrelease1	mychart1	       
+myrelease2	         	true   	common:label              	mychart1	       
+myrelease3	         	true   	                          	mychart1	       
+myrelease4	         	true   	id:myrelease1             	mychart1	       
 `
 	assert.Equal(t, expected, out)
 }
@@ -4576,7 +4582,7 @@ releases:
 		assert.NilError(t, err)
 	})
 
-	expected := `[{"name":"myrelease1","namespace":"","enabled":false,"labels":"id:myrelease1"},{"name":"myrelease2","namespace":"","enabled":true,"labels":""},{"name":"myrelease3","namespace":"","enabled":true,"labels":""},{"name":"myrelease4","namespace":"","enabled":true,"labels":"id:myrelease1"}]
+	expected := `[{"name":"myrelease1","namespace":"","enabled":false,"labels":"id:myrelease1","chart":"mychart1","version":""},{"name":"myrelease2","namespace":"","enabled":true,"labels":"","chart":"mychart1","version":""},{"name":"myrelease3","namespace":"","enabled":true,"labels":"","chart":"mychart1","version":""},{"name":"myrelease4","namespace":"","enabled":true,"labels":"id:myrelease1","chart":"mychart1","version":""}]
 `
 	assert.Equal(t, expected, out)
 }
diff --git a/pkg/helmexec/exec_test.go b/pkg/helmexec/exec_test.go
index 7de1811..13b7110 100644
--- a/pkg/helmexec/exec_test.go
+++ b/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)
-	expected = `exec: helm --kube-context dev diff release chart --timeout 10 --wait
+	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 --wait-for-jobs
 `
 	if buffer.String() != expected {
 		t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected)
diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go
index c46cceb..4e426fe 100644
--- a/pkg/state/state_test.go
+++ b/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{
diff --git a/pkg/state/temp_test.go b/pkg/state/temp_test.go
index 6fde6c3..c19f562 100644
--- a/pkg/state/temp_test.go
+++ b/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 {