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

azure: retry HTTP requests on codes 404, 410, and 429 #1807

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Feb 8, 2024

For some reason, the Azure IMDS server expects clients to retry their HTTP requests even on codes that usually would be considered final. The documented one is 410[1], but let's just match the set from cloud-init, which also includes 404 and 429[2].

Closes: #1806

For some reason, the Azure IMDS server expects clients to retry their
HTTP requests even on codes that usually would be considered final.
The documented one is 410[[1]], but let's just match the set from
cloud-init, which also includes 404 and 429[[2]].

Closes: coreos#1806

[1]: https://learn.microsoft.com/en-us/azure/virtual-machines/instance-metadata-service?tabs=linux#errors-and-debugging
[2]: canonical/cloud-init@c1a2047cf291
Copy link
Member

@travier travier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

retryCodes := []int{
404, // not found
410, // gone
429, // rate-limited
Copy link

@SudoBrendan SudoBrendan Feb 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idea: we should consider respecting the Retry-After header here so we don't cascade 429 errors (if this isn't already being done somewhere - I see a duration value that could be played with).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, yes. We currently do an exponential backoff which will help a lot here at least.

@travier
Copy link
Member

travier commented Feb 9, 2024

We have positive test results that this likely fixes the issue on Azure so I'll merge this PR. We can do a follow-up if that's not enough. Thanks Jonathan for the quick fix.

@travier travier merged commit ab5eeb7 into coreos:main Feb 9, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Retry on HTTP 410 error code for Azure IMDS provider
3 participants