From b1928e585d7ce00875b901ddb1b5f55395837205 Mon Sep 17 00:00:00 2001 From: Anton Bretting Date: Thu, 10 Mar 2022 10:57:58 +0100 Subject: [PATCH] Stop panic when deduplicating releases (#2067) * Stop panic when deduplicating releases * Add testdata for new testcases --- pkg/app/app.go | 9 +-- pkg/app/app_apply_test.go | 61 ++++++++++++++++ .../log | 43 ++++++++++++ .../log | 69 +++++++++++++++++++ 4 files changed, 176 insertions(+), 6 deletions(-) create mode 100644 pkg/app/testdata/testapply_2/select_non_existant_release_with_--allow-no-matching-release/log create mode 100644 pkg/app/testdata/testapply_2/select_single_release_from_helmfile_with_two_duplicates/log diff --git a/pkg/app/app.go b/pkg/app/app.go index 30767b3d..180f6cf1 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -1200,13 +1200,10 @@ func (a *App) getSelectedReleases(r *Run, includeTransitiveNeeds bool) ([]state. // releases in the helmfile config. // Otherwise we can't compute the DAG of releases correctly. r, deduped := selectedIds[id] - if !deduped { - panic(fmt.Errorf("assertion error: release %q has never been selected. This shouldn't happen", id)) + if deduped { + deduplicated = append(deduplicated, r) + dedupedBefore[id] = struct{}{} } - - deduplicated = append(deduplicated, r) - - dedupedBefore[id] = struct{}{} } if err := checkDuplicates(r.helm, r.state, deduplicated); err != nil { diff --git a/pkg/app/app_apply_test.go b/pkg/app/app_apply_test.go index 82ed9a06..6a65e1c1 100644 --- a/pkg/app/app_apply_test.go +++ b/pkg/app/app_apply_test.go @@ -1220,4 +1220,65 @@ releases: concurrency: 1, }) }) + + t.Run("select non existant release with --allow-no-matching-release", func(t *testing.T) { + check(t, testcase{ + files: map[string]string{ + "/path/to/helmfile.yaml": ` +releases: +- name: foo + chart: incubator/raw + namespace: default + labels: + app: test + +- name: bar + chart: incubator/raw + namespace: default + labels: + app: test +`, + }, + selectors: []string{"app=foo"}, + upgraded: []exectest.Release{}, + error: "err: no releases found that matches specified selector(app=foo) and environment(default), in any helmfile", + // as we check for log output, set concurrency to 1 to avoid non-deterministic test result + concurrency: 1, + }) + }) + + t.Run("select single release from helmfile with two duplicates", func(t *testing.T) { + check(t, testcase{ + files: map[string]string{ + "/path/to/helmfile.yaml": ` +releases: +- name: foo + chart: incubator/raw + namespace: default + labels: + app: test + +- name: bar + chart: incubator/raw + namespace: default + labels: + app: build + +- name: bar + chart: incubator/raw + namespace: default + labels: + app: test +`, + }, + selectors: []string{"name=foo"}, + upgraded: []exectest.Release{}, + diffs: map[exectest.DiffKey]error{ + exectest.DiffKey{Name: "foo", Chart: "incubator/raw", Flags: "--kube-contextdefault--namespacedefault--detailed-exitcode"}: helmexec.ExitError{Code: 2}, + }, + error: "", + // as we check for log output, set concurrency to 1 to avoid non-deterministic test result + concurrency: 1, + }) + }) } diff --git a/pkg/app/testdata/testapply_2/select_non_existant_release_with_--allow-no-matching-release/log b/pkg/app/testdata/testapply_2/select_non_existant_release_with_--allow-no-matching-release/log new file mode 100644 index 00000000..48a0477f --- /dev/null +++ b/pkg/app/testdata/testapply_2/select_non_existant_release_with_--allow-no-matching-release/log @@ -0,0 +1,43 @@ +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: releases: + 2: - name: foo + 3: chart: incubator/raw + 4: namespace: default + 5: labels: + 6: app: test + 7: + 8: - name: bar + 9: chart: incubator/raw +10: namespace: default +11: labels: +12: app: test +13: + +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: releases: + 2: - name: foo + 3: chart: incubator/raw + 4: namespace: default + 5: labels: + 6: app: test + 7: + 8: - name: bar + 9: chart: incubator/raw +10: namespace: default +11: labels: +12: app: test +13: + +merged environment: &{default map[] map[]} +0 release(s) matching app=foo found in helmfile.yaml + diff --git a/pkg/app/testdata/testapply_2/select_single_release_from_helmfile_with_two_duplicates/log b/pkg/app/testdata/testapply_2/select_single_release_from_helmfile_with_two_duplicates/log new file mode 100644 index 00000000..5d448c73 --- /dev/null +++ b/pkg/app/testdata/testapply_2/select_single_release_from_helmfile_with_two_duplicates/log @@ -0,0 +1,69 @@ +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: releases: + 2: - name: foo + 3: chart: incubator/raw + 4: namespace: default + 5: labels: + 6: app: test + 7: + 8: - name: bar + 9: chart: incubator/raw +10: namespace: default +11: labels: +12: app: build +13: +14: - name: bar +15: chart: incubator/raw +16: namespace: default +17: labels: +18: app: test +19: + +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: releases: + 2: - name: foo + 3: chart: incubator/raw + 4: namespace: default + 5: labels: + 6: app: test + 7: + 8: - name: bar + 9: chart: incubator/raw +10: namespace: default +11: labels: +12: app: build +13: +14: - name: bar +15: chart: incubator/raw +16: namespace: default +17: labels: +18: app: test +19: + +merged environment: &{default map[] map[]} +1 release(s) matching name=foo found in helmfile.yaml + +Affected releases are: + foo (incubator/raw) UPDATED + +processing 1 groups of releases in this order: +GROUP RELEASES +1 default/default/foo + +processing releases in group 1/1: default/default/foo +getting deployed release version failed:unexpected list key: {^foo$ --kube-contextdefault--deleting--deployed--failed--pending} + +UPDATED RELEASES: +NAME CHART VERSION +foo incubator/raw +