From 3748f32bc7f44b8730c0ba77441c6b0ec8525d7b Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 20 Jun 2023 20:43:08 +0530 Subject: [PATCH] engine-orchestration,vmware: hypervisor migration during start vm migration (#7444) Signed-off-by: Abhishek Kumar --- .../com/cloud/vm/VirtualMachineManager.java | 2 + .../cloud/vm/VirtualMachineManagerImpl.java | 35 +++++++++++++-- .../vm/VirtualMachineManagerImplTest.java | 30 +++++++++++++ .../com/cloud/hypervisor/guru/VMwareGuru.java | 14 +++--- .../cloud/hypervisor/guru/VMwareGuruTest.java | 43 ++++++++++--------- 5 files changed, 93 insertions(+), 31 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 ada94cd057c..82396cf4635 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 @@ public interface VirtualMachineManager extends Manager { 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 1b3d914b27a..db76fec9b86 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,26 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } + 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 = 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 public void orchestrateStart(final String vmUuid, final Map params, final DeploymentPlan planToDeploy, final DeploymentPlanner planner) throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException { @@ -1227,6 +1247,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac resetVmNicsDeviceId(vm.getId()); _networkMgr.prepare(vmProfile, dest, ctx); if (vm.getHypervisorType() != HypervisorType.BareMetal) { + checkAndAttemptMigrateVmAcrossCluster(vm, cluster_id, dest.getStorageForDisks()); volumeMgr.prepare(vmProfile, dest); } @@ -2355,7 +2376,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { return _agentMgr.send(hostId, commandsContainer); } catch (AgentUnavailableException | OperationTimedoutException e) { - throw new CloudRuntimeException(String.format("Failed to migrate VM: %s", vm.getUuid()),e); + s_logger.warn(String.format("Hypervisor migration failed for the VM: %s", vm), e); } } return null; @@ -5712,8 +5733,12 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac 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(); @@ -5731,6 +5756,10 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac 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/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index 742bb3dda89..c4be16ea255 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -838,4 +838,34 @@ public class VirtualMachineManagerImplTest { Mockito.when(templateZoneDao.findByZoneTemplate(dcId, templateId)).thenReturn(Mockito.mock(VMTemplateZoneVO.class)); virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm); } + + @Test + public void checkAndAttemptMigrateVmAcrossClusterNonValid() { + // Below scenarios shouldn't result in VM migration + + 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); + } } 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 0b67074e937..fe35d565088 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 java.util.UUID; 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.DataStoreRole; 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.Volume; 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 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co @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 class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co 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); 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 0db8271c8e5..7398ef99a8c 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.powermock.modules.junit4.PowerMockRunner; 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 class VMwareGuruTest { 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);