resolver: retry oauth token fetch on transient network errors - #7008
resolver: retry oauth token fetch on transient network errors#7008wussh wants to merge 1 commit into
Conversation
Blob/manifest fetches already retry transient errors (ECONNRESET, EOF, 5xx, etc.) via retryhandler, but the OAuth token exchange did not, so a single flaky connection to the auth endpoint failed the whole build. Extract the retry/backoff loop into a generic retryhandler.WithRetry helper and use it for the token fetch requests in authorizer.go. Fixes moby#6981 Signed-off-by: wussh <andrianta.321@gmail.com>
tonistiigi
left a comment
There was a problem hiding this comment.
- This doesn't seem to handle the client side authorizer "session/auth/authprovider/authprovider.go:132" what should be the default path.
- On the pull util/pull/pull.go:153, push util/push/push.go:109 and config paths, retryhandler.New already wraps the handler, creating nested retry behavior atm iiuc.
Not needed in this PR but should be in future follow-ups:
- Looks like cancellation is not quite efficient. If backoff is in sleep context cancellation can't cancel directly but needs to wait for sleep to finish.
- If 429 returns a low retry time (eg. under 10 sec) then we should just wait that time, otherwise error.
|
@tonistiigi Good catches, both confirmed:
Before I push a fix, which direction do you prefer for #2?
And for #1, want it as a follow-up in this PR or a separate one? |
Summary
Blob/manifest fetches already retry transient network errors (ECONNRESET, EOF, 5xx, etc.) via
retryhandler, but the OAuth token exchange during registry auth did not. A single flaky connection to the auth endpoint (e.g.auth.docker.io) fails the whole build even though a retry would succeed.This extracts the retry/backoff loop in
util/resolver/retryhandlerinto a genericWithRetryhelper (used internally by the existingNewhandler, no behavior change there) and applies it to the token fetch requests inutil/resolver/authorizer.go(GET, OAuth POST fallback, and anonymous token fetch).Fixes #6981
Test plan
go build ./util/resolver/...go vet ./util/resolver/...go test ./util/resolver/...(all existing tests pass, includingTestBearerAuthFallsBackToAnonymousTokenWithoutSessionwhich exercises the changed code path)gofmt -sclean on changed files