Improve handling of releases being newly installed by helmfile-apply (#1618)

This improves helmfile-apply with two things:

- Some users had timing-out issues or annoyed by huge output from helm-diff run as part of helmfile-apply on first install. `--skip-diff-on-install` skips running helm-diff for releases being newly installed, so that you can avoid those issues.
- Some users had difficultly or found it not straight-forward to install CRDs and custom resources from separate charts in one helmfile-apply (#1353). The new helmfile.yaml release field `disableValidationOnInstall: true` adds `--disable-validation` to helm-diff only for releases being newly released, which should mostly resolve the issue.

Resolves #1353

main.go

@@ -397,6 +397,10 @@ func main() {
 					Name:  "skip-cleanup",
 					Usage: "Stop cleaning up temporary values generated by helmfile and helm-secrets. Useful for debugging. Don't use in production for security",
 				},
+				cli.BoolFlag{
+					Name:  "skip-diff-on-install",
+					Usage: "Skips running helm-diff on releases being newly installed on this apply. Useful when the release manifests are too huge to be reviewed, or it's too time-consuming to diff at all",
+				},
 				cli.BoolFlag{
 					Name:  "include-tests",
 					Usage: "enable the diffing of the helm test hooks",
@@ -738,6 +742,10 @@ func (c configImpl) SkipCleanup() bool {
 	return c.c.Bool("skip-cleanup")
 }
 
+func (c configImpl) SkipDiffOnInstall() bool {
+	return c.c.Bool("skip-diff-on-install")
+}
+
 func (c configImpl) EmbedValues() bool {
 	return c.c.Bool("embed-values")
 }

pkg/app/app.go

@@ -1088,6 +1088,7 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) {
 		Context:           c.Context(),
 		Set:               c.Set(),
 		SkipCleanup:       c.RetainValuesFiles() || c.SkipCleanup(),
+		SkipDiffOnInstall: c.SkipDiffOnInstall(),
 	}
 
 	infoMsg, releasesToBeUpdated, releasesToBeDeleted, errs := r.diff(false, detailedExitCode, c, diffOpts)

pkg/app/app_test.go

@@ -2313,6 +2313,7 @@ type applyConfig struct {
 	concurrency       int
 	detailedExitcode  bool
 	interactive       bool
+	skipDiffOnInstall bool
 	logger            *zap.SugaredLogger
 }
 
@@ -2380,6 +2381,10 @@ func (a applyConfig) RetainValuesFiles() bool {
 	return a.retainValuesFiles
 }
 
+func (a applyConfig) SkipDiffOnInstall() bool {
+	return a.skipDiffOnInstall
+}
+
 type depsConfig struct {
 	skipRepos bool
 }
@@ -2655,6 +2660,7 @@ func TestApply(t *testing.T) {
 		loc               string
 		ns                string
 		concurrency       int
+		skipDiffOnInstall bool
 		error             string
 		files             map[string]string
 		selectors         []string
@@ -3071,6 +3077,222 @@ baz    stable/mychart3
 bar    stable/mychart2          
 foo    stable/mychart1          
 
+`,
+		},
+		//
+		// install with upgrade
+		//
+		{
+			name: "install-with-upgrade-with-validation-control",
+			loc:  location(),
+			files: map[string]string{
+				"/path/to/helmfile.yaml": `
+releases:
+- name: baz
+  chart: stable/mychart3
+  disableValidationOnInstall: true
+- name: foo
+  chart: stable/mychart1
+  disableValidationOnInstall: true
+  needs:
+  - bar
+- name: bar
+  chart: stable/mychart2
+  disableValidation: true
+`,
+			},
+			diffs: map[exectest.DiffKey]error{
+				exectest.DiffKey{Name: "baz", Chart: "stable/mychart3", Flags: "--kube-contextdefault--detailed-exitcode"}:                     helmexec.ExitError{Code: 2},
+				exectest.DiffKey{Name: "foo", Chart: "stable/mychart1", Flags: "--disable-validation--kube-contextdefault--detailed-exitcode"}: helmexec.ExitError{Code: 2},
+				exectest.DiffKey{Name: "bar", Chart: "stable/mychart2", Flags: "--disable-validation--kube-contextdefault--detailed-exitcode"}: helmexec.ExitError{Code: 2},
+			},
+			lists: map[exectest.ListKey]string{
+				exectest.ListKey{Filter: "^foo$", Flags: "--kube-contextdefault--deployed--failed--pending"}: ``,
+				exectest.ListKey{Filter: "^bar$", Flags: "--kube-contextdefault--deployed--failed--pending"}: `NAME	REVISION	UPDATED                 	STATUS  	CHART        	APP VERSION	NAMESPACE
+bar 	4       	Fri Nov  1 08:40:07 2019	DEPLOYED	mychart2-3.1.0	3.1.0      	default
+`,
+				exectest.ListKey{Filter: "^baz$", Flags: "--kube-contextdefault--deployed--failed--pending"}: `NAME	REVISION	UPDATED                 	STATUS  	CHART        	APP VERSION	NAMESPACE
+baz 	4       	Fri Nov  1 08:40:07 2019	DEPLOYED	mychart3-3.1.0	3.1.0      	default
+`,
+			},
+			upgraded: []exectest.Release{
+				{Name: "baz", Flags: []string{"--kube-context", "default"}},
+				{Name: "bar", Flags: []string{"--kube-context", "default"}},
+				{Name: "foo", Flags: []string{"--kube-context", "default"}},
+			},
+			deleted:     []exectest.Release{},
+			concurrency: 1,
+			log: `processing file "helmfile.yaml" in directory "."
+first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode=<nil>
+first-pass uses: &{default map[] map[]}
+first-pass rendering output of "helmfile.yaml.part.0":
+ 0: 
+ 1: releases:
+ 2: - name: baz
+ 3:   chart: stable/mychart3
+ 4:   disableValidationOnInstall: true
+ 5: - name: foo
+ 6:   chart: stable/mychart1
+ 7:   disableValidationOnInstall: true
+ 8:   needs:
+ 9:   - bar
+10: - name: bar
+11:   chart: stable/mychart2
+12:   disableValidation: true
+13: 
+
+first-pass produced: &{default map[] map[]}
+first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]}
+vals:
+map[]
+defaultVals:[]
+second-pass rendering result of "helmfile.yaml.part.0":
+ 0: 
+ 1: releases:
+ 2: - name: baz
+ 3:   chart: stable/mychart3
+ 4:   disableValidationOnInstall: true
+ 5: - name: foo
+ 6:   chart: stable/mychart1
+ 7:   disableValidationOnInstall: true
+ 8:   needs:
+ 9:   - bar
+10: - name: bar
+11:   chart: stable/mychart2
+12:   disableValidation: true
+13: 
+
+merged environment: &{default map[] map[]}
+3 release(s) found in helmfile.yaml
+
+Affected releases are:
+  bar (stable/mychart2) UPDATED
+  baz (stable/mychart3) UPDATED
+  foo (stable/mychart1) UPDATED
+
+processing 2 groups of releases in this order:
+GROUP RELEASES
+1     baz, bar
+2     foo
+
+processing releases in group 1/2: baz, bar
+processing releases in group 2/2: foo
+getting deployed release version failed:Failed to get the version for:mychart1
+
+UPDATED RELEASES:
+NAME   CHART             VERSION
+baz    stable/mychart3     3.1.0
+bar    stable/mychart2     3.1.0
+foo    stable/mychart1          
+
+`,
+		},
+		//
+		// install with upgrade and --skip-diff-on-install
+		//
+		{
+			name:              "install-with-upgrade-with-skip-diff-on-install",
+			loc:               location(),
+			skipDiffOnInstall: true,
+			files: map[string]string{
+				"/path/to/helmfile.yaml": `
+releases:
+- name: baz
+  chart: stable/mychart3
+  disableValidationOnInstall: true
+- name: foo
+  chart: stable/mychart1
+  disableValidationOnInstall: true
+  needs:
+  - bar
+- name: bar
+  chart: stable/mychart2
+  disableValidation: true
+`,
+			},
+			diffs: map[exectest.DiffKey]error{
+				exectest.DiffKey{Name: "baz", Chart: "stable/mychart3", Flags: "--kube-contextdefault--detailed-exitcode"}:                     helmexec.ExitError{Code: 2},
+				exectest.DiffKey{Name: "bar", Chart: "stable/mychart2", Flags: "--disable-validation--kube-contextdefault--detailed-exitcode"}: helmexec.ExitError{Code: 2},
+			},
+			lists: map[exectest.ListKey]string{
+				exectest.ListKey{Filter: "^foo$", Flags: "--kube-contextdefault--deployed--failed--pending"}: ``,
+				exectest.ListKey{Filter: "^bar$", Flags: "--kube-contextdefault--deployed--failed--pending"}: `NAME	REVISION	UPDATED                 	STATUS  	CHART        	APP VERSION	NAMESPACE
+bar 	4       	Fri Nov  1 08:40:07 2019	DEPLOYED	mychart2-3.1.0	3.1.0      	default
+`,
+				exectest.ListKey{Filter: "^baz$", Flags: "--kube-contextdefault--deployed--failed--pending"}: `NAME	REVISION	UPDATED                 	STATUS  	CHART        	APP VERSION	NAMESPACE
+baz 	4       	Fri Nov  1 08:40:07 2019	DEPLOYED	mychart3-3.1.0	3.1.0      	default
+`,
+			},
+			upgraded: []exectest.Release{
+				{Name: "baz", Flags: []string{"--kube-context", "default"}},
+				{Name: "bar", Flags: []string{"--kube-context", "default"}},
+				{Name: "foo", Flags: []string{"--kube-context", "default"}},
+			},
+			deleted:     []exectest.Release{},
+			concurrency: 1,
+			log: `processing file "helmfile.yaml" in directory "."
+first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode=<nil>
+first-pass uses: &{default map[] map[]}
+first-pass rendering output of "helmfile.yaml.part.0":
+ 0: 
+ 1: releases:
+ 2: - name: baz
+ 3:   chart: stable/mychart3
+ 4:   disableValidationOnInstall: true
+ 5: - name: foo
+ 6:   chart: stable/mychart1
+ 7:   disableValidationOnInstall: true
+ 8:   needs:
+ 9:   - bar
+10: - name: bar
+11:   chart: stable/mychart2
+12:   disableValidation: true
+13: 
+
+first-pass produced: &{default map[] map[]}
+first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]}
+vals:
+map[]
+defaultVals:[]
+second-pass rendering result of "helmfile.yaml.part.0":
+ 0: 
+ 1: releases:
+ 2: - name: baz
+ 3:   chart: stable/mychart3
+ 4:   disableValidationOnInstall: true
+ 5: - name: foo
+ 6:   chart: stable/mychart1
+ 7:   disableValidationOnInstall: true
+ 8:   needs:
+ 9:   - bar
+10: - name: bar
+11:   chart: stable/mychart2
+12:   disableValidation: true
+13: 
+
+merged environment: &{default map[] map[]}
+3 release(s) found in helmfile.yaml
+
+Affected releases are:
+  bar (stable/mychart2) UPDATED
+  baz (stable/mychart3) UPDATED
+  foo (stable/mychart1) UPDATED
+
+processing 2 groups of releases in this order:
+GROUP RELEASES
+1     baz, bar
+2     foo
+
+processing releases in group 1/2: baz, bar
+processing releases in group 2/2: foo
+getting deployed release version failed:Failed to get the version for:mychart1
+
+UPDATED RELEASES:
+NAME   CHART             VERSION
+baz    stable/mychart3     3.1.0
+bar    stable/mychart2     3.1.0
+foo    stable/mychart1          
+
 `,
 		},
 		//
@@ -3921,6 +4143,7 @@ err: "foo" depends on nonexistent release "bar"
 					// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
 					concurrency:       tc.concurrency,
 					logger:            logger,
+					skipDiffOnInstall: tc.skipDiffOnInstall,
 				})
 				if tc.error == "" && applyErr != nil {
 					t.Fatalf("unexpected error for data defined at %s: %v", tc.loc, applyErr)

pkg/app/config.go

@@ -52,6 +52,7 @@ type ApplyConfigProvider interface {
 
 	RetainValuesFiles() bool
 	SkipCleanup() bool
+	SkipDiffOnInstall() bool
 
 	concurrencyConfig
 	interactive

pkg/state/state.go

@@ -211,6 +211,10 @@ type ReleaseSpec struct {
 	// FYI, such diff without `--disable-validation` fails on first install because the K8s cluster doesn't have CRDs registered yet.
 	DisableValidation *bool `yaml:"disableValidation,omitempty"`
 
+	// DisableValidationOnInstall disables the K8s API validation while running helm-diff on the release being newly installed on helmfile-apply.
+	// It is useful when any release contains custom resources for CRDs that is not yet installed onto the cluster.
+	DisableValidationOnInstall *bool `yaml:"disableValidationOnInstall,omitempty"`
+
 	// MissingFileHandler is set to either "Error" or "Warn". "Error" instructs helmfile to fail when unable to find a values or secrets file. When "Warn", it prints the file and continues.
 	// The default value for MissingFileHandler is "Error".
 	MissingFileHandler *string `yaml:"missingFileHandler,omitempty"`
@@ -1422,6 +1426,7 @@ type diffPrepareResult struct {
 	flags                   []string
 	errors                  []*ReleaseError
 	files                   []string
+	upgradeDueToSkippedDiff bool
 }
 
 func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValues []string, concurrency int, detailedExitCode, includeTests, suppressSecrets bool, opt ...DiffOpt) ([]diffPrepareResult, []error) {
@@ -1430,6 +1435,29 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 		o.Apply(opts)
 	}
 
+	mu := &sync.Mutex{}
+	installedReleases := map[string]bool{}
+
+	isInstalled := func(r *ReleaseSpec) bool {
+		mu.Lock()
+		defer mu.Unlock()
+
+		id := ReleaseToID(r)
+
+		if v, ok := installedReleases[id]; ok {
+			return v
+		}
+
+		v, err := st.isReleaseInstalled(st.createHelmContext(r, 0), helm, *r)
+		if err != nil {
+			st.logger.Warnf("confirming if the release is already installed or not: %v", err)
+		} else {
+			installedReleases[id] = v
+		}
+
+		return v
+	}
+
 	releases := []*ReleaseSpec{}
 	for i, _ := range st.Releases {
 		if !st.Releases[i].Desired() {
@@ -1465,10 +1493,17 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 
 				st.ApplyOverrides(release)
 
+				if opts.SkipDiffOnInstall && !isInstalled(release) {
+					results <- diffPrepareResult{release: release, upgradeDueToSkippedDiff: true}
+					continue
+				}
+
+				disableValidation := release.DisableValidationOnInstall != nil && *release.DisableValidationOnInstall && !isInstalled(release)
+
 				// TODO We need a long-term fix for this :)
 				// See https://github.com/roboll/helmfile/issues/737
 				mut.Lock()
-				flags, files, err := st.flagsForDiff(helm, release, workerIndex)
+				flags, files, err := st.flagsForDiff(helm, release, disableValidation, workerIndex)
 				mut.Unlock()
 				if err != nil {
 					errs = append(errs, err)
@@ -1571,6 +1606,8 @@ type DiffOpts struct {
 	Set     []string
 
 	SkipCleanup bool
+
+	SkipDiffOnInstall bool
 }
 
 func (o *DiffOpts) Apply(opts *DiffOpts) {
@@ -1609,6 +1646,9 @@ func (st *HelmState) DiffReleases(helm helmexec.Interface, additionalValues []st
 	rs := []ReleaseSpec{}
 	errs := []error{}
 
+	// The exit code returned by helm-diff when it detected any changes
+	HelmDiffExitCodeChanged := 2
+
 	st.scatterGather(
 		workerLimit,
 		len(preps),
@@ -1622,7 +1662,9 @@ func (st *HelmState) DiffReleases(helm helmexec.Interface, additionalValues []st
 			for prep := range jobQueue {
 				flags := prep.flags
 				release := prep.release
-				if err := helm.DiffRelease(st.createHelmContext(release, workerIndex), release.Name, normalizeChart(st.basePath, release.Chart), suppressDiff, flags...); err != nil {
+				if prep.upgradeDueToSkippedDiff {
+					results <- diffResult{&ReleaseError{ReleaseSpec: release, err: nil, Code: HelmDiffExitCodeChanged}}
+				} else if err := helm.DiffRelease(st.createHelmContext(release, workerIndex), release.Name, normalizeChart(st.basePath, release.Chart), suppressDiff, flags...); err != nil {
 					switch e := err.(type) {
 					case helmexec.ExitError:
 						// Propagate any non-zero exit status from the external command like `helm` that is failed under the hood
@@ -1647,7 +1689,7 @@ func (st *HelmState) DiffReleases(helm helmexec.Interface, additionalValues []st
 				res := <-results
 				if res.err != nil {
 					errs = append(errs, res.err)
-					if res.err.Code == 2 {
+					if res.err.Code == HelmDiffExitCodeChanged {
 						rs = append(rs, *res.err.ReleaseSpec)
 					}
 				}
@@ -2151,7 +2193,7 @@ func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseS
 	return append(flags, common...), files, nil
 }
 
-func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, []string, error) {
+func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec, disableValidation bool, workerIndex int) ([]string, []string, error) {
 	flags := st.chartVersionFlags(release)
 
 	disableOpenAPIValidation := false
@@ -2165,7 +2207,6 @@ func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec,
 		flags = append(flags, "--disable-openapi-validation")
 	}
 
-	disableValidation := false
 	if release.DisableValidation != nil {
 		disableValidation = *release.DisableValidation
 	} else if st.HelmDefaults.DisableValidation != nil {