fix(eks): show eksctl progress and the failure cause during cluster lifecycle

pkg/client/eksctl/client.go

@@ -39,6 +39,20 @@ type EnvironmentRunner interface {
 	) (stdout, stderr []byte, err error)
 }
 
+// ProgressRunner is the optional extension implemented by runners that can write
+// a command's output to progress while it runs, in addition to returning it
+// buffered. A nil environment preserves os/exec's default inheritance.
+type ProgressRunner interface {
+	RunWithProgress(
+		ctx context.Context,
+		name string,
+		args []string,
+		stdin io.Reader,
+		environment []string,
+		progress io.Writer,
+	) (stdout, stderr []byte, err error)
+}
+
 // ExecRunner is the default Runner that shells out via os/exec.
 type ExecRunner struct{}
 
@@ -61,6 +75,31 @@ func (ExecRunner) RunWithEnvironment(
 	args []string,
 	stdin io.Reader,
 	environment []string,
+) ([]byte, []byte, error) {
+	return runCommand(ctx, name, args, stdin, environment, nil)
+}
+
+// RunWithProgress executes the command with environment, writing each complete
+// output line to progress as it arrives while still returning both buffers.
+func (ExecRunner) RunWithProgress(
+	ctx context.Context,
+	name string,
+	args []string,
+	stdin io.Reader,
+	environment []string,
+	progress io.Writer,
+) ([]byte, []byte, error) {
+	return runCommand(ctx, name, args, stdin, environment, progress)
+}
+
+// runCommand executes one eksctl process, optionally mirroring its output to progress.
+func runCommand(
+	ctx context.Context,
+	name string,
+	args []string,
+	stdin io.Reader,
+	environment []string,
+	progress io.Writer,
 ) ([]byte, []byte, error) {
 	// #nosec G204 -- This uses os/exec directly with a program name and argv
 	// slice; it does not invoke a shell, so user-influenced values in args
@@ -74,6 +113,21 @@ func (ExecRunner) RunWithEnvironment(
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr
 
+	if progress != nil {
+		// One line buffer per stream, so stdout and stderr never interleave mid-line. os/exec copies
+		// the two streams on separate goroutines, and a caller's writer need not be safe for
+		// concurrent use, so both buffers write through one shared lock.
+		shared := newLockedWriter(progress)
+		stdoutLines := newLineWriter(shared, nil)
+		stderrLines := newLineWriter(shared, nil)
+
+		defer stdoutLines.Flush()
+		defer stderrLines.Flush()
+
+		cmd.Stdout = io.MultiWriter(&stdout, stdoutLines)
+		cmd.Stderr = io.MultiWriter(&stderr, stderrLines)
+	}
+
 	if stdin != nil {
 		cmd.Stdin = stdin
 	}
@@ -91,6 +145,7 @@ type Client struct {
 	binary      string
 	runner      Runner
 	environment []string
+	progress    io.Writer
 
 	requireCredentialValues bool
 }
@@ -127,6 +182,16 @@ func WithEnvironment(environment []string) Option {
 	}
 }
 
+// WithProgressWriter streams the output of long-running eksctl commands (create,
+// delete, scale, upgrade) to w while they run, with credential values redacted.
+// Read-only listings are never streamed, because their stdout is parsed. A nil
+// writer disables streaming.
+func WithProgressWriter(w io.Writer) Option {
+	return func(c *Client) {
+		c.progress = w
+	}
+}
+
 // RequireCredentialValues makes execution fail closed unless the explicit
 // child environment contains either a profile or a complete static credential
 // pair. Use it when custom source names were configured so an unset alias
@@ -179,14 +244,7 @@ func (c *Client) CheckAvailable() error {
 // escape hatch used by all higher-level methods on this client and can be
 // used directly when a helper has not been written yet.
 func (c *Client) Exec(ctx context.Context, args ...string) ([]byte, []byte, error) {
-	stdout, stderr, err := c.run(ctx, args, nil)
-
-	stderr = c.redactCredentialValues(stderr)
-	if err != nil {
-		return stdout, stderr, wrapExecErr(args, stderr, err)
-	}
-
-	return stdout, stderr, nil
+	return c.exec(ctx, nil, nil, args)
 }
 
 // ExecWithStdin runs eksctl with the given arguments and feeds stdin from the
@@ -196,11 +254,30 @@ func (c *Client) ExecWithStdin(
 	stdin io.Reader,
 	args ...string,
 ) ([]byte, []byte, error) {
-	stdout, stderr, err := c.run(ctx, args, stdin)
+	return c.exec(ctx, stdin, nil, args)
+}
+
+// execWithProgress runs a long-running mutating command, streaming its output to the
+// configured progress writer. Commands whose stdout is parsed must use Exec instead.
+func (c *Client) execWithProgress(ctx context.Context, args ...string) error {
+	_, _, err := c.exec(ctx, nil, c.progress, args)
+
+	return err
+}
+
+// exec runs eksctl, redacts credential values from stderr, and wraps a failure with the
+// trailing output of both streams.
+func (c *Client) exec(
+	ctx context.Context,
+	stdin io.Reader,
+	progress io.Writer,
+	args []string,
+) ([]byte, []byte, error) {
+	stdout, stderr, err := c.run(ctx, args, stdin, progress)
 
 	stderr = c.redactCredentialValues(stderr)
 	if err != nil {
-		return stdout, stderr, wrapExecErr(args, stderr, err)
+		return stdout, stderr, wrapExecErr(args, c.redactCredentialValues(stdout), stderr, err)
 	}
 
 	return stdout, stderr, nil
@@ -211,12 +288,32 @@ func (c *Client) run(
 	ctx context.Context,
 	args []string,
 	stdin io.Reader,
+	progress io.Writer,
 ) ([]byte, []byte, error) {
 	err := c.validateCredentialValues()
 	if err != nil {
 		return nil, nil, err
 	}
 
+	if progressRunner, ok := c.runner.(ProgressRunner); ok && progress != nil {
+		lines := newLineWriter(progress, c.redactCredentialValues)
+		defer lines.Flush()
+
+		stdout, stderr, err := progressRunner.RunWithProgress(
+			ctx,
+			c.binary,
+			args,
+			stdin,
+			cloneStrings(c.environment),
+			lines,
+		)
+		if err != nil {
+			return stdout, stderr, fmt.Errorf("run eksctl with progress: %w", err)
+		}
+
+		return stdout, stderr, nil
+	}
+
 	if c.environment == nil {
 		stdout, stderr, err := c.runner.Run(ctx, c.binary, args, stdin)
 		if err != nil {
@@ -329,18 +426,23 @@ func environmentValues(environment []string) map[string]string {
 	return values
 }
 
-// wrapExecErr annotates an exec failure with the invoked arguments and the
-// first line of stderr (if any) to produce actionable error messages without
-// leaking the full eksctl output into Go error strings.
-func wrapExecErr(args []string, stderr []byte, err error) error {
+// wrapExecErr annotates an exec failure with the invoked arguments, the first
+// line of stderr (if any), and a bounded tail of stdout and stderr. eksctl logs
+// the cause of a failure on stdout and prints only a generic line on stderr, so
+// the tail is what makes the error actionable. Both streams must already be
+// redacted; the tail is capped in both lines and bytes, so neither a long output nor a single
+// very long line can carry the whole of it into an error.
+func wrapExecErr(args []string, stdout, stderr []byte, err error) error {
 	const (
 		firstLineParts = 2
 	)
 
 	firstStderrLine := strings.SplitN(strings.TrimSpace(string(stderr)), "\n", firstLineParts)[0]
-	if firstStderrLine == "" {
-		return fmt.Errorf("eksctl %s: %w", strings.Join(args, " "), err)
+
+	wrapped := fmt.Errorf("eksctl %s: %w", strings.Join(args, " "), err)
+	if firstStderrLine != "" {
+		wrapped = fmt.Errorf("eksctl %s: %w: %s", strings.Join(args, " "), err, firstStderrLine)
 	}
 
-	return fmt.Errorf("eksctl %s: %w: %s", strings.Join(args, " "), err, firstStderrLine)
+	return withOutputTail(wrapped, stdout, stderr, firstStderrLine)
 }

pkg/client/eksctl/client_test.go

@@ -328,7 +328,7 @@ func TestCreateCluster_ConfigRegionIsNotDuplicatedAsAFlag(t *testing.T) {
 
 	assert.Equal(t, "eksctl-under-test", runner.lastName)
 	assert.Equal(t,
-		[]string{"create", "cluster", "--config-file", "eks.yaml"},
+		[]string{"create", "cluster", "--config-file", "eks.yaml", "--timeout", "45m"},
 		runner.lastArgs,
 	)
 }
@@ -343,7 +343,7 @@ func TestCreateCluster_NoRegionOmitsFlag(t *testing.T) {
 	require.NoError(t, err)
 
 	assert.Equal(t,
-		[]string{"create", "cluster", "--config-file", "eks.yaml"},
+		[]string{"create", "cluster", "--config-file", "eks.yaml", "--timeout", "45m"},
 		runner.lastArgs,
 	)
 }
@@ -367,6 +367,31 @@ func TestCreateCluster_WithKubeconfigPinsOutputPath(t *testing.T) {
 			"create", "cluster",
 			"--config-file", "eks.yaml",
 			"--kubeconfig", "/tmp/ksail-kubeconfig",
+			"--timeout", "45m",
+		},
+		runner.lastArgs,
+	)
+}
+
+func TestCreateCluster_PassesExplicitWaitTimeout(t *testing.T) {
+	t.Parallel()
+
+	runner := &fakeRunner{}
+	client := newTestClient(runner)
+
+	err := client.CreateCluster(t.Context(), "eks.yaml", "us-east-1")
+	require.NoError(t, err)
+
+	// Without this flag eksctl falls back to its own default wait. That is what let a
+	// slow provision outlive the smoke workflow's create-step budget and be killed
+	// with no cause in the log (#7078): the step timeout fires first and discards the
+	// diagnosis eksctl was about to print. The value is asserted literally so that
+	// changing the budget has to be a deliberate edit here too.
+	assert.Equal(t,
+		[]string{
+			"create", "cluster",
+			"--config-file", "eks.yaml",
+			"--timeout", "45m",
 		},
 		runner.lastArgs,
 	)

pkg/client/eksctl/commands.go

@@ -21,6 +21,26 @@ const subcommandCluster = "cluster"
 // outputFormatJSON is the eksctl --output value requesting JSON, shared by every listing call.
 const outputFormatJSON = "json"
 
+// flagTimeout is the eksctl flag bounding how long a command waits for its
+// CloudFormation stacks to settle.
+const flagTimeout = "--timeout"
+
+// createClusterWaitTimeout bounds how long `eksctl create cluster` waits before it
+// gives up and says why.
+//
+// Without it eksctl uses its own default, which is not stated in KSail and cannot be
+// relied on to sit below the CI budget. When the workflow's create step expires first
+// it kills eksctl mid-wait, so the run records that create failed but not what it was
+// waiting on — the missing-cause failure in #7078.
+//
+// The value sits above a realistic provision — a live EKS create was still building
+// its node group at 36m22s (run 34999766125) — and below the 60-minute create-step
+// budget the smoke workflow gets in #7008. That ordering is what makes eksctl the one
+// that gives up first, with a message, so #7008 lands before this is fully effective;
+// until it does, the step's current 30-minute budget still expires first and this wait
+// is an upper bound that is never reached.
+const createClusterWaitTimeout = "45m"
+
 // ClusterSummary represents a single cluster entry returned by
 // `eksctl get cluster -o json`. Field tags preserve eksctl's PascalCase
 // JSON keys (eksctl emits these names verbatim).
@@ -82,9 +102,9 @@ func (c *Client) createCluster(
 		args = append(args, "--kubeconfig", path)
 	}
 
-	_, _, err := c.Exec(ctx, args...)
+	args = append(args, flagTimeout, createClusterWaitTimeout)
 
-	return err
+	return c.execWithProgress(ctx, args...)
 }
 
 // CreateNodegroup creates the node groups in a config file. The caller supplies
@@ -94,9 +114,7 @@ func (c *Client) CreateNodegroup(ctx context.Context, configPath string) error {
 		return ErrEmptyConfigPath
 	}
 
-	_, _, err := c.Exec(ctx, "create", "nodegroup", "--config-file", configPath)
-
-	return err
+	return c.execWithProgress(ctx, "create", "nodegroup", "--config-file", configPath)
 }
 
 // DeleteCluster invokes `eksctl delete cluster --name <name> [--region <region>]`.
@@ -125,9 +143,7 @@ func (c *Client) DeleteCluster(
 		args = append(args, "--wait")
 	}
 
-	_, _, err := c.Exec(ctx, args...)
-
-	return err
+	return c.execWithProgress(ctx, args...)
 }
 
 // GetCluster returns the summary for a named cluster. Returns
@@ -267,9 +283,7 @@ func (c *Client) ScaleNodegroup(
 		args = append(args, "--region", region)
 	}
 
-	_, _, err := c.Exec(ctx, args...)
-
-	return err
+	return c.execWithProgress(ctx, args...)
 }
 
 // UpgradeCluster invokes `eksctl upgrade cluster -f <configPath>`.
@@ -289,9 +303,7 @@ func (c *Client) UpgradeCluster(
 		args = append(args, "--approve")
 	}
 
-	_, _, err := c.Exec(ctx, args...)
-
-	return err
+	return c.execWithProgress(ctx, args...)
 }
 
 // parseClusterSummaries unmarshals the JSON output of `eksctl get cluster`.