optimize flag logic about wait (helm#1051)

* optimize flag logic about wait Signed-off-by: yxxhero <aiopsclub@163.com> * fix tests Signed-off-by: yxxhero <aiopsclub@163.com> * add units Signed-off-by: yxxhero <aiopsclub@163.com> * fix little issues Signed-off-by: yxxhero <aiopsclub@163.com> --------- Signed-off-by: yxxhero <aiopsclub@163.com>
Nordix · Oct 19, 2023 · a371e41 · a371e41
1 parent 40d4e3a
commit a371e41
Show file tree

Hide file tree

Showing 3 changed files with 150 additions and 15 deletions.
diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go
@@ -39,6 +39,30 @@ func (st *HelmState) appendPostRenderFlags(flags []string, release *ReleaseSpec,
 	return flags
 }
 
+func (st *HelmState) appendWaitForJobsFlags(flags []string, release *ReleaseSpec, ops *SyncOpts) []string {
+	switch {
+	case release.WaitForJobs != nil && *release.WaitForJobs:
+		flags = append(flags, "--wait-for-jobs")
+	case ops != nil && ops.WaitForJobs:
+		flags = append(flags, "--wait-for-jobs")
+	case release.WaitForJobs == nil && st.HelmDefaults.WaitForJobs:
+		flags = append(flags, "--wait-for-jobs")
+	}
+	return flags
+}
+
+func (st *HelmState) appendWaitFlags(flags []string, release *ReleaseSpec, ops *SyncOpts) []string {
+	switch {
+	case release.Wait != nil && *release.Wait:
+		flags = append(flags, "--wait")
+	case ops != nil && ops.Wait:
+		flags = append(flags, "--wait")
+	case release.Wait == nil && st.HelmDefaults.Wait:
+		flags = append(flags, "--wait")
+	}
+	return flags
+}
+
 // append post-renderer flags to helm flags
 func (st *HelmState) appendCascadeFlags(flags []string, helm helmexec.Interface, release *ReleaseSpec, cascade string) []string {
 	// see https://github.com/helm/helm/releases/tag/v3.12.1

diff --git a/pkg/state/helmx_test.go b/pkg/state/helmx_test.go
@@ -9,6 +9,130 @@ import (
 	"github.com/helmfile/helmfile/pkg/testutil"
 )
 
+func TestAppendWaitForJobsFlags(t *testing.T) {
+	tests := []struct {
+		name     string
+		release  *ReleaseSpec
+		syncOpts *SyncOpts
+		helmSpec HelmSpec
+		expected []string
+	}{
+		{
+			name:     "release wait for jobs",
+			release:  &ReleaseSpec{WaitForJobs: &[]bool{true}[0]},
+			syncOpts: nil,
+			helmSpec: HelmSpec{},
+			expected: []string{"--wait-for-jobs"},
+		},
+		{
+			name:     "cli flags wait for jobs",
+			release:  &ReleaseSpec{},
+			syncOpts: &SyncOpts{WaitForJobs: true},
+			helmSpec: HelmSpec{},
+			expected: []string{"--wait-for-jobs"},
+		},
+		{
+			name:     "helm defaults wait for jobs",
+			release:  &ReleaseSpec{},
+			syncOpts: nil,
+			helmSpec: HelmSpec{WaitForJobs: true},
+			expected: []string{"--wait-for-jobs"},
+		},
+		{
+			name:     "release wait for jobs false",
+			release:  &ReleaseSpec{WaitForJobs: &[]bool{false}[0]},
+			syncOpts: nil,
+			helmSpec: HelmSpec{WaitForJobs: true},
+			expected: []string{},
+		},
+		{
+			name:     "cli flags wait for jobs false",
+			release:  &ReleaseSpec{},
+			syncOpts: &SyncOpts{},
+			helmSpec: HelmSpec{WaitForJobs: true},
+			expected: []string{"--wait-for-jobs"},
+		},
+		{
+			name:     "helm defaults wait for jobs false",
+			release:  &ReleaseSpec{},
+			syncOpts: nil,
+			helmSpec: HelmSpec{WaitForJobs: false},
+			expected: []string{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			st := &HelmState{}
+			st.HelmDefaults = tt.helmSpec
+			got := st.appendWaitForJobsFlags([]string{}, tt.release, tt.syncOpts)
+			require.Equalf(t, tt.expected, got, "appendWaitForJobsFlags() = %v, want %v", got, tt.expected)
+		})
+	}
+}
+
+func TestAppendWaitFlags(t *testing.T) {
+	tests := []struct {
+		name     string
+		release  *ReleaseSpec
+		syncOpts *SyncOpts
+		helmSpec HelmSpec
+		expected []string
+	}{
+		{
+			name:     "release wait",
+			release:  &ReleaseSpec{Wait: &[]bool{true}[0]},
+			syncOpts: nil,
+			helmSpec: HelmSpec{},
+			expected: []string{"--wait"},
+		},
+		{
+			name:     "cli flags wait",
+			release:  &ReleaseSpec{},
+			syncOpts: &SyncOpts{Wait: true},
+			helmSpec: HelmSpec{},
+			expected: []string{"--wait"},
+		},
+		{
+			name:     "helm defaults wait",
+			release:  &ReleaseSpec{},
+			syncOpts: nil,
+			helmSpec: HelmSpec{Wait: true},
+			expected: []string{"--wait"},
+		},
+		{
+			name:     "release wait false",
+			release:  &ReleaseSpec{Wait: &[]bool{false}[0]},
+			syncOpts: nil,
+			helmSpec: HelmSpec{Wait: true},
+			expected: []string{},
+		},
+		{
+			name:     "cli flags wait false",
+			release:  &ReleaseSpec{},
+			syncOpts: &SyncOpts{},
+			helmSpec: HelmSpec{Wait: true},
+			expected: []string{"--wait"},
+		},
+		{
+			name:     "helm defaults wait false",
+			release:  &ReleaseSpec{},
+			syncOpts: nil,
+			helmSpec: HelmSpec{Wait: false},
+			expected: []string{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			st := &HelmState{}
+			st.HelmDefaults = tt.helmSpec
+			got := st.appendWaitFlags([]string{}, tt.release, tt.syncOpts)
+			require.Equalf(t, tt.expected, got, "appendWaitFlags() = %v, want %v", got, tt.expected)
+		})
+	}
+}
+
 func TestAppendCascadeFlags(t *testing.T) {
 	type args struct {
 		flags    []string

diff --git a/pkg/state/state.go b/pkg/state/state.go
@@ -637,14 +637,6 @@ func (st *HelmState) prepareSyncReleases(helm helmexec.Interface, additionalValu
 					flags = append(flags, "--skip-crds")
 				}
 
-				if opts.Wait {
-					flags = append(flags, "--wait")
-				}
-
-				if opts.WaitForJobs {
-					flags = append(flags, "--wait-for-jobs")
-				}
-
 				flags = st.appendValuesControlModeFlag(flags, opts.ReuseValues, opts.ResetValues)
 
 				if len(errs) > 0 {
@@ -2517,13 +2509,8 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp
 		flags = append(flags, "--enable-dns")
 	}
 
-	if release.Wait != nil && *release.Wait || release.Wait == nil && st.HelmDefaults.Wait {
-		flags = append(flags, "--wait")
-	}
-
-	if release.WaitForJobs != nil && *release.WaitForJobs || release.WaitForJobs == nil && st.HelmDefaults.WaitForJobs {
-		flags = append(flags, "--wait-for-jobs")
-	}
+	flags = st.appendWaitFlags(flags, release, opt)
+	flags = st.appendWaitForJobsFlags(flags, release, opt)
 
 	flags = append(flags, st.timeoutFlags(release)...)