Repository: roboll/helmfile
Pull request #1236 (2020-04-30 23:56:13)

feat: Add global `--debug` flag for passing `--debug` to helm
Added a global flag `—debug`  that sets the logging level to debug and passes the global flag `—debug` to Helm through subcommands’ `—args`. See: https://github.com/roboll/helmfile/issues/1104

- Helm accepts the redundancy of the global flag `--debug`
- argparser.GetArgs gets rid of the redundancy which seems to be used to parse all —args
- what do you think about adding an Args field to `state.ReleaseSpec` ?
- what about adding a `Debug` (as we do for `—atomic` and `—wait` e.g.) into `state.HelmSpec` `state.ReleaseSpec` for more granularity ?
- sth still prevents the debug output from being displayed into stdout
- tests



Patch:
diff --git a/Dockerfile.helm3 b/Dockerfile.helm3
index 7e0167a..2cef439 100644
--- a/Dockerfile.helm3
+++ b/Dockerfile.helm3
@@ -11,10 +11,10 @@ FROM alpine:3.11
 
 RUN apk add --no-cache ca-certificates git bash curl jq
 
-ARG HELM_VERSION=v3.2.0
+ARG HELM_VERSION=v3.2.1
 ARG HELM_LOCATION="https://get.helm.sh"
 ARG HELM_FILENAME="helm-${HELM_VERSION}-linux-amd64.tar.gz"
-ARG HELM_SHA256="4c3fd562e64005786ac8f18e7334054a24da34ec04bbd769c206b03b8ed6e457"
+ARG HELM_SHA256="018f9908cb950701a5d59e757653a790c66d8eda288625dbb185354ca6f41f6b"
 RUN wget ${HELM_LOCATION}/${HELM_FILENAME} && \
     echo Verifying ${HELM_FILENAME}... && \
     sha256sum ${HELM_FILENAME} | grep -q "${HELM_SHA256}" && \
diff --git a/go.mod b/go.mod
index e93e0cd..3e34e9b 100644
--- a/go.mod
+++ b/go.mod
@@ -24,7 +24,7 @@ require (
 	github.com/tatsushid/go-prettytable v0.0.0-20141013043238-ed2d14c29939
 	github.com/urfave/cli v1.20.0
 	github.com/variantdev/dag v0.0.0-20191028002400-bb0b3c785363
-	github.com/variantdev/vals v0.4.0
+	github.com/variantdev/vals v0.4.1-0.20200501114609-9cebe482281c
 	go.mozilla.org/sops v0.0.0-20190912205235-14a22d7a7060 // indirect
 	go.uber.org/multierr v1.1.0
 	go.uber.org/zap v1.9.1
diff --git a/go.sum b/go.sum
index adeea41..2800b54 100644
--- a/go.sum
+++ b/go.sum
@@ -778,6 +778,8 @@ github.com/variantdev/dag v0.0.0-20191028002400-bb0b3c785363 h1:KrfQBEUn+wEOQ/6U
 github.com/variantdev/dag v0.0.0-20191028002400-bb0b3c785363/go.mod h1:pH1TQsNSLj2uxMo9NNl9zdGy01Wtn+/2MT96BrKmVyE=
 github.com/variantdev/vals v0.4.0 h1:O1O7/sWhlvozcY2DjZBzlE1notxwVo6UBT1+w7HsO/k=
 github.com/variantdev/vals v0.4.0/go.mod h1:28iJwAamsfHevGLXO7QKmVWmwa79xM06sRjw8eGY73U=
+github.com/variantdev/vals v0.4.1-0.20200501114609-9cebe482281c h1:ODJGhYOTMQJ1s+aeVzFsoJc/2MnseLhMdgB+/mykXe0=
+github.com/variantdev/vals v0.4.1-0.20200501114609-9cebe482281c/go.mod h1:IfLBErvsyYBQU0Jobuzfmft46K0MG0sLO6xz9o9KZdY=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
 github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 go.etcd.io/bbolt v1.3.2 h1:Z/90sZLPOeCy2PwprqkFa25PdkusRzaj9P8zm/KNyvk=
diff --git a/main.go b/main.go
index 8fa37a5..2f14ce3 100644
--- a/main.go
+++ b/main.go
@@ -13,7 +13,6 @@ import (
 	"github.com/roboll/helmfile/pkg/state"
 	"github.com/urfave/cli"
 	"go.uber.org/zap"
-	"go.uber.org/zap/zapcore"
 )
 
 var logger *zap.SugaredLogger
@@ -22,14 +21,11 @@ func configureLogging(c *cli.Context) error {
 	// Valid levels:
 	// https://github.com/uber-go/zap/blob/7e7e266a8dbce911a49554b945538c5b950196b8/zapcore/level.go#L126
 	logLevel := c.GlobalString("log-level")
-	if c.GlobalBool("quiet") {
+	if c.GlobalBool("debug") {
+		logLevel = "debug"
+	} else if c.GlobalBool("quiet") {
 		logLevel = "warn"
 	}
-	var level zapcore.Level
-	err := level.Set(logLevel)
-	if err != nil {
-		return err
-	}
 	logger = helmexec.NewLogger(os.Stderr, logLevel)
 	if c.App.Metadata == nil {
 		// Auto-initialised in 1.19.0
@@ -76,6 +72,10 @@ func main() {
 			Name:  "kube-context",
 			Usage: "Set kubectl context. Uses current context by default",
 		},
+		cli.BoolFlag{
+			Name:  "debug",
+			Usage: "Enable verbose output for Helm and set log-level to debug, this disables --quiet/-q effect",
+		},
 		cli.BoolFlag{
 			Name:  "no-color",
 			Usage: "Output without color",
@@ -539,7 +539,13 @@ func (c configImpl) Values() []string {
 }
 
 func (c configImpl) Args() string {
-	return c.c.String("args")
+	args := c.c.String("args")
+	enableHelmDebug := c.c.GlobalBool("debug")
+
+	if enableHelmDebug {
+		args = fmt.Sprintf("%s %s", args, "--debug")
+	}
+	return args
 }
 
 func (c configImpl) OutputDir() string {
diff --git a/pkg/app/app.go b/pkg/app/app.go
index c23ef3e..42064dd 100644
--- a/pkg/app/app.go
+++ b/pkg/app/app.go
@@ -475,34 +475,44 @@ func (a *App) visitStates(fileOrDir string, defOpts LoadOpts, converge func(*sta
 		}
 		st.Selectors = opts.Selectors
 
-		if len(st.Helmfiles) > 0 {
-			noMatchInSubHelmfiles := true
-			for i, m := range st.Helmfiles {
-				optsForNestedState := LoadOpts{
-					CalleePath:        filepath.Join(d, f),
-					Environment:       m.Environment,
-					Reverse:           defOpts.Reverse,
-					RetainValuesFiles: defOpts.RetainValuesFiles,
-				}
-				//assign parent selector to sub helm selector in legacy mode or do not inherit in experimental mode
-				if (m.Selectors == nil && !isExplicitSelectorInheritanceEnabled()) || m.SelectorsInherited {
-					optsForNestedState.Selectors = opts.Selectors
-				} else {
-					optsForNestedState.Selectors = m.Selectors
-				}
+		visitSubHelmfiles := func() error {
+			if len(st.Helmfiles) > 0 {
+				noMatchInSubHelmfiles := true
+				for i, m := range st.Helmfiles {
+					optsForNestedState := LoadOpts{
+						CalleePath:        filepath.Join(d, f),
+						Environment:       m.Environment,
+						Reverse:           defOpts.Reverse,
+						RetainValuesFiles: defOpts.RetainValuesFiles,
+					}
+					//assign parent selector to sub helm selector in legacy mode or do not inherit in experimental mode
+					if (m.Selectors == nil && !isExplicitSelectorInheritanceEnabled()) || m.SelectorsInherited {
+						optsForNestedState.Selectors = opts.Selectors
+					} else {
+						optsForNestedState.Selectors = m.Selectors
+					}
 
-				if err := a.visitStates(m.Path, optsForNestedState, converge); err != nil {
-					switch err.(type) {
-					case *NoMatchingHelmfileError:
+					if err := a.visitStates(m.Path, optsForNestedState, converge); err != nil {
+						switch err.(type) {
+						case *NoMatchingHelmfileError:
 
-					default:
-						return appError(fmt.Sprintf("in .helmfiles[%d]", i), err)
+						default:
+							return appError(fmt.Sprintf("in .helmfiles[%d]", i), err)
+						}
+					} else {
+						noMatchInSubHelmfiles = false
 					}
-				} else {
-					noMatchInSubHelmfiles = false
 				}
+				noMatchInHelmfiles = noMatchInHelmfiles && noMatchInSubHelmfiles
+			}
+			return nil
+		}
+
+		if !opts.Reverse {
+			err = visitSubHelmfiles()
+			if err != nil {
+				return err
 			}
-			noMatchInHelmfiles = noMatchInHelmfiles && noMatchInSubHelmfiles
 		}
 
 		templated, tmplErr := st.ExecuteTemplates()
@@ -513,6 +523,13 @@ func (a *App) visitStates(fileOrDir string, defOpts LoadOpts, converge func(*sta
 		processed, errs := converge(templated)
 		noMatchInHelmfiles = noMatchInHelmfiles && !processed
 
+		if opts.Reverse {
+			err = visitSubHelmfiles()
+			if err != nil {
+				return err
+			}
+		}
+
 		return context{app: a, st: templated, retainValues: defOpts.RetainValuesFiles}.clean(errs)
 	})
 
@@ -682,7 +699,7 @@ func (a *App) visitStatesWithSelectorsAndRemoteSupport(fileOrDir string, converg
 	return a.visitStates(fileOrDir, opts, converge)
 }
 
-func processFilteredReleases(st *state.HelmState, converge func(st *state.HelmState) []error) (bool, []error) {
+func processFilteredReleases(st *state.HelmState, helm helmexec.Interface, converge func(st *state.HelmState) []error) (bool, []error) {
 	if len(st.Selectors) > 0 {
 		err := st.FilterReleases()
 		if err != nil {
@@ -696,11 +713,15 @@ func processFilteredReleases(st *state.HelmState, converge func(st *state.HelmSt
 
 	releaseNameCounts := map[Key]int{}
 	for _, r := range st.Releases {
-		tillerNamespace := st.HelmDefaults.TillerNamespace
-		if r.TillerNamespace != "" {
-			tillerNamespace = r.TillerNamespace
+		namespace := r.Namespace
+		if !helm.IsHelm3() {
+			if r.TillerNamespace != "" {
+				namespace = r.TillerNamespace
+			} else {
+				namespace = st.HelmDefaults.TillerNamespace
+			}
 		}
-		releaseNameCounts[Key{tillerNamespace, r.Name}]++
+		releaseNameCounts[Key{namespace, r.Name}]++
 	}
 	for name, c := range releaseNameCounts {
 		if c > 1 {
@@ -717,7 +738,7 @@ func processFilteredReleases(st *state.HelmState, converge func(st *state.HelmSt
 
 func (a *App) Wrap(converge func(*state.HelmState, helmexec.Interface) []error) func(st *state.HelmState, helm helmexec.Interface) (bool, []error) {
 	return func(st *state.HelmState, helm helmexec.Interface) (bool, []error) {
-		return processFilteredReleases(st, func(st *state.HelmState) []error {
+		return processFilteredReleases(st, helm, func(st *state.HelmState) []error {
 			return converge(st, helm)
 		})
 	}
@@ -725,7 +746,7 @@ func (a *App) Wrap(converge func(*state.HelmState, helmexec.Interface) []error)
 
 func (a *App) VisitDesiredStatesWithReleasesFiltered(fileOrDir string, converge func(*state.HelmState) []error, o ...LoadOption) error {
 	return a.visitStatesWithSelectorsAndRemoteSupport(fileOrDir, func(st *state.HelmState) (bool, []error) {
-		return processFilteredReleases(st, converge)
+		return processFilteredReleases(st, a.getHelm(st), converge)
 	}, o...)
 }
 
@@ -1142,6 +1163,9 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) {
 	releasesToRender := map[string]state.ReleaseSpec{}
 	for _, r := range toRender {
 		id := state.ReleaseToID(&r)
+		if r.Installed != nil && !*r.Installed {
+			continue
+		}
 		releasesToRender[id] = r
 	}
 
diff --git a/pkg/app/context.go b/pkg/app/context.go
index 1cfff7f..9864ce0 100644
--- a/pkg/app/context.go
+++ b/pkg/app/context.go
@@ -17,6 +17,15 @@ func NewContext() Context {
 func (ctx Context) SyncReposOnce(st *state.HelmState, helm state.RepoUpdater) []error {
 	var errs []error
 
+	hasInstalled := false
+	for _, release := range st.Releases {
+		hasInstalled = hasInstalled || release.Installed == nil || *release.Installed
+	}
+
+	if !hasInstalled {
+		return errs
+	}
+
 	allUpdated := true
 	for _, r := range st.Repositories {
 		_, exists := ctx.updatedRepos[r.Name]
diff --git a/pkg/helmexec/exit_error.go b/pkg/helmexec/exit_error.go
index 6b11638..9419cf4 100644
--- a/pkg/helmexec/exit_error.go
+++ b/pkg/helmexec/exit_error.go
@@ -2,13 +2,12 @@ package helmexec
 
 import (
 	"fmt"
-	"path/filepath"
 	"strings"
 )
 
 func newExitError(helmCmdPath string, exitStatus int, errorMessage string) ExitError {
 	return ExitError{
-		Message: fmt.Sprintf("%s exited with status %d:\n%s", filepath.Base(helmCmdPath), exitStatus, indent(strings.TrimSpace(errorMessage))),
+		Message: fmt.Sprintf("the following cmd exited with status %d:\n%s\n\n%s", exitStatus, indent(strings.TrimSpace(helmCmdPath)), indent(strings.TrimSpace(errorMessage))),
 		Code:    exitStatus,
 	}
 }
diff --git a/pkg/helmexec/runner.go b/pkg/helmexec/runner.go
index 2892afc..179c019 100644
--- a/pkg/helmexec/runner.go
+++ b/pkg/helmexec/runner.go
@@ -64,7 +64,8 @@ func combinedOutput(c *exec.Cmd, logger *zap.SugaredLogger) ([]byte, error) {
 			// so that helmfile could return its own exit code accordingly
 			waitStatus := ee.Sys().(syscall.WaitStatus)
 			exitStatus := waitStatus.ExitStatus()
-			err = newExitError(c.Path, exitStatus, string(e))
+			cmd := fmt.Sprintf("%s %s", c.Path, strings.Join(c.Args, " "))
+			err = newExitError(cmd, exitStatus, string(e))
 		default:
 			panic(fmt.Sprintf("unexpected error: %v", err))
 		}
diff --git a/pkg/state/state.go b/pkg/state/state.go
index 9558b3b..5ca00b7 100644
--- a/pkg/state/state.go
+++ b/pkg/state/state.go
@@ -975,6 +975,9 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 		if !st.Releases[i].Desired() {
 			continue
 		}
+		if st.Releases[i].Installed != nil && !*(st.Releases[i].Installed) {
+			continue
+		}
 		releases = append(releases, &st.Releases[i])
 	}
 
@@ -1388,6 +1391,10 @@ func (st *HelmState) PrepareReleases(helm helmexec.Interface, helmfileCommand st
 	for i := range st.Releases {
 		release := st.Releases[i]
 
+		if release.Installed != nil && !*release.Installed {
+			continue
+		}
+
 		if _, err := st.triggerPrepareEvent(&release, helmfileCommand); err != nil {
 			errs = append(errs, newReleaseFailedError(&release, err))
 			continue
@@ -1484,6 +1491,10 @@ func (st *HelmState) BuildDeps(helm helmexec.Interface) []error {
 			continue
 		}
 
+		if release.Installed != nil && !*release.Installed {
+			continue
+		}
+
 		if isLocalChart(release.Chart) {
 			if err := helm.BuildDeps(release.Name, normalizeChart(st.basePath, release.Chart)); err != nil {
 				errs = append(errs, err)


Test patch:
diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go
index 86582bc..555b928 100644
--- a/pkg/app/app_test.go
+++ b/pkg/app/app_test.go
@@ -45,12 +45,16 @@ func injectFs(app *App, fs *testhelper.TestFs) *App {
 }
 
 func expectNoCallsToHelm(app *App) {
+	expectNoCallsToHelmVersion(app, false)
+}
+
+func expectNoCallsToHelmVersion(app *App, isHelm3 bool) {
 	if app.helms != nil {
 		panic("invalid call to expectNoCallsToHelm")
 	}
 
 	app.helms = map[helmKey]helmexec.Interface{
-		createHelmKey(app.OverrideHelmBinary, app.OverrideKubeContext): &noCallHelmExec{},
+		createHelmKey(app.OverrideHelmBinary, app.OverrideKubeContext): &versionOnlyHelmExec{isHelm3: isHelm3},
 	}
 }
 
@@ -1243,6 +1247,178 @@ releases:
 	}
 }
 
+// See https://github.com/roboll/helmfile/issues/1213
+func TestVisitDesiredStatesWithReleases_DuplicateReleasesHelm2(t *testing.T) {
+	files := map[string]string{
+		"/path/to/helmfile.yaml": `
+releases:
+- name: foo
+  namespace: foo
+  chart: charts/foo
+- name: foo
+  namespace: bar
+  chart: charts/foo
+`,
+	}
+
+	actual := []state.ReleaseSpec{}
+
+	collectReleases := func(st *state.HelmState) []error {
+		for _, r := range st.Releases {
+			actual = append(actual, r)
+		}
+		return []error{}
+	}
+	app := appWithFs(&App{
+		OverrideHelmBinary:  DefaultHelmBinary,
+		OverrideKubeContext: "default",
+		Logger:              helmexec.NewLogger(os.Stderr, "debug"),
+		Namespace:           "",
+		Env:                 "default",
+	}, files)
+
+	expectNoCallsToHelmVersion(app, false)
+
+	err := app.VisitDesiredStatesWithReleasesFiltered(
+		"helmfile.yaml", collectReleases,
+	)
+
+	expected := "in ./helmfile.yaml: duplicate release \"foo\" found in \"\": there were 2 releases named \"foo\" matching specified selector"
+	if err == nil {
+		t.Errorf("error expected but not happened")
+	} else if err.Error() != expected {
+		t.Errorf("unexpected error message: expected=\"%s\", actual=\"%s\"", expected, err.Error())
+	}
+}
+
+// See https://github.com/roboll/helmfile/issues/1213
+func TestVisitDesiredStatesWithReleases_NoDuplicateReleasesHelm2(t *testing.T) {
+	files := map[string]string{
+		"/path/to/helmfile.yaml": `
+releases:
+- name: foo
+  namespace: foo
+  tillerNamespace: tns1
+  chart: charts/foo
+- name: foo
+  namespace: bar
+  tillerNamespace: tns2
+  chart: charts/foo
+`,
+	}
+
+	actual := []state.ReleaseSpec{}
+
+	collectReleases := func(st *state.HelmState) []error {
+		for _, r := range st.Releases {
+			actual = append(actual, r)
+		}
+		return []error{}
+	}
+	app := appWithFs(&App{
+		OverrideHelmBinary:  DefaultHelmBinary,
+		OverrideKubeContext: "default",
+		Logger:              helmexec.NewLogger(os.Stderr, "debug"),
+		Namespace:           "",
+		Env:                 "default",
+	}, files)
+
+	expectNoCallsToHelmVersion(app, false)
+
+	err := app.VisitDesiredStatesWithReleasesFiltered(
+		"helmfile.yaml", collectReleases,
+	)
+
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+// See https://github.com/roboll/helmfile/issues/1213
+func TestVisitDesiredStatesWithReleases_NoDuplicateReleasesHelm3(t *testing.T) {
+	files := map[string]string{
+		"/path/to/helmfile.yaml": `
+releases:
+- name: foo
+  namespace: foo
+  chart: charts/foo
+- name: foo
+  namespace: bar
+  chart: charts/foo
+`,
+	}
+
+	actual := []state.ReleaseSpec{}
+
+	collectReleases := func(st *state.HelmState) []error {
+		for _, r := range st.Releases {
+			actual = append(actual, r)
+		}
+		return []error{}
+	}
+	app := appWithFs(&App{
+		OverrideHelmBinary:  DefaultHelmBinary,
+		OverrideKubeContext: "default",
+		Logger:              helmexec.NewLogger(os.Stderr, "debug"),
+		Namespace:           "",
+		Env:                 "default",
+	}, files)
+
+	expectNoCallsToHelmVersion(app, true)
+
+	err := app.VisitDesiredStatesWithReleasesFiltered(
+		"helmfile.yaml", collectReleases,
+	)
+
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+// See https://github.com/roboll/helmfile/issues/1213
+func TestVisitDesiredStatesWithReleases_DuplicateReleasesHelm3(t *testing.T) {
+	files := map[string]string{
+		"/path/to/helmfile.yaml": `
+releases:
+- name: foo
+  namespace: foo
+  chart: charts/foo
+- name: foo
+  namespace: foo
+  chart: charts/foo
+`,
+	}
+
+	actual := []state.ReleaseSpec{}
+
+	collectReleases := func(st *state.HelmState) []error {
+		for _, r := range st.Releases {
+			actual = append(actual, r)
+		}
+		return []error{}
+	}
+	app := appWithFs(&App{
+		OverrideHelmBinary:  DefaultHelmBinary,
+		OverrideKubeContext: "default",
+		Logger:              helmexec.NewLogger(os.Stderr, "debug"),
+		Namespace:           "",
+		Env:                 "default",
+	}, files)
+
+	expectNoCallsToHelmVersion(app, true)
+
+	err := app.VisitDesiredStatesWithReleasesFiltered(
+		"helmfile.yaml", collectReleases,
+	)
+
+	expected := "in ./helmfile.yaml: duplicate release \"foo\" found in \"foo\": there were 2 releases named \"foo\" matching specified selector"
+	if err == nil {
+		t.Errorf("error expected but not happened")
+	} else if err.Error() != expected {
+		t.Errorf("unexpected error message: expected=\"%s\", actual=\"%s\"", expected, err.Error())
+	}
+}
+
 func TestLoadDesiredStateFromYaml_DuplicateReleaseName(t *testing.T) {
 	yamlFile := "example/path/to/yaml/file"
 	yamlContent := []byte(`releases:
@@ -3751,6 +3927,8 @@ releases:
 		Namespace:           "testNamespace",
 	}, files)
 
+	expectNoCallsToHelm(app)
+
 	out := captureStdout(func() {
 		err := app.PrintState(configImpl{})
 		assert.NilError(t, err)
@@ -3795,6 +3973,9 @@ releases:
 		Logger:              logger,
 		Namespace:           "testNamespace",
 	}, files)
+
+	expectNoCallsToHelm(app)
+
 	out := captureStdout(func() {
 		err := app.PrintState(configImpl{})
 		assert.NilError(t, err)
@@ -3951,6 +4132,8 @@ releases:
 		Env:                 "default",
 	}, files)
 
+	expectNoCallsToHelm(app)
+
 	var specs []state.ReleaseSpec
 	collectReleases := func(st *state.HelmState) []error {
 		specs = append(specs, st.Releases...)
diff --git a/pkg/app/mocks_test.go b/pkg/app/mocks_test.go
index fdd45d4..8d8e7cd 100644
--- a/pkg/app/mocks_test.go
+++ b/pkg/app/mocks_test.go
@@ -5,6 +5,15 @@ import "github.com/roboll/helmfile/pkg/helmexec"
 type noCallHelmExec struct {
 }
 
+type versionOnlyHelmExec struct {
+	*noCallHelmExec
+	isHelm3 bool
+}
+
+func (helm *versionOnlyHelmExec) IsHelm3() bool {
+	return helm.isHelm3
+}
+
 func (helm *noCallHelmExec) doPanic() {
 	panic("unexpected call to helm")
 }