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

IMDS timeout increased to 10 seconds #19389

Closed
wants to merge 1 commit into from

Conversation

shashanksbs45
Copy link

@shashanksbs45 shashanksbs45 commented Oct 19, 2022

In case of stressed servers 1000ms is very aggressive time. This PR is to just trying to get some attention in to this intermittent context deadline issue. Error Im facing is ManagedIdentityCredential: IMDS token request timed out error for long running jobs that makes many requests to azure secrets fetch data.

  • The purpose of this PR is explained in this or a referenced issue.
  • The PR does not update generated files.
  • Tests are included and/or updated for code changes.
  • Updates to CHANGELOG.md are included.
  • MIT license headers are included in each file.

In case of stressed servers 1000ms is very aggressive time. This PR is to just trying to get some attention in to this intermittent context deadline issue
@ghost ghost added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Oct 19, 2022
@ghost
Copy link

ghost commented Oct 19, 2022

Thank you for your contribution shashanksbs! We will review the pull request and get back to you soon.

Copy link
Member

@chlowell chlowell left a comment

Choose a reason for hiding this comment

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

Thanks, this is timely! We (Azure SDK authentication team) recently began discussing ways to improve IMDS timeout behavior for DefaultAzureCredential. Simply lengthening the timeout is one of the options but would regress performance in some scenarios, and I don't want to make any changes here until we've explored other options. Note that only DefaultAzureCredential imposes this timeout. If it's a problem in your application, you can avoid it by using ManagedIdentityCredential directly or with ChainedTokenCredential.

To give some context on the timeout, we added it to limit the worst case running time of DefaultAzureCredential's first auth attempt. This is important when IMDS isn't available, for example in local development. The only way to learn whether IMDS is available is to try connecting to it, which can take longer than 10 seconds to time out on some platforms.

@shashanksbs45
Copy link
Author

shashanksbs45 commented Oct 27, 2022

@chlowell @jhendrixMSFT @kenegozi
how ManagedIdentityCredential or ChainedTokenCredential helpful in mitigating IMDS timeout issue

Pod Identity can take up to a couple minutes to get ready for token requests from new pods. In case of clients that have shorter timeouts, the retries can be terminated and the client will not receive a token in the first attempt. This feature flag can use to prevent timeouts due to that delay. we added https://azure.github.io/aad-pod-identity/docs/configure/feature_flags/#set-retry-after-header-in-nmi-response this check in azure pod identity helm chart.

But IMDS timeout issue still persists and sometimes we are getting no default identity is assigned to this resource error.

these errors occur mostly on stressed servers, on normal day it works fine

Please suggest any resolution for this

@jhendrixMSFT
Copy link
Member

jhendrixMSFT commented Oct 27, 2022

The short timeout that you're changing here is only part of DefaultAzureCredential. If you directly use ManagedIdentityCredential then there is no timeout (outside of how the default HTTP client works).

@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Dec 30, 2022
@ghost
Copy link

ghost commented Dec 30, 2022

Hi @realDeveloper45. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

@ghost ghost closed this Jan 6, 2023
@ghost
Copy link

ghost commented Jan 6, 2023

Hi @realDeveloper45. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing "/reopen" if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the "no-recent-activity" label; otherwise, this is likely to be closed again with the next cleanup pass.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Identity customer-reported Issues that are reported by GitHub users external to the Azure organization. no-recent-activity There has been no recent activity on this issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants