From 0e00cdf2a53289cb971fea4bc8e4dfaa9a7e4358 Mon Sep 17 00:00:00 2001 From: KUOKA Yusuke Date: Wed, 3 Apr 2019 23:41:41 +0900 Subject: [PATCH] fix: `helmfile destroy` should delete releases in the specified tiller namespace (#536) Actually, 4 helm commands including "list", "diff", "status" and "delete" were not taking tiller-rerelated flags into account in helmfile. This fixes all these commands. Fixes #534 --- helmexec/exec.go | 8 +- helmexec/helmexec.go | 4 +- state/state.go | 29 ++++-- state/state_test.go | 213 ++++++++++++++++++++++++++++++++++++++----- 4 files changed, 220 insertions(+), 34 deletions(-) diff --git a/helmexec/exec.go b/helmexec/exec.go index 4d887c1d..6d7400c5 100644 --- a/helmexec/exec.go +++ b/helmexec/exec.go @@ -103,16 +103,16 @@ func (helm *execer) SyncRelease(name, chart string, flags ...string) error { return err } -func (helm *execer) ReleaseStatus(name string) error { +func (helm *execer) ReleaseStatus(name string, flags ...string) error { helm.logger.Infof("Getting status %v", name) - out, err := helm.exec(append([]string{"status", name})...) + out, err := helm.exec(append([]string{"status", name}, flags...)...) helm.write(out) return err } -func (helm *execer) List(filter string) (string, error) { +func (helm *execer) List(filter string, flags ...string) (string, error) { helm.logger.Infof("Listing releases matching %v", filter) - out, err := helm.exec(append([]string{"list", filter})...) + out, err := helm.exec(append([]string{"list", filter}, flags...)...) helm.write(out) return string(out), err } diff --git a/helmexec/helmexec.go b/helmexec/helmexec.go index 5d6c65df..6138a342 100644 --- a/helmexec/helmexec.go +++ b/helmexec/helmexec.go @@ -14,9 +14,9 @@ type Interface interface { TemplateRelease(chart string, flags ...string) error Fetch(chart string, flags ...string) error Lint(chart string, flags ...string) error - ReleaseStatus(name string) error + ReleaseStatus(name string, flags ...string) error DeleteRelease(name string, flags ...string) error TestRelease(name string, flags ...string) error - List(filter string) (string, error) + List(filter string, flags ...string) (string, error) DecryptSecret(name string) (string, error) } diff --git a/state/state.go b/state/state.go index 9195fd81..04481960 100644 --- a/state/state.go +++ b/state/state.go @@ -277,8 +277,8 @@ func (st *HelmState) prepareSyncReleases(helm helmexec.Interface, additionalValu return res, errs } -func isReleaseInstalled(helm helmexec.Interface, release ReleaseSpec) (bool, error) { - out, err := helm.List("^" + release.Name + "$") +func (st *HelmState) isReleaseInstalled(helm helmexec.Interface, release ReleaseSpec) (bool, error) { + out, err := helm.List("^"+release.Name+"$", st.tillerFlags(&release)...) if err != nil { return false, err } else if out != "" { @@ -292,7 +292,7 @@ func (st *HelmState) DetectReleasesToBeDeleted(helm helmexec.Interface) ([]*Rele for i, _ := range st.Releases { release := st.Releases[i] if !release.Desired() { - installed, err := isReleaseInstalled(helm, release) + installed, err := st.isReleaseInstalled(helm, release) if err != nil { return nil, err } else if installed { @@ -330,7 +330,7 @@ func (st *HelmState) SyncReleases(helm helmexec.Interface, additionalValues []st chart := normalizeChart(st.basePath, release.Chart) var relErr *ReleaseError if !release.Desired() { - installed, err := isReleaseInstalled(helm, *release) + installed, err := st.isReleaseInstalled(helm, *release) if err != nil { relErr = &ReleaseError{release, err} } else if installed { @@ -735,7 +735,11 @@ func (st *HelmState) ReleaseStatuses(helm helmexec.Interface, workerLimit int) [ if !release.Desired() { return nil } - return helm.ReleaseStatus(release.Name) + + flags := []string{} + flags = st.appendTillerFlags(flags, &release) + + return helm.ReleaseStatus(release.Name, flags...) }) } @@ -750,7 +754,9 @@ func (st *HelmState) DeleteReleases(helm helmexec.Interface, purge bool) []error if purge { flags = append(flags, "--purge") } - installed, err := isReleaseInstalled(helm, release) + flags = st.appendTillerFlags(flags, &release) + + installed, err := st.isReleaseInstalled(helm, release) if err != nil { return err } @@ -773,6 +779,8 @@ func (st *HelmState) TestReleases(helm helmexec.Interface, cleanup bool, timeout flags = append(flags, "--cleanup") } flags = append(flags, "--timeout", strconv.Itoa(timeout)) + flags = st.appendTillerFlags(flags, &release) + return helm.TestRelease(release.Name, flags...) }) } @@ -976,6 +984,15 @@ func findChartDirectory(topLevelDir string) (string, error) { } func (st *HelmState) appendTillerFlags(flags []string, release *ReleaseSpec) []string { + adds := st.tillerFlags(release) + for _, a := range adds { + flags = append(flags, a) + } + return flags +} + +func (st *HelmState) tillerFlags(release *ReleaseSpec) []string { + flags := []string{} if release.TillerNamespace != "" { flags = append(flags, "--tiller-namespace", release.TillerNamespace) } else if st.HelmDefaults.TillerNamespace != "" { diff --git a/state/state_test.go b/state/state_test.go index fa839b1e..6be5958a 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -631,12 +631,18 @@ func Test_normalizeChart(t *testing.T) { // mocking helmexec.Interface +type listKey struct { + filter string + flags string +} + type mockHelmExec struct { charts []string repo []string releases []mockRelease deleted []mockRelease - lists map[string]string + lists map[listKey]string + diffed []mockRelease } type mockRelease struct { @@ -682,21 +688,22 @@ func (helm *mockHelmExec) SyncRelease(name, chart string, flags ...string) error return nil } func (helm *mockHelmExec) DiffRelease(name, chart string, flags ...string) error { + helm.diffed = append(helm.diffed, mockRelease{name: name, flags: flags}) return nil } -func (helm *mockHelmExec) ReleaseStatus(release string) error { +func (helm *mockHelmExec) ReleaseStatus(release string, flags ...string) error { if strings.Contains(release, "error") { return errors.New("error") } - helm.releases = append(helm.releases, mockRelease{name: release, flags: []string{}}) + helm.releases = append(helm.releases, mockRelease{name: release, flags: flags}) return nil } func (helm *mockHelmExec) DeleteRelease(name string, flags ...string) error { helm.deleted = append(helm.deleted, mockRelease{name: name, flags: flags}) return nil } -func (helm *mockHelmExec) List(filter string) (string, error) { - return helm.lists[filter], nil +func (helm *mockHelmExec) List(filter string, flags ...string) (string, error) { + return helm.lists[listKey{filter: filter, flags: strings.Join(flags, "")}], nil } func (helm *mockHelmExec) DecryptSecret(name string) (string, error) { return "", nil @@ -807,6 +814,18 @@ func TestHelmState_SyncReleases(t *testing.T) { helm: &mockHelmExec{}, wantReleases: []mockRelease{{"releaseName", []string{}}}, }, + { + name: "with tiller args", + releases: []ReleaseSpec{ + { + Name: "releaseName", + Chart: "foo", + TillerNamespace: "tillerns", + }, + }, + helm: &mockHelmExec{}, + wantReleases: []mockRelease{{"releaseName", []string{"--tiller-namespace", "tillerns"}}}, + }, { name: "escaped values", releases: []ReleaseSpec{ @@ -887,6 +906,120 @@ func TestHelmState_SyncReleases(t *testing.T) { } } +func TestHelmState_DiffReleases(t *testing.T) { + tests := []struct { + name string + releases []ReleaseSpec + helm *mockHelmExec + wantReleases []mockRelease + }{ + { + name: "normal release", + releases: []ReleaseSpec{ + { + Name: "releaseName", + Chart: "foo", + }, + }, + helm: &mockHelmExec{}, + wantReleases: []mockRelease{{"releaseName", []string{}}}, + }, + { + name: "with tiller args", + releases: []ReleaseSpec{ + { + Name: "releaseName", + Chart: "foo", + TillerNamespace: "tillerns", + }, + }, + helm: &mockHelmExec{}, + wantReleases: []mockRelease{{"releaseName", []string{"--tiller-namespace", "tillerns"}}}, + }, + { + name: "escaped values", + releases: []ReleaseSpec{ + { + Name: "releaseName", + Chart: "foo", + SetValues: []SetValue{ + { + Name: "someList", + Value: "a,b,c", + }, + { + Name: "json", + Value: "{\"name\": \"john\"}", + }, + }, + }, + }, + helm: &mockHelmExec{}, + wantReleases: []mockRelease{{"releaseName", []string{"--set", "someList=a\\,b\\,c", "--set", "json=\\{\"name\": \"john\"\\}"}}}, + }, + { + name: "set single value from file", + releases: []ReleaseSpec{ + { + Name: "releaseName", + Chart: "foo", + SetValues: []SetValue{ + { + Name: "foo", + Value: "FOO", + }, + { + Name: "bar", + File: "path/to/bar", + }, + { + Name: "baz", + Value: "BAZ", + }, + }, + }, + }, + helm: &mockHelmExec{}, + wantReleases: []mockRelease{{"releaseName", []string{"--set", "foo=FOO", "--set-file", "bar=path/to/bar", "--set", "baz=BAZ"}}}, + }, + { + name: "set single array value in an array", + releases: []ReleaseSpec{ + { + Name: "releaseName", + Chart: "foo", + SetValues: []SetValue{ + { + Name: "foo.bar[0]", + Values: []string{ + "A", + "B", + }, + }, + }, + }, + }, + helm: &mockHelmExec{}, + wantReleases: []mockRelease{{"releaseName", []string{"--set", "foo.bar[0]={A,B}"}}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + state := &HelmState{ + Releases: tt.releases, + logger: logger, + } + _, err := state.DiffReleases(tt.helm, []string{}, 1, false, false, false) + if err != nil { + fmt.Errorf("unexpected error: %v", err) + } + if !reflect.DeepEqual(tt.helm.diffed, tt.wantReleases) { + t.Errorf("HelmState.DiffReleases() for [%s] = %v, want %v", tt.name, tt.helm.releases, tt.wantReleases) + } + }) + } +} + func TestHelmState_SyncReleasesCleanup(t *testing.T) { tests := []struct { name string @@ -1156,6 +1289,17 @@ func TestHelmState_ReleaseStatuses(t *testing.T) { helm: &mockHelmExec{}, wantErr: false, }, + { + name: "with tiller args", + releases: []ReleaseSpec{ + { + Name: "releaseA", + TillerNamespace: "tillerns", + }, + }, + helm: &mockHelmExec{}, + want: []mockRelease{{"releaseA", []string{"--tiller-namespace", "tillerns"}}}, + }, } for _, tt := range tests { i := func(t *testing.T) { @@ -1190,12 +1334,13 @@ func TestHelmState_ReleaseStatuses(t *testing.T) { func TestHelmState_TestReleasesNoCleanUp(t *testing.T) { tests := []struct { - name string - cleanup bool - releases []ReleaseSpec - helm *mockHelmExec - want []mockRelease - wantErr bool + name string + cleanup bool + releases []ReleaseSpec + helm *mockHelmExec + want []mockRelease + wantErr bool + tillerNamespace string }{ { name: "happy path", @@ -1228,6 +1373,17 @@ func TestHelmState_TestReleasesNoCleanUp(t *testing.T) { helm: &mockHelmExec{}, wantErr: true, }, + { + name: "with tiller args", + releases: []ReleaseSpec{ + { + Name: "releaseA", + TillerNamespace: "tillerns", + }, + }, + helm: &mockHelmExec{}, + want: []mockRelease{{"releaseA", []string{"--timeout", "1", "--tiller-namespace", "tillerns"}}}, + }, } for _, tt := range tests { i := func(t *testing.T) { @@ -1297,12 +1453,14 @@ func TestHelmState_NoReleaseMatched(t *testing.T) { func TestHelmState_Delete(t *testing.T) { tests := []struct { - name string - deleted []mockRelease - wantErr bool - desired *bool - installed bool - purge bool + name string + deleted []mockRelease + wantErr bool + desired *bool + installed bool + purge bool + flags string + tillerNamespace string }{ { name: "desired and installed (purge=false)", @@ -1376,12 +1534,23 @@ func TestHelmState_Delete(t *testing.T) { purge: true, deleted: []mockRelease{}, }, + { + name: "with tiller args", + wantErr: false, + desired: nil, + installed: true, + purge: true, + tillerNamespace: "tillerns", + flags: "--tiller-namespacetillerns", + deleted: []mockRelease{{"releaseA", []string{"--purge", "--tiller-namespace", "tillerns"}}}, + }, } for _, tt := range tests { i := func(t *testing.T) { release := ReleaseSpec{ - Name: "releaseA", - Installed: tt.desired, + Name: "releaseA", + Installed: tt.desired, + TillerNamespace: tt.tillerNamespace, } releases := []ReleaseSpec{ release, @@ -1391,15 +1560,15 @@ func TestHelmState_Delete(t *testing.T) { logger: logger, } helm := &mockHelmExec{ - lists: map[string]string{}, + lists: map[listKey]string{}, deleted: []mockRelease{}, } if tt.installed { - helm.lists["^releaseA$"] = "releaseA" + helm.lists[listKey{filter: "^releaseA$", flags: tt.flags}] = "releaseA" } errs := state.DeleteReleases(helm, tt.purge) if (errs != nil) != tt.wantErr { - t.Errorf("DeleteREleases() for %s error = %v, wantErr %v", tt.name, errs, tt.wantErr) + t.Errorf("DeleteReleases() for %s error = %v, wantErr %v", tt.name, errs, tt.wantErr) return } if !reflect.DeepEqual(tt.deleted, helm.deleted) {