-
Notifications
You must be signed in to change notification settings - Fork 40.2k
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
remove VM API call dependency in azure disk WaitForAttach #77483
remove VM API call dependency in azure disk WaitForAttach #77483
Conversation
/test pull-kubernetes-cross |
This PR works well on both Linux and Windows node. |
/hold |
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.
/lgtm
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.
one nit and one big question :-)
if we make this call https://github.com/kubernetes/kubernetes/blob/master/pkg/volume/azure_dd/attacher.go#L84 return lun that was selected on VM model then our getDiskLun and WaitForAttach()
would be a lot simpler, right?
No, the attach disk logic happens in controller manager, anyway it needs VM API call, while this PR fixed the VM API call dependency in The return value of
WaitForAttach() in kubelet would return the real device path according to provided LUN num
|
add comment add unit test for WaitForAttach fnc add unit test for WaitForAttach Func
70af63c
to
9a8f07d
Compare
/test pull-kubernetes-cross |
/test pull-kubernetes-e2e-aks-engine-azure |
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.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andyzhangx, 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 |
What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR removes the VM API call dep in azure disk
WaitForAttach
func, originallydevicePath
is a disk LUN num, and then it could be a real device path due to the volume status update(you could find more details in this PR: #62612), in that condition, return thedevicePath
immediately, get disk lun num by VM API call is actually not necessary.Which issue(s) this PR fixes:
Fixes #77462
Special notes for your reviewer:
release-note:
/assign @feiskyer @khenidak
/priority important-soon
/sig azure