feat: `helmfile repos` updates all repos regardless of selector and releases (#353)

Resolves #338

app_test.go

@@ -4,13 +4,15 @@ import (
 	"fmt"
 	"github.com/roboll/helmfile/helmexec"
 	"github.com/roboll/helmfile/state"
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
 )
 
 // See https://github.com/roboll/helmfile/issues/193
-func TestFindAndIterateOverDesiredStates(t *testing.T) {
+func TestVisitDesiredStatesWithReleasesFiltered(t *testing.T) {
 	absPaths := map[string]string{
 		".": "/path/to",
 		"/path/to/helmfile.d": "/path/to/helmfile.d",
@@ -73,15 +75,6 @@ releases:
 		}
 		return a, nil
 	}
-	app := &app{
-		readFile:          readFile,
-		glob:              glob,
-		abs:               abs,
-		fileExistsAt:      fileExistsAt,
-		directoryExistsAt: directoryExistsAt,
-		kubeContext:       "default",
-		logger:            helmexec.NewLogger(os.Stderr, "debug"),
-	}
 	noop := func(st *state.HelmState, helm helmexec.Interface) []error {
 		return []error{}
 	}
@@ -97,8 +90,20 @@ releases:
 	}
 
 	for _, testcase := range testcases {
-		err := app.FindAndIterateOverDesiredStates(
+		app := &app{
-			"helmfile.yaml", noop, "", []string{fmt.Sprintf("name=%s", testcase.name)}, "default",
+			readFile:          readFile,
+			glob:              glob,
+			abs:               abs,
+			fileExistsAt:      fileExistsAt,
+			directoryExistsAt: directoryExistsAt,
+			kubeContext:       "default",
+			logger:            helmexec.NewLogger(os.Stderr, "debug"),
+			selectors:         []string{fmt.Sprintf("name=%s", testcase.name)},
+			namespace:         "",
+			env:               "default",
+		}
+		err := app.VisitDesiredStatesWithReleasesFiltered(
+			"helmfile.yaml", noop,
 		)
 		if testcase.expectErr && err == nil {
 			t.Errorf("error expected but not happened for name=%s", testcase.name)
@@ -109,7 +114,7 @@ releases:
 }
 
 // See https://github.com/roboll/helmfile/issues/320
-func TestFindAndIterateOverDesiredStates_UndefinedEnv(t *testing.T) {
+func TestVisitDesiredStatesWithReleasesFiltered_UndefinedEnv(t *testing.T) {
 	absPaths := map[string]string{
 		".": "/path/to",
 		"/path/to/helmfile.d": "/path/to/helmfile.d",
@@ -166,15 +171,6 @@ releases:
 		}
 		return a, nil
 	}
-	app := &app{
-		readFile:          readFile,
-		glob:              glob,
-		abs:               abs,
-		fileExistsAt:      fileExistsAt,
-		directoryExistsAt: directoryExistsAt,
-		kubeContext:       "default",
-		logger:            helmexec.NewLogger(os.Stderr, "debug"),
-	}
 	noop := func(st *state.HelmState, helm helmexec.Interface) []error {
 		return []error{}
 	}
@@ -189,8 +185,20 @@ releases:
 	}
 
 	for _, testcase := range testcases {
-		err := app.FindAndIterateOverDesiredStates(
+		app := &app{
-			"helmfile.yaml", noop, "", []string{}, testcase.name,
+			readFile:          readFile,
+			glob:              glob,
+			abs:               abs,
+			fileExistsAt:      fileExistsAt,
+			directoryExistsAt: directoryExistsAt,
+			kubeContext:       "default",
+			logger:            helmexec.NewLogger(os.Stderr, "debug"),
+			namespace:         "",
+			selectors:         []string{},
+			env:               testcase.name,
+		}
+		err := app.VisitDesiredStatesWithReleasesFiltered(
+			"helmfile.yaml", noop,
 		)
 		if testcase.expectErr && err == nil {
 			t.Errorf("error expected but not happened for environment=%s", testcase.name)
@@ -201,7 +209,7 @@ releases:
 }
 
 // See https://github.com/roboll/helmfile/issues/322
-func TestFindAndIterateOverDesiredStates_Selectors(t *testing.T) {
+func TestVisitDesiredStatesWithReleasesFiltered_Selectors(t *testing.T) {
 	absPaths := map[string]string{
 		".": "/path/to",
 		"/path/to/helmfile.d": "/path/to/helmfile.d",
@@ -229,6 +237,14 @@ releases:
 releases:
 - name: grafana
   chart: stable/grafana
+- name: foo
+  chart: charts/foo
+  labels:
+    duplicated: yes
+- name: foo
+  chart: charts/foo
+  labels:
+    duplicated: yes
 `,
 	}
 	globMatches := map[string][]string{
@@ -264,6 +280,31 @@ releases:
 		}
 		return a, nil
 	}
+
+	testcases := []struct {
+		label         string
+		expectedCount int
+		expectErr     bool
+		errMsg        string
+	}{
+		{label: "name=prometheus", expectedCount: 1, expectErr: false},
+		{label: "name=", expectedCount: 0, expectErr: true, errMsg: "failed processing /path/to/helmfile.d/a1.yaml: Malformed label: name=. Expected label in form k=v or k!=v"},
+		{label: "name!=", expectedCount: 0, expectErr: true, errMsg: "failed processing /path/to/helmfile.d/a1.yaml: Malformed label: name!=. Expected label in form k=v or k!=v"},
+		{label: "name", expectedCount: 0, expectErr: true, errMsg: "failed processing /path/to/helmfile.d/a1.yaml: Malformed label: name. Expected label in form k=v or k!=v"},
+		// See https://github.com/roboll/helmfile/issues/193
+		{label: "duplicated=yes", expectedCount: 0, expectErr: true, errMsg: "failed processing /path/to/helmfile.d/b.yaml: duplicate release \"foo\" found: there were 2 releases named \"foo\" matching specified selector"},
+	}
+
+	for _, testcase := range testcases {
+		actual := []string{}
+
+		collectReleases := func(st *state.HelmState, helm helmexec.Interface) []error {
+			for _, r := range st.Releases {
+				actual = append(actual, r.Name)
+			}
+			return []error{}
+		}
+
 		app := &app{
 			readFile:          readFile,
 			glob:              glob,
@@ -272,40 +313,31 @@ releases:
 			directoryExistsAt: directoryExistsAt,
 			kubeContext:       "default",
 			logger:            helmexec.NewLogger(os.Stderr, "debug"),
-	}
+			namespace:         "",
-	noop := func(st *state.HelmState, helm helmexec.Interface) []error {
+			selectors:         []string{testcase.label},
-		return []error{}
+			env:               "default",
 		}
 
-	testcases := []struct {
+		err := app.VisitDesiredStatesWithReleasesFiltered(
-		label     string
+			"helmfile.yaml", collectReleases,
-		expectErr bool
-		errMsg    string
-	}{
-		{label: "name=prometheus", expectErr: false},
-		{label: "name=", expectErr: true, errMsg: "Malformed label: name=. Expected label in form k=v or k!=v"},
-		{label: "name!=", expectErr: true, errMsg: "Malformed label: name!=. Expected label in form k=v or k!=v"},
-		{label: "name", expectErr: true, errMsg: "Malformed label: name. Expected label in form k=v or k!=v"},
-	}
-
-	for _, testcase := range testcases {
-		err := app.FindAndIterateOverDesiredStates(
-			"helmfile.yaml", noop, "", []string{testcase.label}, "default",
 		)
 		if testcase.expectErr {
 			if err == nil {
-				t.Errorf("error expected but not happened for name=%s", testcase.label)
+				t.Errorf("error expected but not happened for selector %s", testcase.label)
 			} else if err.Error() != testcase.errMsg {
 				t.Errorf("unexpected error message: expected=\"%s\", actual=\"%s\"", testcase.errMsg, err.Error())
 			}
 		} else if !testcase.expectErr && err != nil {
-			t.Errorf("unexpected error for name=%s: %v", testcase.label, err)
+			t.Errorf("unexpected error for selector %s: %v", testcase.label, err)
+		}
+		if len(actual) != testcase.expectedCount {
+			t.Errorf("unexpected release count for selector %s: expected=%d, actual=%d", testcase.label, testcase.expectedCount, len(actual))
 		}
 	}
 }
 
 // See https://github.com/roboll/helmfile/issues/312
-func TestFindAndIterateOverDesiredStates_ReverseOrder(t *testing.T) {
+func TestVisitDesiredStatesWithReleasesFiltered_ReverseOrder(t *testing.T) {
 	absPaths := map[string]string{
 		".": "/path/to",
 		"/path/to/helmfile.d": "/path/to/helmfile.d",
@@ -399,9 +431,12 @@ releases:
 			kubeContext:       "default",
 			logger:            helmexec.NewLogger(os.Stderr, "debug"),
 			reverse:           testcase.reverse,
+			namespace:         "",
+			selectors:         []string{},
+			env:               "default",
 		}
-		err := app.FindAndIterateOverDesiredStates(
+		err := app.VisitDesiredStatesWithReleasesFiltered(
-			"helmfile.yaml", collectReleases, "", []string{}, "default",
+			"helmfile.yaml", collectReleases,
 		)
 		if err != nil {
 			t.Errorf("unexpected error: %v", err)
@@ -411,3 +446,29 @@ releases:
 		}
 	}
 }
+
+func TestLoadDesiredStateFromYaml_DuplicateReleaseName(t *testing.T) {
+	yamlFile := "example/path/to/yaml/file"
+	yamlContent := []byte(`releases:
+- name: myrelease1
+  chart: mychart1
+  labels:
+    stage: pre
+    foo: bar
+- name: myrelease1
+  chart: mychart2
+  labels:
+    stage: post
+`)
+	app := &app{
+		readFile:    ioutil.ReadFile,
+		glob:        filepath.Glob,
+		abs:         filepath.Abs,
+		kubeContext: "default",
+		logger:      logger,
+	}
+	_, err := app.loadDesiredStateFromYaml(yamlContent, yamlFile, "default", "default")
+	if err != nil {
+		t.Error("unexpected error")
+	}
+}

main.go

@@ -10,8 +10,6 @@ import (
 	"sort"
 	"syscall"
 
-	"os/exec"
-
 	"io/ioutil"
 
 	"strings"
@@ -24,6 +22,7 @@ import (
 	"github.com/urfave/cli"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
+	"os/exec"
 )
 
 const (
@@ -115,13 +114,17 @@ func main() {
 				},
 			},
 			Action: func(c *cli.Context) error {
-				return findAndIterateOverDesiredStatesUsingFlags(c, func(state *state.HelmState, helm helmexec.Interface) []error {
+				return visitAllDesiredStates(c, func(state *state.HelmState, helm helmexec.Interface) (bool, []error) {
 					args := args.GetArgs(c.String("args"), state)
 					if len(args) > 0 {
 						helm.SetExtraArgs(args...)
 					}
 
-					return state.SyncRepos(helm)
+					errs := state.SyncRepos(helm)
+
+					ok := len(state.Repositories) > 0 && len(errs) == 0
+
+					return ok, errs
 				})
 			},
 		},
@@ -566,16 +569,19 @@ type app struct {
 	fileExistsAt      func(string) bool
 	directoryExistsAt func(string) bool
 	reverse           bool
+	env               string
+	namespace         string
+	selectors         []string
 }
 
 func findAndIterateOverDesiredStatesUsingFlags(c *cli.Context, converge func(*state.HelmState, helmexec.Interface) []error) error {
 	return findAndIterateOverDesiredStatesUsingFlagsWithReverse(c, false, converge)
 }
 
-func findAndIterateOverDesiredStatesUsingFlagsWithReverse(c *cli.Context, reverse bool, converge func(*state.HelmState, helmexec.Interface) []error) error {
+func initAppEntry(c *cli.Context, reverse bool) (*app, string, error) {
 	if c.NArg() > 0 {
 		cli.ShowAppHelp(c)
-		return fmt.Errorf("err: extraneous arguments: %s", strings.Join(c.Args(), ", "))
+		return nil, "", fmt.Errorf("err: extraneous arguments: %s", strings.Join(c.Args(), ", "))
 	}
 
 	fileOrDir := c.GlobalString("file")
@@ -598,21 +604,66 @@ func findAndIterateOverDesiredStatesUsingFlagsWithReverse(c *cli.Context, revers
 		kubeContext:       kubeContext,
 		logger:            logger,
 		reverse:           reverse,
+		env:               env,
+		namespace:         namespace,
+		selectors:         selectors,
 	}
+
+	return app, fileOrDir, nil
+}
+
+func visitAllDesiredStates(c *cli.Context, converge func(*state.HelmState, helmexec.Interface) (bool, []error)) error {
+	app, fileOrDir, err := initAppEntry(c, false)
+	if err != nil {
+		return err
+	}
+
+	convergeWithHelmBinary := func(st *state.HelmState, helm helmexec.Interface) (bool, []error) {
+		if c.GlobalString("helm-binary") != "" {
+			helm.SetHelmBinary(c.GlobalString("helm-binary"))
+		}
+		return converge(st, helm)
+	}
+
+	err = app.VisitDesiredStates(fileOrDir, convergeWithHelmBinary)
+
+	return toCliError(err)
+}
+
+func toCliError(err error) error {
+	if err != nil {
+		switch e := err.(type) {
+		case *noMatchingHelmfileError:
+			return cli.NewExitError(e.Error(), 2)
+		case *exec.ExitError:
+			// Propagate any non-zero exit status from the external command like `helm` that is failed under the hood
+			status := e.Sys().(syscall.WaitStatus)
+			return cli.NewExitError(e.Error(), status.ExitStatus())
+		case *state.DiffError:
+			return cli.NewExitError(e.Error(), e.Code)
+		default:
+			return cli.NewExitError(e.Error(), 1)
+		}
+	}
+	return err
+}
+
+func findAndIterateOverDesiredStatesUsingFlagsWithReverse(c *cli.Context, reverse bool, converge func(*state.HelmState, helmexec.Interface) []error) error {
+	app, fileOrDir, err := initAppEntry(c, reverse)
+	if err != nil {
+		return err
+	}
+
 	convergeWithHelmBinary := func(st *state.HelmState, helm helmexec.Interface) []error {
 		if c.GlobalString("helm-binary") != "" {
 			helm.SetHelmBinary(c.GlobalString("helm-binary"))
 		}
 		return converge(st, helm)
 	}
-	if err := app.FindAndIterateOverDesiredStates(fileOrDir, convergeWithHelmBinary, namespace, selectors, env); err != nil {
+
-		switch e := err.(type) {
+	err = app.VisitDesiredStatesWithReleasesFiltered(fileOrDir, convergeWithHelmBinary)
-		case *noMatchingHelmfileError:
+
-			return cli.NewExitError(e.Error(), 2)
+	return toCliError(err)
-		}
-		return err
-	}
-	return nil
 }
 
 type noMatchingHelmfileError struct {
@@ -690,11 +741,12 @@ func (r *twoPassRenderer) renderTemplate(content []byte) (*bytes.Buffer, error)
 	return yamlBuf, nil
 }
 
-func (a *app) FindAndIterateOverDesiredStates(fileOrDir string, converge func(*state.HelmState, helmexec.Interface) []error, namespace string, selectors []string, env string) error {
+func (a *app) VisitDesiredStates(fileOrDir string, converge func(*state.HelmState, helmexec.Interface) (bool, []error)) error {
 	desiredStateFiles, err := a.findDesiredStateFiles(fileOrDir)
 	if err != nil {
 		return err
 	}
+
 	noMatchInHelmfiles := true
 	for _, f := range desiredStateFiles {
 		a.logger.Debugf("Processing %s", f)
@@ -706,8 +758,8 @@ func (a *app) FindAndIterateOverDesiredStates(fileOrDir string, converge func(*s
 		// render template, in two runs
 		r := &twoPassRenderer{
 			reader:    a.readFile,
-			env:       env,
+			env:       a.env,
-			namespace: namespace,
+			namespace: a.namespace,
 			filename:  f,
 			logger:    a.logger,
 			abs:       a.abs,
@@ -717,13 +769,13 @@ func (a *app) FindAndIterateOverDesiredStates(fileOrDir string, converge func(*s
 			return fmt.Errorf("error during %s parsing: %v", f, err)
 		}
 
-		st, noMatchInThisHelmfile, err := a.loadDesiredStateFromYaml(
+		st, err := a.loadDesiredStateFromYaml(
 			yamlBuf.Bytes(),
 			f,
-			namespace,
+			a.namespace,
-			selectors,
+			a.env,
-			env,
 		)
+
 		helm := helmexec.New(a.logger, a.kubeContext)
 
 		if err != nil {
@@ -746,7 +798,7 @@ func (a *app) FindAndIterateOverDesiredStates(fileOrDir string, converge func(*s
 		if len(st.Helmfiles) > 0 {
 			noMatchInSubHelmfiles := true
 			for _, m := range st.Helmfiles {
-				if err := a.FindAndIterateOverDesiredStates(m, converge, namespace, selectors, env); err != nil {
+				if err := a.VisitDesiredStates(m, converge); err != nil {
 					switch err.(type) {
 					case *noMatchingHelmfileError:
 
@@ -759,11 +811,9 @@ func (a *app) FindAndIterateOverDesiredStates(fileOrDir string, converge func(*s
 			}
 			noMatchInHelmfiles = noMatchInHelmfiles && noMatchInSubHelmfiles
 		} else {
-			noMatchInHelmfiles = noMatchInHelmfiles && noMatchInThisHelmfile
+			var processed bool
-			if noMatchInThisHelmfile {
+			processed, errs = converge(st, helm)
-				continue
+			noMatchInHelmfiles = noMatchInHelmfiles && !processed
-			}
-			errs = converge(st, helm)
 		}
 
 		if err := clean(st, errs); err != nil {
@@ -771,7 +821,40 @@ func (a *app) FindAndIterateOverDesiredStates(fileOrDir string, converge func(*s
 		}
 	}
 	if noMatchInHelmfiles {
-		return &noMatchingHelmfileError{selectors, env}
+		return &noMatchingHelmfileError{selectors: a.selectors, env: a.env}
+	}
+	return nil
+}
+
+func (a *app) VisitDesiredStatesWithReleasesFiltered(fileOrDir string, converge func(*state.HelmState, helmexec.Interface) []error) error {
+	selectors := a.selectors
+
+	err := a.VisitDesiredStates(fileOrDir, func(st *state.HelmState, helm helmexec.Interface) (bool, []error) {
+		if len(selectors) > 0 {
+			err := st.FilterReleases(selectors)
+			if err != nil {
+				return false, []error{err}
+			}
+		}
+
+		releaseNameCounts := map[string]int{}
+		for _, r := range st.Releases {
+			releaseNameCounts[r.Name]++
+		}
+		for name, c := range releaseNameCounts {
+			if c > 1 {
+				return false, []error{fmt.Errorf("duplicate release \"%s\" found: there were %d releases named \"%s\" matching specified selector", name, c, name)}
+			}
+		}
+
+		errs := converge(st, helm)
+
+		processed := len(st.Releases) != 0 && len(errs) == 0
+
+		return processed, errs
+	})
+	if err != nil {
+		return err
 	}
 	return nil
 }
@@ -833,11 +916,11 @@ func directoryExistsAt(path string) bool {
 	return err == nil && fileInfo.Mode().IsDir()
 }
 
-func (a *app) loadDesiredStateFromYaml(yaml []byte, file string, namespace string, labels []string, env string) (*state.HelmState, bool, error) {
+func (a *app) loadDesiredStateFromYaml(yaml []byte, file string, namespace string, env string) (*state.HelmState, error) {
 	c := state.NewCreator(a.logger, a.readFile, a.abs)
 	st, err := c.CreateFromYaml(yaml, file, env)
 	if err != nil {
-		return nil, false, err
+		return nil, err
 	}
 
 	helmfiles := []string{}
@@ -845,7 +928,7 @@ func (a *app) loadDesiredStateFromYaml(yaml []byte, file string, namespace strin
 		helmfileRelativePattern := st.JoinBase(globPattern)
 		matches, err := a.glob(helmfileRelativePattern)
 		if err != nil {
-			return nil, false, fmt.Errorf("failed processing %s: %v", globPattern, err)
+			return nil, fmt.Errorf("failed processing %s: %v", globPattern, err)
 		}
 		sort.Strings(matches)
 
@@ -876,23 +959,6 @@ func (a *app) loadDesiredStateFromYaml(yaml []byte, file string, namespace strin
 		st.Namespace = namespace
 	}
 
-	if len(labels) > 0 {
-		err = st.FilterReleases(labels)
-		if err != nil {
-			return nil, false, err
-		}
-	}
-
-	releaseNameCounts := map[string]int{}
-	for _, r := range st.Releases {
-		releaseNameCounts[r.Name]++
-	}
-	for name, c := range releaseNameCounts {
-		if c > 1 {
-			return nil, false, fmt.Errorf("duplicate release \"%s\" found: there were %d releases named \"%s\" matching specified selector", name, c, name)
-		}
-	}
-
 	sigs := make(chan os.Signal, 1)
 	signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
 	go func() {
@@ -902,7 +968,7 @@ func (a *app) loadDesiredStateFromYaml(yaml []byte, file string, namespace strin
 		clean(st, errs)
 	}()
 
-	return st, len(st.Releases) == 0, nil
+	return st, nil
 }
 
 func clean(st *state.HelmState, errs []error) error {
@@ -924,16 +990,7 @@ func clean(st *state.HelmState, errs []error) error {
 				fmt.Printf("err: %v\n", e)
 			}
 		}
-		switch e := errs[0].(type) {
+		return errs[0]
-		case *exec.ExitError:
-			// Propagate any non-zero exit status from the external command like `helm` that is failed under the hood
-			status := e.Sys().(syscall.WaitStatus)
-			os.Exit(status.ExitStatus())
-		case *state.DiffError:
-			os.Exit(e.Code)
-		default:
-			os.Exit(1)
-		}
 	}
 	return nil
 }

main_test.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"fmt"
-	"io/ioutil"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -11,36 +10,6 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
-// See https://github.com/roboll/helmfile/issues/193
-func TestReadFromYaml_DuplicateReleaseName(t *testing.T) {
-	yamlFile := "example/path/to/yaml/file"
-	yamlContent := []byte(`releases:
-- name: myrelease1
-  chart: mychart1
-  labels:
-    stage: pre
-    foo: bar
-- name: myrelease1
-  chart: mychart2
-  labels:
-    stage: post
-`)
-	app := &app{
-		readFile:    ioutil.ReadFile,
-		glob:        filepath.Glob,
-		abs:         filepath.Abs,
-		kubeContext: "default",
-		logger:      logger,
-	}
-	_, _, err := app.loadDesiredStateFromYaml(yamlContent, yamlFile, "default", []string{}, "default")
-	if err == nil {
-		t.Error("error expected but not happened")
-	}
-	if err.Error() != "duplicate release \"myrelease1\" found: there were 2 releases named \"myrelease1\" matching specified selector" {
-		t.Errorf("unexpected error happened: %v", err)
-	}
-}
-
 func makeRenderer(readFile func(string) ([]byte, error), env string) *twoPassRenderer {
 	return &twoPassRenderer{
 		reader:    readFile,

state/state.go

@@ -788,7 +788,7 @@ func (state *HelmState) Clean() []error {
 // FilterReleases allows for the execution of helm commands against a subset of the releases in the helmfile.
 func (state *HelmState) FilterReleases(labels []string) error {
 	var filteredReleases []ReleaseSpec
-	releaseSet := map[string]ReleaseSpec{}
+	releaseSet := map[string][]ReleaseSpec{}
 	filters := []ReleaseFilter{}
 	for _, label := range labels {
 		f, err := ParseLabels(label)
@@ -808,13 +808,13 @@ func (state *HelmState) FilterReleases(labels []string) error {
 				r.Labels = map[string]string{}
 			}
 			if f.Match(r) {
-				releaseSet[r.Name] = r
+				releaseSet[r.Name] = append(releaseSet[r.Name], r)
 				continue
 			}
 		}
 	}
 	for _, r := range releaseSet {
-		filteredReleases = append(filteredReleases, r)
+		filteredReleases = append(filteredReleases, r...)
 	}
 	state.Releases = filteredReleases
 	numFound := len(filteredReleases)