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

Add retry #750

Open
sophiewigmore opened this issue Jul 15, 2024 · 5 comments · May be fixed by #803
Open

Add retry #750

sophiewigmore opened this issue Jul 15, 2024 · 5 comments · May be fixed by #803

Comments

@sophiewigmore
Copy link

sophiewigmore commented Jul 15, 2024

Hey! Occasionally, our login step fails due to rate-limiting or the system we're logging into being unavailable. It would be super useful to have a retry flag of some kind. Based on similar functionality for other actions in the space, imagining it might look like:

- name: Login
   uses: docker/login-action@v3
   with:
       registry: ...
       username: ...
       password: ...
       retries: 3
@ashwinkhode
Copy link

+1

@nephatrine
Copy link

This would be fantastic. I have actions occasionally fail with Client.Timeout exceeded while awaiting headers and just rerunning the step does the trick in those instances.

@szofar
Copy link

szofar commented Sep 11, 2024

same! our Harbor registry times out ALL the time and I have to go manually re-trigger the workflow

@crazy-max
Copy link
Member

our login step fails due to rate-limiting

Hammering a registry of requests because it's unavailable is not a short or long term solution. Registries often implement rate limiting to prevent abuse and ensure fair usage among all users. If a login attempt fails due to rate limiting, retrying the login multiple times in quick succession can exacerbate the problem. This can lead to further rate limiting, potentially locking out the user for a longer period or affecting other users who are trying to access the registry. If the registry is temporarily unavailable due to maintenance or an outage, retrying the login multiple times will not resolve the issue. This can result in unnecessary load on the registry once it becomes available again, as multiple clients may attempt to reconnect simultaneously.

While retry logic can be useful, it's important to implement it thoughtfully to avoid exacerbating issues like rate limiting and system unavailability. Using smth like exponential backoff, circuit breaker patterns could be a solution.

@fedordikarev fedordikarev linked a pull request Oct 29, 2024 that will close this issue
@ealogar
Copy link

ealogar commented Oct 31, 2024

I would consider to add more status code for the retries as an optional property in the action.
There are situations where a 401 is returned from a custom registry as a security mechanism to avoid giving hints and you may want to retry those as well (of course with a backoff retry policy in place...). Maybe not the default but overridable would be great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants