Skip to content

Skip deleting instance with delete protection during account cleanup #12901

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.

Sign up for GitHub

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

Draft
sudo87 wants to merge 1 commit into
base: 4.22
Choose a base branch
from
+6 −0
Draft

Skip deleting instance with delete protection during account cleanup #12901

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions server/src/main/java/com/cloud/user/AccountManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,12 @@ protected boolean cleanupAccount(AccountVO account, long callerUserId, Account c
}

for (UserVmVO vm : vms) {
if (vm.isDeleteProtection()) {
logger.warn("Instance [id = {}, name = {}] has delete protection enabled and cannot be deleted.",
vm.getUuid(), vm.getName());
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a VM has delete protection enabled, this loop now continues without destroying/expunging it, but it also leaves accountCleanupNeeded unchanged. That means the cleanup can still be marked as complete (needsCleanup=false) even though protected instances remain under a removed account, potentially leaving orphaned VMs/resources with no follow-up cleanup attempts (e.g., in basic/shared-network setups where later network cleanup may still succeed).

Consider treating this as an incomplete cleanup: set accountCleanupNeeded = true when delete-protected VMs are found and/or fail the account/domain deletion with a clear error so operators can remove protection first, rather than silently completing cleanup.

Suggested change
vm.getUuid(), vm.getName());
vm.getUuid(), vm.getName());
accountCleanupNeeded = true;

Copilot uses AI. Check for mistakes.
continue;
}
Comment on lines +1003 to +1007
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces new behavior for account cleanup when deleteProtection is enabled, but there is no unit test covering it. Please add a test (e.g., in server/src/test/java/com/cloud/user/AccountManagerImplTest) that provides a UserVmVO with isDeleteProtection()==true and asserts the VM is not expunged (and that the account is handled as expected, e.g., marked for cleanup or the delete operation fails).

Copilot uses AI. Check for mistakes.

if (vm.getState() != VirtualMachine.State.Destroyed && vm.getState() != VirtualMachine.State.Expunging) {
try {
_vmMgr.destroyVm(vm.getId(), false);
Expand Down
Loading