cmd/testwrapper: add a max retry time across all failures (#20453)

We've occasionally seen CI jobs retry broken commits for a long time
because we only implement a budget per test. Add a cap to ensure we
never spend an unreasonable amount of time on retries.

Updates tailscale/corp#43604

Signed-off-by: Tom Proctor <tomhjp@users.noreply.github.com>

cmd/testwrapper/testwrapper.go

@@ -71,6 +71,11 @@
 	// minRetries is the minimum number of retry attempts we make for a failed
 	// test, regardless of perTestBudget. Override via TS_TESTWRAPPER_MIN_RETRIES.
 	minRetries = envInt("TS_TESTWRAPPER_MIN_RETRIES", 2)
+	// maxRetryTime is the maximum wall-clock time we are willing to spend on the
+	// whole retry phase. It ensures a run with many flakes or real failures
+	// doesn't block for an unreasonable amount of time. Override via
+	// TS_TESTWRAPPER_MAX_RETRY_TIME (a time.Duration string).
+	maxRetryTime = envDuration("TS_TESTWRAPPER_MAX_RETRY_TIME", 10*time.Minute)
 )
 
 func envDuration(key string, def time.Duration) time.Duration {
@@ -572,7 +577,7 @@ func computePerAttemptTimeout(firstFail time.Duration) time.Duration {
 }
 
 // retryFailedTest runs the per-test retry loop for ft. It updates ft in place.
-func retryFailedTest(ctx context.Context, ft *failedTest, goTestArgs, testArgs []string) {
+func retryFailedTest(ctx context.Context, ft *failedTest, goTestArgs, testArgs []string, deadline time.Time) {
 	perAttempt := computePerAttemptTimeout(ft.firstFailDuration)
 	for {
 		if ft.everPassed {
@@ -584,6 +589,14 @@ func retryFailedTest(ctx context.Context, ft *failedTest, goTestArgs, testArgs [
 		if ft.attempts >= minRetries && ft.totalRetryElapsed >= perTestBudget {
 			return
 		}
+		if remaining := time.Until(deadline); remaining < perAttempt {
+			// perAttempt represents a reasonable guess of how long the test might take
+			// to run, so don't even attempt to retry a test that is likely to take us
+			// past our overall deadline.
+			log.Printf("testwrapper: not retrying %s.%s because its timeout (%.1fs) would exceed the remaining max retry time (%.1fs)",
+				ft.pkg, ft.testName, perAttempt.Seconds(), remaining.Seconds())
+			return
+		}
 
 		// FlakeAttemptEnv is 1-indexed counting the first pass as attempt 1.
 		// Retry attempt N is FlakeAttemptEnv = 1 + N.
@@ -845,9 +858,10 @@ func main() {
 
 	// Second pass: retry each failed test serially with its per-test budget.
 	if len(failed) > 0 {
+		deadline := time.Now().Add(maxRetryTime)
 		fmt.Printf("\n\nRetrying %d failed test(s) to detect flakiness...\n\n", len(failed))
 		for _, ft := range failed {
-			retryFailedTest(ctx, ft, goTestArgs, testArgs)
+			retryFailedTest(ctx, ft, goTestArgs, testArgs, deadline)
 		}
 	}
 

cmd/testwrapper/testwrapper_test.go

@@ -450,6 +450,55 @@ func TestCached(t *testing.T) {}
 	}
 }
 
+// TestMaxRetryTime verifies the suite-wide retry deadline
+// (TS_TESTWRAPPER_MAX_RETRY_TIME) is a hard backstop: when the remaining time
+// is smaller than a single attempt's timeout, testwrapper skips the retry
+// entirely — even below minRetries — and reports the test as a permanent
+// failure. Because computePerAttemptTimeout floors each attempt at 30s, any
+// deadline under 30s deterministically cuts off the first retry with no timing
+// race.
+func TestMaxRetryTime(t *testing.T) {
+	t.Parallel()
+
+	testfile := filepath.Join(t.TempDir(), "maxretrytime_test.go")
+	code := []byte(`package maxretrytime_test
+
+import "testing"
+
+func TestAlwaysFail(t *testing.T) {
+	t.Fatal("always fails")
+}
+`)
+	if err := os.WriteFile(testfile, code, 0o644); err != nil {
+		t.Fatalf("writing package: %s", err)
+	}
+
+	cmd := cmdTestwrapper(t, "-v", testfile)
+	cmd.Env = append(cmd.Env, "TS_TESTWRAPPER_MAX_RETRY_TIME=1ms")
+	out, err := cmd.CombinedOutput()
+	if code, ok := errExitCode(err); !ok || code != 1 {
+		t.Fatalf("testwrapper %s: expected exit code 1, got %v; output:\n%s", testfile, err, out)
+	}
+	if !bytes.Contains(out, []byte("permanent test failures JSON:")) {
+		t.Errorf("missing permanent test failures JSON line in output:\n%s", out)
+	}
+	if !bytes.Contains(out, []byte("would exceed the remaining max retry time")) {
+		t.Errorf("missing retry-deadline skip message in output:\n%s", out)
+	}
+	// The deadline overrides minRetries, so no retry attempt should run: the
+	// test executes exactly once (the first pass).
+	if got := bytes.Count(out, []byte("[retry ")); got != 0 {
+		t.Errorf("expected no retry attempts, but %d were made; output:\n%s", got, out)
+	}
+	if runs := bytes.Count(out, []byte("=== RUN   TestAlwaysFail")); runs != 1 {
+		t.Errorf("expected TestAlwaysFail to run exactly once, ran %d times; output:\n%s", runs, out)
+	}
+
+	if testing.Verbose() {
+		t.Logf("success - output:\n%s", out)
+	}
+}
+
 func errExitCode(err error) (int, bool) {
 	if exit, ok := errors.AsType[*exec.ExitError](err); ok {
 		return exit.ExitCode(), true