diff --git a/pkg/app/app.go b/pkg/app/app.go index a0a8ed1c..2c8d5bfa 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -98,13 +98,13 @@ func Init(app *App) *App { } func (a *App) Deps(c DepsConfigProvider) error { - return a.ForEachState(func(run *Run) []error { + return a.ForEachStateFiltered(func(run *Run) []error { return run.Deps(c) }) } func (a *App) Repos(c ReposConfigProvider) error { - return a.ForEachState(func(run *Run) []error { + return a.ForEachStateFiltered(func(run *Run) []error { return run.Repos(c) }) } @@ -116,68 +116,68 @@ func (a *App) reverse() *App { } func (a *App) DeprecatedSyncCharts(c DeprecatedChartsConfigProvider) error { - return a.ForEachState(func(run *Run) []error { + return a.ForEachStateFiltered(func(run *Run) []error { return run.DeprecatedSyncCharts(c) }) } func (a *App) Diff(c DiffConfigProvider) error { - return a.ForEachState(func(run *Run) []error { + return a.ForEachStateFiltered(func(run *Run) []error { return run.Diff(c) }) } func (a *App) Template(c TemplateConfigProvider) error { - return a.ForEachState(func(run *Run) []error { + return a.ForEachStateFiltered(func(run *Run) []error { return run.Template(c) }) } func (a *App) Lint(c LintConfigProvider) error { - return a.ForEachState(func(run *Run) []error { + return a.ForEachStateFiltered(func(run *Run) []error { return run.Lint(c) }) } func (a *App) Sync(c SyncConfigProvider) error { - return a.ForEachState(func(run *Run) []error { + return a.ForEachStateFiltered(func(run *Run) []error { return run.Sync(c) }) } func (a *App) Apply(c ApplyConfigProvider) error { - return a.ForEachState(func(run *Run) []error { + return a.ForEachState(func(run *Run) (bool, []error) { return a.apply(run, c) }) } func (a *App) Status(c StatusesConfigProvider) error { - return a.ForEachState(func(run *Run) []error { + return a.ForEachStateFiltered(func(run *Run) []error { return run.Status(c) }) } func (a *App) Delete(c DeleteConfigProvider) error { - return a.reverse().ForEachState(func(run *Run) []error { + return a.reverse().ForEachStateFiltered(func(run *Run) []error { return run.Delete(c) }, true) } func (a *App) Destroy(c DestroyConfigProvider) error { - return a.reverse().ForEachState(func(run *Run) []error { + return a.reverse().ForEachStateFiltered(func(run *Run) []error { return run.Destroy(c) }, true) } func (a *App) Test(c TestConfigProvider) error { - return a.ForEachState(func(run *Run) []error { + return a.ForEachStateFiltered(func(run *Run) []error { return run.Test(c) }) } func (a *App) PrintState(c StateConfigProvider) error { - return a.ForEachState(func(run *Run) []error { + return a.ForEachStateFiltered(func(run *Run) []error { state, err := run.state.ToYaml() if err != nil { return []error{err} @@ -191,7 +191,7 @@ func (a *App) ListReleases(c StateConfigProvider) error { table := uitable.New() table.AddRow("NAME", "NAMESPACE", "INSTALLED", "LABELS") - err := a.ForEachState(func(run *Run) []error { + err := a.ForEachStateFiltered(func(run *Run) []error { //var releases m for _, r := range run.state.Releases { labels := "" @@ -399,7 +399,7 @@ func (a *App) visitStates(fileOrDir string, defOpts LoadOpts, converge func(*sta return nil } -func (a *App) ForEachState(do func(*Run) []error, dagEnabled ...bool) error { +func (a *App) ForEachStateFiltered(do func(*Run) []error, dagEnabled ...bool) error { ctx := NewContext() err := a.VisitDesiredStatesWithReleasesFiltered(a.FileOrDir, func(st *state.HelmState, helm helmexec.Interface) []error { run := NewRun(st, helm, ctx) @@ -414,6 +414,20 @@ func (a *App) ForEachState(do func(*Run) []error, dagEnabled ...bool) error { return err } +func (a *App) ForEachState(do func(*Run) (bool, []error)) error { + ctx := NewContext() + err := a.visitStatesWithSelectorsAndRemoteSupport(a.FileOrDir, func(st *state.HelmState, helm helmexec.Interface) (bool, []error) { + run := NewRun(st, helm, ctx) + return do(run) + }) + + if err != nil && a.ErrorHandler != nil { + return a.ErrorHandler(err) + } + + return err +} + func printBatches(batches [][]state.Release) string { buf := &bytes.Buffer{} @@ -644,7 +658,7 @@ func (a *App) findDesiredStateFiles(specifiedPath string) ([]string, error) { return files, nil } -func (a *App) apply(r *Run, c ApplyConfigProvider) []error { +func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, []error) { st := r.state helm := r.helm ctx := r.ctx @@ -652,14 +666,14 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) []error { affectedReleases := state.AffectedReleases{} if !c.SkipDeps() { if errs := ctx.SyncReposOnce(st, helm); errs != nil && len(errs) > 0 { - return errs + return false, errs } if errs := st.BuildDeps(helm); errs != nil && len(errs) > 0 { - return errs + return false, errs } } if errs := st.PrepareReleases(helm, "apply"); errs != nil && len(errs) > 0 { - return errs + return false, errs } // helm must be 2.11+ and helm-diff should be provided `--detailed-exitcode` in order for `helmfile apply` to work properly @@ -671,29 +685,53 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) []error { Set: c.Set(), } - changedReleases, errs := st.DiffReleases(helm, c.Values(), c.Concurrency(), detailedExitCode, c.SuppressSecrets(), false, diffOpts) + allReleases := st.Releases - deletingReleases, err := st.DetectReleasesToBeDeleted(helm, st.Releases) - if err != nil { - errs = append(errs, err) + var changedReleases []state.ReleaseSpec + var deletingReleases []state.ReleaseSpec + var planningErrs []error + + // TODO Better way to detect diff on only filtered releases + { + if len(st.Selectors) > 0 { + releases, err := st.GetFilteredReleases() + if err != nil { + return false, []error{err} + } + if len(releases) == 0 { + return false, nil + } + st.Releases = releases + } + + changedReleases, planningErrs = st.DiffReleases(helm, c.Values(), c.Concurrency(), detailedExitCode, c.SuppressSecrets(), false, diffOpts) + + var err error + deletingReleases, err = st.DetectReleasesToBeDeleted(helm, st.Releases) + if err != nil { + planningErrs = append(planningErrs, err) + } } + st.Releases = allReleases + fatalErrs := []error{} - noError := true - for _, e := range errs { + for _, e := range planningErrs { switch err := e.(type) { case *state.ReleaseError: if err.Code != 2 { - noError = false fatalErrs = append(fatalErrs, e) } default: - noError = false fatalErrs = append(fatalErrs, e) } } + if len(fatalErrs) > 0 { + return false, fatalErrs + } + releasesToBeDeleted := map[string]state.ReleaseSpec{} for _, r := range deletingReleases { id := state.ReleaseToID(&r) @@ -711,90 +749,90 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) []error { } // sync only when there are changes - if noError { - if len(releasesToBeUpdated) == 0 && len(releasesToBeDeleted) == 0 { - // TODO better way to get the logger - logger := c.Logger() - logger.Infof("") - logger.Infof("No affected releases") - } else { - names := []string{} - for _, r := range releasesToBeUpdated { - names = append(names, fmt.Sprintf(" %s (%s) UPDATED", r.Name, r.Chart)) - } - for _, r := range releasesToBeDeleted { - names = append(names, fmt.Sprintf(" %s (%s) DELETED", r.Name, r.Chart)) - } - // Make the output deterministic for testing purpose - sort.Strings(names) + if len(releasesToBeUpdated) == 0 && len(releasesToBeDeleted) == 0 { + // TODO better way to get the logger + logger := c.Logger() + logger.Infof("") + logger.Infof("No affected releases") + return true, nil + } - infoMsg := fmt.Sprintf(`Affected releases are: + names := []string{} + for _, r := range releasesToBeUpdated { + names = append(names, fmt.Sprintf(" %s (%s) UPDATED", r.Name, r.Chart)) + } + for _, r := range releasesToBeDeleted { + names = append(names, fmt.Sprintf(" %s (%s) DELETED", r.Name, r.Chart)) + } + // Make the output deterministic for testing purpose + sort.Strings(names) + + infoMsg := fmt.Sprintf(`Affected releases are: %s `, strings.Join(names, "\n")) - confMsg := fmt.Sprintf(`%s + confMsg := fmt.Sprintf(`%s Do you really want to apply? Helmfile will apply all your changes, as shown above. `, infoMsg) - interactive := c.Interactive() - if !interactive { - a.Logger.Debug(infoMsg) + interactive := c.Interactive() + if !interactive { + a.Logger.Debug(infoMsg) + } + + syncErrs := []error{} + + if !interactive || interactive && r.askForConfirmation(confMsg) { + r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...) + + // We deleted releases by traversing the DAG in reverse order + if len(releasesToBeDeleted) > 0 { + _, deletionErrs := withDAG(st, helm, a.Logger, true, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { + var rs []state.ReleaseSpec + + for _, r := range subst.Releases { + if _, ok := releasesToBeDeleted[state.ReleaseToID(&r)]; ok { + rs = append(rs, r) + } + } + + subst.Releases = rs + + return subst.DeleteReleasesForSync(&affectedReleases, helm, c.Concurrency()) + })) + + if deletionErrs != nil && len(deletionErrs) > 0 { + syncErrs = append(syncErrs, deletionErrs...) } - if !interactive || interactive && r.askForConfirmation(confMsg) { - r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...) + } - // We deleted releases by traversing the DAG in reverse order - if len(releasesToBeDeleted) > 0 { - _, errs := withDAG(st, helm, a.Logger, true, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { - var rs []state.ReleaseSpec + // We upgrade releases by traversing the DAG + if len(releasesToBeUpdated) > 0 { + _, updateErrs := withDAG(st, helm, a.Logger, false, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { + var rs []state.ReleaseSpec - for _, r := range subst.Releases { - if _, ok := releasesToBeDeleted[state.ReleaseToID(&r)]; ok { - rs = append(rs, r) - } - } - - subst.Releases = rs - - return subst.DeleteReleasesForSync(&affectedReleases, helm, c.Concurrency()) - })) - - if errs != nil && len(errs) > 0 { - return errs + for _, r := range subst.Releases { + if _, ok := releasesToBeUpdated[state.ReleaseToID(&r)]; ok { + rs = append(rs, r) } } - // We upgrade releases by traversing the DAG - if len(releasesToBeUpdated) > 0 { - _, errs := withDAG(st, helm, a.Logger, false, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { - var rs []state.ReleaseSpec + subst.Releases = rs - for _, r := range subst.Releases { - if _, ok := releasesToBeUpdated[state.ReleaseToID(&r)]; ok { - rs = append(rs, r) - } - } - - subst.Releases = rs - - syncOpts := state.SyncOpts{ - Set: c.Set(), - } - return subst.SyncReleases(&affectedReleases, helm, c.Values(), c.Concurrency(), &syncOpts) - })) - - if errs != nil && len(errs) > 0 { - return errs - } + syncOpts := state.SyncOpts{ + Set: c.Set(), } + return subst.SyncReleases(&affectedReleases, helm, c.Values(), c.Concurrency(), &syncOpts) + })) - return nil + if updateErrs != nil && len(updateErrs) > 0 { + syncErrs = append(syncErrs, updateErrs...) } } } affectedReleases.DisplayAffectedReleases(c.Logger()) - return fatalErrs + return true, syncErrs } func fileExistsAt(path string) bool { diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 16d93e80..fbf94adf 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2061,6 +2061,7 @@ func TestApply(t *testing.T) { concurrency int error string files map[string]string + selectors []string lists map[exectest.ListKey]string diffs map[exectest.DiffKey]error upgraded []exectest.Release @@ -2361,6 +2362,22 @@ worker 1/1 finished worker 1/1 started getting deployed release version failed:unexpected list key: {^frontend-v3$ --kube-contextdefault} worker 1/1 finished + +UPDATED RELEASES: +NAME CHART VERSION +front-proxy stable/envoy +logging charts/fluent-bit +database charts/mysql +servicemesh charts/istio +anotherbackend charts/anotherbackend +backend-v2 charts/backend +frontend-v3 charts/frontend + + +DELETED RELEASES: +NAME +frontend-v1 +backend-v1 `, }, // @@ -2487,6 +2504,13 @@ worker 1/1 finished worker 1/1 started getting deployed release version failed:unexpected list key: {^foo$ --kube-contextdefault} worker 1/1 finished + +UPDATED RELEASES: +NAME CHART VERSION +bar mychart2 +baz mychart3 +foo mychart1 + `, }, // @@ -2756,6 +2780,12 @@ worker 1/1 finished worker 1/1 started getting deployed release version failed:unexpected list key: {^bar$ --tiller-namespacetns2--kube-contextdefault} worker 1/1 finished + +UPDATED RELEASES: +NAME CHART VERSION +foo mychart1 +bar mychart2 + `, }, // @@ -2967,6 +2997,251 @@ bar 4 Fri Nov 1 08:40:07 2019 DEPLOYED mychart2-3.1.0 3.1.0 defau }, }, // + // upgrades with selector + // + { + // see https://github.com/roboll/helmfile/issues/919#issuecomment-549831747 + name: "upgrades with good selector", + loc: location(), + files: map[string]string{ + "/path/to/helmfile.yaml": ` +{{ $mark := "a" }} + +releases: +- name: kubernetes-external-secrets + chart: incubator/raw + namespace: kube-system + +- name: external-secrets + chart: incubator/raw + namespace: default + labels: + app: test + needs: + - kube-system/kubernetes-external-secrets + +- name: my-release + chart: incubator/raw + namespace: default + labels: + app: test + needs: + - default/external-secrets +`, + }, + selectors: []string{"app=test"}, + diffs: map[exectest.DiffKey]error{ + exectest.DiffKey{Name: "external-secrets", Chart: "incubator/raw", Flags: "--kube-contextdefault--namespacedefault--detailed-exitcode"}: helmexec.ExitError{Code: 2}, + exectest.DiffKey{Name: "my-release", Chart: "incubator/raw", Flags: "--kube-contextdefault--namespacedefault--detailed-exitcode"}: helmexec.ExitError{Code: 2}, + }, + upgraded: []exectest.Release{ + {Name: "external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, + {Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, + }, + // as we check for log output, set concurrency to 1 to avoid non-deterministic test result + concurrency: 1, + log: `processing file "helmfile.yaml" in directory "." +first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode= +first-pass uses: &{default map[] map[]} +first-pass rendering output of "helmfile.yaml.part.0": + 0: + 1: + 2: + 3: releases: + 4: - name: kubernetes-external-secrets + 5: chart: incubator/raw + 6: namespace: kube-system + 7: + 8: - name: external-secrets + 9: chart: incubator/raw +10: namespace: default +11: labels: +12: app: test +13: needs: +14: - kube-system/kubernetes-external-secrets +15: +16: - name: my-release +17: chart: incubator/raw +18: namespace: default +19: labels: +20: app: test +21: needs: +22: - default/external-secrets +23: + +first-pass produced: &{default map[] map[]} +first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]} +vals: +map[] +defaultVals:[] +second-pass rendering result of "helmfile.yaml.part.0": + 0: + 1: + 2: + 3: releases: + 4: - name: kubernetes-external-secrets + 5: chart: incubator/raw + 6: namespace: kube-system + 7: + 8: - name: external-secrets + 9: chart: incubator/raw +10: namespace: default +11: labels: +12: app: test +13: needs: +14: - kube-system/kubernetes-external-secrets +15: +16: - name: my-release +17: chart: incubator/raw +18: namespace: default +19: labels: +20: app: test +21: needs: +22: - default/external-secrets +23: + +merged environment: &{default map[] map[]} +worker 1/1 started +worker 1/1 finished +worker 1/1 started +worker 1/1 finished +Affected releases are: + external-secrets (incubator/raw) UPDATED + my-release (incubator/raw) UPDATED + +processing 3 groups of releases in this order: +GROUP RELEASES +1 kube-system/kubernetes-external-secrets +2 default/external-secrets +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 + +worker 1/1 started +worker 1/1 finished +worker 1/1 started +getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault} +worker 1/1 finished +processing releases in group 3/3: default/my-release +1 release(s) matching app=test found in helmfile.yaml + +worker 1/1 started +worker 1/1 finished +worker 1/1 started +getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault} +worker 1/1 finished + +UPDATED RELEASES: +NAME CHART VERSION +external-secrets incubator/raw +my-release incubator/raw + +`, + }, + { + // see https://github.com/roboll/helmfile/issues/919#issuecomment-549831747 + name: "upgrades with bad selector", + loc: location(), + files: map[string]string{ + "/path/to/helmfile.yaml": ` +{{ $mark := "a" }} + +releases: +- name: kubernetes-external-secrets + chart: incubator/raw + namespace: kube-system + +- name: external-secrets + chart: incubator/raw + namespace: default + labels: + app: test + needs: + - kube-system/kubernetes-external-secrets + +- name: my-release + chart: incubator/raw + namespace: default + labels: + app: test + needs: + - default/external-secrets +`, + }, + selectors: []string{"app=test_non_existent"}, + diffs: map[exectest.DiffKey]error{}, + upgraded: []exectest.Release{}, + error: "err: no releases found that matches specified selector(app=test_non_existent) and environment(default), in any helmfile", + // as we check for log output, set concurrency to 1 to avoid non-deterministic test result + concurrency: 1, + log: `processing file "helmfile.yaml" in directory "." +first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode= +first-pass uses: &{default map[] map[]} +first-pass rendering output of "helmfile.yaml.part.0": + 0: + 1: + 2: + 3: releases: + 4: - name: kubernetes-external-secrets + 5: chart: incubator/raw + 6: namespace: kube-system + 7: + 8: - name: external-secrets + 9: chart: incubator/raw +10: namespace: default +11: labels: +12: app: test +13: needs: +14: - kube-system/kubernetes-external-secrets +15: +16: - name: my-release +17: chart: incubator/raw +18: namespace: default +19: labels: +20: app: test +21: needs: +22: - default/external-secrets +23: + +first-pass produced: &{default map[] map[]} +first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]} +vals: +map[] +defaultVals:[] +second-pass rendering result of "helmfile.yaml.part.0": + 0: + 1: + 2: + 3: releases: + 4: - name: kubernetes-external-secrets + 5: chart: incubator/raw + 6: namespace: kube-system + 7: + 8: - name: external-secrets + 9: chart: incubator/raw +10: namespace: default +11: labels: +12: app: test +13: needs: +14: - kube-system/kubernetes-external-secrets +15: +16: - name: my-release +17: chart: incubator/raw +18: namespace: default +19: labels: +20: app: test +21: needs: +22: - default/external-secrets +23: + +merged environment: &{default map[] map[]} +`, + }, + // // error cases // { @@ -3101,6 +3376,10 @@ err: "foo" has dependency to inexistent release "bar" app.Namespace = tc.ns } + if tc.selectors != nil { + app.Selectors = tc.selectors + } + applyErr := app.Apply(applyConfig{ // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. concurrency: tc.concurrency, diff --git a/pkg/state/state.go b/pkg/state/state.go index e9191545..2f0d781e 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1258,11 +1258,10 @@ func MarkFilteredReleases(releases []ReleaseSpec, selectors []string) ([]Release return filteredReleases, nil } -// FilterReleases allows for the execution of helm commands against a subset of the releases in the helmfile. -func (st *HelmState) FilterReleases() error { +func (st *HelmState) GetFilteredReleases() ([]ReleaseSpec, error) { filteredReleases, err := MarkFilteredReleases(st.Releases, st.Selectors) if err != nil { - return err + return nil, err } var releases []ReleaseSpec for _, r := range filteredReleases { @@ -1270,8 +1269,17 @@ func (st *HelmState) FilterReleases() error { releases = append(releases, r.ReleaseSpec) } } + return releases, nil +} + +// FilterReleases allows for the execution of helm commands against a subset of the releases in the helmfile. +func (st *HelmState) FilterReleases() error { + releases, err := st.GetFilteredReleases() + if err != nil { + return err + } st.Releases = releases - st.logger.Debugf("%d release(s) matching %s found in %s\n", len(filteredReleases), strings.Join(st.Selectors, ","), st.FilePath) + st.logger.Debugf("%d release(s) matching %s found in %s\n", len(releases), strings.Join(st.Selectors, ","), st.FilePath) return nil } @@ -1852,9 +1860,9 @@ func renderValsSecrets(e vals.Evaluator, input ...string) ([]string, error) { // DisplayAffectedReleases logs the upgraded, deleted and in error releases func (ar *AffectedReleases) DisplayAffectedReleases(logger *zap.SugaredLogger) { - if ar.Upgraded != nil { - logger.Info("\nList of updated releases :") - tbl, _ := prettytable.NewTable(prettytable.Column{Header: "RELEASE"}, + if ar.Upgraded != nil && len(ar.Upgraded) > 0 { + logger.Info("\nUPDATED RELEASES:") + tbl, _ := prettytable.NewTable(prettytable.Column{Header: "NAME"}, prettytable.Column{Header: "CHART", MinWidth: 6}, prettytable.Column{Header: "VERSION", AlignRight: true}, ) @@ -1862,18 +1870,18 @@ func (ar *AffectedReleases) DisplayAffectedReleases(logger *zap.SugaredLogger) { for _, release := range ar.Upgraded { tbl.AddRow(release.Name, release.Chart, release.installedVersion) } - tbl.Print() + logger.Info(tbl.String()) } - if ar.Deleted != nil { - logger.Info("\nList of deleted releases :") - logger.Info("RELEASE") + if ar.Deleted != nil && len(ar.Deleted) > 0 { + logger.Info("\nDELETED RELEASES:") + logger.Info("NAME") for _, release := range ar.Deleted { logger.Info(release.Name) } } - if ar.Failed != nil { - logger.Info("\nList of releases in error :") - logger.Info("RELEASE") + if ar.Failed != nil && len(ar.Failed) > 0 { + logger.Info("\nFAILED RELEASES:") + logger.Info("NAME") for _, release := range ar.Failed { logger.Info(release.Name) }