-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
Allow Kubelet to run with no Azure identity #77906
Conversation
useInstanceMetadata should be enabled and Kubelet would use IMDS to get node's information.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: feiskyer The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/priority important-soon /hold @andyzhangx When validating this with #77851, I find AzureDisk is not working as expected. The error message is:
As the error message indicated, the lun is -1, which is weird. Do you think this is related to #77483? |
The above issue is fixed by #77912. /hold cancel |
if metadata.Compute.VMSize != "" { | ||
return metadata.Compute.VMSize, nil | ||
if !isLocalInstance { | ||
if az.vmSet != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do this code change? not use metadata.Compute.VMSize, nil
directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when this is not local instance, and credentials not provided, we should report errors.
the original logic has been moved to L331.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@feiskyer the change LGTM
/lgtm
/hold
but there is a good chance the release note with { .. }
might break anago (the release tool)
possibly trim it to?
Kubelet could be run with no Azure identity now.
Thanks, let me remove the {} and wrap the example in quotes ``. |
@neolit123 looked at the old release notes, |
ok, understood. |
What type of PR is this?
/kind bug
/sig azure
What this PR does / why we need it:
Allow Kubelet to run with no Azure identity. useInstanceMetadata should be enabled and Kubelet would use IMDS to get node's information.
Which issue(s) this PR fixes:
Fixes #77309
Special notes for your reviewer:
#77851 is also required for no identity working.
Does this PR introduce a user-facing change?: