fix domain delection check, add limit of 10 to the return instance list

Author: sudo87

engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java

@@ -21,6 +21,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import com.cloud.hypervisor.Hypervisor;
 import com.cloud.utils.Pair;
@@ -195,5 +196,5 @@ List<VMInstanceVO> searchRemovedByRemoveDate(final Date startDate, final Date en
 
     List<VMInstanceVO> listDeleteProtectedVmsByAccountId(long accountId);
 
-    List<VMInstanceVO> listDeleteProtectedVmsByDomainIds(List<Long> domainIds);
+    List<VMInstanceVO> listDeleteProtectedVmsByDomainIds(Set<Long> domainIds);
 }

engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java

@@ -25,6 +25,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import javax.annotation.PostConstruct;
@@ -1318,14 +1319,16 @@ public List<VMInstanceVO> listDeleteProtectedVmsByAccountId(long accountId)  {
         SearchCriteria<VMInstanceVO> sc = DeleteProtectedVmSearchByAccount.create();
         sc.setParameters(ApiConstants.ACCOUNT_ID, accountId);
         sc.setParameters(ApiConstants.DELETE_PROTECTION, true);
-        return listBy(sc);
+        Filter filter = new Filter(VMInstanceVO.class, null, false, 0L, 10L);
+        return listBy(sc, filter);
     }
 
     @Override
-    public List<VMInstanceVO> listDeleteProtectedVmsByDomainIds(List<Long> domainIds)  {
+    public List<VMInstanceVO> listDeleteProtectedVmsByDomainIds(Set<Long> domainIds)  {
         SearchCriteria<VMInstanceVO> sc = DeleteProtectedVmSearchByDomainIds.create();
         sc.setParameters(ApiConstants.DOMAIN_IDS, domainIds.toArray());
         sc.setParameters(ApiConstants.DELETE_PROTECTION, true);
-        return listBy(sc);
+        Filter filter = new Filter(VMInstanceVO.class, null, false, 0L, 10L);
+        return listBy(sc, filter);
     }
 }

server/src/main/java/com/cloud/user/AccountManagerImpl.java

@@ -2142,7 +2142,7 @@ protected boolean isDeleteNeeded(AccountVO account, long accountId, Account call
     private void validateNoDeleteProtectedVmsForAccount(Account account) {
         long accountId = account.getId();
         List<VMInstanceVO> deleteProtectedVms = _vmDao.listDeleteProtectedVmsByAccountId(accountId);
-        if (deleteProtectedVms.isEmpty()) {
+        if (CollectionUtils.isEmpty(deleteProtectedVms)) {
             return;
         }
 

server/src/main/java/com/cloud/user/DomainManagerImpl.java

@@ -366,6 +366,8 @@ public boolean deleteDomain(long domainId, Boolean cleanup) {
         }
 
         _accountMgr.checkAccess(caller, domain);
+        // Check across the domain hierarchy (current + children) for any delete-protected instances
+        validateNoDeleteProtectedVmsForDomain(domain);
 
         return deleteDomain(domain, cleanup);
     }
@@ -624,6 +626,9 @@ protected boolean cleanupDomain(Long domainId, Long ownerId) throws ConcurrentOp
         DomainVO domainHandle = _domainDao.findById(domainId);
         logger.debug("Cleaning up domain {}", domainHandle);
         {
+            domainHandle.setState(Domain.State.Inactive);
+            _domainDao.update(domainId, domainHandle);
+
             SearchCriteria<DomainVO> sc = _domainDao.createSearchCriteria();
             sc.addAnd("parent", SearchCriteria.Op.EQ, domainId);
             List<DomainVO> domains = _domainDao.search(sc, null);
@@ -632,12 +637,6 @@ protected boolean cleanupDomain(Long domainId, Long ownerId) throws ConcurrentOp
             sc1.addAnd("path", SearchCriteria.Op.LIKE, "%" + "replace(" + domainHandle.getPath() + ", '%', '[%]')" + "%");
             List<DomainVO> domainsToBeInactivated = _domainDao.search(sc1, null);
 
-            // Validate that no Instance in this domain or its subdomains has delete protection
-            validateNoDeleteProtectedVmsForDomain(domainHandle, domainsToBeInactivated);
-
-            domainHandle.setState(Domain.State.Inactive);
-            _domainDao.update(domainId, domainHandle);
-
             // update all subdomains to inactive so no accounts/users can be created
             for (DomainVO domain : domainsToBeInactivated) {
                 domain.setState(Domain.State.Inactive);
@@ -729,23 +728,20 @@ protected boolean cleanupDomain(Long domainId, Long ownerId) throws ConcurrentOp
         return success && deleteDomainSuccess;
     }
 
-    private void validateNoDeleteProtectedVmsForDomain(Domain domainHandle, List<DomainVO> subDomains) {
-        List<Long> allDomainIds = subDomains.stream().map(Domain::getId).collect(Collectors.toList());
-        allDomainIds.add(domainHandle.getId());
-
+    private void validateNoDeleteProtectedVmsForDomain(Domain parentDomain) {
+        Set<Long> allDomainIds = getDomainChildrenIds(parentDomain.getPath());
         List<VMInstanceVO> deleteProtectedVms = vmInstanceDao.listDeleteProtectedVmsByDomainIds(allDomainIds);
-        if (deleteProtectedVms.isEmpty()) {
+        if (CollectionUtils.isEmpty(deleteProtectedVms)) {
             return;
         }
-
         if (logger.isDebugEnabled()) {
             List<String> vmUuids = deleteProtectedVms.stream().map(VMInstanceVO::getUuid).collect(Collectors.toList());
-            logger.debug("Cannot delete Domain {}, it has delete protection enabled for Instances: {}", domainHandle, vmUuids);
+            logger.debug("Cannot delete Domain {}, it has delete protection enabled for Instances: {}", parentDomain, vmUuids);
         }
 
         throw new InvalidParameterValueException(
                 String.format("Cannot delete Domain '%s'. One or more Instances have delete protection enabled.",
-                        domainHandle.getName()));
+                        parentDomain.getName()));
     }
 
     @Override

server/src/test/java/com/cloud/user/DomainManagerImplTest.java

@@ -216,6 +216,7 @@ public void testDeleteDomainRootDomain() {
     @Test
     public void testDeleteDomainNoCleanup() {
         Mockito.when(_configMgr.releaseDomainSpecificVirtualRanges(Mockito.any())).thenReturn(true);
+        Mockito.doReturn(Collections.emptySet()).when(domainManager).getDomainChildrenIds(Mockito.any());
         domainManager.deleteDomain(DOMAIN_ID, testDomainCleanup);
         Mockito.verify(domainManager).deleteDomain(domain, testDomainCleanup);
         Mockito.verify(domainManager).removeDomainWithNoAccountsForCleanupNetworksOrDedicatedResources(domain);
@@ -281,6 +282,7 @@ public void deleteDomain() {
         Mockito.when(_dedicatedDao.listByDomainId(Mockito.anyLong())).thenReturn(new ArrayList<DedicatedResourceVO>());
         Mockito.when(domainDaoMock.remove(Mockito.anyLong())).thenReturn(true);
         Mockito.when(_configMgr.releaseDomainSpecificVirtualRanges(Mockito.any())).thenReturn(true);
+        Mockito.doReturn(Collections.emptySet()).when(domainManager).getDomainChildrenIds(Mockito.any());
 
         try {
             Assert.assertTrue(domainManager.deleteDomain(20l, false));