diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index bd4c311e3cd5..1fc92ad0e8ee 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -1469,6 +1469,7 @@ protected boolean zoneWideVolumeRequiresStorageMotion(PrimaryDataStore volumeDat */ Ternary, Integer>, List, Map> getTechnicallyCompatibleHosts( final VirtualMachine vm, + final Host srcHost, final Long startIndex, final Long pageSize, final String keyword) { @@ -1479,31 +1480,6 @@ Ternary, Integer>, List, Map(new Pair<>(new ArrayList<>(), 0), new ArrayList<>(), new HashMap<>()); } - final long srcHostId = vm.getHostId(); - final Host srcHost = _hostDao.findById(srcHostId); - if (srcHost == null) { - if (logger.isDebugEnabled()) { - logger.debug("Unable to find the host with ID: " + srcHostId + " of this Instance: " + vm); - } - final InvalidParameterValueException ex = new InvalidParameterValueException("Unable to find the host (with specified ID) of instance with specified ID"); - ex.addProxyObject(String.valueOf(srcHostId), "hostId"); - ex.addProxyObject(vm.getUuid(), "vmId"); - throw ex; - } - - String srcHostVersion = srcHost.getHypervisorVersion(); - if (HypervisorType.KVM.equals(srcHost.getHypervisorType()) && srcHostVersion == null) { - srcHostVersion = ""; - } - - // Check if the vm can be migrated with storage. - boolean canMigrateWithStorage = false; - - List hypervisorTypes = Arrays.asList(new HypervisorType[]{HypervisorType.VMware, HypervisorType.KVM}); - if (VirtualMachine.Type.User.equals(vm.getType()) || hypervisorTypes.contains(vm.getHypervisorType())) { - canMigrateWithStorage = _hypervisorCapabilitiesDao.isStorageMotionSupported(srcHost.getHypervisorType(), srcHostVersion); - } - // Check if the vm is using any disks on local storage. final VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(vm, null, _offeringDao.findById(vm.getId(), vm.getServiceOfferingId()), null, null); final List volumes = _volumeDao.findCreatedByInstance(vmProfile.getId()); @@ -1517,10 +1493,12 @@ Ternary, Integer>, List, Map, Integer> allHostsPair = null; List allHosts = null; @@ -1596,6 +1574,23 @@ Ternary, Integer>, List, Map(allHostsPairResult, filteredHosts, requiresStorageMotion); } + protected boolean isStorageMigrationSupported(final VirtualMachine vm, final Host srcHost) { + final List hypervisorTypes = Arrays.asList(HypervisorType.VMware, HypervisorType.KVM); + if (VirtualMachine.Type.User.equals(vm.getType()) || hypervisorTypes.contains(vm.getHypervisorType())) { + final String srcHostVersion = getHypervisorVersionOfHost(srcHost); + return _hypervisorCapabilitiesDao.isStorageMotionSupported(srcHost.getHypervisorType(), srcHostVersion); + } + return false; + } + + protected String getHypervisorVersionOfHost(final Host host) { + final String version = host.getHypervisorVersion(); + if (version == null && HypervisorType.KVM.equals(host.getHypervisorType())) { + return ""; + } + return version; + } + /** * Apply affinity group constraints and other exclusion rules for VM migration. * This builds an ExcludeList based on affinity groups, DPDK requirements, and dedicated resources. @@ -1692,9 +1687,19 @@ public Ternary, Integer>, List, Map, Integer>, List, Map> compatibilityResult = - getTechnicallyCompatibleHosts(vm, startIndex, pageSize, keyword); + getTechnicallyCompatibleHosts(vm, srcHost, startIndex, pageSize, keyword); Pair, Integer> allHostsPair = compatibilityResult.first(); List filteredHosts = compatibilityResult.second(); @@ -1707,9 +1712,7 @@ public Ternary, Integer>, List, Map, Integer>, List, Map(otherHosts, suitableHosts, requiresStorageMotion); } + protected DataCenterDeployment createDeploymentPlanForMigrationListing(final VirtualMachine vm, final Host srcHost) { + final boolean canMigrateWithStorage = isStorageMigrationSupported(vm, srcHost); + if (canMigrateWithStorage) { + return new DataCenterDeployment(srcHost.getDataCenterId(), srcHost.getPodId(), null, null, null, null); + } + return new DataCenterDeployment(srcHost.getDataCenterId(), srcHost.getPodId(), srcHost.getClusterId(), null, null, null); + } + /** * Add non DPDK enabled hosts to the avoid list */ diff --git a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java index da2005f61368..2f3e97716a03 100644 --- a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java +++ b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java @@ -20,6 +20,7 @@ import com.cloud.dc.DataCenterVO; import com.cloud.dc.Vlan.VlanType; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.deploy.DataCenterDeployment; import com.cloud.deploy.DeploymentPlanningManager; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; @@ -227,7 +228,7 @@ public void setup() throws IllegalAccessException, NoSuchFieldException { apiDBUtilsMock = Mockito.mockStatic(ApiDBUtils.class); // Return empty list to avoid architecture filtering in most tests apiDBUtilsMock.when(() -> ApiDBUtils.listZoneClustersArchs(Mockito.anyLong())) - .thenReturn(new ArrayList<>()); + .thenReturn(new ArrayList<>()); } @After @@ -246,7 +247,7 @@ private void overrideDefaultConfigValue(final ConfigKey configKey, final String } @Test(expected = InvalidParameterValueException.class) - public void testDuplicateRegistraitons(){ + public void testDuplicateRegistrations() { String accountName = "account"; String publicKeyString = "ssh-rsa very public"; String publicKeyMaterial = spy.getPublicKeyFromKeyKeyMaterial(publicKeyString); @@ -826,9 +827,13 @@ public void testListHostsForMigrationOfVMLxcUserVM() { @Test public void testListHostsForMigrationOfVMGpuEnabled() { VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + long hostId = vm.getHostId(); + HostVO srcHost = mockHost(hostId, 4L, 5L, 6L, HypervisorType.KVM); Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.doReturn(srcHost).when(hostDao).findById(hostId); // Mock GPU detail Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) @@ -888,7 +893,7 @@ public void testListHostsForMigrationOfVMWithSystemVM() { spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); // Verify storage motion capability was checked - Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null); + Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.VMware, null); // Verify result structure and data Assert.assertNotNull(result); @@ -952,7 +957,7 @@ public void testListHostsForMigrationOfVMWithDomainRouter() { spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); // Verify hypervisor capabilities were checked - Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.KVM, ""); + Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.KVM, ""); // Verify result contains expected hosts Assert.assertNotNull(result); @@ -1097,7 +1102,7 @@ public void testListHostsForMigrationOfVMKVMWithNullHypervisorVersion() { spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); // Verify KVM null version was converted to empty string - Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.KVM, ""); + Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.KVM, ""); // Verify result data Assert.assertNotNull(result); @@ -1416,7 +1421,7 @@ public void testListHostsForMigrationOfVMStorageMotionCapabilityCheck() { spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); // Verify storage motion capability was checked for User VM - Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null); + Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.VMware, null); // Verify response data Assert.assertNotNull(result); @@ -1481,7 +1486,7 @@ public void testListHostsForMigrationOfVMWithAllSupportedHypervisors() { spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); // Verify hypervisor is in supported hypervisors list - Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(hypervisorType, version); + Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(hypervisorType, version); // Verify validation passed for this hypervisor Assert.assertNotNull("Result should not be null for " + hypervisorType, result); @@ -1508,8 +1513,6 @@ public void testListHostsForMigrationOfVMSourceHostNotFound() { Account caller = mockRootAdminAccount(); Mockito.doReturn(caller).when(spy).getCaller(); Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); - Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) - .thenReturn(null); Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(null); spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); @@ -1589,7 +1592,7 @@ public void testListHostsForMigrationOfVMStorageMotionCheckForSystemVM() { spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); // Verify that storage motion capability was checked for system VM (VMware is in hypervisorTypes list) - Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null); + Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.VMware, null); // Verify response structure Assert.assertNotNull(result); @@ -1642,7 +1645,7 @@ public void testListHostsForMigrationOfVMStorageMotionCheckForUserVM() { spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); // Verify User VM can migrate with storage (User VM type always checks) - Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.KVM, ""); + Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.KVM, ""); // Verify response data Assert.assertNotNull(result); @@ -1695,7 +1698,7 @@ public void testListHostsForMigrationOfVMWithoutStorageMotionClusterScope() { spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); // Verify XenServer without storage motion was checked - Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.XenServer, null); + Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.XenServer, null); // Verify cluster-scoped search was used (not zone-wide) Mockito.verify(spy).searchForServers( Mockito.eq(0L), Mockito.eq(20L), Mockito.isNull(), Mockito.any(Type.class), @@ -1845,14 +1848,14 @@ public void testListHostsForMigrationOfVMVmwareStorageMotionCheck() { Mockito.doReturn(caller).when(spy).getCaller(); Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) - .thenReturn(null); + .thenReturn(null); HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.VMware); Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); // VMware with DomainRouter should still check storage motion Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.VMware, null)) - .thenReturn(true); + .thenReturn(true); ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); @@ -1880,7 +1883,7 @@ public void testListHostsForMigrationOfVMVmwareStorageMotionCheck() { spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); // Verify VMware always checks storage motion (hypervisorTypes list includes VMware) - Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null); + Mockito.verify(hypervisorCapabilitiesDao, Mockito.atLeastOnce()).isStorageMotionSupported(HypervisorType.VMware, null); // Verify response Assert.assertNotNull(result); @@ -2074,4 +2077,32 @@ private DiskOfferingVO mockSharedDiskOffering(Long id) { Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(false); return diskOffering; } + + @Test + public void createDeploymentPlanForMigrationListingTestAllocatesInAnyClusterWhenStorageMigrationIsSupported() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + HostVO srcHost = mockHost(vm.getHostId(), 1L, 2L, 3L, HypervisorType.KVM); + + Mockito.doReturn(true).when(spy).isStorageMigrationSupported(vm, srcHost); + + DataCenterDeployment deploymentPlan = spy.createDeploymentPlanForMigrationListing(vm, srcHost); + + Assert.assertEquals(3L, deploymentPlan.getDataCenterId()); + Assert.assertEquals(2L, (long) deploymentPlan.getPodId()); + Assert.assertNull(deploymentPlan.getClusterId()); + } + + @Test + public void createDeploymentPlanForMigrationListingTestAllocatesInSourceClusterWhenStorageMigrationIsNotSupported() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.XenServer); + HostVO srcHost = mockHost(vm.getHostId(), 4L, 5L, 6L, HypervisorType.XenServer); + + Mockito.doReturn(false).when(spy).isStorageMigrationSupported(vm, srcHost); + + DataCenterDeployment deploymentPlan = spy.createDeploymentPlanForMigrationListing(vm, srcHost); + + Assert.assertEquals(6L, deploymentPlan.getDataCenterId()); + Assert.assertEquals(5L, (long) deploymentPlan.getPodId()); + Assert.assertEquals(4L, (long) deploymentPlan.getClusterId()); + } }