diff --git a/pkg/app/app.go b/pkg/app/app.go index ad006a8b..837a8616 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -102,34 +102,42 @@ func Init(app *App) *App { } func (a *App) Deps(c DepsConfigProvider) error { - return a.ForEachStateFiltered(func(run *Run) (errs []error) { - prepErrs := run.withReposAndPreparedCharts(false, c.SkipRepos(), "deps", func() { + return a.ForEachState(func(run *Run) (_ bool, errs []error) { + prepErr := run.withPreparedCharts(false, c.SkipRepos(), "deps", func() { errs = run.Deps(c) }) - errs = append(errs, prepErrs...) + if prepErr != nil { + errs = append(errs, prepErr) + } return - }) + }, SetFilter(true)) } func (a *App) Repos(c ReposConfigProvider) error { - return a.ForEachStateFiltered(func(run *Run) (errs []error) { - err := run.withPreparedCharts(false, "repos", func() { - errs = run.Repos(c) + return a.ForEachState(func(run *Run) (_ bool, errs []error) { + var reposErr error + + err := run.withPreparedCharts(false, true, "repos", func() { + reposErr = run.Repos(c) }) + if reposErr != nil { + errs = append(errs, reposErr) + } + if err != nil { errs = append(errs, err) } return - }) + }, SetFilter(true)) } func (a *App) DeprecatedSyncCharts(c DeprecatedChartsConfigProvider) error { - return a.ForEachStateFiltered(func(run *Run) (errs []error) { - err := run.withPreparedCharts(false, "charts", func() { + return a.ForEachState(func(run *Run) (_ bool, errs []error) { + err := run.withPreparedCharts(false, true, "charts", func() { errs = run.DeprecatedSyncCharts(c) }) @@ -138,7 +146,7 @@ func (a *App) DeprecatedSyncCharts(c DeprecatedChartsConfigProvider) error { } return - }) + }, SetFilter(true)) } func (a *App) Diff(c DiffConfigProvider) error { @@ -155,15 +163,17 @@ func (a *App) Diff(c DiffConfigProvider) error { var errs []error - prepErrs := run.withReposAndPreparedCharts(false, c.SkipDeps(), "diff", func() { - msg, matched, affected, errs = run.Diff(c) + prepErr := run.withPreparedCharts(false, c.SkipDeps(), "diff", func() { + msg, matched, affected, errs = a.diff(run, c) }) if msg != nil { a.Logger.Info(*msg) } - errs = append(errs, prepErrs...) + if prepErr != nil { + errs = append(errs, prepErr) + } affectedAny = affectedAny || affected @@ -206,35 +216,41 @@ func (a *App) Diff(c DiffConfigProvider) error { func (a *App) Template(c TemplateConfigProvider) error { return a.ForEachState(func(run *Run) (ok bool, errs []error) { - prepErrs := run.withReposAndPreparedCharts(true, c.SkipDeps(), "template", func() { + prepErr := run.withPreparedCharts(true, c.SkipDeps(), "template", func() { ok, errs = a.template(run, c) }) - errs = append(errs, prepErrs...) + if prepErr != nil { + errs = append(errs, prepErr) + } return }) } func (a *App) Lint(c LintConfigProvider) error { - return a.ForEachStateFiltered(func(run *Run) (errs []error) { - prepErrs := run.withReposAndPreparedCharts(true, c.SkipDeps(), "lint", func() { + return a.ForEachState(func(run *Run) (_ bool, errs []error) { + prepErr := run.withPreparedCharts(true, c.SkipDeps(), "lint", func() { errs = run.Lint(c) }) - errs = append(errs, prepErrs...) + if prepErr != nil { + errs = append(errs, prepErr) + } return - }) + }, SetFilter(true)) } func (a *App) Sync(c SyncConfigProvider) error { return a.ForEachState(func(run *Run) (ok bool, errs []error) { - prepErrs := run.withReposAndPreparedCharts(false, c.SkipDeps(), "sync", func() { + prepErr := run.withPreparedCharts(false, c.SkipDeps(), "sync", func() { ok, errs = a.sync(run, c) }) - errs = append(errs, prepErrs...) + if prepErr != nil { + errs = append(errs, prepErr) + } return }) @@ -250,7 +266,7 @@ func (a *App) Apply(c ApplyConfigProvider) error { opts = append(opts, SetRetainValuesFiles(c.RetainValuesFiles())) err := a.ForEachState(func(run *Run) (ok bool, errs []error) { - prepErrs := run.withReposAndPreparedCharts(false, c.SkipDeps(), "apply", func() { + prepErr := run.withPreparedCharts(false, c.SkipDeps(), "apply", func() { matched, updated, es := a.apply(run, c) mut.Lock() @@ -260,7 +276,9 @@ func (a *App) Apply(c ApplyConfigProvider) error { ok, errs = matched, es }) - errs = append(errs, prepErrs...) + if prepErr != nil { + errs = append(errs, prepErr) + } return }, opts...) @@ -279,8 +297,8 @@ func (a *App) Apply(c ApplyConfigProvider) error { } func (a *App) Status(c StatusesConfigProvider) error { - return a.ForEachStateFiltered(func(run *Run) (errs []error) { - err := run.withPreparedCharts(false, "status", func() { + return a.ForEachState(func(run *Run) (_ bool, errs []error) { + err := run.withPreparedCharts(false, true, "status", func() { errs = run.Status(c) }) @@ -289,12 +307,12 @@ func (a *App) Status(c StatusesConfigProvider) error { } return - }) + }, SetFilter(true)) } func (a *App) Delete(c DeleteConfigProvider) error { return a.ForEachState(func(run *Run) (ok bool, errs []error) { - err := run.withPreparedCharts(false, "delete", func() { + err := run.withPreparedCharts(false, true, "delete", func() { ok, errs = a.delete(run, c.Purge(), c) }) @@ -308,7 +326,7 @@ func (a *App) Delete(c DeleteConfigProvider) error { func (a *App) Destroy(c DestroyConfigProvider) error { return a.ForEachState(func(run *Run) (ok bool, errs []error) { - err := run.withPreparedCharts(false, "destroy", func() { + err := run.withPreparedCharts(false, true, "destroy", func() { ok, errs = a.delete(run, true, c) }) @@ -321,14 +339,14 @@ func (a *App) Destroy(c DestroyConfigProvider) error { } func (a *App) Test(c TestConfigProvider) error { - return a.ForEachStateFiltered(func(run *Run) (errs []error) { + return a.ForEachState(func(run *Run) (_ bool, errs []error) { if c.Cleanup() && run.helm.IsHelm3() { a.Logger.Warnf("warn: requested cleanup will not be applied. " + "To clean up test resources with Helm 3, you have to remove them manually " + "or set helm.sh/hook-delete-policy\n") } - err := run.withPreparedCharts(false, "test", func() { + err := run.withPreparedCharts(false, true, "test", func() { errs = run.Test(c) }) @@ -337,18 +355,18 @@ func (a *App) Test(c TestConfigProvider) error { } return - }) + }, SetFilter(true)) } func (a *App) PrintState(c StateConfigProvider) error { - return a.VisitDesiredStatesWithReleasesFiltered(a.FileOrDir, func(st *state.HelmState) (errs []error) { - err := NewRun(st, a.getHelm(st), NewContext()).withPreparedCharts(false, "build", func() { - state, err := st.ToYaml() + return a.ForEachState(func(run *Run) (_ bool, errs []error) { + err := run.withPreparedCharts(false, true, "build", func() { + state, err := run.state.ToYaml() if err != nil { errs = []error{err} return } - fmt.Printf("---\n# Source: %s\n\n%+v", st.FilePath, state) + fmt.Printf("---\n# Source: %s\n\n%+v", run.state.FilePath, state) errs = []error{} }) @@ -358,17 +376,17 @@ func (a *App) PrintState(c StateConfigProvider) error { } return - }) + }, SetFilter(true)) } func (a *App) ListReleases(c ListConfigProvider) error { var releases []*HelmRelease - err := a.VisitDesiredStatesWithReleasesFiltered(a.FileOrDir, func(st *state.HelmState) []error { - err := NewRun(st, a.getHelm(st), NewContext()).withPreparedCharts(false, "list", func() { + err := a.ForEachState(func(run *Run) (_ bool, errs []error) { + err := run.withPreparedCharts(false, true, "list", func() { //var releases m - for _, r := range st.Releases { + for _, r := range run.state.Releases { labels := "" for k, v := range r.Labels { labels = fmt.Sprintf("%s,%s:%s", labels, k, v) @@ -384,14 +402,12 @@ func (a *App) ListReleases(c ListConfigProvider) error { } }) - var errs []error - if err != nil { errs = append(errs, err) } - return errs - }) + return + }, SetFilter(true)) if err != nil { return err @@ -656,20 +672,6 @@ func (a *App) visitStates(fileOrDir string, defOpts LoadOpts, converge func(*sta return nil } -func (a *App) ForEachStateFiltered(do func(*Run) []error) error { - ctx := NewContext() - - err := a.VisitDesiredStatesWithReleasesFiltered(a.FileOrDir, func(st *state.HelmState) []error { - helm := a.getHelm(st) - - run := NewRun(st, helm, ctx) - - return do(run) - }) - - return err -} - type LoadOption func(o *LoadOpts) var ( @@ -684,6 +686,12 @@ var ( o.RetainValuesFiles = true } } + + SetFilter = func(f bool) func(o *LoadOpts) { + return func(o *LoadOpts) { + o.Filter = f + } + } ) func (a *App) ForEachState(do func(*Run) (bool, []error), o ...LoadOption) error { @@ -797,7 +805,17 @@ func (a *App) visitStatesWithSelectorsAndRemoteSupport(fileOrDir string, converg a.remote = remote.NewRemote(a.Logger, dir, a.readFile, a.directoryExistsAt, a.fileExistsAt) - return a.visitStates(fileOrDir, opts, converge) + f := converge + if opts.Filter { + f = func(st *state.HelmState) (bool, []error) { + return processFilteredReleases(st, a.getHelm(st), func(st *state.HelmState) []error { + _, err := converge(st) + return err + }) + } + } + + return a.visitStates(fileOrDir, opts, f) } func processFilteredReleases(st *state.HelmState, helm helmexec.Interface, converge func(st *state.HelmState) []error) (bool, []error) { @@ -845,12 +863,6 @@ 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, a.getHelm(st), converge) - }, o...) -} - func (a *App) findDesiredStateFiles(specifiedPath string, opts LoadOpts) ([]string, error) { path, err := a.remote.Locate(specifiedPath) if err != nil { @@ -913,13 +925,30 @@ func (a *App) findDesiredStateFiles(specifiedPath string, opts LoadOpts) ([]stri return files, nil } +func (a *App) getSelectedReleases(r *Run) ([]state.ReleaseSpec, error) { + releases, err := r.state.GetSelectedReleasesWithOverrides() + if err != nil { + return nil, err + } + + var extra string + + if len(r.state.Selectors) > 0 { + extra = " matching " + strings.Join(r.state.Selectors, ",") + } + + a.Logger.Debugf("%d release(s)%s found in %s\n", len(releases), extra, r.state.FilePath) + + return releases, nil +} + func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { st := r.state helm := r.helm allReleases := st.GetReleasesWithOverrides() - toApply, err := st.GetSelectedReleasesWithOverrides() + toApply, err := a.getSelectedReleases(r) if err != nil { return false, false, []error{err} } @@ -1039,7 +1068,7 @@ func (a *App) delete(r *Run, purge bool, c DestroyConfigProvider) (bool, []error affectedReleases := state.AffectedReleases{} - toSync, err := st.GetSelectedReleasesWithOverrides() + toSync, err := a.getSelectedReleases(r) if err != nil { return false, []error{err} } @@ -1106,7 +1135,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { allReleases := st.GetReleasesWithOverrides() - toSync, err := st.GetSelectedReleasesWithOverrides() + toSync, err := a.getSelectedReleases(r) if err != nil { return false, []error{err} } @@ -1228,7 +1257,7 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) { allReleases := st.GetReleasesWithOverrides() - toRender, err := st.GetSelectedReleasesWithOverrides() + toRender, err := a.getSelectedReleases(r) if err != nil { return false, []error{err} } diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index e3df3ce5..859a583d 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "fmt" + "github.com/google/go-cmp/cmp" "github.com/roboll/helmfile/pkg/remote" "io" "log" @@ -90,19 +91,21 @@ releases: Logger: helmexec.NewLogger(os.Stderr, "debug"), Namespace: "", Env: "default", + FileOrDir: "helmfile.yaml", } expectNoCallsToHelm(app) app = injectFs(app, fs) actualOrder := []string{} - noop := func(st *state.HelmState) []error { - actualOrder = append(actualOrder, st.FilePath) - return []error{} + noop := func(run *Run) (bool, []error) { + actualOrder = append(actualOrder, run.state.FilePath) + return false, []error{} } - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", noop, + err := app.ForEachState( + noop, + SetFilter(true), ) if err != nil { t.Errorf("unexpected error: %v", err) @@ -114,6 +117,10 @@ releases: } } +func Noop(_ *Run) (bool, []error) { + return false, []error{} +} + func TestVisitDesiredStatesWithReleasesFiltered_EnvValuesFileOrder(t *testing.T) { files := map[string]string{ "/path/to/helmfile.yaml": ` @@ -140,17 +147,16 @@ BAZ: 4 Logger: helmexec.NewLogger(os.Stderr, "debug"), Namespace: "", Env: "default", + FileOrDir: "helmfile.yaml", } expectNoCallsToHelm(app) app = injectFs(app, fs) - noop := func(st *state.HelmState) []error { - return []error{} - } - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", noop, + err := app.ForEachState( + Noop, + SetFilter(true), ) if err != nil { t.Errorf("unexpected error: %v", err) @@ -182,17 +188,16 @@ releases: Logger: helmexec.NewLogger(os.Stderr, "debug"), Namespace: "", Env: "default", + FileOrDir: "helmfile.yaml", } expectNoCallsToHelm(app) app = injectFs(app, fs) - noop := func(st *state.HelmState) []error { - return []error{} - } - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", noop, + err := app.ForEachState( + Noop, + SetFilter(true), ) if err == nil { t.Fatal("expected error did not occur") @@ -232,12 +237,10 @@ releases: expectNoCallsToHelm(app) app = injectFs(app, fs) - noop := func(st *state.HelmState) []error { - return []error{} - } - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", noop, + err := app.ForEachState( + Noop, + SetFilter(true), ) if err != nil { t.Errorf("unexpected error: %v", err) @@ -279,17 +282,16 @@ releases: Logger: helmexec.NewLogger(os.Stderr, "debug"), Namespace: "", Env: "default", + FileOrDir: "helmfile.yaml", } expectNoCallsToHelm(app) app = injectFs(app, fs) - noop := func(st *state.HelmState) []error { - return []error{} - } - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", noop, + err := app.ForEachState( + Noop, + SetFilter(true), ) if testcase.expectErr && err == nil { t.Fatal("expected error did not occur") @@ -346,17 +348,16 @@ releases: Selectors: []string{fmt.Sprintf("name=%s", testcase.name)}, Namespace: "", Env: "default", + FileOrDir: "helmfile.yaml", } expectNoCallsToHelm(app) app = injectFs(app, fs) - noop := func(st *state.HelmState) []error { - return []error{} - } - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", noop, + err := app.ForEachState( + Noop, + SetFilter(true), ) if testcase.expectErr && err == nil { t.Errorf("error expected but not happened for name=%s", testcase.name) @@ -385,9 +386,6 @@ releases: chart: stable/zipkin `, } - noop := func(st *state.HelmState) []error { - return []error{} - } testcases := []struct { name string @@ -406,12 +404,14 @@ releases: Namespace: "", Selectors: []string{}, Env: testcase.name, + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", noop, + err := app.ForEachState( + Noop, + SetFilter(true), ) if testcase.expectErr && err == nil { t.Errorf("error expected but not happened for environment=%s", testcase.name) @@ -495,11 +495,11 @@ releases: t.Run(testcase.label, func(t *testing.T) { actual := []string{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { actual = append(actual, r.Name) } - return []error{} + return false, []error{} } app := appWithFs(&App{ @@ -509,12 +509,14 @@ releases: Namespace: "", Selectors: []string{testcase.label}, Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) if testcase.expectErr { if err == nil { @@ -734,11 +736,11 @@ func runFilterSubHelmFilesTests(testcases []struct { for _, testcase := range testcases { actual := []string{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { actual = append(actual, r.Name) } - return []error{} + return false, []error{} } app := appWithFs(&App{ @@ -748,12 +750,14 @@ func runFilterSubHelmFilesTests(testcases []struct { Namespace: "", Selectors: []string{testcase.label}, Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) if testcase.expectErr { if err == nil { @@ -832,21 +836,23 @@ tillerNs: INLINE_TILLER_NS_2 Namespace: "", Selectors: []string{}, Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) processed := []state.ReleaseSpec{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { processed = append(processed, r) } - return []error{} + return false, []error{} } - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) if err != nil { @@ -927,11 +933,11 @@ releases: for _, testcase := range testcases { actual := []string{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { actual = append(actual, r.Name) } - return []error{} + return false, []error{} } app := appWithFs(&App{ OverrideHelmBinary: DefaultHelmBinary, @@ -940,13 +946,15 @@ releases: Namespace: "", Selectors: []string{}, Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, SetReverse(testcase.reverse), + SetFilter(true), ) if err != nil { t.Errorf("unexpected error: %v", err) @@ -988,11 +996,11 @@ bar: "bar1" for _, testcase := range testcases { actual := []string{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { actual = append(actual, r.Name) } - return []error{} + return false, []error{} } app := appWithFs(&App{ OverrideHelmBinary: DefaultHelmBinary, @@ -1003,12 +1011,14 @@ bar: "bar1" Env: "default", ValuesFiles: []string{"overrides.yaml"}, Set: map[string]interface{}{"bar": "bar2", "baz": "baz1"}, + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1108,11 +1118,11 @@ x: actual := []state.ReleaseSpec{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { actual = append(actual, r) } - return []error{} + return false, []error{} } app := appWithFs(&App{ OverrideHelmBinary: DefaultHelmBinary, @@ -1123,12 +1133,14 @@ x: Env: testcase.env, ValuesFiles: []string{"overrides.yaml"}, Set: map[string]interface{}{"x": map[string]interface{}{"hoge": "hoge_set", "fuga": "fuga_set"}}, + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1160,11 +1172,11 @@ releases: actual := []state.ReleaseSpec{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { actual = append(actual, r) } - return []error{} + return false, []error{} } app := appWithFs(&App{ OverrideHelmBinary: DefaultHelmBinary, @@ -1173,12 +1185,14 @@ releases: Namespace: "", Env: "default", Selectors: []string{}, + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1215,11 +1229,11 @@ releases: actual := []state.ReleaseSpec{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { actual = append(actual, r) } - return []error{} + return false, []error{} } app := appWithFs(&App{ OverrideHelmBinary: DefaultHelmBinary, @@ -1228,12 +1242,14 @@ releases: Namespace: "", Selectors: []string{}, Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1264,11 +1280,11 @@ releases: actual := []state.ReleaseSpec{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { actual = append(actual, r) } - return []error{} + return false, []error{} } app := appWithFs(&App{ OverrideHelmBinary: DefaultHelmBinary, @@ -1276,12 +1292,14 @@ releases: Logger: helmexec.NewLogger(os.Stderr, "debug"), Namespace: "", Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelmVersion(app, false) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) expected := "in ./helmfile.yaml: duplicate release \"foo\" found in \"\": there were 2 releases named \"foo\" matching specified selector" @@ -1310,11 +1328,11 @@ releases: actual := []state.ReleaseSpec{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { actual = append(actual, r) } - return []error{} + return false, []error{} } app := appWithFs(&App{ OverrideHelmBinary: DefaultHelmBinary, @@ -1322,12 +1340,14 @@ releases: Logger: helmexec.NewLogger(os.Stderr, "debug"), Namespace: "", Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelmVersion(app, false) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) if err != nil { @@ -1351,11 +1371,11 @@ releases: actual := []state.ReleaseSpec{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { actual = append(actual, r) } - return []error{} + return false, []error{} } app := appWithFs(&App{ OverrideHelmBinary: DefaultHelmBinary, @@ -1363,12 +1383,14 @@ releases: Logger: helmexec.NewLogger(os.Stderr, "debug"), Namespace: "", Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelmVersion(app, true) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) if err != nil { @@ -1392,11 +1414,11 @@ releases: actual := []state.ReleaseSpec{} - collectReleases := func(st *state.HelmState) []error { - for _, r := range st.Releases { + collectReleases := func(run *Run) (bool, []error) { + for _, r := range run.state.Releases { actual = append(actual, r) } - return []error{} + return false, []error{} } app := appWithFs(&App{ OverrideHelmBinary: DefaultHelmBinary, @@ -1404,12 +1426,14 @@ releases: Logger: helmexec.NewLogger(os.Stderr, "debug"), Namespace: "", Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelmVersion(app, true) - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) expected := "in ./helmfile.yaml: duplicate release \"foo\" found in \"foo\": there were 2 releases named \"foo\" matching specified selector" @@ -2150,8 +2174,9 @@ services: } type configImpl struct { - set []string - output string + set []string + output string + skipDeps bool } func (c configImpl) Set() []string { @@ -2171,7 +2196,7 @@ func (c configImpl) Validate() bool { } func (c configImpl) SkipDeps() bool { - return true + return c.skipDeps } func (c configImpl) OutputDir() string { @@ -2293,6 +2318,7 @@ type listKey struct { type mockHelmExec struct { templated []mockTemplates + repos []mockRepo updateDepsCallbacks map[string]func(string) error } @@ -2302,6 +2328,10 @@ type mockTemplates struct { flags []string } +type mockRepo struct { + Name string +} + func (helm *mockHelmExec) TemplateRelease(name, chart string, flags ...string) error { helm.templated = append(helm.templated, mockTemplates{name: name, chart: chart, flags: flags}) return nil @@ -2322,6 +2352,7 @@ func (helm *mockHelmExec) SetHelmBinary(bin string) { return } func (helm *mockHelmExec) AddRepo(name, repository, cafile, certfile, keyfile, username, password string) error { + helm.repos = append(helm.repos, mockRepo{Name: name}) return nil } func (helm *mockHelmExec) UpdateRepo() error { @@ -2371,11 +2402,23 @@ func (helm *mockHelmExec) IsVersionAtLeast(major int, minor int) bool { func TestTemplate_SingleStateFile(t *testing.T) { files := map[string]string{ "/path/to/helmfile.yaml": ` +repositories: +- name: stable + url: https://kubernetes-charts.storage.googleapis.com +- name: stable2 + url: https://kubernetes-charts.storage.googleapis.com + releases: - name: myrelease1 chart: stable/mychart1 + labels: + group: one - name: myrelease2 chart: stable/mychart2 + labels: + group: one +- name: myrelease3 + chart: stable2/mychart3 `, } @@ -2385,6 +2428,10 @@ releases: {name: "myrelease2", chart: "stable/mychart2", flags: []string{"--namespace", "testNamespace", "--set", "foo=a", "--set", "bar=b", "--output-dir", "output/subdir/helmfile-[a-z0-9]{8}-myrelease2"}}, } + var wantRepos = []mockRepo{ + {Name: "stable"}, + } + var buffer bytes.Buffer logger := helmexec.NewLogger(&buffer, "debug") @@ -2405,12 +2452,19 @@ releases: }, Namespace: "testNamespace", valsRuntime: valsRuntime, + Selectors: []string{ + "group=one", + }, }, files) - if err := app.Template(configImpl{set: []string{"foo=a", "bar=b"}}); err != nil { + if err := app.Template(configImpl{set: []string{"foo=a", "bar=b"}, skipDeps: false}); err != nil { t.Fatalf("%v", err) } + if diff := cmp.Diff(wantRepos, helm.repos); diff != "" { + t.Errorf("unexpected add repo:\n%s", diff) + } + for i := range wantReleases { if wantReleases[i].name != helm.templated[i].name { t.Errorf("name = [%v], want %v", helm.templated[i].name, wantReleases[i].name) @@ -2428,7 +2482,6 @@ releases: t.Errorf("HelmState.TemplateReleases() = [%v], want %v", helm.templated[i].flags[j], wantReleases[i].flags[j]) } } - } } @@ -3511,15 +3564,9 @@ GROUP RELEASES 3 default/my-release processing releases in group 1/3: kube-system/kubernetes-external-secrets -0 release(s) matching app=test found in helmfile.yaml - processing releases in group 2/3: default/external-secrets -1 release(s) matching app=test found in helmfile.yaml - getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault--deployed--failed--pending} processing releases in group 3/3: default/my-release -1 release(s) matching app=test found in helmfile.yaml - getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault--deployed--failed--pending} UPDATED RELEASES: @@ -4082,18 +4129,20 @@ releases: OverrideKubeContext: "default", Logger: helmexec.NewLogger(os.Stderr, "debug"), Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) var specs []state.ReleaseSpec - collectReleases := func(st *state.HelmState) []error { - specs = append(specs, st.Releases...) - return nil + collectReleases := func(run *Run) (bool, []error) { + specs = append(specs, run.state.Releases...) + return false, nil } - err := app.VisitDesiredStatesWithReleasesFiltered( - "helmfile.yaml", collectReleases, + err := app.ForEachState( + collectReleases, + SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/pkg/app/context.go b/pkg/app/context.go index 9864ce06..592bcba2 100644 --- a/pkg/app/context.go +++ b/pkg/app/context.go @@ -5,40 +5,21 @@ import ( ) type Context struct { - updatedRepos map[string]struct{} + updatedRepos map[string]bool } func NewContext() Context { return Context{ - updatedRepos: map[string]struct{}{}, + updatedRepos: map[string]bool{}, } } -func (ctx Context) SyncReposOnce(st *state.HelmState, helm state.RepoUpdater) []error { - var errs []error +func (ctx Context) SyncReposOnce(st *state.HelmState, helm state.RepoUpdater) error { + updated, err := st.SyncRepos(helm, ctx.updatedRepos) - hasInstalled := false - for _, release := range st.Releases { - hasInstalled = hasInstalled || release.Installed == nil || *release.Installed + for _, r := range updated { + ctx.updatedRepos[r] = true } - if !hasInstalled { - return errs - } - - allUpdated := true - for _, r := range st.Repositories { - _, exists := ctx.updatedRepos[r.Name] - allUpdated = allUpdated && exists - } - - if !allUpdated { - errs = st.SyncRepos(helm) - - for _, r := range st.Repositories { - ctx.updatedRepos[r.Name] = struct{}{} - } - } - - return errs + return err } diff --git a/pkg/app/load_opts.go b/pkg/app/load_opts.go index 589e593a..7b3b2ae2 100644 --- a/pkg/app/load_opts.go +++ b/pkg/app/load_opts.go @@ -15,6 +15,8 @@ type LoadOpts struct { CalleePath string Reverse bool + + Filter bool } func (o LoadOpts) DeepCopy() LoadOpts { diff --git a/pkg/app/run.go b/pkg/app/run.go index 68fe79f5..2898caa0 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -36,26 +36,18 @@ func (r *Run) askForConfirmation(msg string) bool { return AskForConfirmation(msg) } -func (r *Run) withReposAndPreparedCharts(forceDownload bool, skipRepos bool, helmfileCommand string, f func()) []error { - if !skipRepos { - ctx := r.ctx - if errs := ctx.SyncReposOnce(r.state, r.helm); errs != nil && len(errs) > 0 { - return errs - } - } - - if err := r.withPreparedCharts(forceDownload, helmfileCommand, f); err != nil { - return []error{err} - } - - return nil -} - -func (r *Run) withPreparedCharts(forceDownload bool, helmfileCommand string, f func()) error { +func (r *Run) withPreparedCharts(forceDownload, skipRepos bool, helmfileCommand string, f func()) error { if r.ReleaseToChart != nil { panic("Run.PrepareCharts can be called only once") } + if !skipRepos { + ctx := r.ctx + if err := ctx.SyncReposOnce(r.state, r.helm); err != nil { + return err + } + } + // Create tmp directory and bail immediately if it fails dir, err := ioutil.TempDir("", "") if err != nil { @@ -94,7 +86,7 @@ func (r *Run) Deps(c DepsConfigProvider) []error { return r.state.UpdateDeps(r.helm) } -func (r *Run) Repos(c ReposConfigProvider) []error { +func (r *Run) Repos(c ReposConfigProvider) error { r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...) return r.ctx.SyncReposOnce(r.state, r.helm) @@ -118,13 +110,13 @@ func (r *Run) Status(c StatusesConfigProvider) []error { return r.state.ReleaseStatuses(r.helm, workers) } -func (r *Run) Diff(c DiffConfigProvider) (*string, bool, bool, []error) { +func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error) { st := r.state helm := r.helm allReleases := st.GetReleasesWithOverrides() - toDiff, err := st.GetSelectedReleasesWithOverrides() + toDiff, err := a.getSelectedReleases(r) if err != nil { return nil, false, false, []error{err} } diff --git a/pkg/state/state.go b/pkg/state/state.go index aa717f3e..ac3bd7b0 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -299,24 +299,57 @@ type RepoUpdater interface { UpdateRepo() error } -// SyncRepos will update the given helm releases -func (st *HelmState) SyncRepos(helm RepoUpdater) []error { - errs := []error{} +// getRepositoriesToSync returns the names of repositories to be updated +func (st *HelmState) getRepositoriesToSync() (map[string]bool, error) { + releases, err := st.GetSelectedReleasesWithOverrides() + if err != nil { + return nil, err + } - for _, repo := range st.Repositories { - if err := helm.AddRepo(repo.Name, repo.URL, repo.CaFile, repo.CertFile, repo.KeyFile, repo.Username, repo.Password); err != nil { - errs = append(errs, err) + repositoriesToUpdate := map[string]bool{} + + if len(releases) == 0 { + for _, repo := range st.Repositories { + repositoriesToUpdate[repo.Name] = true + } + + return repositoriesToUpdate, nil + } + + for _, release := range releases { + if release.Installed == nil || *release.Installed { + chart := strings.Split(release.Chart, "/") + if len(chart) == 1 { + continue + } + repositoriesToUpdate[chart[0]] = true } } - if len(errs) != 0 { - return errs + return repositoriesToUpdate, nil +} + +func (st *HelmState) SyncRepos(helm RepoUpdater, shouldSkip map[string]bool) ([]string, error) { + shouldUpdate, err := st.getRepositoriesToSync() + if err != nil { + return nil, err } - if err := helm.UpdateRepo(); err != nil { - return []error{err} + var updated []string + + for _, repo := range st.Repositories { + if !shouldUpdate[repo.Name] || shouldSkip[repo.Name] { + continue + } + + if err := helm.AddRepo(repo.Name, repo.URL, repo.CaFile, repo.CertFile, repo.KeyFile, repo.Username, repo.Password); err != nil { + return nil, err + } + + updated = append(updated, repo.Name) } - return nil + + return updated, nil } type syncResult struct { @@ -1462,14 +1495,6 @@ func (st *HelmState) GetSelectedReleasesWithOverrides() ([]ReleaseSpec, error) { } } - var extra string - - if len(st.Selectors) > 0 { - extra = " matching " + strings.Join(st.Selectors, ",") - } - - st.logger.Debugf("%d release(s)%s found in %s\n", len(releases), extra, st.FilePath) - return releases, nil } diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 840d71e3..b4e65b53 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -923,7 +923,7 @@ func TestHelmState_SyncRepos(t *testing.T) { state := &HelmState{ Repositories: tt.repos, } - if _ = state.SyncRepos(tt.helm); !reflect.DeepEqual(tt.helm.Repo, tt.want) { + if _, _ = state.SyncRepos(tt.helm, map[string]bool{}); !reflect.DeepEqual(tt.helm.Repo, tt.want) { t.Errorf("HelmState.SyncRepos() for [%s] = %v, want %v", tt.name, tt.helm.Repo, tt.want) } })