Merge pull request #31421 from benoittgt/improve-doc-wait-strategy

Unify --wait strategy inputs, clarify defaults and docs; avoid SDK timeout footgun

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'",
+		"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)
@@ -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":
@@ -89,11 +89,11 @@ 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:
-		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.HookOnlyStrategy, kube.LegacyStrategy)
 	}
 }
 

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
@@ -165,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))
 	}
 }
 

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,6 +62,9 @@ func alwaysReady(_ *unstructured.Unstructured) (*status.Result, error) {
 }
 
 func (w *statusWaiter) WatchUntilReady(resourceList ResourceList, timeout time.Duration) error {
+	if timeout == 0 {
+		timeout = DefaultStatusWatcherTimeout
+	}
 	ctx, cancel := w.contextWithTimeout(timeout)
 	defer cancel()
 	slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout)
@@ -76,6 +86,9 @@ func (w *statusWaiter) WatchUntilReady(resourceList ResourceList, timeout time.D
 }
 
 func (w *statusWaiter) Wait(resourceList ResourceList, timeout time.Duration) error {
+	if timeout == 0 {
+		timeout = DefaultStatusWatcherTimeout
+	}
 	ctx, cancel := w.contextWithTimeout(timeout)
 	defer cancel()
 	slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout)
@@ -84,6 +97,9 @@ func (w *statusWaiter) Wait(resourceList ResourceList, timeout time.Duration) er
 }
 
 func (w *statusWaiter) WaitWithJobs(resourceList ResourceList, timeout time.Duration) error {
+	if timeout == 0 {
+		timeout = DefaultStatusWatcherTimeout
+	}
 	ctx, cancel := w.contextWithTimeout(timeout)
 	defer cancel()
 	slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout)
@@ -95,6 +111,9 @@ func (w *statusWaiter) WaitWithJobs(resourceList ResourceList, timeout time.Dura
 }
 
 func (w *statusWaiter) WaitForDelete(resourceList ResourceList, timeout time.Duration) error {
+	if timeout == 0 {
+		timeout = DefaultStatusWatcherTimeout
+	}
 	ctx, cancel := w.contextWithTimeout(timeout)
 	defer cancel()
 	slog.Debug("waiting for resources to be deleted", "count", len(resourceList), "timeout", timeout)