This is an automated email from the ASF dual-hosted git repository. sureshanaparti pushed a commit to branch 4.19 in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push: new 2bf36ef9353 DRS: Ensure the destination host is part of the same cluster (#9245) 2bf36ef9353 is described below commit 2bf36ef935342e983aae3a84bccb3f0dcdb25b5d Author: Vishesh <vishes...@gmail.com> AuthorDate: Wed Jun 26 02:09:26 2024 +0530 DRS: Ensure the destination host is part of the same cluster (#9245) * DRS: Ensure the destination host is part of the same cluster * Add and fix unit tests --- .../cloudstack/cluster/ClusterDrsServiceImpl.java | 2 +- .../cluster/ClusterDrsServiceImplTest.java | 60 +++++++++++++++++++--- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java index 9fe00fade61..8542b52eb50 100644 --- a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java @@ -467,7 +467,7 @@ public class ClusterDrsServiceImpl extends ManagerBase implements ClusterDrsServ Map<Host, Boolean> requiresStorageMotion = hostsForMigrationOfVM.third(); for (Host destHost : compatibleDestinationHosts) { - if (!suitableDestinationHosts.contains(destHost)) { + if (!suitableDestinationHosts.contains(destHost) || cluster.getId() != destHost.getClusterId()) { continue; } Ternary<Double, Double, Double> metrics = algorithm.getMetrics(cluster.getId(), vm, diff --git a/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java b/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java index 8aed790af0a..b86819f9389 100644 --- a/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java @@ -73,6 +73,7 @@ import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; @RunWith(MockitoJUnitRunner.class) public class ClusterDrsServiceImplTest { @@ -353,6 +354,7 @@ public class ClusterDrsServiceImplTest { Mockito.when(cluster.getId()).thenReturn(1L); HostVO destHost = Mockito.mock(HostVO.class); + Mockito.when(destHost.getClusterId()).thenReturn(1L); HostVO host = Mockito.mock(HostVO.class); Mockito.when(host.getId()).thenReturn(2L); @@ -386,13 +388,9 @@ public class ClusterDrsServiceImplTest { } Mockito.when(managementServer.listHostsForMigrationOfVM(vm1, 0L, 500L, null, vmList)).thenReturn( - new Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Host, Boolean>>( - new Pair<>(List.of(destHost), 1), List.of(destHost), Map.of(destHost, - false))); + new Ternary<>(new Pair<>(List.of(destHost), 1), List.of(destHost), Map.of(destHost, false))); Mockito.when(managementServer.listHostsForMigrationOfVM(vm2, 0L, 500L, null, vmList)).thenReturn( - new Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Host, Boolean>>( - new Pair<>(List.of(destHost), 1), List.of(destHost), Map.of(destHost, - false))); + new Ternary<>(new Pair<>(List.of(destHost), 1), List.of(destHost), Map.of(destHost, false))); Mockito.when(balancedAlgorithm.getMetrics(cluster.getId(), vm1, serviceOffering, destHost, new HashMap<>(), new HashMap<>(), false)).thenReturn(new Ternary<>(1.0, 0.5, 1.5)); @@ -406,6 +404,56 @@ public class ClusterDrsServiceImplTest { assertEquals(vm1, bestMigration.first()); } + @Test + public void testGetBestMigrationDifferentCluster() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getId()).thenReturn(1L); + + HostVO destHost = Mockito.mock(HostVO.class); + Mockito.when(destHost.getClusterId()).thenReturn(2L); + + HostVO host = Mockito.mock(HostVO.class); + Mockito.when(host.getId()).thenReturn(2L); + + VMInstanceVO vm1 = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm1.getId()).thenReturn(1L); + Mockito.when(vm1.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm1.getState()).thenReturn(VirtualMachine.State.Running); + Mockito.when(vm1.getDetails()).thenReturn(Collections.emptyMap()); + + VMInstanceVO vm2 = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm2.getId()).thenReturn(2L); + Mockito.when(vm2.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm2.getState()).thenReturn(VirtualMachine.State.Running); + Mockito.when(vm2.getDetails()).thenReturn(Collections.emptyMap()); + + List<VirtualMachine> vmList = new ArrayList<>(); + vmList.add(vm1); + vmList.add(vm2); + + Map<Long, List<VirtualMachine>> hostVmMap = new HashMap<>(); + hostVmMap.put(host.getId(), new ArrayList<>()); + hostVmMap.get(host.getId()).add(vm1); + hostVmMap.get(host.getId()).add(vm2); + + Map<Long, ServiceOffering> vmIdServiceOfferingMap = new HashMap<>(); + + ServiceOffering serviceOffering = Mockito.mock(ServiceOffering.class); + for (VirtualMachine vm : vmList) { + vmIdServiceOfferingMap.put(vm.getId(), serviceOffering); + } + + Mockito.when(managementServer.listHostsForMigrationOfVM(vm1, 0L, 500L, null, vmList)).thenReturn( + new Ternary<>(new Pair<>(List.of(destHost), 1), List.of(destHost), Map.of(destHost, false))); + Mockito.when(managementServer.listHostsForMigrationOfVM(vm2, 0L, 500L, null, vmList)).thenReturn( + new Ternary<>(new Pair<>(List.of(destHost), 1), List.of(destHost), Map.of(destHost, false))); + Pair<VirtualMachine, Host> bestMigration = clusterDrsService.getBestMigration(cluster, balancedAlgorithm, + vmList, vmIdServiceOfferingMap, new HashMap<>(), new HashMap<>()); + + assertNull(bestMigration.second()); + assertNull(bestMigration.first()); + } + @Test public void testSavePlan() { Mockito.when(drsPlanDao.persist(Mockito.any(ClusterDrsPlanVO.class))).thenReturn(