-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix VMWare leftovers when deleting VM without root disk #9735
base: 4.19
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #9735 +/- ##
============================================
+ Coverage 4.76% 15.08% +10.31%
- Complexity 0 11188 +11188
============================================
Files 366 5403 +5037
Lines 29525 473174 +443649
Branches 5167 59662 +54495
============================================
+ Hits 1408 71379 +69971
- Misses 28111 393852 +365741
- Partials 6 7943 +7937
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
clgtm, needs testing.
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.
Code LGTM
s_logger.debug(msg); | ||
return new Answer(cmd, true, msg); | ||
} | ||
vmMo.destroy(); |
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.
just curious, why not do it in the process for DestroyCommand ?
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.
DestroyCommand centers around the volume. I guess I could have just added conditions around it, but keeping them separate seemed fitting.
Description
The MS sends the destroy/delete command of a VM to vCenter when cleaning up its volumes. If the VM does not have a root volume, the VM with its folder and metadata files remain in vCenter.
To reproduce, deploy a VM, detach its volume, and delete the VM. Then, check vCenter and the VM will still be there.
This PR fixes it by implementing finalizeExpunge for VMWareGuru, which queues a Cleanup Command that, if the VM still exists, sends a destroy command to vCenter to cleanup whatever remained.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?