This is an automated email from the ASF dual-hosted git repository.
winterhazel pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new ec2d3ea1e6d Fix `findHostsForMigration` never returning hosts from
other clusters (#13452)
ec2d3ea1e6d is described below
commit ec2d3ea1e6d186964760c8d2ab33d3164715e1b5
Author: Fabricio Duarte <[email protected]>
AuthorDate: Fri Jul 10 13:22:58 2026 -0300
Fix `findHostsForMigration` never returning hosts from other clusters
(#13452)
---
.../com/cloud/server/ManagementServerImpl.java | 69 +++++++++++++---------
.../com/cloud/server/ManagementServerImplTest.java | 61 ++++++++++++++-----
2 files changed, 86 insertions(+), 44 deletions(-)
diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
index bd4c311e3cd..1fc92ad0e8e 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 @@ public class ManagementServerImpl extends ManagerBase
implements ManagementServe
*/
Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>,
Map<Host, Boolean>> getTechnicallyCompatibleHosts(
final VirtualMachine vm,
+ final Host srcHost,
final Long startIndex,
final Long pageSize,
final String keyword) {
@@ -1479,31 +1480,6 @@ public class ManagementServerImpl extends ManagerBase
implements ManagementServe
return new Ternary<>(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<HypervisorType> 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<VolumeVO> volumes =
_volumeDao.findCreatedByInstance(vmProfile.getId());
@@ -1517,10 +1493,12 @@ public class ManagementServerImpl extends ManagerBase
implements ManagementServe
}
}
+ boolean canMigrateWithStorage = isStorageMigrationSupported(vm,
srcHost);
if (!canMigrateWithStorage && usesLocal) {
throw new InvalidParameterValueException("Unsupported operation,
instance uses Local storage, cannot migrate");
}
+ final String srcHostVersion = getHypervisorVersionOfHost(srcHost);
final Type hostType = srcHost.getType();
Pair<List<HostVO>, Integer> allHostsPair = null;
List<HostVO> allHosts = null;
@@ -1596,6 +1574,23 @@ public class ManagementServerImpl extends ManagerBase
implements ManagementServe
return new Ternary<>(allHostsPairResult, filteredHosts,
requiresStorageMotion);
}
+ protected boolean isStorageMigrationSupported(final VirtualMachine vm,
final Host srcHost) {
+ final List<HypervisorType> 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 class ManagementServerImpl extends ManagerBase
implements ManagementServe
validateVmForHostMigration(vm);
+ final long srcHostId = vm.getHostId();
+ final Host srcHost = _hostDao.findById(srcHostId);
+ if (srcHost == null) {
+ logger.debug("Unable to find the host with ID: {} of this
Instance: {}", srcHostId, 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;
+ }
+
// Get technically compatible hosts (storage, hypervisor, UEFI)
Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>,
Map<Host, Boolean>> compatibilityResult =
- getTechnicallyCompatibleHosts(vm, startIndex, pageSize, keyword);
+ getTechnicallyCompatibleHosts(vm, srcHost, startIndex, pageSize,
keyword);
Pair<List<? extends Host>, Integer> allHostsPair =
compatibilityResult.first();
List<? extends Host> filteredHosts = compatibilityResult.second();
@@ -1707,9 +1712,7 @@ public class ManagementServerImpl extends ManagerBase
implements ManagementServe
}
// Create deployment plan and VM profile
- final Host srcHost = _hostDao.findById(vm.getHostId());
- final DataCenterDeployment plan = new DataCenterDeployment(
- srcHost.getDataCenterId(), srcHost.getPodId(),
srcHost.getClusterId(), null, null, null);
+ final DataCenterDeployment plan =
createDeploymentPlanForMigrationListing(vm, srcHost);
final VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(
vm, null, _offeringDao.findById(vm.getId(),
vm.getServiceOfferingId()), null, null);
@@ -1723,6 +1726,14 @@ public class ManagementServerImpl extends ManagerBase
implements ManagementServe
return new Ternary<>(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 da2005f6136..2f3e97716a0 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.api.ApiDBUtils;
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 class ManagementServerImplTest {
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 @@ public class ManagementServerImplTest {
}
@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 class ManagementServerImplTest {
@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 class ManagementServerImplTest {
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 class ManagementServerImplTest {
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 class ManagementServerImplTest {
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 class ManagementServerImplTest {
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 class ManagementServerImplTest {
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 class ManagementServerImplTest {
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 class ManagementServerImplTest {
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 class ManagementServerImplTest {
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 class ManagementServerImplTest {
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 class ManagementServerImplTest {
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 class ManagementServerImplTest {
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 @@ public class ManagementServerImplTest {
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());
+ }
}
