Pull 8875: Added UT for findAffectedVmsForStorageScopeChange and remo…

…ved listByPoolIdVMStatesNotInCluster
apache · sureshanaparti · Jun 29, 2024 · Apr 1, 2024 · Apr 2, 2024 · Apr 2, 2024
commit 35c8b00db6d593bf58eb7a0cf2761499ca615d66
diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java
@@ -27,7 +27,6 @@
 import com.cloud.utils.Pair;
 import com.cloud.utils.db.GenericDao;
 import com.cloud.utils.fsm.StateDao;
-import com.cloud.vm.VirtualMachine;
 
 public interface VolumeDao extends GenericDao<VolumeVO, Long>, StateDao<Volume.State, Volume.Event, Volume> {
 
@@ -156,6 +155,4 @@ public interface VolumeDao extends GenericDao<VolumeVO, Long>, StateDao<Volume.S
     VolumeVO findByPoolIdAndPath(long id, String path);
 
     List<VolumeVO> listByIds(List<Long> ids);
-
-    public List<VolumeVO> listByPoolIdVMStatesNotInCluster(long clusterId, List<VirtualMachine.State> states, long poolId);
 }
diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
@@ -24,16 +24,13 @@
 import java.util.Date;
 import java.util.List;
 
-import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
 import com.cloud.exception.InvalidParameterValueException;
-import com.cloud.host.HostVO;
-import com.cloud.host.dao.HostDao;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.server.ResourceTag.ResourceObjectType;
 import com.cloud.storage.ScopeType;
@@ -48,17 +45,13 @@
 import com.cloud.utils.db.DB;
 import com.cloud.utils.db.GenericDaoBase;
 import com.cloud.utils.db.GenericSearchBuilder;
-import com.cloud.utils.db.JoinBuilder.JoinType;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
 import com.cloud.utils.db.SearchCriteria.Func;
 import com.cloud.utils.db.SearchCriteria.Op;
 import com.cloud.utils.db.TransactionLegacy;
 import com.cloud.utils.db.UpdateBuilder;
 import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.vm.VMInstanceVO;
