From 1ff042cfa73232db6694dcd72061507a37996154 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 25 Apr 2026 01:56:04 -0500 Subject: [PATCH 1/3] fix(e2e): add SSH retry logic for flaky Windows WSL tests SSH connections to devcontainers on Windows+WSL runners can fail transiently when the devsy agent binary injection has not completed. Retry exit-code-1 failures with exponential backoff (3 attempts, ~5s/~10s waits) while letting permanent failures propagate immediately. --- e2e/framework/command.go | 4 ++- e2e/framework/retry.go | 60 +++++++++++++++++++++++++++++++++++++ e2e/framework/retry_test.go | 35 ++++++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) diff --git a/e2e/framework/command.go b/e2e/framework/command.go index 0d1955ed1..069c8eb33 100644 --- a/e2e/framework/command.go +++ b/e2e/framework/command.go @@ -145,7 +145,9 @@ func (f *Framework) DevsySSH( workspace string, command string, ) (string, error) { - out, err := f.ExecCommandOutput(ctx, []string{"ssh", workspace, "--command", command}) + out, err := execWithSSHRetry(ctx, workspace, func(ctx context.Context) (string, error) { + return f.ExecCommandOutput(ctx, []string{"ssh", workspace, "--command", command}) + }) if err != nil { return "", fmt.Errorf("devsy ssh failed: %s", err.Error()) } diff --git a/e2e/framework/retry.go b/e2e/framework/retry.go index fa8141e1d..3b1d04fc5 100644 --- a/e2e/framework/retry.go +++ b/e2e/framework/retry.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os/exec" "strings" "time" @@ -20,6 +21,16 @@ var dockerPullBackoff = wait.Backoff{ Jitter: 0.1, } +// sshBackoff defines retry timing for transient SSH failures on Windows+WSL +// runners where the devsy agent binary injection can intermittently fail. +// 3 total attempts (1 initial + 2 retries) with waits of ~5s, ~10s. +var sshBackoff = wait.Backoff{ + Steps: 3, + Duration: 5 * time.Second, + Factor: 2.0, + Jitter: 0.1, +} + // retryableDockerPatterns are stderr substrings indicating a transient Docker // registry error that is worth retrying. var retryableDockerPatterns = []string{ @@ -31,6 +42,17 @@ var retryableDockerPatterns = []string{ "503 Service Unavailable", } +// isRetryableSSHError returns true when the error indicates a transient SSH +// failure (exit code 1) typically caused by the devsy agent not yet being +// ready inside the container. +func isRetryableSSHError(err error) bool { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + return exitErr.ExitCode() == 1 + } + return false +} + // isRetryableDockerError returns true if stderr contains a transient Docker // registry error (rate limits, timeouts, connection resets). func isRetryableDockerError(stderr string) bool { @@ -78,3 +100,41 @@ func execWithDockerRetry( } return lastStdout, lastStderr, err } + +// execWithSSHRetry runs fn and retries if the error indicates a transient SSH +// failure (exit status 1). This handles the case where the devsy agent binary +// injection into a WSL container has not completed yet. +func execWithSSHRetry( + ctx context.Context, + workspace string, + fn func(ctx context.Context) (string, error), +) (string, error) { + var lastOut string + var lastErr error + attempt := 0 + + err := wait.ExponentialBackoffWithContext(ctx, sshBackoff, + func(ctx context.Context) (bool, error) { + attempt++ + lastOut, lastErr = fn(ctx) + if lastErr == nil { + return true, nil // success + } + if isRetryableSSHError(lastErr) { + ginkgo.GinkgoWriter.Printf( + "[retry] ssh %s: attempt %d failed with transient error, retrying: %s\n", + workspace, attempt, lastErr, + ) + return false, nil // retry + } + return false, lastErr // non-retryable, stop immediately + }, + ) + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return lastOut, err + } + if err != nil && lastErr != nil { + return lastOut, fmt.Errorf("after %d attempts: %w", attempt, lastErr) + } + return lastOut, err +} diff --git a/e2e/framework/retry_test.go b/e2e/framework/retry_test.go index 50fb59f46..3271a2b35 100644 --- a/e2e/framework/retry_test.go +++ b/e2e/framework/retry_test.go @@ -1,11 +1,46 @@ package framework import ( + "fmt" + "os/exec" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +// exitError runs a shell command that exits with the given code and returns +// the resulting *exec.ExitError. +func exitError(t *testing.T, code string) *exec.ExitError { + t.Helper() + // #nosec G204 -- test helper with controlled exit code argument + err := exec.Command("sh", "-c", "exit "+code).Run() + require.Error(t, err) + var exitErr *exec.ExitError + require.ErrorAs(t, err, &exitErr) + return exitErr +} + +func TestIsRetryableSSHError_ExitStatus1(t *testing.T) { + assert.True(t, isRetryableSSHError(exitError(t, "1"))) +} + +func TestIsRetryableSSHError_ExitStatus10(t *testing.T) { + assert.False(t, isRetryableSSHError(exitError(t, "10"))) +} + +func TestIsRetryableSSHError_ExitStatus127(t *testing.T) { + assert.False(t, isRetryableSSHError(exitError(t, "127"))) +} + +func TestIsRetryableSSHError_Nil(t *testing.T) { + assert.False(t, isRetryableSSHError(nil)) +} + +func TestIsRetryableSSHError_NonExitError(t *testing.T) { + assert.False(t, isRetryableSSHError(fmt.Errorf("some other error"))) +} + func TestIsRetryableDockerError_RateLimit(t *testing.T) { stderr := `GET https://index.docker.io/v2/library/ubuntu/manifests/latest: ` + `TOOMANYREQUESTS: You have reached your unauthenticated pull rate limit.` From 85d0512eaa647d58db1d53b417263624822c611f Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 25 Apr 2026 07:29:16 -0500 Subject: [PATCH 2/3] fix(e2e): narrow SSH retry to actual connection failures, not any exit code 1 isRetryableSSHError was treating any exit code 1 as a transient SSH error, but remote commands like `cat nonexistent-file` also return exit code 1. This caused the postAttachCommand test to fail because cat retries masked the expected file-not-found error. Now requires both exit code 1 AND SSH-specific stderr patterns (connection refused, connection reset, tunnel to container, etc.) to trigger a retry. DevsySSH uses ExecCommandCapture to thread stderr through to the check. --- e2e/framework/command.go | 4 +-- e2e/framework/retry.go | 43 +++++++++++++++++++++++-------- e2e/framework/retry_test.go | 51 ++++++++++++++++++++++++++++++++----- 3 files changed, 80 insertions(+), 18 deletions(-) diff --git a/e2e/framework/command.go b/e2e/framework/command.go index 069c8eb33..7e84b51e8 100644 --- a/e2e/framework/command.go +++ b/e2e/framework/command.go @@ -145,8 +145,8 @@ func (f *Framework) DevsySSH( workspace string, command string, ) (string, error) { - out, err := execWithSSHRetry(ctx, workspace, func(ctx context.Context) (string, error) { - return f.ExecCommandOutput(ctx, []string{"ssh", workspace, "--command", command}) + out, err := execWithSSHRetry(ctx, workspace, func(ctx context.Context) (string, string, error) { + return f.ExecCommandCapture(ctx, []string{"ssh", workspace, "--command", command}) }) if err != nil { return "", fmt.Errorf("devsy ssh failed: %s", err.Error()) diff --git a/e2e/framework/retry.go b/e2e/framework/retry.go index 3b1d04fc5..1387d69eb 100644 --- a/e2e/framework/retry.go +++ b/e2e/framework/retry.go @@ -42,13 +42,35 @@ var retryableDockerPatterns = []string{ "503 Service Unavailable", } +// retryableSSHPatterns are stderr substrings indicating a transient SSH +// connection error (as opposed to a remote command that legitimately failed). +var retryableSSHPatterns = []string{ + "connection refused", + "connection reset", + "ssh: connect to host", + "tunnel to container", + "no such container", + "connection timed out", + "broken pipe", +} + // isRetryableSSHError returns true when the error indicates a transient SSH -// failure (exit code 1) typically caused by the devsy agent not yet being -// ready inside the container. -func isRetryableSSHError(err error) bool { +// connection failure. Both exit code 1 AND an SSH-specific pattern in stderr +// are required so that remote command failures (e.g. cat on a missing file) +// are not mistakenly retried. +func isRetryableSSHError(err error, stderr string) bool { var exitErr *exec.ExitError - if errors.As(err, &exitErr) { - return exitErr.ExitCode() == 1 + if !errors.As(err, &exitErr) { + return false + } + if exitErr.ExitCode() != 1 { + return false + } + lower := strings.ToLower(stderr) + for _, pattern := range retryableSSHPatterns { + if strings.Contains(lower, pattern) { + return true + } } return false } @@ -102,25 +124,26 @@ func execWithDockerRetry( } // execWithSSHRetry runs fn and retries if the error indicates a transient SSH -// failure (exit status 1). This handles the case where the devsy agent binary -// injection into a WSL container has not completed yet. +// connection failure. The fn callback must return stdout, stderr, and error so +// that stderr can be inspected for SSH-specific patterns. func execWithSSHRetry( ctx context.Context, workspace string, - fn func(ctx context.Context) (string, error), + fn func(ctx context.Context) (stdout, stderr string, err error), ) (string, error) { var lastOut string + var lastStderr string var lastErr error attempt := 0 err := wait.ExponentialBackoffWithContext(ctx, sshBackoff, func(ctx context.Context) (bool, error) { attempt++ - lastOut, lastErr = fn(ctx) + lastOut, lastStderr, lastErr = fn(ctx) if lastErr == nil { return true, nil // success } - if isRetryableSSHError(lastErr) { + if isRetryableSSHError(lastErr, lastStderr) { ginkgo.GinkgoWriter.Printf( "[retry] ssh %s: attempt %d failed with transient error, retrying: %s\n", workspace, attempt, lastErr, diff --git a/e2e/framework/retry_test.go b/e2e/framework/retry_test.go index 3271a2b35..4f6f8b793 100644 --- a/e2e/framework/retry_test.go +++ b/e2e/framework/retry_test.go @@ -21,24 +21,63 @@ func exitError(t *testing.T, code string) *exec.ExitError { return exitErr } -func TestIsRetryableSSHError_ExitStatus1(t *testing.T) { - assert.True(t, isRetryableSSHError(exitError(t, "1"))) +func TestIsRetryableSSHError_ConnectionRefused(t *testing.T) { + assert.True( + t, + isRetryableSSHError( + exitError(t, "1"), + "ssh: connect to host 127.0.0.1 port 22: Connection refused", + ), + ) +} + +func TestIsRetryableSSHError_ConnectionReset(t *testing.T) { + assert.True(t, isRetryableSSHError(exitError(t, "1"), "Connection reset by peer")) +} + +func TestIsRetryableSSHError_TunnelToContainer(t *testing.T) { + assert.True(t, isRetryableSSHError(exitError(t, "1"), "error: tunnel to container failed")) +} + +func TestIsRetryableSSHError_ConnectionTimedOut(t *testing.T) { + assert.True( + t, + isRetryableSSHError( + exitError(t, "1"), + "ssh: connect to host 10.0.0.1 port 22: Connection timed out", + ), + ) +} + +func TestIsRetryableSSHError_ExitCode1_NoSSHPattern(t *testing.T) { + // Remote command failure (e.g. cat on missing file) — should NOT be retried. + assert.False( + t, + isRetryableSSHError( + exitError(t, "1"), + "cat: /home/user/post-attach.out: No such file or directory", + ), + ) +} + +func TestIsRetryableSSHError_ExitCode1_EmptyStderr(t *testing.T) { + assert.False(t, isRetryableSSHError(exitError(t, "1"), "")) } func TestIsRetryableSSHError_ExitStatus10(t *testing.T) { - assert.False(t, isRetryableSSHError(exitError(t, "10"))) + assert.False(t, isRetryableSSHError(exitError(t, "10"), "connection refused")) } func TestIsRetryableSSHError_ExitStatus127(t *testing.T) { - assert.False(t, isRetryableSSHError(exitError(t, "127"))) + assert.False(t, isRetryableSSHError(exitError(t, "127"), "connection refused")) } func TestIsRetryableSSHError_Nil(t *testing.T) { - assert.False(t, isRetryableSSHError(nil)) + assert.False(t, isRetryableSSHError(nil, "connection refused")) } func TestIsRetryableSSHError_NonExitError(t *testing.T) { - assert.False(t, isRetryableSSHError(fmt.Errorf("some other error"))) + assert.False(t, isRetryableSSHError(fmt.Errorf("some other error"), "connection refused")) } func TestIsRetryableDockerError_RateLimit(t *testing.T) { From 6a1123e724642e3e7fbc0caaf56e73afd1145d59 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 25 Apr 2026 08:02:59 -0500 Subject: [PATCH 3/3] fix(e2e): remove 'tunnel to container' from retryable SSH patterns classifyTunnelErrors() wraps every container command failure with 'tunnel to container:', so this pattern matched remote command exits (like cat on a missing file), not just SSH connection failures. --- e2e/framework/retry.go | 1 - e2e/framework/retry_test.go | 16 ++++++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/e2e/framework/retry.go b/e2e/framework/retry.go index 1387d69eb..fec0e9b79 100644 --- a/e2e/framework/retry.go +++ b/e2e/framework/retry.go @@ -48,7 +48,6 @@ var retryableSSHPatterns = []string{ "connection refused", "connection reset", "ssh: connect to host", - "tunnel to container", "no such container", "connection timed out", "broken pipe", diff --git a/e2e/framework/retry_test.go b/e2e/framework/retry_test.go index 4f6f8b793..836dd70a2 100644 --- a/e2e/framework/retry_test.go +++ b/e2e/framework/retry_test.go @@ -35,8 +35,20 @@ func TestIsRetryableSSHError_ConnectionReset(t *testing.T) { assert.True(t, isRetryableSSHError(exitError(t, "1"), "Connection reset by peer")) } -func TestIsRetryableSSHError_TunnelToContainer(t *testing.T) { - assert.True(t, isRetryableSSHError(exitError(t, "1"), "error: tunnel to container failed")) +func TestIsRetryableSSHError_TunnelToContainer_NotRetryable(t *testing.T) { + assert.False(t, isRetryableSSHError(exitError(t, "1"), "error: tunnel to container failed")) +} + +func TestIsRetryableSSHError_RemoteCommandFailure(t *testing.T) { + // The devsy CLI wraps all command failures with "tunnel to container:" in stderr. + // This must NOT be treated as a transient SSH error. + assert.False( + t, + isRetryableSSHError( + exitError(t, "1"), + "tunnel to container: run in container: ssh session: Process exited with status 1", + ), + ) } func TestIsRetryableSSHError_ConnectionTimedOut(t *testing.T) {