From f6c603b28db79b81bf689e0a6ce09f772a92a4e7 Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Thu, 18 Jun 2026 17:17:30 -0300 Subject: [PATCH] Fix `findHostsForMigration` never returning hosts from other clusters --- .../cloud/server/ManagementServerImpl.java | 59 ++++++++++++------- .../server/ManagementServerImplTest.java | 57 ++++++++++++++---- 2 files changed, 83 insertions(+), 33 deletions(-) diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index bd4c311e3cd5..32034e1425aa 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -1490,19 +1490,7 @@ Ternary, Integer>, List, Map 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); - } + final String srcHostVersion = getHypervisorVersionOfHost(srcHost); // 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); @@ -1517,6 +1505,7 @@ Ternary, Integer>, List, Map, Integer>, List, Map(allHostsPairResult, filteredHosts, requiresStorageMotion); } + protected boolean isStorageMigrationSupported(final VirtualMachine vm) { + final Host srcHost = _hostDao.findById(vm.getHostId()); + if (srcHost == null) { + throw new CloudRuntimeException(String.format("Unable to find the host where Instance [%s] is running.", vm.getInstanceName())); + } + + 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. @@ -1642,7 +1653,6 @@ public ExcludeList applyAffinityConstraints(VirtualMachine vm, VirtualMachinePro * @param plan The deployment plan * @param compatibleHosts List of technically compatible hosts * @param excludes ExcludeList with hosts to avoid - * @param srcHost Source host (for architecture filtering) * @return List of suitable hosts with capacity */ protected List getCapableSuitableHosts( @@ -1650,8 +1660,7 @@ protected List getCapableSuitableHosts( final VirtualMachineProfile vmProfile, final DataCenterDeployment plan, final List compatibleHosts, - final ExcludeList excludes, - final Host srcHost) { + final ExcludeList excludes) { List suitableHosts = new ArrayList<>(); @@ -1677,6 +1686,7 @@ protected List getCapableSuitableHosts( // Only list hosts of the same architecture as the source Host in a multi-arch zone if (!suitableHosts.isEmpty()) { + Host srcHost = _hostDao.findById(vm.getHostId()); List clusterArchs = ApiDBUtils.listZoneClustersArchs(vm.getDataCenterId()); if (CollectionUtils.isNotEmpty(clusterArchs) && clusterArchs.size() > 1) { suitableHosts = suitableHosts.stream().filter(h -> h.getArch() == srcHost.getArch()).collect(Collectors.toList()); @@ -1707,9 +1717,7 @@ public Ternary, Integer>, List, Map, Integer>, List, Map suitableHosts = getCapableSuitableHosts(vm, vmProfile, plan, filteredHosts, excludes, srcHost); + List suitableHosts = getCapableSuitableHosts(vm, vmProfile, plan, filteredHosts, excludes); final Pair, Integer> otherHosts = new Pair<>(allHostsPair.first(), allHostsPair.second()); return new Ternary<>(otherHosts, suitableHosts, requiresStorageMotion); } + protected DataCenterDeployment createDeploymentPlanForMigrationListing(final VirtualMachine vm) { + final Host srcHost = _hostDao.findById(vm.getHostId()); + + final boolean canMigrateWithStorage = isStorageMigrationSupported(vm); + 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..412ed1dde09a 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); @@ -888,7 +889,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 +953,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 +1098,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 +1417,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 +1482,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); @@ -1589,7 +1590,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 +1643,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 +1696,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 +1846,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 +1881,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 +2075,34 @@ private DiskOfferingVO mockSharedDiskOffering(Long id) { Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(false); return diskOffering; } + + @Test + public void createDeploymentPlanForMigrationListingTestAllocatesInAnyClusterWhenStorageMigrationIsSupported() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + Mockito.doReturn(true).when(spy).isStorageMigrationSupported(vm); + + HostVO srcHost = mockHost(100L, 1L, 2L, 3L, HypervisorType.KVM); + Mockito.doReturn(srcHost).when(hostDao).findById(Mockito.anyLong()); + + DataCenterDeployment deploymentPlan = spy.createDeploymentPlanForMigrationListing(vm); + + 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); + Mockito.doReturn(false).when(spy).isStorageMigrationSupported(vm); + + HostVO srcHost = mockHost(200L, 4L, 5L, 6L, HypervisorType.XenServer); + Mockito.doReturn(srcHost).when(hostDao).findById(Mockito.anyLong()); + + DataCenterDeployment deploymentPlan = spy.createDeploymentPlanForMigrationListing(vm); + + Assert.assertEquals(6L, deploymentPlan.getDataCenterId()); + Assert.assertEquals(5L, (long) deploymentPlan.getPodId()); + Assert.assertEquals(4L, (long) deploymentPlan.getClusterId()); + } }