From 5cbd9b3035cee869566491c686c1f90ce1b52171 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Fri, 24 Oct 2025 18:34:28 +0200 Subject: [PATCH 01/11] While testing SDK features for v4. I was surprised with the error: > "reporter failed to start: event funnel closed: context deadline exceeded" It happens when you forget to set a minimal timeout: ```go upgradeClient := action.NewUpgrade(actionConfig) upgradeClient.WaitStrategy = kube.StatusWatcherStrategy // When Timeout is zero, the status wait uses a context with zero timeout which // immediately expires causing "context deadline exceeded" errors. upgradeClient.Timeout = 2 * time.Minute ``` Also maybe it might be worth documenting this more clearly. Initial [PR](https://github.com/helm/helm/pull/13604) say: > I have not written any docs, I assume that can be done when we are closer to Helm 4, a lot of it is covered by linking to - https://github.com/kubernetes-sigs/cli-utils/blob/master/pkg/kstatus/README.md Related: - https://github.com/helm/helm/pull/31411#issuecomment-3443925663 Signed-off-by: Benoit Tigeot --- pkg/kube/client.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 68f1e6475..56de52651 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -98,12 +98,23 @@ type Client struct { var _ Interface = (*Client)(nil) +// WaitStrategy represents the algorithm used to wait for Kubernetes +// resources to reach their desired state. type WaitStrategy string const ( + // StatusWatcherStrategy: event-driven waits using kstatus (watches + aggregated readers). + // Default for --wait. More accurate and responsive; waits CRs and full reconciliation. + // Requires: reachable API server, list+watch RBAC on deployed resources, and a non-zero timeout. StatusWatcherStrategy WaitStrategy = "watcher" - LegacyStrategy WaitStrategy = "legacy" - HookOnlyStrategy WaitStrategy = "hookOnly" + + // LegacyStrategy: Helm 3-style periodic polling until ready or timeout. + // Use when watches aren’t available/reliable, or for compatibility/simple CI. + // Requires only list RBAC for polled resources. + LegacyStrategy WaitStrategy = "legacy" + + // HookOnlyStrategy: wait only for hook Pods/Jobs to complete; does not wait for general chart resources. + HookOnlyStrategy WaitStrategy = "hookOnly" ) type FieldValidationDirective string From 95e1ee108936f15657f644e2e87f0b4c8319edb2 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Fri, 24 Oct 2025 18:54:59 +0200 Subject: [PATCH 02/11] Increase documentation of --wait flag Signed-off-by: Benoit Tigeot --- pkg/cmd/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index b20772ef9..941602211 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -59,7 +59,7 @@ func AddWaitFlag(cmd *cobra.Command, wait *kube.WaitStrategy) { cmd.Flags().Var( newWaitValue(kube.HookOnlyStrategy, wait), "wait", - "if specified, will wait until all resources are in the expected state before marking the operation as successful. It will wait for as long as --timeout. Valid inputs are 'watcher' and 'legacy'", + "wait until resources are ready (up to --timeout). Values: 'watcher' (default), 'legacy'. '--wait' without value = 'watcher'. Booleans deprecated; '--wait=false' does hook-only.", ) // Sets the strategy to use the watcher strategy if `--wait` is used without an argument cmd.Flags().Lookup("wait").NoOptDefVal = string(kube.StatusWatcherStrategy) From e8b0cff45a21de6fe144789830e46c188e713313 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Sat, 25 Oct 2025 11:15:06 +0200 Subject: [PATCH 03/11] Update pkg/cmd/flags.go Co-authored-by: George Jenkins Signed-off-by: Benoit Tigeot Signed-off-by: Benoit Tigeot --- pkg/cmd/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index 941602211..4f416ddf0 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -59,7 +59,7 @@ func AddWaitFlag(cmd *cobra.Command, wait *kube.WaitStrategy) { cmd.Flags().Var( newWaitValue(kube.HookOnlyStrategy, wait), "wait", - "wait until resources are ready (up to --timeout). Values: 'watcher' (default), 'legacy'. '--wait' without value = 'watcher'. Booleans deprecated; '--wait=false' does hook-only.", + "wait until resources are ready (up to --timeout). Values: 'watcher' (default), 'legacy', and 'hookOnly'", ) // Sets the strategy to use the watcher strategy if `--wait` is used without an argument cmd.Flags().Lookup("wait").NoOptDefVal = string(kube.StatusWatcherStrategy) From a5e110fccd9b8ce6f0dc110d2c83ee079f15e8e4 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Sat, 25 Oct 2025 13:25:22 +0200 Subject: [PATCH 04/11] Make wait strategy selection more obvious It is not clear enough that you need to set --wait=false for a hook only strategy. Signed-off-by: Benoit Tigeot --- pkg/cmd/flags.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index 4f416ddf0..1aef6430a 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -81,7 +81,7 @@ func (ws *waitValue) String() string { func (ws *waitValue) Set(s string) error { switch s { - case string(kube.StatusWatcherStrategy), string(kube.LegacyStrategy): + case string(kube.StatusWatcherStrategy), string(kube.LegacyStrategy), string(kube.HookOnlyStrategy): *ws = waitValue(s) return nil case "true": @@ -93,7 +93,7 @@ func (ws *waitValue) Set(s string) error { *ws = waitValue(kube.HookOnlyStrategy) return nil default: - return fmt.Errorf("invalid wait input %q. Valid inputs are %s, and %s", s, kube.StatusWatcherStrategy, kube.LegacyStrategy) + return fmt.Errorf("invalid wait input %q. Valid inputs are %s, %s, and %s", s, kube.StatusWatcherStrategy, kube.LegacyStrategy, kube.HookOnlyStrategy) } } From 1836f37f4d5a59f8e8fa9b78bb2517e6069292ac Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Sat, 25 Oct 2025 13:27:35 +0200 Subject: [PATCH 05/11] The default is not HookOnlyStrategy but WaitStrategy Signed-off-by: Benoit Tigeot --- pkg/cmd/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index 1aef6430a..55633e62b 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -89,7 +89,7 @@ func (ws *waitValue) Set(s string) error { *ws = waitValue(kube.StatusWatcherStrategy) return nil case "false": - slog.Warn("--wait=false is deprecated (boolean value) and can be replaced by omitting the --wait flag") + slog.Warn("--wait=false is deprecated (boolean value) and can be replaced with --wait=hookOnly") *ws = waitValue(kube.HookOnlyStrategy) return nil default: From 8535e9f4ab5c143fcbd429f45a0f15cf510a0cc0 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Sat, 25 Oct 2025 13:34:31 +0200 Subject: [PATCH 06/11] Avoid confusion between `--wait` (watcher) and no --wait (hookOnly) Signed-off-by: Benoit Tigeot --- pkg/cmd/flags.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index 55633e62b..ee4806280 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -57,9 +57,9 @@ func addValueOptionsFlags(f *pflag.FlagSet, v *values.Options) { func AddWaitFlag(cmd *cobra.Command, wait *kube.WaitStrategy) { cmd.Flags().Var( - newWaitValue(kube.HookOnlyStrategy, wait), + newWaitValue(kube.StatusWatcherStrategy, wait), "wait", - "wait until resources are ready (up to --timeout). Values: 'watcher' (default), 'legacy', and 'hookOnly'", + "wait until resources are ready (up to --timeout). Values: 'watcher' (default), 'hookOnly', and 'legacy'", ) // Sets the strategy to use the watcher strategy if `--wait` is used without an argument cmd.Flags().Lookup("wait").NoOptDefVal = string(kube.StatusWatcherStrategy) From 11128659aa76b09aae2e7903c7790dabbe7a4c95 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 27 Oct 2025 10:50:12 +0100 Subject: [PATCH 07/11] Provide more help for SDK user when setting up WaitStrategy Signed-off-by: Benoit Tigeot --- pkg/kube/client.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 56de52651..a660d7075 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -176,8 +176,10 @@ func (c *Client) GetWaiter(strategy WaitStrategy) (Waiter, error) { return nil, err } return &hookOnlyWaiter{sw: sw}, nil + case "": + return nil, errors.New("wait strategy not set. Choose one of: " + string(StatusWatcherStrategy) + ", " + string(HookOnlyStrategy) + ", " + string(LegacyStrategy)) default: - return nil, errors.New("unknown wait strategy") + return nil, errors.New("unknown wait strategy (s" + string(strategy) + "). Valid values are: " + string(StatusWatcherStrategy) + ", " + string(HookOnlyStrategy) + ", " + string(LegacyStrategy)) } } From 52a282832fad5691bbcfb28cf8c3dfade914a597 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 27 Oct 2025 14:58:40 +0100 Subject: [PATCH 08/11] Do not change the default waiting strategy when --wait is not set It is too much a breaking change for a new RC of beta V4. See comment: https://github.com/helm/helm/pull/31421#discussion_r2465121438 Signed-off-by: Benoit Tigeot --- pkg/cmd/flags.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index ee4806280..5b8b8cf21 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -57,9 +57,9 @@ func addValueOptionsFlags(f *pflag.FlagSet, v *values.Options) { func AddWaitFlag(cmd *cobra.Command, wait *kube.WaitStrategy) { cmd.Flags().Var( - newWaitValue(kube.StatusWatcherStrategy, wait), + newWaitValue(kube.HookOnlyStrategy, wait), "wait", - "wait until resources are ready (up to --timeout). Values: 'watcher' (default), 'hookOnly', and 'legacy'", + "wait until resources are ready (up to --timeout). Values: 'watcher', 'hookOnly' (default), and 'legacy'", ) // Sets the strategy to use the watcher strategy if `--wait` is used without an argument cmd.Flags().Lookup("wait").NoOptDefVal = string(kube.StatusWatcherStrategy) From 5f6fa437b2c2954092b6c3f8af5b83aef7dcb874 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 27 Oct 2025 15:19:09 +0100 Subject: [PATCH 09/11] Prevent surprising failure with SDK when timeout is not set While testing SDK features for v4. I was surprised with the error: "reporter failed to start: event funnel closed: context deadline exceeded" This occurs when no timeout is set: ``` upgradeClient := action.NewUpgrade(actionConfig) upgradeClient.WaitStrategy = kube.StatusWatcherStrategy // When Timeout is zero, the status wait uses a context with zero timeout which // immediately expires causing "context deadline exceeded" errors. upgradeClient.Timeout = 2 * time.Minute ``` With this patch it will work without specifying. Signed-off-by: Benoit Tigeot --- pkg/kube/statuswait.go | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/pkg/kube/statuswait.go b/pkg/kube/statuswait.go index cd9722eda..31d4f4444 100644 --- a/pkg/kube/statuswait.go +++ b/pkg/kube/statuswait.go @@ -47,6 +47,13 @@ type statusWaiter struct { ctx context.Context } +// DefaultStatusWatcherTimeout is the timeout used by the status waiter when a +// zero timeout is provided. This prevents callers from accidentally passing a +// zero value (which would immediately cancel the context) and getting +// "context deadline exceeded" errors. SDK callers can rely on this default +// when they don't set a timeout. +var DefaultStatusWatcherTimeout = 30 * time.Second + func alwaysReady(_ *unstructured.Unstructured) (*status.Result, error) { return &status.Result{ Status: status.CurrentStatus, @@ -55,7 +62,10 @@ func alwaysReady(_ *unstructured.Unstructured) (*status.Result, error) { } func (w *statusWaiter) WatchUntilReady(resourceList ResourceList, timeout time.Duration) error { - ctx, cancel := w.contextWithTimeout(timeout) + if timeout == 0 { + timeout = DefaultStatusWatcherTimeout + } + ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout) sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper) @@ -76,7 +86,16 @@ func (w *statusWaiter) WatchUntilReady(resourceList ResourceList, timeout time.D } func (w *statusWaiter) Wait(resourceList ResourceList, timeout time.Duration) error { +<<<<<<< HEAD ctx, cancel := w.contextWithTimeout(timeout) +||||||| parent of 86f98f870 (Prevent surprising failure with SDK when timeout is not set) + ctx, cancel := context.WithTimeout(context.TODO(), timeout) +======= + if timeout == 0 { + timeout = DefaultStatusWatcherTimeout + } + ctx, cancel := context.WithTimeout(context.TODO(), timeout) +>>>>>>> 86f98f870 (Prevent surprising failure with SDK when timeout is not set) defer cancel() slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout) sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper) @@ -84,7 +103,16 @@ func (w *statusWaiter) Wait(resourceList ResourceList, timeout time.Duration) er } func (w *statusWaiter) WaitWithJobs(resourceList ResourceList, timeout time.Duration) error { +<<<<<<< HEAD ctx, cancel := w.contextWithTimeout(timeout) +||||||| parent of 86f98f870 (Prevent surprising failure with SDK when timeout is not set) + ctx, cancel := context.WithTimeout(context.TODO(), timeout) +======= + if timeout == 0 { + timeout = DefaultStatusWatcherTimeout + } + ctx, cancel := context.WithTimeout(context.TODO(), timeout) +>>>>>>> 86f98f870 (Prevent surprising failure with SDK when timeout is not set) defer cancel() slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout) sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper) @@ -95,7 +123,16 @@ func (w *statusWaiter) WaitWithJobs(resourceList ResourceList, timeout time.Dura } func (w *statusWaiter) WaitForDelete(resourceList ResourceList, timeout time.Duration) error { +<<<<<<< HEAD ctx, cancel := w.contextWithTimeout(timeout) +||||||| parent of 86f98f870 (Prevent surprising failure with SDK when timeout is not set) + ctx, cancel := context.WithTimeout(context.TODO(), timeout) +======= + if timeout == 0 { + timeout = DefaultStatusWatcherTimeout + } + ctx, cancel := context.WithTimeout(context.TODO(), timeout) +>>>>>>> 86f98f870 (Prevent surprising failure with SDK when timeout is not set) defer cancel() slog.Debug("waiting for resources to be deleted", "count", len(resourceList), "timeout", timeout) sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper) From 277c140c871ed112da916d8997c7d243e39f41b3 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 27 Oct 2025 15:39:13 +0100 Subject: [PATCH 10/11] Error strategy list match help Signed-off-by: Benoit Tigeot --- pkg/cmd/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index 5b8b8cf21..3269720a1 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -93,7 +93,7 @@ func (ws *waitValue) Set(s string) error { *ws = waitValue(kube.HookOnlyStrategy) return nil default: - return fmt.Errorf("invalid wait input %q. Valid inputs are %s, %s, and %s", s, kube.StatusWatcherStrategy, kube.LegacyStrategy, kube.HookOnlyStrategy) + return fmt.Errorf("invalid wait input %q. Valid inputs are %s, %s, and %s", s, kube.StatusWatcherStrategy, kube.HookOnlyStrategy, kube.LegacyStrategy) } } From 02312a1cf2124abc27e2cd261d8df8dcc0c6f1e4 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 29 Oct 2025 23:00:36 +0100 Subject: [PATCH 11/11] Update pkg/cmd/flags.go Co-authored-by: George Jenkins Signed-off-by: Benoit Tigeot Signed-off-by: Benoit Tigeot --- pkg/cmd/flags.go | 2 +- pkg/kube/statuswait.go | 26 ++++---------------------- 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index 3269720a1..efe0be6e7 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -59,7 +59,7 @@ func AddWaitFlag(cmd *cobra.Command, wait *kube.WaitStrategy) { cmd.Flags().Var( newWaitValue(kube.HookOnlyStrategy, wait), "wait", - "wait until resources are ready (up to --timeout). Values: 'watcher', 'hookOnly' (default), and 'legacy'", + "if specified, wait until resources are ready (up to --timeout). Values: 'watcher' (default), 'hookOnly', and 'legacy'.", ) // Sets the strategy to use the watcher strategy if `--wait` is used without an argument cmd.Flags().Lookup("wait").NoOptDefVal = string(kube.StatusWatcherStrategy) diff --git a/pkg/kube/statuswait.go b/pkg/kube/statuswait.go index 31d4f4444..225321f6e 100644 --- a/pkg/kube/statuswait.go +++ b/pkg/kube/statuswait.go @@ -65,7 +65,7 @@ func (w *statusWaiter) WatchUntilReady(resourceList ResourceList, timeout time.D if timeout == 0 { timeout = DefaultStatusWatcherTimeout } - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := w.contextWithTimeout(timeout) defer cancel() slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout) sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper) @@ -86,16 +86,10 @@ func (w *statusWaiter) WatchUntilReady(resourceList ResourceList, timeout time.D } func (w *statusWaiter) Wait(resourceList ResourceList, timeout time.Duration) error { -<<<<<<< HEAD - ctx, cancel := w.contextWithTimeout(timeout) -||||||| parent of 86f98f870 (Prevent surprising failure with SDK when timeout is not set) - ctx, cancel := context.WithTimeout(context.TODO(), timeout) -======= if timeout == 0 { timeout = DefaultStatusWatcherTimeout } - ctx, cancel := context.WithTimeout(context.TODO(), timeout) ->>>>>>> 86f98f870 (Prevent surprising failure with SDK when timeout is not set) + ctx, cancel := w.contextWithTimeout(timeout) defer cancel() slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout) sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper) @@ -103,16 +97,10 @@ func (w *statusWaiter) Wait(resourceList ResourceList, timeout time.Duration) er } func (w *statusWaiter) WaitWithJobs(resourceList ResourceList, timeout time.Duration) error { -<<<<<<< HEAD - ctx, cancel := w.contextWithTimeout(timeout) -||||||| parent of 86f98f870 (Prevent surprising failure with SDK when timeout is not set) - ctx, cancel := context.WithTimeout(context.TODO(), timeout) -======= if timeout == 0 { timeout = DefaultStatusWatcherTimeout } - ctx, cancel := context.WithTimeout(context.TODO(), timeout) ->>>>>>> 86f98f870 (Prevent surprising failure with SDK when timeout is not set) + ctx, cancel := w.contextWithTimeout(timeout) defer cancel() slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout) sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper) @@ -123,16 +111,10 @@ func (w *statusWaiter) WaitWithJobs(resourceList ResourceList, timeout time.Dura } func (w *statusWaiter) WaitForDelete(resourceList ResourceList, timeout time.Duration) error { -<<<<<<< HEAD - ctx, cancel := w.contextWithTimeout(timeout) -||||||| parent of 86f98f870 (Prevent surprising failure with SDK when timeout is not set) - ctx, cancel := context.WithTimeout(context.TODO(), timeout) -======= if timeout == 0 { timeout = DefaultStatusWatcherTimeout } - ctx, cancel := context.WithTimeout(context.TODO(), timeout) ->>>>>>> 86f98f870 (Prevent surprising failure with SDK when timeout is not set) + ctx, cancel := w.contextWithTimeout(timeout) defer cancel() slog.Debug("waiting for resources to be deleted", "count", len(resourceList), "timeout", timeout) sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper)