From 2c38611acd07a4f22693c35630c68af673f5be66 Mon Sep 17 00:00:00 2001 From: Connor Hindley Date: Tue, 11 Feb 2025 17:28:37 -0600 Subject: [PATCH] feat: Add support for --wait-retries flag. (#1922) * 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 --- docs/index.md | 3 ++ 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 | 67 ++++++++++++++++++++++++++++++++++++++++- pkg/state/state.go | 8 ++++- pkg/state/temp_test.go | 12 ++++---- 10 files changed, 125 insertions(+), 9 deletions(-) diff --git a/docs/index.md b/docs/index.md index 88c819b7..22bf5fbe 100644 --- a/docs/index.md +++ b/docs/index.md @@ -195,6 +195,8 @@ helmDefaults: keyring: path/to/keyring.gpg # wait for k8s resources via --wait. (default false) wait: true + # if set and --wait enabled, will retry any failed check on resource state subject to the specified number of retries (default 0) + waitRetries: 3 # 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 (default false, Implemented in Helm3.5) waitForJobs: true # time in seconds to wait for any individual Kubernetes operation (like Jobs for hooks, and waits on pod/pvc/svc/deployment readiness) (default 300) @@ -312,6 +314,7 @@ releases: verify: true keyring: path/to/keyring.gpg wait: true + waitRetries: 3 waitForJobs: true timeout: 60 recreatePods: true 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..d60cd225 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,74 @@ 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"}, + }, + { + name: "release wait default retries", + release: &ReleaseSpec{Wait: &[]bool{true}[0]}, + syncOpts: nil, + helm: testutil.NewVersionHelmExec("3.15.0"), + helmSpec: HelmSpec{WaitRetries: 4}, + expected: []string{"--wait", "--wait-retries", "4"}, + }, + { + name: "release retries default wait", + release: &ReleaseSpec{WaitRetries: &[]int{5}[0]}, + syncOpts: nil, + helm: testutil.NewVersionHelmExec("3.15.0"), + helmSpec: HelmSpec{Wait: true}, + expected: []string{"--wait", "--wait-retries", "5"}, + }, } 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 diff --git a/pkg/state/temp_test.go b/pkg/state/temp_test.go index cc6769be..596d2dde 100644 --- a/pkg/state/temp_test.go +++ b/pkg/state/temp_test.go @@ -38,39 +38,39 @@ func TestGenerateID(t *testing.T) { run(testcase{ subject: "baseline", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, - want: "foo-values-5b58697694", + want: "foo-values-669d45cd7b", }) run(testcase{ subject: "different bytes content", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, data: []byte(`{"k":"v"}`), - want: "foo-values-58bff47d77", + want: "foo-values-67d8c67fcf", }) run(testcase{ subject: "different map content", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, data: map[string]any{"k": "v"}, - want: "foo-values-5fb8948f75", + want: "foo-values-b9bc64677", }) run(testcase{ subject: "different chart", release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"}, - want: "foo-values-784b76684f", + want: "foo-values-585c4565f5", }) run(testcase{ subject: "different name", release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"}, - want: "bar-values-f48df5f49", + want: "bar-values-c94846459", }) run(testcase{ subject: "specific ns", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"}, - want: "myns-foo-values-6b68696b8c", + want: "myns-foo-values-798d69477", }) for id, n := range ids {