-
Notifications
You must be signed in to change notification settings - Fork 295
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
⚠️ Perform guest shutdown if VMware tools installed when deleting VM #1982
Conversation
Welcome @laozc! |
Hi @laozc. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
Should we also introduce a timeout in case the guest shutdown stalls? |
Definitely. |
Can we have a more detailed release note highlighting the option? |
As far as I can tell the last remaing point is this one: #1982 (comment) Otherwise lgtm from my side |
This looks good now. I'll let someone else do another pass. /approve |
I think using the api/ version was the last one. But we can follow-up for that. @laozc Can you please squash the commits? Then we would merge /lgtm |
Will the tide/merge-method-squash not label not work? |
/retest |
Sure. PR was squashed and rebased. |
Signed-off-by: Zhongcheng Lao <[email protected]>
@laozc: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
/unhold Thanks very much for persisting amongst all the reviews. /approve |
/remove-label tide/merge-method-squash |
Absolutely agree. Thank you very much! nice work :) /lgtm |
LGTM label has been added. Git tree hash: 962d23e7f339fa0744ee268bf874876fd5872e2c
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: randomvariable, sbueringer 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 |
Thank you for your reviews :) |
What this PR does / why we need it:
Currently a hard power off is initiated when the vSphere VM gets deleted.
This will not allow the VM to perform a graceful shutdown for the OS and all the services running in the OS.
This PR addresses the issue by:
The guest shutdown may stall for a long while and we'll address the issue in a later PR.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #1981
Special notes for your reviewer:
Guest Shutdown API does not have an associated task in the vCenter. Therefore in the PR we use a local task to mimic the wait and update process as like other vCenter Task based operations.
Release note: