From 5a68e553c58df2ec5ee85498ff60952fe279461a Mon Sep 17 00:00:00 2001 From: Connor Hindley Date: Tue, 11 Feb 2025 14:31:21 -0600 Subject: [PATCH] feat: Add support for --wait-retries flag. This change wires up waitRetries option to set the helm --wait-retries flag. --wait-retries was added in helm 3.15.0 and makes waiting more robust to registry errors. https://github.com/helm/helm/commit/fc74964 https://github.com/helm/helm/releases/tag/v3.15.0 Resolves #1522 Signed-off-by: Connor Hindley --- pkg/app/app.go | 4 ++++ pkg/app/app_test.go | 5 ++++ pkg/app/config.go | 2 ++ pkg/config/apply.go | 7 ++++++ pkg/config/sync.go | 7 ++++++ pkg/state/helmx.go | 19 ++++++++++++++- pkg/state/helmx_test.go | 51 ++++++++++++++++++++++++++++++++++++++++- pkg/state/state.go | 8 ++++++- 8 files changed, 100 insertions(+), 3 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index 91beba57..23304dfc 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -385,6 +385,7 @@ func (a *App) Sync(c SyncConfigProvider) error { SkipRefresh: c.SkipRefresh(), SkipDeps: c.SkipDeps(), Wait: c.Wait(), + WaitRetries: c.WaitRetries(), WaitForJobs: c.WaitForJobs(), IncludeCRDs: &includeCRDs, IncludeTransitiveNeeds: c.IncludeNeeds(), @@ -419,6 +420,7 @@ func (a *App) Apply(c ApplyConfigProvider) error { SkipRefresh: c.SkipRefresh(), SkipDeps: c.SkipDeps(), Wait: c.Wait(), + WaitRetries: c.WaitRetries(), WaitForJobs: c.WaitForJobs(), IncludeCRDs: &includeCRDs, SkipCleanup: c.RetainValuesFiles() || c.SkipCleanup(), @@ -1546,6 +1548,7 @@ Do you really want to apply? SkipCleanup: c.RetainValuesFiles() || c.SkipCleanup(), SkipCRDs: c.SkipCRDs(), Wait: c.Wait(), + WaitRetries: c.WaitRetries(), WaitForJobs: c.WaitForJobs(), ReuseValues: c.ReuseValues(), ResetValues: c.ResetValues(), @@ -1943,6 +1946,7 @@ Do you really want to sync? Set: c.Set(), SkipCRDs: c.SkipCRDs(), Wait: c.Wait(), + WaitRetries: c.WaitRetries(), WaitForJobs: c.WaitForJobs(), ReuseValues: c.ReuseValues(), ResetValues: c.ResetValues(), diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index df79ee51..90c3f1ce 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2244,6 +2244,7 @@ type applyConfig struct { diffArgs string logger *zap.SugaredLogger wait bool + waitRetries int waitForJobs bool reuseValues bool postRenderer string @@ -2272,6 +2273,10 @@ func (a applyConfig) Wait() bool { return a.wait } +func (a applyConfig) WaitRetries() int { + return a.waitRetries +} + func (a applyConfig) WaitForJobs() bool { return a.waitForJobs } diff --git a/pkg/app/config.go b/pkg/app/config.go index b6e3a064..ecccc0dc 100644 --- a/pkg/app/config.go +++ b/pkg/app/config.go @@ -63,6 +63,7 @@ type ApplyConfigProvider interface { SkipDeps() bool SkipRefresh() bool Wait() bool + WaitRetries() int WaitForJobs() bool IncludeTests() bool @@ -113,6 +114,7 @@ type SyncConfigProvider interface { SkipDeps() bool SkipRefresh() bool Wait() bool + WaitRetries() int WaitForJobs() bool SyncArgs() string diff --git a/pkg/config/apply.go b/pkg/config/apply.go index 4d67aa61..1b34a88b 100644 --- a/pkg/config/apply.go +++ b/pkg/config/apply.go @@ -50,6 +50,8 @@ type ApplyOptions struct { SuppressDiff bool // Wait is true if the helm command should wait for the release to be deployed Wait bool + // WaitRetries is the number of times to retry waiting for the release to be deployed + WaitRetries int // WaitForJobs is true if the helm command should wait for the jobs to be completed WaitForJobs bool // Propagate '--skipSchemaValidation' to helmv3 template and helm install @@ -213,6 +215,11 @@ func (a *ApplyImpl) Wait() bool { return a.ApplyOptions.Wait } +// WaitRetries returns the wait retries. +func (a *ApplyImpl) WaitRetries() int { + return a.ApplyOptions.WaitRetries +} + // WaitForJobs returns the wait for jobs. func (a *ApplyImpl) WaitForJobs() bool { return a.ApplyOptions.WaitForJobs diff --git a/pkg/config/sync.go b/pkg/config/sync.go index c8927b72..aaea12bb 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -20,6 +20,8 @@ type SyncOptions struct { SkipCRDs bool // Wait is the wait flag Wait bool + // WaitRetries is the wait retries flag + WaitRetries int // WaitForJobs is the wait for jobs flag WaitForJobs bool // ReuseValues is true if the helm command should reuse the values @@ -110,6 +112,11 @@ func (t *SyncImpl) Wait() bool { return t.SyncOptions.Wait } +// WaitRetries returns the wait retries +func (t *SyncImpl) WaitRetries() int { + return t.SyncOptions.WaitRetries +} + // WaitForJobs returns the wait for jobs func (t *SyncImpl) WaitForJobs() bool { return t.SyncOptions.WaitForJobs diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go index 88e8e98a..2988a605 100644 --- a/pkg/state/helmx.go +++ b/pkg/state/helmx.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strconv" "github.com/helmfile/chartify" @@ -105,15 +106,31 @@ func (st *HelmState) appendWaitForJobsFlags(flags []string, release *ReleaseSpec return flags } -func (st *HelmState) appendWaitFlags(flags []string, release *ReleaseSpec, ops *SyncOpts) []string { +func (st *HelmState) appendWaitFlags(flags []string, helm helmexec.Interface, release *ReleaseSpec, ops *SyncOpts) []string { + var hasWait bool switch { case release.Wait != nil && *release.Wait: + hasWait = true flags = append(flags, "--wait") case ops != nil && ops.Wait: + hasWait = true flags = append(flags, "--wait") case release.Wait == nil && st.HelmDefaults.Wait: + hasWait = true flags = append(flags, "--wait") } + // see https://github.com/helm/helm/releases/tag/v3.15.0 + // https://github.com/helm/helm/commit/fc74964 + if hasWait && helm.IsVersionAtLeast("3.15.0") { + switch { + case release.WaitRetries != nil && *release.WaitRetries > 0: + flags = append(flags, "--wait-retries", strconv.Itoa(*release.WaitRetries)) + case ops != nil && ops.WaitRetries > 0: + flags = append(flags, "--wait-retries", strconv.Itoa(ops.WaitRetries)) + case release.WaitRetries == nil && st.HelmDefaults.WaitRetries > 0: + flags = append(flags, "--wait-retries", strconv.Itoa(st.HelmDefaults.WaitRetries)) + } + } return flags } diff --git a/pkg/state/helmx_test.go b/pkg/state/helmx_test.go index c9cdc44f..7db6c37d 100644 --- a/pkg/state/helmx_test.go +++ b/pkg/state/helmx_test.go @@ -76,13 +76,16 @@ func TestAppendWaitFlags(t *testing.T) { name string release *ReleaseSpec syncOpts *SyncOpts + helm helmexec.Interface helmSpec HelmSpec expected []string }{ + // --wait { name: "release wait", release: &ReleaseSpec{Wait: &[]bool{true}[0]}, syncOpts: nil, + helm: testutil.NewVersionHelmExec("3.11.0"), helmSpec: HelmSpec{}, expected: []string{"--wait"}, }, @@ -90,6 +93,7 @@ func TestAppendWaitFlags(t *testing.T) { name: "cli flags wait", release: &ReleaseSpec{}, syncOpts: &SyncOpts{Wait: true}, + helm: testutil.NewVersionHelmExec("3.11.0"), helmSpec: HelmSpec{}, expected: []string{"--wait"}, }, @@ -97,6 +101,7 @@ func TestAppendWaitFlags(t *testing.T) { name: "helm defaults wait", release: &ReleaseSpec{}, syncOpts: nil, + helm: testutil.NewVersionHelmExec("3.11.0"), helmSpec: HelmSpec{Wait: true}, expected: []string{"--wait"}, }, @@ -104,6 +109,7 @@ func TestAppendWaitFlags(t *testing.T) { name: "release wait false", release: &ReleaseSpec{Wait: &[]bool{false}[0]}, syncOpts: nil, + helm: testutil.NewVersionHelmExec("3.11.0"), helmSpec: HelmSpec{Wait: true}, expected: []string{}, }, @@ -111,6 +117,7 @@ func TestAppendWaitFlags(t *testing.T) { name: "cli flags wait false", release: &ReleaseSpec{}, syncOpts: &SyncOpts{}, + helm: testutil.NewVersionHelmExec("3.11.0"), helmSpec: HelmSpec{Wait: true}, expected: []string{"--wait"}, }, @@ -118,16 +125,58 @@ func TestAppendWaitFlags(t *testing.T) { name: "helm defaults wait false", release: &ReleaseSpec{}, syncOpts: nil, + helm: testutil.NewVersionHelmExec("3.11.0"), helmSpec: HelmSpec{Wait: false}, expected: []string{}, }, + // --wait-retries + { + name: "release wait and retry unsupported", + release: &ReleaseSpec{Wait: &[]bool{true}[0], WaitRetries: &[]int{1}[0]}, + syncOpts: nil, + helm: testutil.NewVersionHelmExec("3.11.0"), + helmSpec: HelmSpec{}, + expected: []string{"--wait"}, + }, + { + name: "release wait and retry supported", + release: &ReleaseSpec{Wait: &[]bool{true}[0], WaitRetries: &[]int{1}[0]}, + syncOpts: nil, + helm: testutil.NewVersionHelmExec("3.15.0"), + helmSpec: HelmSpec{}, + expected: []string{"--wait", "--wait-retries", "1"}, + }, + { + name: "no wait retry", + release: &ReleaseSpec{WaitRetries: &[]int{1}[0]}, + syncOpts: nil, + helm: testutil.NewVersionHelmExec("3.15.0"), + helmSpec: HelmSpec{}, + expected: []string{}, + }, + { + name: "cli flags wait and retry", + release: &ReleaseSpec{}, + syncOpts: &SyncOpts{Wait: true, WaitRetries: 2}, + helm: testutil.NewVersionHelmExec("3.15.0"), + helmSpec: HelmSpec{}, + expected: []string{"--wait", "--wait-retries", "2"}, + }, + { + name: "helm defaults wait retry", + release: &ReleaseSpec{}, + syncOpts: nil, + helm: testutil.NewVersionHelmExec("3.15.0"), + helmSpec: HelmSpec{Wait: true, WaitRetries: 3}, + expected: []string{"--wait", "--wait-retries", "3"}, + }, } 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) + got := st.appendWaitFlags([]string{}, tt.helm, tt.release, tt.syncOpts) require.Equalf(t, tt.expected, got, "appendWaitFlags() = %v, want %v", got, tt.expected) }) } diff --git a/pkg/state/state.go b/pkg/state/state.go index a28c05d9..38215228 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -167,6 +167,8 @@ type HelmSpec struct { Devel bool `yaml:"devel"` // Wait, if set to true, will wait until all Pods, PVCs, Services, and minimum number of Pods of a Deployment are in a ready state before marking the release as successful Wait bool `yaml:"wait"` + // WaitRetries, if set and --wait enabled, will retry any failed check on resource state, except if HTTP status code < 500 is received, subject to the specified number of retries + WaitRetries int `yaml:"waitRetries"` // WaitForJobs, if set and --wait enabled, will wait until all Jobs have been completed before marking the release as successful. It will wait for as long as --timeout WaitForJobs bool `yaml:"waitForJobs"` // Timeout is the time in seconds to wait for any individual Kubernetes operation (like Jobs for hooks, and waits on pod/pvc/svc/deployment readiness) (default 300) @@ -262,6 +264,8 @@ type ReleaseSpec struct { Devel *bool `yaml:"devel,omitempty"` // Wait, if set to true, will wait until all Pods, PVCs, Services, and minimum number of Pods of a Deployment are in a ready state before marking the release as successful Wait *bool `yaml:"wait,omitempty"` + // WaitRetries, if set and --wait enabled, will retry any failed check on resource state, except if HTTP status code < 500 is received, subject to the specified number of retries + WaitRetries *int `yaml:"waitRetries,omitempty"` // WaitForJobs, if set and --wait enabled, will wait until all Jobs have been completed before marking the release as successful. It will wait for as long as --timeout WaitForJobs *bool `yaml:"waitForJobs,omitempty"` // Timeout is the time in seconds to wait for any individual Kubernetes operation (like Jobs for hooks, and waits on pod/pvc/svc/deployment readiness) (default 300) @@ -781,6 +785,7 @@ type SyncOpts struct { SkipCleanup bool SkipCRDs bool Wait bool + WaitRetries int WaitForJobs bool ReuseValues bool ResetValues bool @@ -1122,6 +1127,7 @@ type ChartPrepareOptions struct { Validate bool IncludeCRDs *bool Wait bool + WaitRetries int WaitForJobs bool OutputDir string OutputDirTemplate string @@ -2730,7 +2736,7 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp flags = append(flags, "--enable-dns") } - flags = st.appendWaitFlags(flags, release, opt) + flags = st.appendWaitFlags(flags, helm, release, opt) flags = st.appendWaitForJobsFlags(flags, release, opt) // non-OCI chart should be verified here