-import com.cloud.vm.VirtualMachine;
-import com.cloud.vm.dao.VMInstanceDao;
 
 @Component
 public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements VolumeDao {
@@ -79,14 +72,9 @@ public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements Vol
     protected GenericSearchBuilder<VolumeVO, SumCount> primaryStorageSearch2;
     protected GenericSearchBuilder<VolumeVO, SumCount> secondaryStorageSearch;
     private final SearchBuilder<VolumeVO> poolAndPathSearch;
-    protected SearchBuilder<VolumeVO> volumePoolNotInClusterSearch;
 
     @Inject
     ResourceTagDao tagsDao;
-    @Inject
-    HostDao hostDao;
-    @Inject
-    VMInstanceDao vmDao;
 
     protected static final String SELECT_VM_SQL = "SELECT DISTINCT instance_id from volumes v where v.host_id = ? and v.mirror_state = ?";
     // need to account for zone-wide primary storage where storage_pool has
@@ -508,19 +496,6 @@ public VolumeDaoImpl() {
         poolAndPathSearch.done();
     }
 
-    @PostConstruct
-    public void init() {
-        volumePoolNotInClusterSearch = createSearchBuilder();
-        volumePoolNotInClusterSearch.and("poolId", volumePoolNotInClusterSearch.entity().getPoolId(), Op.EQ);
-        SearchBuilder<VMInstanceVO> vmSearch = vmDao.createSearchBuilder();
-        vmSearch.and("vmStates", vmSearch.entity().getState(), Op.IN);
-        SearchBuilder<HostVO> hostSearch = hostDao.createSearchBuilder();
-        hostSearch.and("clusterId", hostSearch.entity().getClusterId(), SearchCriteria.Op.NEQ);
-        vmSearch.join("hostSearch", hostSearch, hostSearch.entity().getId(), vmSearch.entity().getHostId(), JoinType.INNER);
-        volumePoolNotInClusterSearch.join("vmSearch", vmSearch, vmSearch.entity().getId(), volumePoolNotInClusterSearch.entity().getInstanceId(), JoinType.INNER);
-        volumePoolNotInClusterSearch.done();
-    }
-
     @Override
     @DB()
     public Pair<Long, Long> getCountAndTotalByPool(long poolId) {
@@ -866,13 +841,4 @@ public List<VolumeVO> listByIds(List<Long> ids) {
         sc.setParameters("idIN", ids.toArray());
         return listBy(sc, null);
     }
-
-    @Override
-    public List<VolumeVO> listByPoolIdVMStatesNotInCluster(long clusterId, List<VirtualMachine.State> states, long poolId) {
-        SearchCriteria<VolumeVO> sc = volumePoolNotInClusterSearch.create();
-        sc.setParameters("poolId", poolId);
-        sc.setParameters("vmSearch", "vmStates", states);
-        sc.setJoinParameters("hostSearch", "clusterId", clusterId);
-        return listBy(sc, null);
-    }
 }
diff --git a/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java b/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java
@@ -20,16 +20,22 @@
 import com.cloud.api.query.dao.TemplateJoinDao;
 import com.cloud.api.query.vo.EventJoinVO;
 import com.cloud.api.query.vo.TemplateJoinVO;
+import com.cloud.dc.ClusterVO;
+import com.cloud.dc.dao.ClusterDao;
 import com.cloud.event.EventVO;
 import com.cloud.event.dao.EventDao;
 import com.cloud.event.dao.EventJoinDao;
 import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.exception.PermissionDeniedException;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
 import com.cloud.network.Network;
 import com.cloud.network.VNF;
 import com.cloud.network.dao.NetworkVO;
 import com.cloud.server.ResourceTag;
 import com.cloud.storage.BucketVO;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.StoragePoolStatus;
 import com.cloud.storage.dao.BucketDao;
 import com.cloud.user.Account;
 import com.cloud.user.AccountManager;
@@ -41,20 +47,27 @@
 import com.cloud.utils.db.Filter;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
+import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.VMInstanceDao;
+
 import org.apache.cloudstack.acl.SecurityChecker;
 import org.apache.cloudstack.api.ApiCommandResourceType;
 import org.apache.cloudstack.api.command.admin.storage.ListObjectStoragePoolsCmd;
+import org.apache.cloudstack.api.command.admin.vm.FindAffectedVmsForStorageScopeChangeCmd;
 import org.apache.cloudstack.api.command.user.bucket.ListBucketsCmd;
 import org.apache.cloudstack.api.command.user.event.ListEventsCmd;
 import org.apache.cloudstack.api.command.user.resource.ListDetailOptionsCmd;
 import org.apache.cloudstack.api.response.DetailOptionsResponse;
 import org.apache.cloudstack.api.response.EventResponse;
 import org.apache.cloudstack.api.response.ListResponse;
 import org.apache.cloudstack.api.response.ObjectStoreResponse;
+import org.apache.cloudstack.api.response.VirtualMachineResponse;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.storage.datastore.db.ObjectStoreDao;
 import org.apache.cloudstack.storage.datastore.db.ObjectStoreVO;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -65,6 +78,7 @@
 import org.mockito.Mockito;
 import org.mockito.Spy;
 import org.mockito.junit.MockitoJUnitRunner;
+import org.springframework.test.util.ReflectionTestUtils;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -112,6 +126,18 @@ public class QueryManagerImplTest {
     @Mock
     ObjectStoreDao objectStoreDao;
 
+    @Mock
+    VMInstanceDao vmInstanceDao;
+
+    @Mock
+    PrimaryDataStoreDao storagePoolDao;
+
+    @Mock
+    HostDao hostDao;
+
+    @Mock
+    ClusterDao clusterDao;
+
     @Mock
     BucketDao bucketDao;
 
@@ -352,4 +378,43 @@ public void testSearchForBuckets() {
         when(bucketDao.searchAndCount(any(), any())).thenReturn(new Pair<>(buckets, 2));
         queryManagerImplSpy.searchForBuckets(listBucketsCmd);
     }
+
+    @Test
+    public void testFindAffectedVmsForScopeChange() {
+        Long clusterId = 1L;
+        Long poolId = 2L;
+        Long hostId = 3L;
+        FindAffectedVmsForStorageScopeChangeCmd cmd = new FindAffectedVmsForStorageScopeChangeCmd();
+        ReflectionTestUtils.setField(cmd, "clusterIdForScopeChange", clusterId);
+        ReflectionTestUtils.setField(cmd, "storageId", poolId);
+
+        StoragePoolVO pool = Mockito.mock(StoragePoolVO.class);
+        Mockito.when(pool.getStatus()).thenReturn(StoragePoolStatus.Up);
+        Mockito.when(pool.getScope()).thenReturn(ScopeType.ZONE);
+        Mockito.when(storagePoolDao.findById(poolId)).thenReturn(pool);
+        try {
+            queryManager.findAffectedVmsForStorageScopeChange(cmd);
+            Assert.fail();
+        } catch (InvalidParameterValueException ex) {
+            Assert.assertEquals(ex.getMessage(), "Scope change of Storage pool is only allowed in Disabled state");
+        }
+
+        VMInstanceVO instance = Mockito.mock(VMInstanceVO.class);
+        String instanceUuid = String.valueOf(UUID.randomUUID());
+        Pair<List<VMInstanceVO>, Integer> vms = new Pair<>(List.of(instance), 1);
+        HostVO host = Mockito.mock(HostVO.class);
+        ClusterVO cluster = Mockito.mock(ClusterVO.class);
+
+        Mockito.when(pool.getStatus()).thenReturn(StoragePoolStatus.Disabled);
+        Mockito.when(instance.getUuid()).thenReturn(instanceUuid);
+        Mockito.when(instance.getType()).thenReturn(VirtualMachine.Type.Instance);
+        Mockito.when(instance.getHostId()).thenReturn(hostId);
+        Mockito.when(vmInstanceDao.listByVmsNotInClusterUsingPool(clusterId, poolId)).thenReturn(vms);
+        Mockito.when(hostDao.findById(hostId)).thenReturn(host);
+        Mockito.when(host.getClusterId()).thenReturn(clusterId);
+        Mockito.when(clusterDao.findById(clusterId)).thenReturn(cluster);
+
+        ListResponse<VirtualMachineResponse> response = queryManager.findAffectedVmsForStorageScopeChange(cmd);
+        Assert.assertEquals(response.getResponses().get(0).getId(), instanceUuid);
+    }
 }
diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java
@@ -29,9 +29,9 @@
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.storage.dao.VolumeDao;
 import com.cloud.user.AccountManagerImpl;
+import com.cloud.utils.Pair;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.VMInstanceVO;
-import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.dao.VMInstanceDao;
 
 import org.apache.cloudstack.framework.config.ConfigDepot;
@@ -50,7 +50,6 @@
 import org.springframework.test.util.ReflectionTestUtils;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -323,10 +322,9 @@ public void testChangeStoragePoolScopeToClusterVolumesPresentException() {
         final ClusterVO cluster = new ClusterVO();
         Mockito.when(clusterDao.findById(1L)).thenReturn(cluster);
 
-        List<VolumeVO> volumes = new ArrayList<>();
-        volumes.add(new VolumeVO());
-        List<VirtualMachine.State> states = Arrays.asList(VirtualMachine.State.Starting, VirtualMachine.State.Running, VirtualMachine.State.Stopping, VirtualMachine.State.Migrating, VirtualMachine.State.Restoring);
-        Mockito.when(_volumeDao.listByPoolIdVMStatesNotInCluster(1L, states, 1L)).thenReturn(volumes);
+        VMInstanceVO instance = Mockito.mock(VMInstanceVO.class);
+        Pair<List<VMInstanceVO>, Integer> vms = new Pair<>(List.of(instance), 1);
+        Mockito.when(vmInstanceDao.listByVmsNotInClusterUsingPool(1L, 1L)).thenReturn(vms);
 
         ChangeStoragePoolScopeCmd cmd = mockChangeStoragePooolScopeCmd("CLUSTER");
         storageManagerImpl.changeStoragePoolScope(cmd);