-
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
Feature: Allow adding delete protection for VMs & volumes #9633
Feature: Allow adding delete protection for VMs & volumes #9633
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9633 +/- ##
============================================
- Coverage 15.57% 15.57% -0.01%
Complexity 12056 12056
============================================
Files 5506 5506
Lines 482919 482995 +76
Branches 61184 60564 -620
============================================
+ Hits 75233 75242 +9
- Misses 399375 399439 +64
- Partials 8311 8314 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10952 |
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10959 |
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10965 |
@blueorangutan test |
@vishesh92 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@blueorangutan package |
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 11003 |
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11009 |
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, simple and very useful feature. Didn't test it though.
@blueorangutan test |
@rohityadavcloud a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-11383)
|
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 looks good. One query abt DB change
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.vm_instance', 'delete_protection', 'boolean DEFAULT FALSE COMMENT "delete protection for vm" '); | ||
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.volumes', 'delete_protection', 'boolean DEFAULT FALSE COMMENT "delete protection for volumes" '); |
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.
question - will it make a lot of difference if these flags are stored as details in vm_details/volume_details tables?
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.
I don't think it will create a lot of impact. Just a few extra queries & joins to check the value from details table.
@JoaoJandre This is a small feature. Can we include this PR in 4.20 release as well? |
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11038 |
@blueorangutan test |
@vishesh92 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Sure! but next week the main branch is getting frozen, so the PR will need to be merged by then. |
Thanks Joao. I will try to get both the PRs merged by Monday. |
[SF] Trillian test result (tid-11413)
|
@rohityadavcloud a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
UI build: ✔️ |
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, manually tested it
Co-authored-by: Suresh Kumar Anaparti <[email protected]>
Docs PR: apache/cloudstack-documentation#433
Description
This PR allows adding deletion protection for a VM & volume. If the flag is enabled for a VM or a volume, the deletion of that resource fails. This feature helps user prevent deletion of critical resources by mistake.
If the resource is part of an Autoscaling group, CKS or any other plugin/feature, deletion protection flag is ignored. i.e. Deletion protection works only while deletion of resource via
destroyVirtualMachine
&deleteVolume
command.As of now, delete protection can be enabled only after the resource has been created/deployed. i.e. only updateVirtualMachine & updateVolume commands can set the deleteprotection for a resource.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?