Retry throttled bootstrap data requests - #282
Conversation
|
Thanks for the contribution! This pull request comes from a fork, so the Azure E2E workflow is intentionally skipped for security reasons. Merge will remain blocked until the E2E tests have been run from a branch in the Maintainer options:
We do not run Azure E2E directly from fork PR code because it requires Azure OIDC access. |
There was a problem hiding this comment.
Pull request overview
This PR adds resilient client-side handling for AKS RP subscription-scoped throttling on listBootstrapData, so FlexNode nodes can successfully join even when the first bootstrap-data request receives HTTP 429 responses.
Changes:
- Add bounded retry logic for HTTP 429 responses, honoring
Retry-Afterand applying exponential full jitter to spread concurrent retries. - Refresh ARM access tokens before each retry and stop retrying promptly on context cancellation/deadlines.
- Add unit tests covering retry behavior, token refresh, Retry-After parsing, and retry budget constraints.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/bootstrapdata/bootstrap_data.go | Implements 429 retry loop with jittered backoff, Retry-After parsing, and token refresh per retry. |
| pkg/bootstrapdata/bootstrap_data_test.go | Adds test coverage for throttling retries, deadlines/cancellation, and Retry-After/backoff behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if response.StatusCode != http.StatusTooManyRequests || retry == maxRetries { | ||
| return response, nil | ||
| } | ||
|
|
||
| delay := throttleRetryDelay(response.Header.Get("Retry-After"), retry, time.Now(), deps.jitter) | ||
| if deadline, ok := ctx.Deadline(); ok && time.Until(deadline) <= delay { | ||
| return response, nil | ||
| } |
| if request.Header.Get("Authorization") != "Bearer arm-token" { | ||
| t.Error("missing token") | ||
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
pkg/bootstrapdata/bootstrap_data.go:120
- defaultDependencies() used to configure an http.Client that disallowed redirects for the bootstrap-data call. With the switch to the ARM SDK client, redirects can again be followed by the default transport, which changes prior behavior and can risk leaking the bearer token on same-host redirects. Consider restoring the redirect prohibition by defaulting the injected Transporter to an http.Client with CheckRedirect rejecting redirects.
func defaultDependencies() dependencies {
return dependencies{credential: newCredential}
}
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
| "net/http" | ||
| "net/url" |
GitHub Actions
Inactive
|
Superseded by same-repository PR #283 so the Azure E2E workflow can run. |
Summary
listBootstrapDatathrough the generatedarmcontainerservice/v9AgentPoolsClientRetry-AfterContext
AKS RP is adding subscription-scoped throttling to
listBootstrapDatawith a 100-request burst and one-token-per-second refill. Without client retries, a throttled first-boot request exits before writing bootstrap data and the node does not join.Validation
go test ./...go vet ./...go test -race ./pkg/bootstrapdatagolangci-lint run --timeout=5m(v2.13.0)git diff --check