From c1195f01626c5cd67e61f1570b255e4790c90866 Mon Sep 17 00:00:00 2001
From: Santhosh Kumar Somarapu <somarapu.santhosh91@gmail.com>
Date: Wed, 5 Aug 2026 07:00:52 -0700
Subject: [PATCH] driver/kubernetes: jitter the dial backoff so builders do not
 retry in lockstep

calculateBackoff was a pure function of the attempt number, so every builder
retrying the same condition waited exactly the same durations. That matters
here because CSR approval lagging node readiness is a cluster-wide event:
concurrent builds scheduled onto newly-ready nodes hit the transient TLS error
at the same moment, then retry in unison against an API server already working
through the approval backlog.

Add jitter drawn from [d, 2d], capped by maxDelay, where d is the exponential
value for the attempt. The exponential value is the floor rather than the
midpoint, so a retry is never issued sooner than the schedule would have on its
own. Centring it would let the first retry fire at baseDelay/2, which undercuts
a configured minimum at exactly the wrong moment. With maxRetries=5 and
baseDelay=500ms the delays used are 500ms through 4s, so the 10s cap is never
reached in practice.

Marked the math/rand call with the same #nosec pattern podchooser uses.

Signed-off-by: Santhosh Kumar Somarapu <somarapu.santhosh91@gmail.com>
---
 driver/kubernetes/driver.go      | 33 ++++++++++++++++--
 driver/kubernetes/driver_test.go | 58 ++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 2 deletions(-)
 create mode 100644 driver/kubernetes/driver_test.go

diff --git a/driver/kubernetes/driver.go b/driver/kubernetes/driver.go
index 2f76e0d0365b..4736e369c36a 100644
--- a/driver/kubernetes/driver.go
+++ b/driver/kubernetes/driver.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	stderrors "errors"
 	"fmt"
+	"math/rand"
 	"net"
 	"strings"
 	"syscall"
@@ -389,9 +390,37 @@ func isTransientConnectionError(err error) bool {
 	return false
 }
 
-// calculateBackoff calculates the delay for the given attempt with exponential backoff.
+// calculateBackoff calculates the delay for the given attempt with exponential
+// backoff and additive jitter, drawing from [d, 2d] capped by maxDelay, where d
+// is the exponential value for the attempt. The exponential value is the floor
+// rather than the midpoint, so a retry is never issued sooner than the schedule
+// would have on its own.
+//
+// The exponential component alone is a pure function of the attempt number, so
+// every builder retrying the same condition waits for exactly the same durations.
+// That matters for the case this backoff exists to handle: CSR approval lagging
+// node readiness is a cluster-wide event, so concurrent builds scheduled onto
+// newly-ready nodes hit the transient TLS error at the same moment and would
+// then retry in unison, concentrating load on the API server while it is already
+// working through the approval backlog.
+//
+// The jitter is added to the interval rather than centred on it, so a retry is
+// never issued sooner than the exponential schedule intended. Centring it would
+// let the first retry fire after baseDelay/2, undercutting a configured minimum
+// while the API server is still working through the CSR backlog. The extra is
+// bounded by the remaining headroom so the result never exceeds maxDelay.
 func calculateBackoff(attempt int, baseDelay, maxDelay time.Duration) time.Duration {
-	return min(time.Duration(1<<uint(attempt))*baseDelay, maxDelay)
+	d := min(time.Duration(1<<uint(attempt))*baseDelay, maxDelay)
+
+	extra := d
+	if headroom := maxDelay - d; headroom < extra {
+		extra = headroom
+	}
+	if extra <= 0 {
+		return d
+	}
+
+	return d + time.Duration(rand.Int63n(int64(extra)+1)) // #nosec G404 -- no strong randomness required for retry jitter
 }
 
 func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client.Client, error) {
diff --git a/driver/kubernetes/driver_test.go b/driver/kubernetes/driver_test.go
new file mode 100644
index 000000000000..55ccd3b464b9
--- /dev/null
+++ b/driver/kubernetes/driver_test.go
@@ -0,0 +1,58 @@
+package kubernetes
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestCalculateBackoffNeverShorterThanSchedule(t *testing.T) {
+	const (
+		baseDelay = 500 * time.Millisecond
+		maxDelay  = 10 * time.Second
+	)
+
+	for attempt := range 6 {
+		schedule := min(time.Duration(1<<uint(attempt))*baseDelay, maxDelay)
+		ceiling := min(2*schedule, maxDelay)
+
+		for range 500 {
+			got := calculateBackoff(attempt, baseDelay, maxDelay)
+			require.GreaterOrEqual(t, got, schedule,
+				"attempt %d must never wait less than the exponential schedule", attempt)
+			require.LessOrEqual(t, got, ceiling,
+				"attempt %d must not exceed twice the schedule, nor maxDelay", attempt)
+		}
+	}
+}
+
+func TestCalculateBackoffRespectsMaxDelay(t *testing.T) {
+	const (
+		baseDelay = 500 * time.Millisecond
+		maxDelay  = 10 * time.Second
+	)
+
+	for range 500 {
+		require.LessOrEqual(t, calculateBackoff(20, baseDelay, maxDelay), maxDelay,
+			"a large attempt count must stay capped at maxDelay")
+	}
+}
+
+func TestCalculateBackoffVaries(t *testing.T) {
+	const (
+		baseDelay = 500 * time.Millisecond
+		maxDelay  = 10 * time.Second
+	)
+
+	seen := make(map[time.Duration]struct{})
+	for range 500 {
+		seen[calculateBackoff(3, baseDelay, maxDelay)] = struct{}{}
+	}
+
+	// A deterministic implementation returns a single value. The range at
+	// attempt 3 is two seconds wide, so one distinct value across 500 draws
+	// would not be chance.
+	require.Greater(t, len(seen), 1,
+		"backoff must vary so concurrent builders do not retry in lockstep")
+}