From 5d43b30a7ca34587e5205bc711235c5fb58ee313 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Tue, 20 Apr 2021 23:06:51 +0900 Subject: [PATCH] Add --{include,skip}-needs to various helmfile commands (#1772) * Add --{include,skip}-needs to helmfile-sync and helmfile-apply * Add --include-needs to helmfile-template * Add TODO related to #1018 * Add a few new test files to cover new functionalities * Update apply test to incorporate the change that the destroy and sync steps target affected releases only --- go.mod | 2 +- go.sum | 2 + main.go | 36 ++ pkg/app/app.go | 177 ++++-- pkg/app/app_apply_test.go | 1064 +++++++++++++++++++++++++++++++++++++ pkg/app/app_sync_test.go | 790 +++++++++++++++++++++++++++ pkg/app/app_test.go | 181 ++++++- pkg/app/config.go | 10 + pkg/app/diff_test.go | 142 ++++- pkg/state/state_run.go | 74 ++- 10 files changed, 2384 insertions(+), 94 deletions(-) create mode 100644 pkg/app/app_apply_test.go create mode 100644 pkg/app/app_sync_test.go diff --git a/go.mod b/go.mod index 215d6b6d..be06756a 100644 --- a/go.mod +++ b/go.mod @@ -29,7 +29,7 @@ require ( github.com/tatsushid/go-prettytable v0.0.0-20141013043238-ed2d14c29939 github.com/urfave/cli v1.22.5 github.com/variantdev/chartify v0.8.0 - github.com/variantdev/dag v0.0.0-20191028002400-bb0b3c785363 + github.com/variantdev/dag v1.0.0 github.com/variantdev/vals v0.13.0 go.uber.org/multierr v1.6.0 go.uber.org/zap v1.16.0 diff --git a/go.sum b/go.sum index 6c0a7662..349e1dbe 100644 --- a/go.sum +++ b/go.sum @@ -585,6 +585,8 @@ github.com/variantdev/chartify v0.8.0 h1:yIBsS/dIUeMjWP8U6JWlT3PM0Lky7en9QBi+MgD github.com/variantdev/chartify v0.8.0/go.mod h1:qF4XzQlkfH/6k2jAi1hLas+lK4zSCa8kY+r5JdmLA68= github.com/variantdev/dag v0.0.0-20191028002400-bb0b3c785363 h1:KrfQBEUn+wEOQ/6UIfoqRDvn+Q/wZridQ7t0G1vQqKE= github.com/variantdev/dag v0.0.0-20191028002400-bb0b3c785363/go.mod h1:pH1TQsNSLj2uxMo9NNl9zdGy01Wtn+/2MT96BrKmVyE= +github.com/variantdev/dag v1.0.0 h1:7SFjATxHtrYV20P3tx53yNDBMegz6RT4jv8JPHqAHdM= +github.com/variantdev/dag v1.0.0/go.mod h1:pH1TQsNSLj2uxMo9NNl9zdGy01Wtn+/2MT96BrKmVyE= github.com/variantdev/vals v0.13.0 h1:zdtTBjoWKkUGdFauxETkDVjqWXdjUNwI+ggWcUmpxv8= github.com/variantdev/vals v0.13.0/go.mod h1:pBwm+vPLQALN6otkNqiT1fUKdWHfjAm4070UkrNLsVA= github.com/vektra/mockery v1.1.2/go.mod h1:VcfZjKaFOPO+MpN4ZvwPjs4c48lkq1o3Ym8yHZJu0jU= diff --git a/main.go b/main.go index ed4185f3..0ab52a06 100644 --- a/main.go +++ b/main.go @@ -199,6 +199,14 @@ func main() { Name: "include-tests", Usage: "enable the diffing of the helm test hooks", }, + cli.BoolFlag{ + Name: "skip-needs", + Usage: `do not automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when when --selector/-l flag is not provided`, + }, + cli.BoolFlag{ + Name: "include-needs", + Usage: `automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when when --selector/-l flag is not provided`, + }, cli.BoolFlag{ Name: "suppress-secrets", Usage: "suppress secrets in the output. highly recommended to specify on CI/CD use-cases", @@ -260,6 +268,10 @@ func main() { Name: "include-crds", Usage: "include CRDs in the templated output", }, + cli.BoolFlag{ + Name: "include-needs", + Usage: `automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when when --selector/-l flag is not provided`, + }, cli.BoolFlag{ Name: "skip-deps", Usage: `skip running "helm repo update" and "helm dependency build"`, @@ -386,6 +398,14 @@ func main() { Name: "skip-deps", Usage: `skip running "helm repo update" and "helm dependency build"`, }, + cli.BoolFlag{ + Name: "skip-needs", + Usage: `do not automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when when --selector/-l flag is not provided`, + }, + cli.BoolFlag{ + Name: "include-needs", + Usage: `automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when when --selector/-l flag is not provided`, + }, cli.BoolFlag{ Name: "wait", Usage: `Override helmDefaults.wait setting "helm upgrade --install --wait"`, @@ -442,6 +462,14 @@ func main() { Name: "skip-crds", Usage: "if set, no CRDs will be installed on sync. By default, CRDs are installed if not already present", }, + cli.BoolFlag{ + Name: "skip-needs", + Usage: `do not automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when when --selector/-l flag is not provided`, + }, + cli.BoolFlag{ + Name: "include-needs", + Usage: `automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when when --selector/-l flag is not provided`, + }, cli.BoolFlag{ Name: "skip-diff-on-install", Usage: "Skips running helm-diff on releases being newly installed on this apply. Useful when the release manifests are too huge to be reviewed, or it's too time-consuming to diff at all", @@ -710,6 +738,14 @@ func (c configImpl) HasCommandName(name string) bool { return c.c.Command.HasName(name) } +func (c configImpl) SkipNeeds() bool { + return c.c.Bool("skip-needs") +} + +func (c configImpl) IncludeNeeds() bool { + return c.c.Bool("include-needs") +} + // DiffConfig func (c configImpl) SkipDeps() bool { diff --git a/pkg/app/app.go b/pkg/app/app.go index 61207796..638c118a 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -868,12 +868,16 @@ func printBatches(batches [][]state.Release) string { return buf.String() } -func withDAG(templated *state.HelmState, helm helmexec.Interface, logger *zap.SugaredLogger, reverse bool, converge func(*state.HelmState, helmexec.Interface) (bool, []error)) (bool, []error) { - batches, err := templated.PlanReleases(reverse) +func withDAG(templated *state.HelmState, helm helmexec.Interface, logger *zap.SugaredLogger, opts state.PlanOptions, converge func(*state.HelmState, helmexec.Interface) (bool, []error)) (bool, []error) { + batches, err := templated.PlanReleases(opts) if err != nil { return false, []error{err} } + return withBatches(templated, batches, helm, logger, converge) +} + +func withBatches(templated *state.HelmState, batches [][]state.Release, helm helmexec.Interface, logger *zap.SugaredLogger, converge func(*state.HelmState, helmexec.Interface) (bool, []error)) (bool, []error) { numBatches := len(batches) logger.Debugf("processing %d groups of releases in this order:\n%s", numBatches, printBatches(batches)) @@ -966,12 +970,24 @@ func processFilteredReleases(st *state.HelmState, helm helmexec.Interface, conve } } + if err := checkDuplicates(helm, st, st.Releases); err != nil { + return false, []error{err} + } + + errs := converge(st) + + processed := len(st.Releases) != 0 && len(errs) == 0 + + return processed, errs +} + +func checkDuplicates(helm helmexec.Interface, st *state.HelmState, releases []state.ReleaseSpec) error { type Key struct { TillerNamespace, Name, KubeContext string } releaseNameCounts := map[Key]int{} - for _, r := range st.Releases { + for _, r := range releases { namespace := r.Namespace if !helm.IsHelm3() { if r.TillerNamespace != "" { @@ -994,15 +1010,11 @@ func processFilteredReleases(st *state.HelmState, helm helmexec.Interface, conve msg += fmt.Sprintf(" in kubecontext %q", name.KubeContext) } - return false, []error{fmt.Errorf("duplicate release %q found%s: there were %d releases named \"%s\" matching specified selector", name.Name, msg, c, name.Name)} + return fmt.Errorf("duplicate release %q found%s: there were %d releases named \"%s\" matching specified selector", name.Name, msg, c, name.Name) } } - errs := converge(st) - - processed := len(st.Releases) != 0 && len(errs) == 0 - - return processed, errs + return nil } func (a *App) Wrap(converge func(*state.HelmState, helmexec.Interface) []error) func(st *state.HelmState, helm helmexec.Interface) (bool, []error) { @@ -1013,6 +1025,14 @@ func (a *App) Wrap(converge func(*state.HelmState, helmexec.Interface) []error) } } +func (a *App) WrapWithoutSelector(converge func(*state.HelmState, helmexec.Interface) []error) func(st *state.HelmState, helm helmexec.Interface) (bool, []error) { + return func(st *state.HelmState, helm helmexec.Interface) (bool, []error) { + errs := converge(st, helm) + processed := len(st.Releases) != 0 && len(errs) == 0 + return processed, errs + } +} + func (a *App) findDesiredStateFiles(specifiedPath string, opts LoadOpts) ([]string, error) { path, err := a.remote.Locate(specifiedPath) if err != nil { @@ -1081,6 +1101,10 @@ func (a *App) getSelectedReleases(r *Run) ([]state.ReleaseSpec, error) { return nil, err } + if err := checkDuplicates(r.helm, r.state, releases); err != nil { + return nil, err + } + var extra string if len(r.state.Selectors) > 0 { @@ -1106,9 +1130,22 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { return false, false, nil } + plan, err := st.PlanReleases(state.PlanOptions{Reverse: false, SelectedReleases: toApply, SkipNeeds: c.SkipNeeds(), IncludeNeeds: c.IncludeNeeds()}) + if err != nil { + return false, false, []error{err} + } + + var toApplyWithNeeds []state.ReleaseSpec + + for _, rs := range plan { + for _, r := range rs { + toApplyWithNeeds = append(toApplyWithNeeds, r.ReleaseSpec) + } + } + // Do build deps and prepare only on selected releases so that we won't waste time // on running various helm commands on unnecessary releases - st.Releases = toApply + st.Releases = toApplyWithNeeds // helm must be 2.11+ and helm-diff should be provided `--detailed-exitcode` in order for `helmfile apply` to work properly detailedExitCode := true @@ -1126,11 +1163,21 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { return false, false, errs } + var toDelete []state.ReleaseSpec + for _, r := range releasesToBeDeleted { + toDelete = append(toDelete, r) + } + + var toUpdate []state.ReleaseSpec + for _, r := range releasesToBeUpdated { + toUpdate = append(toUpdate, r) + } + releasesWithNoChange := map[string]state.ReleaseSpec{} - for _, r := range toApply { + for _, r := range toApplyWithNeeds { id := state.ReleaseToID(&r) - _, uninstalled := releasesToBeUpdated[id] - _, updated := releasesToBeDeleted[id] + _, uninstalled := releasesToBeDeleted[id] + _, updated := releasesToBeUpdated[id] if !uninstalled && !updated { releasesWithNoChange[id] = r } @@ -1174,7 +1221,7 @@ Do you really want to apply? // 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 { + _, deletionErrs := withDAG(st, helm, a.Logger, state.PlanOptions{Reverse: true, SelectedReleases: toDelete, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { var rs []state.ReleaseSpec for _, r := range subst.Releases { @@ -1195,7 +1242,7 @@ Do you really want to apply? // 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 { + _, updateErrs := withDAG(st, helm, a.Logger, state.PlanOptions{SelectedReleases: toUpdate, Reverse: false, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { var rs []state.ReleaseSpec for _, r := range subst.Releases { @@ -1286,7 +1333,7 @@ Do you really want to delete? r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...) if len(releasesToDelete) > 0 { - _, deletionErrs := withDAG(st, helm, a.Logger, true, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { + _, deletionErrs := withDAG(st, helm, a.Logger, state.PlanOptions{Reverse: true, SkipNeeds: true}, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { var rs []state.ReleaseSpec for _, r := range subst.Releases { @@ -1338,13 +1385,22 @@ func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error) // Validate all releases for missing `needs` targets st.Releases = allReleases - if _, err := st.PlanReleases(false); err != nil { + plan, err := st.PlanReleases(state.PlanOptions{Reverse: false, SelectedReleases: toDiff, SkipNeeds: c.SkipNeeds(), IncludeNeeds: c.IncludeNeeds()}) + if err != nil { return nil, false, false, []error{err} } + var toDiffWithNeeds []state.ReleaseSpec + + for _, rs := range plan { + for _, r := range rs { + toDiffWithNeeds = append(toDiffWithNeeds, r.ReleaseSpec) + } + } + // Diff only targeted releases - st.Releases = toDiff + st.Releases = toDiffWithNeeds filtered := &Run{ state: st, @@ -1400,7 +1456,7 @@ func (a *App) lint(r *Run, c LintConfigProvider) (bool, []error) { } if len(releasesToRender) > 0 { - _, templateErrs := withDAG(st, helm, a.Logger, false, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { + _, templateErrs := withDAG(st, helm, a.Logger, state.PlanOptions{Reverse: false, SkipNeeds: true}, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { var rs []state.ReleaseSpec for _, r := range subst.Releases { @@ -1466,7 +1522,7 @@ func (a *App) status(r *Run, c StatusesConfigProvider) (bool, []error) { } if len(releasesToRender) > 0 { - _, templateErrs := withDAG(st, helm, a.Logger, false, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { + _, templateErrs := withDAG(st, helm, a.Logger, state.PlanOptions{Reverse: false, SkipNeeds: true}, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { var rs []state.ReleaseSpec for _, r := range subst.Releases { @@ -1501,11 +1557,24 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { return false, nil } + batches, err := st.PlanReleases(state.PlanOptions{Reverse: false, SelectedReleases: toSync, IncludeNeeds: c.IncludeNeeds(), SkipNeeds: c.SkipNeeds()}) + if err != nil { + return false, []error{err} + } + + var toSyncWithNeeds []state.ReleaseSpec + + for _, rs := range batches { + for _, r := range rs { + toSyncWithNeeds = append(toSyncWithNeeds, r.ReleaseSpec) + } + } + // Do build deps and prepare only on selected releases so that we won't waste time // on running various helm commands on unnecessary releases st.Releases = toSync - toDelete, err := st.DetectReleasesToBeDeletedForSync(helm, toSync) + toDelete, err := st.DetectReleasesToBeDeletedForSync(helm, toSyncWithNeeds) if err != nil { return false, []error{err} } @@ -1517,9 +1586,15 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { } var toUpdate []state.ReleaseSpec - for _, r := range toSync { + for _, r := range toSyncWithNeeds { if _, deleted := releasesToDelete[state.ReleaseToID(&r)]; !deleted { - toUpdate = append(toUpdate, r) + if r.Installed == nil || *r.Installed { + toUpdate = append(toUpdate, r) + } else { + // TODO Emit error when the user opted to fail when the needed release is disabled, + // instead of silently ignoring it. + // See https://github.com/roboll/helmfile/issues/1018 + } } } @@ -1530,7 +1605,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { } releasesWithNoChange := map[string]state.ReleaseSpec{} - for _, r := range toSync { + for _, r := range toSyncWithNeeds { id := state.ReleaseToID(&r) _, uninstalled := releasesToDelete[id] _, updated := releasesToUpdate[id] @@ -1572,7 +1647,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { affectedReleases := state.AffectedReleases{} if len(releasesToDelete) > 0 { - _, deletionErrs := withDAG(st, helm, a.Logger, true, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { + _, deletionErrs := withDAG(st, helm, a.Logger, state.PlanOptions{Reverse: true, SelectedReleases: toDelete, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { var rs []state.ReleaseSpec for _, r := range subst.Releases { @@ -1592,12 +1667,12 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { } if len(releasesToUpdate) > 0 { - _, syncErrs := withDAG(st, helm, a.Logger, false, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error { + _, syncErrs := withDAG(st, helm, a.Logger, state.PlanOptions{SelectedReleases: toUpdate, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { var rs []state.ReleaseSpec for _, r := range subst.Releases { - if r2, ok := releasesToUpdate[state.ReleaseToID(&r)]; ok { - rs = append(rs, r2) + if _, ok := releasesToDelete[state.ReleaseToID(&r)]; !ok { + rs = append(rs, r) } } @@ -1626,25 +1701,37 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) { allReleases := st.GetReleasesWithOverrides() - toRender, err := a.getSelectedReleases(r) + selectedReleases, err := a.getSelectedReleases(r) if err != nil { return false, []error{err} } - if len(toRender) == 0 { + if len(selectedReleases) == 0 { return false, nil } - // Do build deps and prepare only on selected releases so that we won't waste time - // on running various helm commands on unnecessary releases - st.Releases = toRender + batches, err := st.PlanReleases(state.PlanOptions{Reverse: false, SelectedReleases: selectedReleases, IncludeNeeds: c.IncludeNeeds(), SkipNeeds: !c.IncludeNeeds()}) + if err != nil { + return false, []error{err} + } - releasesToRender := map[string]state.ReleaseSpec{} - for _, r := range toRender { + var selectedReleasesWithNeeds []state.ReleaseSpec + + for _, rs := range batches { + for _, r := range rs { + selectedReleasesWithNeeds = append(selectedReleasesWithNeeds, r.ReleaseSpec) + } + } + + var toRender []state.ReleaseSpec + + releasesDisabled := map[string]state.ReleaseSpec{} + for _, r := range selectedReleasesWithNeeds { id := state.ReleaseToID(&r) if r.Installed != nil && !*r.Installed { - continue + releasesDisabled[id] = r + } else { + toRender = append(toRender, r) } - releasesToRender[id] = r } var errs []error @@ -1661,18 +1748,8 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) { helm.SetExtraArgs(args...) } - if len(releasesToRender) > 0 { - _, templateErrs := 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 r2, ok := releasesToRender[state.ReleaseToID(&r)]; ok { - rs = append(rs, r2) - } - } - - subst.Releases = rs - + if len(toRender) > 0 { + _, templateErrs := withDAG(st, helm, a.Logger, state.PlanOptions{SelectedReleases: toRender, Reverse: false, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { opts := &state.TemplateOpts{ Set: c.Set(), IncludeCRDs: c.IncludeCRDs(), @@ -1682,7 +1759,7 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) { return subst.TemplateReleases(helm, c.OutputDir(), c.Values(), args, c.Concurrency(), c.Validate(), opts) })) - if templateErrs != nil && len(templateErrs) > 0 { + if len(templateErrs) > 0 { errs = append(errs, templateErrs...) } } diff --git a/pkg/app/app_apply_test.go b/pkg/app/app_apply_test.go new file mode 100644 index 00000000..cd9eb288 --- /dev/null +++ b/pkg/app/app_apply_test.go @@ -0,0 +1,1064 @@ +package app + +import ( + "bufio" + "bytes" + "io" + "path/filepath" + "sync" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/roboll/helmfile/pkg/exectest" + "github.com/roboll/helmfile/pkg/helmexec" + "github.com/roboll/helmfile/pkg/testhelper" + "github.com/variantdev/vals" +) + +func TestApply_2(t *testing.T) { + type fields struct { + skipNeeds bool + includeNeeds bool + } + + type testcase struct { + fields fields + ns string + concurrency int + skipDiffOnInstall bool + error string + files map[string]string + selectors []string + lists map[exectest.ListKey]string + diffs map[exectest.DiffKey]error + upgraded []exectest.Release + deleted []exectest.Release + log string + } + + check := func(t *testing.T, tc testcase) { + t.Helper() + + wantUpgrades := tc.upgraded + wantDeletes := tc.deleted + + var helm = &exectest.Helm{ + FailOnUnexpectedList: true, + FailOnUnexpectedDiff: true, + Lists: tc.lists, + Diffs: tc.diffs, + DiffMutex: &sync.Mutex{}, + ChartsMutex: &sync.Mutex{}, + ReleasesMutex: &sync.Mutex{}, + } + + bs := &bytes.Buffer{} + + func() { + t.Helper() + + logReader, logWriter := io.Pipe() + + logFlushed := &sync.WaitGroup{} + // Ensure all the log is consumed into `bs` by calling `logWriter.Close()` followed by `logFlushed.Wait()` + logFlushed.Add(1) + go func() { + scanner := bufio.NewScanner(logReader) + for scanner.Scan() { + bs.Write(scanner.Bytes()) + bs.WriteString("\n") + } + logFlushed.Done() + }() + + defer func() { + // This is here to avoid data-trace on bytes buffer `bs` to capture logs + if err := logWriter.Close(); err != nil { + panic(err) + } + logFlushed.Wait() + }() + + logger := helmexec.NewLogger(logWriter, "debug") + + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + if err != nil { + t.Errorf("unexpected error creating vals runtime: %v", err) + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + glob: filepath.Glob, + abs: filepath.Abs, + OverrideKubeContext: "default", + Env: "default", + Logger: logger, + helms: map[helmKey]helmexec.Interface{ + createHelmKey("helm", "default"): helm, + }, + valsRuntime: valsRuntime, + }, tc.files) + + if tc.ns != "" { + app.Namespace = tc.ns + } + + if tc.selectors != nil { + app.Selectors = tc.selectors + } + + syncErr := app.Apply(applyConfig{ + // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. + concurrency: tc.concurrency, + logger: logger, + skipDiffOnInstall: tc.skipDiffOnInstall, + skipNeeds: tc.fields.skipNeeds, + includeNeeds: tc.fields.includeNeeds, + }) + + var gotErr string + if syncErr != nil { + gotErr = syncErr.Error() + } + + if d := cmp.Diff(tc.error, gotErr); d != "" { + t.Fatalf("unexpected error: want (-), got (+): %s", d) + } + + if len(wantUpgrades) > len(helm.Releases) { + t.Fatalf("insufficient number of upgrades: got %d, want %d", len(helm.Releases), len(wantUpgrades)) + } + + for relIdx := range wantUpgrades { + if wantUpgrades[relIdx].Name != helm.Releases[relIdx].Name { + t.Errorf("releases[%d].name: got %q, want %q", relIdx, helm.Releases[relIdx].Name, wantUpgrades[relIdx].Name) + } + for flagIdx := range wantUpgrades[relIdx].Flags { + if wantUpgrades[relIdx].Flags[flagIdx] != helm.Releases[relIdx].Flags[flagIdx] { + t.Errorf("releaes[%d].flags[%d]: got %v, want %v", relIdx, flagIdx, helm.Releases[relIdx].Flags[flagIdx], wantUpgrades[relIdx].Flags[flagIdx]) + } + } + } + + if len(wantDeletes) > len(helm.Deleted) { + t.Fatalf("insufficient number of deletes: got %d, want %d", len(helm.Deleted), len(wantDeletes)) + } + + for relIdx := range wantDeletes { + if wantDeletes[relIdx].Name != helm.Deleted[relIdx].Name { + t.Errorf("releases[%d].name: got %q, want %q", relIdx, helm.Deleted[relIdx].Name, wantDeletes[relIdx].Name) + } + for flagIdx := range wantDeletes[relIdx].Flags { + if wantDeletes[relIdx].Flags[flagIdx] != helm.Deleted[relIdx].Flags[flagIdx] { + t.Errorf("releaes[%d].flags[%d]: got %v, want %v", relIdx, flagIdx, helm.Deleted[relIdx].Flags[flagIdx], wantDeletes[relIdx].Flags[flagIdx]) + } + } + } + }() + + if tc.log != "" { + actual := bs.String() + + diff, exists := testhelper.Diff(tc.log, actual, 3) + if exists { + t.Errorf("unexpected log:\nDIFF\n%s\nEOD", diff) + } + } + } + + t.Run("skip-needs=true", func(t *testing.T) { + check(t, testcase{ + fields: fields{ + skipNeeds: true, + }, + 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"}, + upgraded: []exectest.Release{ + {Name: "external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, + {Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, + }, + 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}, + }, + // 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[]} +2 release(s) matching app=test found in helmfile.yaml + +Affected releases are: + external-secrets (incubator/raw) UPDATED + my-release (incubator/raw) UPDATED + +processing 2 groups of releases in this order: +GROUP RELEASES +1 default/external-secrets +2 default/my-release + +processing releases in group 1/2: default/external-secrets +getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault--deployed--failed--pending} +processing releases in group 2/2: default/my-release +getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault--deployed--failed--pending} + +UPDATED RELEASES: +NAME CHART VERSION +external-secrets incubator/raw +my-release incubator/raw + +`, + }) + }) + + t.Run("skip-needs=true with no diff on a release", func(t *testing.T) { + check(t, testcase{ + fields: fields{ + skipNeeds: true, + }, + 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"}, + upgraded: []exectest.Release{ + {Name: "external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, + }, + lists: map[exectest.ListKey]string{ + exectest.ListKey{Filter: "^external-secrets$", Flags: "--kube-contextdefault--deployed--failed--pending"}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE +^external-secrets$ 4 Fri Nov 1 08:40:07 2019 DEPLOYED raw-3.1.0 3.1.0 default +`, + }, + 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"}: nil, + }, + // 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[]} +2 release(s) matching app=test found in helmfile.yaml + +Affected releases are: + external-secrets (incubator/raw) UPDATED + +processing 1 groups of releases in this order: +GROUP RELEASES +1 default/external-secrets + +processing releases in group 1/1: default/external-secrets + +UPDATED RELEASES: +NAME CHART VERSION +external-secrets incubator/raw 3.1.0 + +`, + }) + }) + + t.Run("skip-needs=false include-needs=true", func(t *testing.T) { + check(t, testcase{ + fields: fields{ + skipNeeds: false, + includeNeeds: true, + }, + error: ``, + 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"}, + upgraded: []exectest.Release{}, + diffs: map[exectest.DiffKey]error{ + exectest.DiffKey{Name: "kubernetes-external-secrets", Chart: "incubator/raw", Flags: "--kube-contextdefault--namespacekube-system--detailed-exitcode"}: helmexec.ExitError{Code: 2}, + 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}, + }, + // 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[]} +2 release(s) matching app=test found in helmfile.yaml + +Affected releases are: + external-secrets (incubator/raw) UPDATED + kubernetes-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 +getting deployed release version failed:unexpected list key: {^kubernetes-external-secrets$ --kube-contextdefault--deployed--failed--pending} +processing releases in group 2/3: default/external-secrets +getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault--deployed--failed--pending} +processing releases in group 3/3: default/my-release +getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault--deployed--failed--pending} + +UPDATED RELEASES: +NAME CHART VERSION +kubernetes-external-secrets incubator/raw +external-secrets incubator/raw +my-release incubator/raw + +`, + }) + }) + + t.Run("skip-needs=false include-needs=true but no diff on needed release", func(t *testing.T) { + check(t, testcase{ + fields: fields{ + skipNeeds: false, + includeNeeds: true, + }, + error: ``, + 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"}, + upgraded: []exectest.Release{}, + lists: nil, + diffs: map[exectest.DiffKey]error{ + exectest.DiffKey{Name: "kubernetes-external-secrets", Chart: "incubator/raw", Flags: "--kube-contextdefault--namespacekube-system--detailed-exitcode"}: nil, + 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}, + }, + // 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[]} +2 release(s) matching app=test found in helmfile.yaml + +Affected releases are: + external-secrets (incubator/raw) UPDATED + my-release (incubator/raw) UPDATED + +processing 2 groups of releases in this order: +GROUP RELEASES +1 default/external-secrets +2 default/my-release + +processing releases in group 1/2: default/external-secrets +getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault--deployed--failed--pending} +processing releases in group 2/2: default/my-release +getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault--deployed--failed--pending} + +UPDATED RELEASES: +NAME CHART VERSION +external-secrets incubator/raw +my-release incubator/raw + +`, + }) + }) + + t.Run("skip-needs=false include-needs=true with installed but disabled release", func(t *testing.T) { + check(t, testcase{ + fields: fields{ + skipNeeds: false, + includeNeeds: true, + }, + error: ``, + files: map[string]string{ + "/path/to/helmfile.yaml": ` +{{ $mark := "a" }} + +releases: +- name: kubernetes-external-secrets + chart: incubator/raw + namespace: kube-system + installed: false + +- 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"}, + upgraded: []exectest.Release{}, + lists: map[exectest.ListKey]string{ + // delete frontend-v1 and backend-v1 + exectest.ListKey{Filter: "^kubernetes-external-secrets$", Flags: "--kube-contextdefault--deployed--failed--pending"}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE +^kubernetes-external-secrets$ 4 Fri Nov 1 08:40:07 2019 DEPLOYED backend-3.1.0 3.1.0 default +`, + }, + 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}, + }, + // 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: installed: false + 8: + 9: - name: external-secrets +10: chart: incubator/raw +11: namespace: default +12: labels: +13: app: test +14: needs: +15: - kube-system/kubernetes-external-secrets +16: +17: - name: my-release +18: chart: incubator/raw +19: namespace: default +20: labels: +21: app: test +22: needs: +23: - default/external-secrets +24: + +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: installed: false + 8: + 9: - name: external-secrets +10: chart: incubator/raw +11: namespace: default +12: labels: +13: app: test +14: needs: +15: - kube-system/kubernetes-external-secrets +16: +17: - name: my-release +18: chart: incubator/raw +19: namespace: default +20: labels: +21: app: test +22: needs: +23: - default/external-secrets +24: + +merged environment: &{default map[] map[]} +2 release(s) matching app=test found in helmfile.yaml + +Affected releases are: + external-secrets (incubator/raw) UPDATED + kubernetes-external-secrets (incubator/raw) DELETED + my-release (incubator/raw) UPDATED + +processing 1 groups of releases in this order: +GROUP RELEASES +1 kube-system/kubernetes-external-secrets + +processing releases in group 1/1: kube-system/kubernetes-external-secrets +processing 2 groups of releases in this order: +GROUP RELEASES +1 default/external-secrets +2 default/my-release + +processing releases in group 1/2: default/external-secrets +getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault--deployed--failed--pending} +processing releases in group 2/2: default/my-release +getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault--deployed--failed--pending} + +UPDATED RELEASES: +NAME CHART VERSION +external-secrets incubator/raw +my-release incubator/raw + + +DELETED RELEASES: +NAME +kubernetes-external-secrets +`, + }) + }) + + t.Run("skip-needs=false include-needs=true with not installed and disabled release", func(t *testing.T) { + check(t, testcase{ + fields: fields{ + skipNeeds: false, + includeNeeds: true, + }, + error: ``, + files: map[string]string{ + "/path/to/helmfile.yaml": ` +{{ $mark := "a" }} + +releases: +- name: kubernetes-external-secrets + chart: incubator/raw + namespace: kube-system + installed: false + +- 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"}, + upgraded: []exectest.Release{}, + lists: map[exectest.ListKey]string{ + // delete frontend-v1 and backend-v1 + exectest.ListKey{Filter: "^kubernetes-external-secrets$", Flags: "--kube-contextdefault--deployed--failed--pending"}: ``, + }, + 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}, + }, + // 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: installed: false + 8: + 9: - name: external-secrets +10: chart: incubator/raw +11: namespace: default +12: labels: +13: app: test +14: needs: +15: - kube-system/kubernetes-external-secrets +16: +17: - name: my-release +18: chart: incubator/raw +19: namespace: default +20: labels: +21: app: test +22: needs: +23: - default/external-secrets +24: + +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: installed: false + 8: + 9: - name: external-secrets +10: chart: incubator/raw +11: namespace: default +12: labels: +13: app: test +14: needs: +15: - kube-system/kubernetes-external-secrets +16: +17: - name: my-release +18: chart: incubator/raw +19: namespace: default +20: labels: +21: app: test +22: needs: +23: - default/external-secrets +24: + +merged environment: &{default map[] map[]} +2 release(s) matching app=test found in helmfile.yaml + +Affected releases are: + external-secrets (incubator/raw) UPDATED + my-release (incubator/raw) UPDATED + +processing 2 groups of releases in this order: +GROUP RELEASES +1 default/external-secrets +2 default/my-release + +processing releases in group 1/2: default/external-secrets +getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault--deployed--failed--pending} +processing releases in group 2/2: default/my-release +getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault--deployed--failed--pending} + +UPDATED RELEASES: +NAME CHART VERSION +external-secrets incubator/raw +my-release incubator/raw + +`, + }) + }) + + t.Run("bad --selector", func(t *testing.T) { + check(t, testcase{ + 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"}, + 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[]} +0 release(s) matching app=test_non_existent found in helmfile.yaml + +`, + }) + }) +} diff --git a/pkg/app/app_sync_test.go b/pkg/app/app_sync_test.go new file mode 100644 index 00000000..1971e22e --- /dev/null +++ b/pkg/app/app_sync_test.go @@ -0,0 +1,790 @@ +package app + +import ( + "bufio" + "bytes" + "io" + "path/filepath" + "sync" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/roboll/helmfile/pkg/exectest" + "github.com/roboll/helmfile/pkg/helmexec" + "github.com/roboll/helmfile/pkg/testhelper" + "github.com/variantdev/vals" +) + +func TestSync(t *testing.T) { + type fields struct { + skipNeeds bool + includeNeeds bool + } + + type testcase struct { + fields fields + ns string + concurrency int + skipDiffOnInstall bool + error string + files map[string]string + selectors []string + lists map[exectest.ListKey]string + upgraded []exectest.Release + deleted []exectest.Release + log string + } + + check := func(t *testing.T, tc testcase) { + t.Helper() + + wantUpgrades := tc.upgraded + wantDeletes := tc.deleted + + var helm = &exectest.Helm{ + FailOnUnexpectedList: true, + FailOnUnexpectedDiff: true, + Lists: tc.lists, + DiffMutex: &sync.Mutex{}, + ChartsMutex: &sync.Mutex{}, + ReleasesMutex: &sync.Mutex{}, + } + + bs := &bytes.Buffer{} + + func() { + t.Helper() + + logReader, logWriter := io.Pipe() + + logFlushed := &sync.WaitGroup{} + // Ensure all the log is consumed into `bs` by calling `logWriter.Close()` followed by `logFlushed.Wait()` + logFlushed.Add(1) + go func() { + scanner := bufio.NewScanner(logReader) + for scanner.Scan() { + bs.Write(scanner.Bytes()) + bs.WriteString("\n") + } + logFlushed.Done() + }() + + defer func() { + // This is here to avoid data-trace on bytes buffer `bs` to capture logs + if err := logWriter.Close(); err != nil { + panic(err) + } + logFlushed.Wait() + }() + + logger := helmexec.NewLogger(logWriter, "debug") + + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + if err != nil { + t.Errorf("unexpected error creating vals runtime: %v", err) + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + glob: filepath.Glob, + abs: filepath.Abs, + OverrideKubeContext: "default", + Env: "default", + Logger: logger, + helms: map[helmKey]helmexec.Interface{ + createHelmKey("helm", "default"): helm, + }, + valsRuntime: valsRuntime, + }, tc.files) + + if tc.ns != "" { + app.Namespace = tc.ns + } + + if tc.selectors != nil { + app.Selectors = tc.selectors + } + + syncErr := app.Sync(applyConfig{ + // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. + concurrency: tc.concurrency, + logger: logger, + skipDiffOnInstall: tc.skipDiffOnInstall, + skipNeeds: tc.fields.skipNeeds, + includeNeeds: tc.fields.includeNeeds, + }) + + var gotErr string + if syncErr != nil { + gotErr = syncErr.Error() + } + + if d := cmp.Diff(tc.error, gotErr); d != "" { + t.Fatalf("unexpected error: want (-), got (+): %s", d) + } + + if len(wantUpgrades) > len(helm.Releases) { + t.Fatalf("insufficient number of upgrades: got %d, want %d", len(helm.Releases), len(wantUpgrades)) + } + + for relIdx := range wantUpgrades { + if wantUpgrades[relIdx].Name != helm.Releases[relIdx].Name { + t.Errorf("releases[%d].name: got %q, want %q", relIdx, helm.Releases[relIdx].Name, wantUpgrades[relIdx].Name) + } + for flagIdx := range wantUpgrades[relIdx].Flags { + if wantUpgrades[relIdx].Flags[flagIdx] != helm.Releases[relIdx].Flags[flagIdx] { + t.Errorf("releaes[%d].flags[%d]: got %v, want %v", relIdx, flagIdx, helm.Releases[relIdx].Flags[flagIdx], wantUpgrades[relIdx].Flags[flagIdx]) + } + } + } + + if len(wantDeletes) > len(helm.Deleted) { + t.Fatalf("insufficient number of deletes: got %d, want %d", len(helm.Deleted), len(wantDeletes)) + } + + for relIdx := range wantDeletes { + if wantDeletes[relIdx].Name != helm.Deleted[relIdx].Name { + t.Errorf("releases[%d].name: got %q, want %q", relIdx, helm.Deleted[relIdx].Name, wantDeletes[relIdx].Name) + } + for flagIdx := range wantDeletes[relIdx].Flags { + if wantDeletes[relIdx].Flags[flagIdx] != helm.Deleted[relIdx].Flags[flagIdx] { + t.Errorf("releaes[%d].flags[%d]: got %v, want %v", relIdx, flagIdx, helm.Deleted[relIdx].Flags[flagIdx], wantDeletes[relIdx].Flags[flagIdx]) + } + } + } + }() + + if tc.log != "" { + actual := bs.String() + + diff, exists := testhelper.Diff(tc.log, actual, 3) + if exists { + t.Errorf("unexpected log:\nDIFF\n%s\nEOD", diff) + } + } + } + + t.Run("skip-needs=true", func(t *testing.T) { + check(t, testcase{ + fields: fields{ + skipNeeds: true, + }, + 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"}, + 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[]} +2 release(s) matching app=test found in helmfile.yaml + +Affected releases are: + external-secrets (incubator/raw) UPDATED + my-release (incubator/raw) UPDATED + +processing 2 groups of releases in this order: +GROUP RELEASES +1 default/external-secrets +2 default/my-release + +processing releases in group 1/2: default/external-secrets +getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault--deployed--failed--pending} +processing releases in group 2/2: default/my-release +getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault--deployed--failed--pending} + +UPDATED RELEASES: +NAME CHART VERSION +external-secrets incubator/raw +my-release incubator/raw + +`, + }) + }) + + t.Run("skip-needs=false include-needs=true", func(t *testing.T) { + check(t, testcase{ + fields: fields{ + skipNeeds: false, + includeNeeds: true, + }, + error: ``, + 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"}, + upgraded: []exectest.Release{}, + // 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[]} +2 release(s) matching app=test found in helmfile.yaml + +Affected releases are: + external-secrets (incubator/raw) UPDATED + kubernetes-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 +getting deployed release version failed:unexpected list key: {^kubernetes-external-secrets$ --kube-contextdefault--deployed--failed--pending} +processing releases in group 2/3: default/external-secrets +getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault--deployed--failed--pending} +processing releases in group 3/3: default/my-release +getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault--deployed--failed--pending} + +UPDATED RELEASES: +NAME CHART VERSION +kubernetes-external-secrets incubator/raw +external-secrets incubator/raw +my-release incubator/raw + +`, + }) + }) + + t.Run("skip-needs=false include-needs=true with installed but disabled release", func(t *testing.T) { + check(t, testcase{ + fields: fields{ + skipNeeds: false, + includeNeeds: true, + }, + error: ``, + files: map[string]string{ + "/path/to/helmfile.yaml": ` +{{ $mark := "a" }} + +releases: +- name: kubernetes-external-secrets + chart: incubator/raw + namespace: kube-system + installed: false + +- 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"}, + upgraded: []exectest.Release{}, + lists: map[exectest.ListKey]string{ + // delete frontend-v1 and backend-v1 + exectest.ListKey{Filter: "^kubernetes-external-secrets$", Flags: "--kube-contextdefault--deployed--failed--pending"}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE +^kubernetes-external-secrets$ 4 Fri Nov 1 08:40:07 2019 DEPLOYED backend-3.1.0 3.1.0 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: installed: false + 8: + 9: - name: external-secrets +10: chart: incubator/raw +11: namespace: default +12: labels: +13: app: test +14: needs: +15: - kube-system/kubernetes-external-secrets +16: +17: - name: my-release +18: chart: incubator/raw +19: namespace: default +20: labels: +21: app: test +22: needs: +23: - default/external-secrets +24: + +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: installed: false + 8: + 9: - name: external-secrets +10: chart: incubator/raw +11: namespace: default +12: labels: +13: app: test +14: needs: +15: - kube-system/kubernetes-external-secrets +16: +17: - name: my-release +18: chart: incubator/raw +19: namespace: default +20: labels: +21: app: test +22: needs: +23: - default/external-secrets +24: + +merged environment: &{default map[] map[]} +2 release(s) matching app=test found in helmfile.yaml + +Affected releases are: + external-secrets (incubator/raw) UPDATED + kubernetes-external-secrets (incubator/raw) DELETED + my-release (incubator/raw) UPDATED + +processing 1 groups of releases in this order: +GROUP RELEASES +1 kube-system/kubernetes-external-secrets + +processing releases in group 1/1: kube-system/kubernetes-external-secrets +processing 2 groups of releases in this order: +GROUP RELEASES +1 default/external-secrets +2 default/my-release + +processing releases in group 1/2: default/external-secrets +getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault--deployed--failed--pending} +processing releases in group 2/2: default/my-release +getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault--deployed--failed--pending} + +UPDATED RELEASES: +NAME CHART VERSION +external-secrets incubator/raw +my-release incubator/raw + + +DELETED RELEASES: +NAME +kubernetes-external-secrets +`, + }) + }) + + t.Run("skip-needs=false include-needs=true with not installed and disabled release", func(t *testing.T) { + check(t, testcase{ + fields: fields{ + skipNeeds: false, + includeNeeds: true, + }, + error: ``, + files: map[string]string{ + "/path/to/helmfile.yaml": ` +{{ $mark := "a" }} + +releases: +- name: kubernetes-external-secrets + chart: incubator/raw + namespace: kube-system + installed: false + +- 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"}, + upgraded: []exectest.Release{}, + lists: map[exectest.ListKey]string{ + // delete frontend-v1 and backend-v1 + exectest.ListKey{Filter: "^kubernetes-external-secrets$", Flags: "--kube-contextdefault--deployed--failed--pending"}: ``, + }, + // 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: installed: false + 8: + 9: - name: external-secrets +10: chart: incubator/raw +11: namespace: default +12: labels: +13: app: test +14: needs: +15: - kube-system/kubernetes-external-secrets +16: +17: - name: my-release +18: chart: incubator/raw +19: namespace: default +20: labels: +21: app: test +22: needs: +23: - default/external-secrets +24: + +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: installed: false + 8: + 9: - name: external-secrets +10: chart: incubator/raw +11: namespace: default +12: labels: +13: app: test +14: needs: +15: - kube-system/kubernetes-external-secrets +16: +17: - name: my-release +18: chart: incubator/raw +19: namespace: default +20: labels: +21: app: test +22: needs: +23: - default/external-secrets +24: + +merged environment: &{default map[] map[]} +2 release(s) matching app=test found in helmfile.yaml + +Affected releases are: + external-secrets (incubator/raw) UPDATED + my-release (incubator/raw) UPDATED + +processing 2 groups of releases in this order: +GROUP RELEASES +1 default/external-secrets +2 default/my-release + +processing releases in group 1/2: default/external-secrets +getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault--deployed--failed--pending} +processing releases in group 2/2: default/my-release +getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault--deployed--failed--pending} + +UPDATED RELEASES: +NAME CHART VERSION +external-secrets incubator/raw +my-release incubator/raw + +`, + }) + }) + + t.Run("bad --selector", func(t *testing.T) { + check(t, testcase{ + 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"}, + 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[]} +0 release(s) matching app=test_non_existent found in helmfile.yaml + +`, + }) + }) +} diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index c0aef2d4..410acb9e 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2245,6 +2245,9 @@ type configImpl struct { skipCleanup bool skipCRDs bool skipDeps bool + + skipNeeds bool + includeNeeds bool } func (a configImpl) Selectors() []string { @@ -2279,6 +2282,14 @@ func (c configImpl) SkipDeps() bool { return c.skipDeps } +func (c configImpl) SkipNeeds() bool { + return c.skipNeeds +} + +func (c configImpl) IncludeNeeds() bool { + return c.includeNeeds +} + func (c configImpl) OutputDir() string { return "output/subdir" } @@ -2312,6 +2323,8 @@ type applyConfig struct { skipCleanup bool skipCRDs bool skipDeps bool + skipNeeds bool + includeNeeds bool includeTests bool suppressSecrets bool showSecrets bool @@ -2363,6 +2376,14 @@ func (a applyConfig) SkipDeps() bool { return a.skipDeps } +func (c applyConfig) SkipNeeds() bool { + return c.skipNeeds +} + +func (c applyConfig) IncludeNeeds() bool { + return c.includeNeeds +} + func (a applyConfig) IncludeTests() bool { return a.includeTests } @@ -2696,9 +2717,14 @@ releases: } func TestApply(t *testing.T) { + type fields struct { + skipNeeds bool + includeNeeds bool + } testcases := []struct { name string loc string + fields fields ns string concurrency int skipDiffOnInstall bool @@ -2946,26 +2972,20 @@ Affected releases are: logging (charts/fluent-bit) UPDATED servicemesh (charts/istio) UPDATED -processing 5 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES -1 frontend-v1, frontend-v2, frontend-v3 -2 backend-v1, backend-v2 -3 anotherbackend -4 database, servicemesh -5 logging, front-proxy +1 frontend-v1 +2 backend-v1 -processing releases in group 1/5: frontend-v1, frontend-v2, frontend-v3 -processing releases in group 2/5: backend-v1, backend-v2 -processing releases in group 3/5: anotherbackend -processing releases in group 4/5: database, servicemesh -processing releases in group 5/5: logging, front-proxy +processing releases in group 1/2: frontend-v1 +processing releases in group 2/2: backend-v1 processing 5 groups of releases in this order: GROUP RELEASES 1 logging, front-proxy 2 database, servicemesh 3 anotherbackend -4 backend-v1, backend-v2 -5 frontend-v1, frontend-v2, frontend-v3 +4 backend-v2 +5 frontend-v3 processing releases in group 1/5: logging, front-proxy getting deployed release version failed:unexpected list key: {^logging$ --kube-contextdefault--deployed--failed--pending} @@ -2975,9 +2995,9 @@ getting deployed release version failed:unexpected list key: {^database$ --kube- getting deployed release version failed:unexpected list key: {^servicemesh$ --kube-contextdefault--deployed--failed--pending} processing releases in group 3/5: anotherbackend getting deployed release version failed:unexpected list key: {^anotherbackend$ --kube-contextdefault--deployed--failed--pending} -processing releases in group 4/5: backend-v1, backend-v2 +processing releases in group 4/5: backend-v2 getting deployed release version failed:unexpected list key: {^backend-v2$ --kube-contextdefault--deployed--failed--pending} -processing releases in group 5/5: frontend-v1, frontend-v2, frontend-v3 +processing releases in group 5/5: frontend-v3 getting deployed release version failed:unexpected list key: {^frontend-v3$ --kube-contextdefault--deployed--failed--pending} UPDATED RELEASES: @@ -3814,8 +3834,11 @@ bar 4 Fri Nov 1 08:40:07 2019 DEPLOYED mychart2-3.1.0 3.1.0 defau // { // see https://github.com/roboll/helmfile/issues/919#issuecomment-549831747 - name: "upgrades with good selector", + name: "upgrades with good selector with --skip-needs=true", loc: location(), + fields: fields{ + skipNeeds: true, + }, files: map[string]string{ "/path/to/helmfile.yaml": ` {{ $mark := "a" }} @@ -3920,16 +3943,14 @@ Affected releases are: external-secrets (incubator/raw) UPDATED my-release (incubator/raw) UPDATED -processing 3 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES -1 kube-system/kubernetes-external-secrets -2 default/external-secrets -3 default/my-release +1 default/external-secrets +2 default/my-release -processing releases in group 1/3: kube-system/kubernetes-external-secrets -processing releases in group 2/3: default/external-secrets +processing releases in group 1/2: default/external-secrets getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault--deployed--failed--pending} -processing releases in group 3/3: default/my-release +processing releases in group 2/2: default/my-release getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault--deployed--failed--pending} UPDATED RELEASES: @@ -3937,6 +3958,115 @@ NAME CHART VERSION external-secrets incubator/raw my-release incubator/raw +`, + }, + { + // see https://github.com/roboll/helmfile/issues/919#issuecomment-549831747 + name: "upgrades with good selector with --skip-needs=false --include-needs=true", + loc: location(), + fields: fields{ + skipNeeds: false, + includeNeeds: true, + }, + error: `in ./helmfile.yaml: release "default/external-secrets" depends on "kube-system/kubernetes-external-secrets" which does not match the selectors. Please add a selector like "--selector name=kubernetes-external-secrets", or indicate whether to skip (--skip-needs) or include (--include-needs) these dependencies`, + 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{}, + // 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[]} +2 release(s) matching app=test found in helmfile.yaml + +err: release "default/external-secrets" depends on "kube-system/kubernetes-external-secrets" which does not match the selectors. Please add a selector like "--selector name=kubernetes-external-secrets", or indicate whether to skip (--skip-needs) or include (--include-needs) these dependencies `, }, { @@ -4102,10 +4232,6 @@ second-pass rendering result of "helmfile.yaml.part.0": merged environment: &{default map[] map[]} 2 release(s) found in helmfile.yaml -Affected releases are: - baz (mychart3) UPDATED - foo (mychart1) UPDATED - err: "foo" depends on nonexistent release "bar" `, }, @@ -4185,6 +4311,7 @@ err: "foo" depends on nonexistent release "bar" concurrency: tc.concurrency, logger: logger, skipDiffOnInstall: tc.skipDiffOnInstall, + skipNeeds: tc.fields.skipNeeds, }) if tc.error == "" && applyErr != nil { t.Fatalf("unexpected error for data defined at %s: %v", tc.loc, applyErr) diff --git a/pkg/app/config.go b/pkg/app/config.go index c22574fd..358a2eec 100644 --- a/pkg/app/config.go +++ b/pkg/app/config.go @@ -59,6 +59,9 @@ type ApplyConfigProvider interface { SkipCleanup() bool SkipDiffOnInstall() bool + SkipNeeds() bool + IncludeNeeds() bool + concurrencyConfig interactive loggingConfig @@ -74,6 +77,9 @@ type SyncConfigProvider interface { Wait() bool WaitForJobs() bool + SkipNeeds() bool + IncludeNeeds() bool + concurrencyConfig loggingConfig } @@ -92,6 +98,9 @@ type DiffConfigProvider interface { ShowSecrets() bool SuppressDiff() bool + SkipNeeds() bool + IncludeNeeds() bool + DetailedExitcode() bool NoColor() bool Context() int @@ -155,6 +164,7 @@ type TemplateConfigProvider interface { SkipCleanup() bool OutputDir() string IncludeCRDs() bool + IncludeNeeds() bool concurrencyConfig } diff --git a/pkg/app/diff_test.go b/pkg/app/diff_test.go index 5008dea1..721dae19 100644 --- a/pkg/app/diff_test.go +++ b/pkg/app/diff_test.go @@ -8,6 +8,7 @@ import ( "sync" "testing" + "github.com/google/go-cmp/cmp" "github.com/roboll/helmfile/pkg/exectest" "github.com/roboll/helmfile/pkg/helmexec" "github.com/roboll/helmfile/pkg/testhelper" @@ -24,6 +25,8 @@ type diffConfig struct { skipCRDs bool skipDeps bool includeTests bool + includeNeeds bool + skipNeeds bool suppressSecrets bool showSecrets bool suppressDiff bool @@ -63,6 +66,14 @@ func (a diffConfig) IncludeTests() bool { return a.includeTests } +func (a diffConfig) IncludeNeeds() bool { + return a.includeNeeds +} + +func (a diffConfig) SkipNeeds() bool { + return a.skipNeeds +} + func (a diffConfig) SuppressSecrets() bool { return a.suppressSecrets } @@ -104,6 +115,10 @@ func (a diffConfig) RetainValuesFiles() bool { } func TestDiff(t *testing.T) { + type flags struct { + skipNeeds bool + } + testcases := []struct { name string loc string @@ -111,6 +126,7 @@ func TestDiff(t *testing.T) { concurrency int detailedExitcode bool error string + flags flags files map[string]string selectors []string lists map[exectest.ListKey]string @@ -899,8 +915,9 @@ bar 4 Fri Nov 1 08:40:07 2019 DEPLOYED mychart2-3.1.0 3.1.0 defau // { // see https://github.com/roboll/helmfile/issues/919#issuecomment-549831747 - name: "upgrades with good selector", - loc: location(), + name: "upgrades with good selector with --skip-needs=true", + loc: location(), + flags: flags{skipNeeds: true}, files: map[string]string{ "/path/to/helmfile.yaml": ` {{ $mark := "a" }} @@ -1004,6 +1021,112 @@ Affected releases are: external-secrets (incubator/raw) UPDATED my-release (incubator/raw) UPDATED +`, + }, + { + name: "upgrades with good selector with --skip-needs=false", + loc: location(), + flags: flags{skipNeeds: false}, + 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"}, + detailedExitcode: true, + 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{}, + // as we check for log output, set concurrency to 1 to avoid non-deterministic test result + concurrency: 1, + error: `in ./helmfile.yaml: release "default/external-secrets" depends on "kube-system/kubernetes-external-secrets" which does not match the selectors. Please add a selector like "--selector name=kubernetes-external-secrets", or indicate whether to skip (--skip-needs) or include (--include-needs) these dependencies`, + 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[]} +2 release(s) matching app=test found in helmfile.yaml + +err: release "default/external-secrets" depends on "kube-system/kubernetes-external-secrets" which does not match the selectors. Please add a selector like "--selector name=kubernetes-external-secrets", or indicate whether to skip (--skip-needs) or include (--include-needs) these dependencies `, }, { @@ -1250,13 +1373,16 @@ err: "foo" depends on nonexistent release "bar" concurrency: tc.concurrency, logger: logger, detailedExitcode: tc.detailedExitcode, + skipNeeds: tc.flags.skipNeeds, }) - if tc.error == "" && diffErr != nil { - t.Fatalf("unexpected error for data defined at %s: %v", tc.loc, diffErr) - } else if tc.error != "" && diffErr == nil { - t.Fatalf("expected error did not occur for data defined at %s", tc.loc) - } else if tc.error != "" && diffErr != nil && tc.error != diffErr.Error() { - t.Fatalf("invalid error: expected %q, got %q", tc.error, diffErr.Error()) + + var diffErrStr string + if diffErr != nil { + diffErrStr = diffErr.Error() + } + + if d := cmp.Diff(tc.error, diffErrStr); d != "" { + t.Fatalf("invalid error: want (-), got (+): %s", d) } if len(wantUpgrades) > len(helm.Releases) { diff --git a/pkg/state/state_run.go b/pkg/state/state_run.go index cf6880f3..020ed7b5 100644 --- a/pkg/state/state_run.go +++ b/pkg/state/state_run.go @@ -1,8 +1,10 @@ package state import ( + "errors" "fmt" "sort" + "strings" "sync" "github.com/roboll/helmfile/pkg/helmexec" @@ -97,17 +99,20 @@ func (st *HelmState) iterateOnReleases(helm helmexec.Interface, concurrency int, return nil } -type PlanOpts struct { - Reverse bool +type PlanOptions struct { + Reverse bool + IncludeNeeds bool + SkipNeeds bool + SelectedReleases []ReleaseSpec } -func (st *HelmState) PlanReleases(reverse bool) ([][]Release, error) { +func (st *HelmState) PlanReleases(opts PlanOptions) ([][]Release, error) { marked, err := st.SelectReleasesWithOverrides() if err != nil { return nil, err } - groups, err := SortedReleaseGroups(marked, reverse) + groups, err := SortedReleaseGroups(marked, opts) if err != nil { return nil, err } @@ -115,8 +120,10 @@ func (st *HelmState) PlanReleases(reverse bool) ([][]Release, error) { return groups, nil } -func SortedReleaseGroups(releases []Release, reverse bool) ([][]Release, error) { - groups, err := GroupReleasesByDependency(releases) +func SortedReleaseGroups(releases []Release, opts PlanOptions) ([][]Release, error) { + reverse := opts.Reverse + + groups, err := GroupReleasesByDependency(releases, opts) if err != nil { return nil, err } @@ -130,7 +137,7 @@ func SortedReleaseGroups(releases []Release, reverse bool) ([][]Release, error) return groups, nil } -func GroupReleasesByDependency(releases []Release) ([][]Release, error) { +func GroupReleasesByDependency(releases []Release, opts PlanOptions) ([][]Release, error) { idToReleases := map[string][]Release{} idToIndex := map[string]int{} @@ -159,8 +166,59 @@ func GroupReleasesByDependency(releases []Release) ([][]Release, error) { } } - plan, err := d.Plan() + var selectedReleaseIDs []string + + for _, r := range opts.SelectedReleases { + id := ReleaseToID(&r) + selectedReleaseIDs = append(selectedReleaseIDs, id) + } + + plan, err := d.Plan(dag.SortOptions{ + Only: selectedReleaseIDs, + WithDependencies: opts.IncludeNeeds, + WithoutDependencies: opts.SkipNeeds, + }) if err != nil { + if ude, ok := err.(*dag.UnhandledDependencyError); ok { + msgs := make([]string, len(ude.UnhandledDependencies)) + for i, ud := range ude.UnhandledDependencies { + id := ud.Id + + ds := make([]string, len(ud.Dependents)) + for i, d := range ud.Dependents { + ds[i] = fmt.Sprintf("%q", d) + } + + var dsHumanized string + if len(ds) < 3 { + dsHumanized = strings.Join(ds, " and ") + } else { + dsHumanized = strings.Join(ds[:len(ds)-1], ", ") + dsHumanized += ", and " + ds[len(ds)-1] + } + + var verb string + if len(ds) == 1 { + verb = "depends" + } else { + verb = "depend" + } + + idComponents := strings.Split(id, "/") + name := idComponents[len(idComponents)-1] + + msg := fmt.Sprintf( + "release %s %s on %q which does not match the selectors. "+ + "Please add a selector like \"--selector name=%s\", or indicate whether to skip (--skip-needs) or include (--include-needs) these dependencies", + dsHumanized, + verb, + id, + name, + ) + msgs[i] = msg + } + return nil, errors.New(msgs[0]) + } return nil, err }