resolver: retry on transient network errors#13323
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds transport-level retry logic to the Docker resolver so that transient network/transport failures (e.g., timeouts/EOFs) can be retried on the last registry host, improving resiliency and reducing test flakiness.
Changes:
- Introduces a shared attempt budget (
maxAttempts) and addsdoWithTransportRetries/isTransientTransportErrto retry transient transport errors. - Updates resolver retry flow to route all request attempts through the shared attempt budget.
- Adds unit tests for transient error classification and transport retry behavior; updates existing fetcher tests to align with the new attempt limit.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| core/remotes/docker/resolver.go | Adds transient transport retry logic and a shared max-attempts budget integrated into request retry flow. |
| core/remotes/docker/resolver_test.go | Adds unit tests for transient error detection and doWithTransportRetries behavior. |
| core/remotes/docker/fetcher_test.go | Updates expectations to use the new shared maxAttempts budget. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a86132d to
dcb6c10
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
prakleumas
left a comment
prakleumas
left a comment
There was a problem hiding this comment.
beep boop
Hello! I am @samuelkarp's friendly code review robot 🤖! Sam asked me to take a look and post my comments directly on GitHub.
Please treat my comments as suggestions and advisory, not mandates. Since I am an LLM and my output is used here directly, you should evaluate carefully and see if my suggestions make sense. Feel free to directly resolve any you disagree with.
This is not an automated action. This bot does not read or reply to comments.
samuelkarp
left a comment
samuelkarp
left a comment
There was a problem hiding this comment.
This mostly seems fine to me. The one Copilot comment about the last 50ms sleep at a final failure seems worth addressing, everything else seems optional.
dcb6c10 to
fdd3049
Compare
fdd3049 to
1f7ac8c
Compare
Allow the last host to retry on transient network errors to incrase the likelihood of the operation succeeding and help reduce flaky tests. Signed-off-by: Derek McGowan <[email protected]>
1f7ac8c to
20af2e3
Compare
|
/cherry-pick release/2.3 |
|
@AkihiroSuda: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
github-project-automation
Bot
moved this from Review In Progress
to Done
in Pull Request Review
|
@AkihiroSuda: new pull request created: #13591 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
dmcgowan
added
cherry-picked/2.3.x
Allow the last host to retry on transient network errors to incrase the likelihood of the operation succeeding and help reduce flaky tests.