Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enhancement]: define more errors as non-retryable during image pull #2502

Closed
p-jahn opened this issue Apr 19, 2024 · 1 comment · Fixed by #2506
Closed

[Enhancement]: define more errors as non-retryable during image pull #2502

p-jahn opened this issue Apr 19, 2024 · 1 comment · Fixed by #2506
Labels
enhancement New feature or request

Comments

@p-jahn
Copy link
Contributor

p-jahn commented Apr 19, 2024

Proposal

If the docker client returns an error during image pull then the command is retried, except for ErrNotFound.

There are more error types defined in https://github.com/moby/moby/blob/master/errdefs/defs.go that are also permanent, as far as I can tell.

I ran into the issue that, with invalid credentials, the pull was retried until I got temp blocked by the container registry. Pulling with invalid credentials should not be tried again.

Suggested errors to be seen as permanent/non-retryable:

  • ErrInvalidParameter
  • ErrUnauthorized
  • ErrForbidden
  • ErrNotImplemented

I'm unsure if all types might be returned by ImagePull (e.g. ErrNotImplemented seems unlikely). I also didn't check if there are similar places where the docker client is called within a retry mechanism. If so, it might be worth using a central definition that just contains all permanent types.

Happy to provide a PR if you like the suggestion.

@p-jahn p-jahn added the enhancement New feature or request label Apr 19, 2024
@mdelapenya
Copy link
Collaborator

I like the idea of being more selective with the retries. Thanks for raising your concerns and opening the issue 🙇

If you have to bandwidth for it, I'll be more than happy to review it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants