From 707ce7f6f81a3b73aba6df9563e603a9f8951f7c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 17 Apr 2023 10:46:51 +0530 Subject: [PATCH 1/5] wip Signed-off-by: Abhishek Kumar --- .../cloud/vm/VirtualMachineManagerImpl.java | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 1b3d914b27a7..8cdbb1eebda9 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1054,6 +1054,32 @@ protected void checkIfTemplateNeededForCreatingVmVolumes(VMInstanceVO vm) { } } + protected void checkAndAttemptMigrateVmAcrossCluster(final VMInstanceVO vm, final Long destinationClusterId, final Map volumePoolMap) { + if (!HypervisorType.VMware.equals(vm.getHypervisorType()) || vm.getLastHostId() == null) { + return; + } + Host lastHost = _hostDao.findById(vm.getLastHostId()); + if (destinationClusterId.equals(lastHost.getClusterId())) { + return; + } + if (volumePoolMap.values().stream().noneMatch(s -> destinationClusterId.equals(s.getClusterId()))) { + return; + } + Answer[] answer; + String warnMessage = "Hypervisor inter-cluster migration during VM start failed"; + try { + answer = attemptHypervisorMigration(vm, volumePoolMap, lastHost.getId()); + if (answer == null) { + s_logger.warn(warnMessage); + return; + } + // Other network related updates will be done using caller + markVolumesInPool(vm, answer); + } catch (CloudRuntimeException cre) { + s_logger.warn(warnMessage, cre); + } + } + @Override public void orchestrateStart(final String vmUuid, final Map params, final DeploymentPlan planToDeploy, final DeploymentPlanner planner) throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException { @@ -1227,6 +1253,7 @@ public void orchestrateStart(final String vmUuid, final Map Date: Tue, 18 Apr 2023 17:07:04 +0530 Subject: [PATCH 2/5] changes for target cluster host Signed-off-by: Abhishek Kumar --- .../java/com/cloud/vm/VirtualMachineManager.java | 2 ++ .../com/cloud/vm/VirtualMachineManagerImpl.java | 12 ++++++++++-- .../java/com/cloud/hypervisor/guru/VMwareGuru.java | 14 +++++++------- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java index ada94cd057c1..82396cf46353 100644 --- a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java +++ b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java @@ -264,6 +264,8 @@ static String getHypervisorHostname(String name) { Pair findClusterAndHostIdForVm(long vmId); + Pair findClusterAndHostIdForVm(VirtualMachine vm, boolean skipCurrentHostForStartingVm); + /** * Obtains statistics for a list of VMs; CPU and network utilization * @param hostId ID of the host diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 8cdbb1eebda9..8198072cc6e8 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -5739,8 +5739,12 @@ private Pair findClusterAndHostIdForVmFromVolumes(long vmId) { return new Pair<>(clusterId, hostId); } - private Pair findClusterAndHostIdForVm(VirtualMachine vm) { - Long hostId = vm.getHostId(); + @Override + public Pair findClusterAndHostIdForVm(VirtualMachine vm, boolean skipCurrentHostForStartingVm) { + Long hostId = null; + if (!skipCurrentHostForStartingVm || !State.Starting.equals(vm.getState())) { + hostId = vm.getHostId(); + } Long clusterId = null; if(hostId == null) { hostId = vm.getLastHostId(); @@ -5758,6 +5762,10 @@ private Pair findClusterAndHostIdForVm(VirtualMachine vm) { return new Pair<>(clusterId, hostId); } + private Pair findClusterAndHostIdForVm(VirtualMachine vm) { + return findClusterAndHostIdForVm(vm, false); + } + @Override public Pair findClusterAndHostIdForVm(long vmId) { VMInstanceVO vm = _vmDao.findById(vmId); diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java index 0b67074e9372..fe35d565088c 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java @@ -27,9 +27,6 @@ import javax.inject.Inject; -import com.cloud.storage.StorageManager; -import com.cloud.storage.StoragePoolHostVO; -import com.cloud.storage.dao.StoragePoolHostDao; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.backup.Backup; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; @@ -112,7 +109,9 @@ import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.GuestOSVO; import com.cloud.storage.Storage; +import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; +import com.cloud.storage.StoragePoolHostVO; import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.VMTemplateVO; @@ -120,6 +119,7 @@ import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.GuestOSDao; +import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.storage.dao.VolumeDao; @@ -1149,10 +1149,10 @@ private String getHostGuidInTargetCluster(boolean isInterClusterMigration, Long @Override public List finalizeMigrate(VirtualMachine vm, Map volumeToPool) { - List commands = new ArrayList(); + List commands = new ArrayList<>(); // OfflineVmwareMigration: specialised migration command - List> volumeToFilerTo = new ArrayList>(); + List> volumeToFilerTo = new ArrayList<>(); Long poolClusterId = null; StoragePool targetLocalPoolForVM = null; for (Map.Entry entry : volumeToPool.entrySet()) { @@ -1166,10 +1166,10 @@ public List finalizeMigrate(VirtualMachine vm, Map if (volume.getVolumeType().equals(Volume.Type.ROOT) && pool.isLocal()) { targetLocalPoolForVM = pool; } - volumeToFilerTo.add(new Pair(volumeTo, filerTo)); + volumeToFilerTo.add(new Pair<>(volumeTo, filerTo)); } final Long destClusterId = poolClusterId; - final Long srcClusterId = vmManager.findClusterAndHostIdForVm(vm.getId()).first(); + final Long srcClusterId = vmManager.findClusterAndHostIdForVm(vm, true).first(); final boolean isInterClusterMigration = isInterClusterMigration(destClusterId, srcClusterId); String targetHostGuid = getTargetHostGuid(targetLocalPoolForVM, destClusterId, isInterClusterMigration); From d02c7692b50658772c91fc55f7a4a4fbfb580536 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 18 Apr 2023 18:56:31 +0530 Subject: [PATCH 3/5] test fix Signed-off-by: Abhishek Kumar --- .../cloud/hypervisor/guru/VMwareGuruTest.java | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java index 0db8271c8e59..7398ef99a8cd 100644 --- a/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java +++ b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java @@ -16,20 +16,13 @@ // under the License. package com.cloud.hypervisor.guru; -import com.cloud.agent.api.Command; -import com.cloud.agent.api.MigrateVmToPoolCommand; -import com.cloud.dc.ClusterDetailsDao; -import com.cloud.host.HostVO; -import com.cloud.host.dao.HostDao; -import com.cloud.storage.Storage.ProvisioningType; -import com.cloud.storage.StoragePool; -import com.cloud.storage.StoragePoolHostVO; -import com.cloud.storage.Volume; -import com.cloud.storage.VolumeVO; -import com.cloud.storage.dao.StoragePoolHostDao; -import com.cloud.utils.Pair; -import com.cloud.vm.VirtualMachine; -import com.cloud.vm.VirtualMachineManager; +import static org.junit.Assert.assertEquals; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.junit.Assert; @@ -46,12 +39,20 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.support.AnnotationConfigContextLoader; -import static org.junit.Assert.assertEquals; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import com.cloud.agent.api.Command; +import com.cloud.agent.api.MigrateVmToPoolCommand; +import com.cloud.dc.ClusterDetailsDao; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; +import com.cloud.storage.Storage.ProvisioningType; +import com.cloud.storage.StoragePool; +import com.cloud.storage.StoragePoolHostVO; +import com.cloud.storage.Volume; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.StoragePoolHostDao; +import com.cloud.utils.Pair; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VirtualMachineManager; @RunWith(PowerMockRunner.class) @PrepareForTest({VMwareGuru.class}) @@ -105,7 +106,7 @@ public void finalizeMigrateForLocalStorageToHaveTargetHostGuid(){ Mockito.when(localStorage.isLocal()).thenReturn(true); Pair clusterAndHost = new Pair<>(1L, 1L); - Mockito.when(vmManager.findClusterAndHostIdForVm(1L)).thenReturn(clusterAndHost); + Mockito.when(vmManager.findClusterAndHostIdForVm(vm, true)).thenReturn(clusterAndHost); List storagePoolHostVOS = new ArrayList<>(); storagePoolHostVOS.add(storagePoolHostVO); From e026e2969f825e8c80580ff749f1d717ffbc740e Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 19 Apr 2023 16:42:28 +0530 Subject: [PATCH 4/5] add test Signed-off-by: Abhishek Kumar --- .../cloud/vm/VirtualMachineManagerImpl.java | 18 ++++------ .../vm/VirtualMachineManagerImplTest.java | 34 +++++++++++++++++++ 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 8198072cc6e8..db76fec9b86d 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1065,19 +1065,13 @@ protected void checkAndAttemptMigrateVmAcrossCluster(final VMInstanceVO vm, fina if (volumePoolMap.values().stream().noneMatch(s -> destinationClusterId.equals(s.getClusterId()))) { return; } - Answer[] answer; - String warnMessage = "Hypervisor inter-cluster migration during VM start failed"; - try { - answer = attemptHypervisorMigration(vm, volumePoolMap, lastHost.getId()); - if (answer == null) { - s_logger.warn(warnMessage); - return; - } - // Other network related updates will be done using caller - markVolumesInPool(vm, answer); - } catch (CloudRuntimeException cre) { - s_logger.warn(warnMessage, cre); + Answer[] answer = attemptHypervisorMigration(vm, volumePoolMap, lastHost.getId()); + if (answer == null) { + s_logger.warn("Hypervisor inter-cluster migration during VM start failed"); + return; } + // Other network related updates will be done using caller + markVolumesInPool(vm, answer); } @Override diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index 742bb3dda897..4dfcd2bb7fa6 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -63,6 +63,7 @@ import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.hypervisor.HypervisorGuruManager; import com.cloud.offering.ServiceOffering; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; @@ -151,6 +152,8 @@ public class VirtualMachineManagerImplTest { private UserVmDao userVmDaoMock; @Mock private UserVmVO userVmMock; + @Mock + HypervisorGuruManager hypervisorGuruManager; @Before public void setup() { @@ -838,4 +841,35 @@ public void checkIfTemplateNeededForCreatingVmVolumesTemplateAvailable() { Mockito.when(templateZoneDao.findByZoneTemplate(dcId, templateId)).thenReturn(Mockito.mock(VMTemplateZoneVO.class)); virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm); } + + @Test + public void checkAndAttemptMigrateVmAcrossClusterNonValid() { + // If any of the non-valid cases try migration exception is thrown + Mockito.when(hypervisorGuruManager.getGuru(Mockito.any())).thenThrow(CloudRuntimeException.class); + + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getHypervisorType()).thenReturn(HypervisorType.KVM); + virtualMachineManagerImpl.checkAndAttemptMigrateVmAcrossCluster(vm, 1L, new HashMap<>()); + + Mockito.when(vm.getHypervisorType()).thenReturn(HypervisorType.VMware); + Mockito.when(vm.getLastHostId()).thenReturn(null); + virtualMachineManagerImpl.checkAndAttemptMigrateVmAcrossCluster(vm, 1L, new HashMap<>()); + + Long destinationClusterId = 10L; + Mockito.when(vm.getLastHostId()).thenReturn(1L); + HostVO hostVO = Mockito.mock(HostVO.class); + Mockito.when(hostVO.getClusterId()).thenReturn(destinationClusterId); + Mockito.when(hostDaoMock.findById(1L)).thenReturn(hostVO); + virtualMachineManagerImpl.checkAndAttemptMigrateVmAcrossCluster(vm, destinationClusterId, new HashMap<>()); + + destinationClusterId = 20L; + Map map = new HashMap<>(); + StoragePool pool1 = Mockito.mock(StoragePool.class); + Mockito.when(pool1.getClusterId()).thenReturn(10L); + map.put(Mockito.mock(Volume.class), pool1); + StoragePool pool2 = Mockito.mock(StoragePool.class); + Mockito.when(pool2.getClusterId()).thenReturn(null); + map.put(Mockito.mock(Volume.class), pool2); + virtualMachineManagerImpl.checkAndAttemptMigrateVmAcrossCluster(vm, destinationClusterId, map); + } } From d7cfefb5d19a165fb1fbda5f1f1f0c42d853c0ad Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 19 Apr 2023 18:05:06 +0530 Subject: [PATCH 5/5] fix Signed-off-by: Abhishek Kumar --- .../java/com/cloud/vm/VirtualMachineManagerImplTest.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index 4dfcd2bb7fa6..c4be16ea2556 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -63,7 +63,6 @@ import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; -import com.cloud.hypervisor.HypervisorGuruManager; import com.cloud.offering.ServiceOffering; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; @@ -152,8 +151,6 @@ public class VirtualMachineManagerImplTest { private UserVmDao userVmDaoMock; @Mock private UserVmVO userVmMock; - @Mock - HypervisorGuruManager hypervisorGuruManager; @Before public void setup() { @@ -844,8 +841,7 @@ public void checkIfTemplateNeededForCreatingVmVolumesTemplateAvailable() { @Test public void checkAndAttemptMigrateVmAcrossClusterNonValid() { - // If any of the non-valid cases try migration exception is thrown - Mockito.when(hypervisorGuruManager.getGuru(Mockito.any())).thenThrow(CloudRuntimeException.class); + // Below scenarios shouldn't result in VM migration VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); Mockito.when(vm.getHypervisorType()).thenReturn(HypervisorType.KVM);