diff --git a/state/state.go b/state/state.go index 88654b32..b95ddd4a 100644 --- a/state/state.go +++ b/state/state.go @@ -239,6 +239,17 @@ func (st *HelmState) prepareSyncReleases(helm helmexec.Interface, additionalValu for release := range jobs { st.applyDefaultsTo(release) + // If `installed: false`, the only potential operation on this release would be uninstalling. + // We skip generating values files in that case, because for an uninstall with `helm delete`, we don't need to those. + // The values files are for `helm upgrade -f values.yaml` calls that happens when the release has `installed: true`. + // This logic addresses: + // - https://github.com/roboll/helmfile/issues/519 + // - https://github.com/roboll/helmfile/issues/616 + if !release.Desired() { + results <- syncPrepareResult{release: release, flags: []string{}, errors: []*ReleaseError{}} + continue + } + flags, flagsErr := st.flagsForUpgrade(helm, release, workerIndex) if flagsErr != nil { results <- syncPrepareResult{errors: []*ReleaseError{newReleaseError(release, flagsErr)}} diff --git a/state/state_test.go b/state/state_test.go index 70f2bb24..8715202b 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -16,6 +16,12 @@ import ( var logger = helmexec.NewLogger(os.Stdout, "warn") +func injectFs(st *HelmState, fs *TestFs) *HelmState { + st.readFile = fs.ReadFile + st.fileExists = fs.FileExists + return st +} + func TestLabelParsing(t *testing.T) { cases := []struct { labelString string @@ -948,6 +954,117 @@ func TestHelmState_SyncReleases(t *testing.T) { } } +func TestHelmState_SyncReleases_MissingValuesFileForUndesiredRelease(t *testing.T) { + no := false + tests := []struct { + name string + release ReleaseSpec + listResult string + expectedError string + }{ + { + name: "should install", + release: ReleaseSpec{ + Name: "foo", + Chart: "../../foo-bar", + }, + listResult: ``, + expectedError: ``, + }, + { + name: "should upgrade", + release: ReleaseSpec{ + Name: "foo", + Chart: "../../foo-bar", + }, + listResult: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE + foo 1 Wed Apr 17 17:39:04 2019 DEPLOYED foo-bar-2.0.4 0.1.0 default`, + expectedError: ``, + }, + { + name: "should uninstall", + release: ReleaseSpec{ + Name: "foo", + Chart: "../../foo-bar", + Installed: &no, + }, + listResult: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE + foo 1 Wed Apr 17 17:39:04 2019 DEPLOYED foo-bar-2.0.4 0.1.0 default`, + expectedError: ``, + }, + { + name: "should fail installing due to missing values file", + release: ReleaseSpec{ + Name: "foo", + Chart: "../../foo-bar", + Values: []interface{}{"noexistent.values.yaml"}, + }, + listResult: ``, + expectedError: `failed processing release foo: file does not exist: noexistent.values.yaml`, + }, + { + name: "should fail upgrading due to missing values file", + release: ReleaseSpec{ + Name: "foo", + Chart: "../../foo-bar", + Values: []interface{}{"noexistent.values.yaml"}, + }, + listResult: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE + foo 1 Wed Apr 17 17:39:04 2019 DEPLOYED foo-bar-2.0.4 0.1.0 default`, + expectedError: `failed processing release foo: file does not exist: noexistent.values.yaml`, + }, + { + name: "should uninstall even when there is a missing values file", + release: ReleaseSpec{ + Name: "foo", + Chart: "../../foo-bar", + Values: []interface{}{"noexistent.values.yaml"}, + Installed: &no, + }, + listResult: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE + foo 1 Wed Apr 17 17:39:04 2019 DEPLOYED foo-bar-2.0.4 0.1.0 default`, + expectedError: ``, + }, + } + for i := range tests { + tt := tests[i] + t.Run(tt.name, func(t *testing.T) { + state := &HelmState{ + Releases: []ReleaseSpec{tt.release}, + logger: logger, + } + fs := NewTestFs(map[string]string{}) + state = injectFs(state, fs) + helm := &mockHelmExec{ + lists: map[listKey]string{}, + } + //simulate the helm.list call result + helm.lists[listKey{filter: "^" + tt.release.Name + "$"}] = tt.listResult + + affectedReleases := AffectedReleases{} + errs := state.SyncReleases(&affectedReleases, helm, []string{}, 1) + + if tt.expectedError != "" { + if len(errs) == 0 { + t.Fatalf("expected error not occurred: expected=%s, got none", tt.expectedError) + } + if len(errs) != 1 { + t.Fatalf("too many errors: expected %d, got %d: %v", 1, len(errs), errs) + } + err := errs[0] + if err.Error() != tt.expectedError { + t.Fatalf("unexpected error: expected=%s, got=%v", tt.expectedError, err) + } + } else { + if len(errs) > 0 { + t.Fatalf("unexpected error(s): expected=0, got=%d: %v", len(errs), errs) + } + } + + }) + } +} + func TestHelmState_SyncReleasesAffectedRealeases(t *testing.T) { no := false tests := []struct { diff --git a/state/testfs.go b/state/testfs.go index 31e55aca..ed6c5380 100644 --- a/state/testfs.go +++ b/state/testfs.go @@ -44,6 +44,10 @@ func (f *TestFs) FileExistsAt(path string) bool { return ok } +func (f *TestFs) FileExists(path string) (bool, error) { + return f.FileExistsAt(path), nil +} + func (f *TestFs) DirectoryExistsAt(path string) bool { var ok bool if strings.Contains(path, "/") {