Jitter kubernetes dial backoff so builders do not retry in lockstep

[1991santhu]

The problem

calculateBackoff is a pure function of the attempt number:

return min(time.Duration(1<<uint(attempt))*baseDelay, maxDelay)

so every builder retrying the same condition waits for exactly 500ms, 1s, 2s, 4s, 8s.

That matters specifically because of the condition this retry exists to handle. From the comment on tryWithBackoff:

// This handles the race condition where Kubernetes marks nodes as "Ready" before their
// Certificate Signing Requests (CSRs) are approved, causing transient TLS errors.

CSR approval lag is a cluster-wide, time-correlated event rather than a per-pod accident. When a node group comes up, every build scheduled onto those nodes hits the transient TLS error at roughly the same moment — and then retries at identical instants. In CI, where parallel builds are the normal case, that concentrates retries on the API server precisely while it is still working through the approval backlog.

The backoff limits how often each builder retries. It does not stop builders retrying together.

The change

Equal jitter — retain half the computed interval as a floor, randomise the remainder:

d := min(time.Duration(1<<uint(attempt))*baseDelay, maxDelay)
half := d / 2

return half + time.Duration(rand.Int63n(int64(d-half)+1))

Half is kept rather than using full jitter so a retry is never issued immediately after a TLS failure. The result never exceeds maxDelay, so no builder waits longer than it does today — the change only removes the alignment.

Testing

go test -count=2 ./driver/kubernetes/... passes, go vet clean.

The package had no test file, so this adds one covering: the result stays within [d/2, d] for attempts 0–5 across 500 samples each; a large attempt count stays capped at maxDelay; and successive calls vary, which fails if the implementation regresses to deterministic.

Note

math/rand rather than crypto/rand — this is load spreading, not a security boundary. Happy to change if the project prefers otherwise. Context for the original retry: #2668.

[1991santhu]

Corrected the direction of the jitter in my own patch.

It was equal jitter, [d/2, d]. At attempt 0 that lets a retry fire at 250ms when baseDelay is 500ms, so it hits the API server sooner than configured while it's still working through the CSR backlog. It adds now, [d, 2d], bounded by the headroom to maxDelay. With maxRetries=5 the delays used are 500ms through 4s, so the 10s cap never comes into play.

I ran the numbers rather than just asserting the problem. 100 builders hitting the same CSR lag: without jitter all 100 dial inside the same 100ms window on every attempt, with it about 28 on the first retry and 7 by the fourth. Simulated arrival times, not something I've measured on a real cluster.

go test -count=2 -run TestCalculateBackoff ./driver/kubernetes/ passes.

[crazy-max]

The direction makes sense to me.

One small wording nit: this is additive jitter, [d, 2d] capped by maxDelay, not equal jitter anymore. That behavior is fine if preserving the current exponential delay as the floor is intentional, but the description/comment should call it that directly.

Also, please squash the commits before merge.

[crazy-max]

The new math/rand call should use the same gosec suppression pattern as podchooser.RandomPodChooser, since retry jitter doesn't need crypto randomness:

return d + time.Duration(rand.Int63n(int64(extra)+1)) // #nosec G404 -- no strong randomness required for retry jitter

[1991santhu]

Thanks, all three done.

Switched to the same pattern as podchooser.RandomPodChooser:

return d + time.Duration(rand.Int63n(int64(extra)+1)) // #nosec G404 -- no strong randomness required for retry jitter

You're right about the wording. The doc comment said "backoff and jitter" without saying which kind, which is worse than useless here since the whole point of the change is that the jitter is additive rather than centred. It now says so directly: additive jitter drawing from [d, 2d] capped by maxDelay, with the exponential value as the floor rather than the midpoint. That is intentional, for the reason the rest of the comment gives, so a retry can never fire sooner than the schedule alone would have.

Squashed to one commit.

Verified with golangci-lint v2.8.0 built from source against Go 1.26, matching how the lint job builds it: 0 issues. go test -count=2 -run TestCalculateBackoff ./driver/kubernetes/ passes.