From f2b610afdfa95b336e8b41e60a97330308677a0c Mon Sep 17 00:00:00 2001 From: KUOKA Yusuke Date: Thu, 27 Sep 2018 01:44:05 +0900 Subject: [PATCH] feat: `helmfile repos` updates all repos regardless of selector and releases (#353) Resolves #338 --- app_test.go | 163 +++++++++++++++++++++++++++++++-------------- main.go | 175 ++++++++++++++++++++++++++++++++----------------- main_test.go | 31 --------- state/state.go | 6 +- 4 files changed, 231 insertions(+), 144 deletions(-) diff --git a/app_test.go b/app_test.go index 5b361ec8..16f6873b 100644 --- a/app_test.go +++ b/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( - "helmfile.yaml", noop, "", []string{fmt.Sprintf("name=%s", testcase.name)}, "default", + app := &app{ + 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( - "helmfile.yaml", noop, "", []string{}, testcase.name, + app := &app{ + 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,48 +280,64 @@ 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{} - } testcases := []struct { - label string - expectErr bool - errMsg string + label string + expectedCount int + 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"}, + {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 { - err := app.FindAndIterateOverDesiredStates( - "helmfile.yaml", noop, "", []string{testcase.label}, "default", + 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, + abs: abs, + fileExistsAt: fileExistsAt, + directoryExistsAt: directoryExistsAt, + kubeContext: "default", + logger: helmexec.NewLogger(os.Stderr, "debug"), + namespace: "", + selectors: []string{testcase.label}, + env: "default", + } + + err := app.VisitDesiredStatesWithReleasesFiltered( + "helmfile.yaml", collectReleases, ) 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( - "helmfile.yaml", collectReleases, "", []string{}, "default", + err := app.VisitDesiredStatesWithReleasesFiltered( + "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") + } +} diff --git a/main.go b/main.go index f32f820f..fb02dc13 100644 --- a/main.go +++ b/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) { - case *noMatchingHelmfileError: - return cli.NewExitError(e.Error(), 2) - } - return err - } - return nil + + err = app.VisitDesiredStatesWithReleasesFiltered(fileOrDir, convergeWithHelmBinary) + + return toCliError(err) } 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, - namespace: namespace, + env: a.env, + 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, - selectors, - env, + a.namespace, + a.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 - if noMatchInThisHelmfile { - continue - } - errs = converge(st, helm) + var processed bool + processed, errs = converge(st, helm) + noMatchInHelmfiles = noMatchInHelmfiles && !processed } 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) { - 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 errs[0] } return nil } diff --git a/main_test.go b/main_test.go index bb1dd74c..a4ed0029 100644 --- a/main_test.go +++ b/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, diff --git a/state/state.go b/state/state.go index 2a4a71f1..3e1d480d 100644 --- a/state/state.go +++ b/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)