engine-orchestration,vmware: hypervisor migration during start vm migration (#7444)

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java

@@ -264,6 +264,8 @@ public interface VirtualMachineManager extends Manager {
 
     Pair<Long, Long> findClusterAndHostIdForVm(long vmId);
 
+    Pair<Long, Long> findClusterAndHostIdForVm(VirtualMachine vm, boolean skipCurrentHostForStartingVm);
+
     /**
      * Obtains statistics for a list of VMs; CPU and network utilization
      * @param hostId ID of the host

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<Volume, StoragePool> 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<VirtualMachineProfile.Param, Object> 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<Long, Long> findClusterAndHostIdForVm(VirtualMachine vm) {
+    @Override
-        Long hostId = vm.getHostId();
+    public Pair<Long, Long> 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<Long, Long> findClusterAndHostIdForVm(VirtualMachine vm) {
+        return findClusterAndHostIdForVm(vm, false);
+    }
+
     @Override
     public Pair<Long, Long> findClusterAndHostIdForVm(long vmId) {
         VMInstanceVO vm = _vmDao.findById(vmId);

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<Volume, StoragePool> 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);
+    }
 }

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<Command> finalizeMigrate(VirtualMachine vm, Map<Volume, StoragePool> volumeToPool) {
-        List<Command> commands = new ArrayList<Command>();
+        List<Command> commands = new ArrayList<>();
 
         // OfflineVmwareMigration: specialised migration command
-        List<Pair<VolumeTO, StorageFilerTO>> volumeToFilerTo = new ArrayList<Pair<VolumeTO, StorageFilerTO>>();
+        List<Pair<VolumeTO, StorageFilerTO>> volumeToFilerTo = new ArrayList<>();
         Long poolClusterId = null;
         StoragePool targetLocalPoolForVM = null;
         for (Map.Entry<Volume, StoragePool> 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, StorageFilerTO>(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);
 

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 static org.junit.Assert.assertEquals;
-import com.cloud.agent.api.MigrateVmToPoolCommand;
+
-import com.cloud.dc.ClusterDetailsDao;
+import java.util.ArrayList;
-import com.cloud.host.HostVO;
+import java.util.HashMap;
-import com.cloud.host.dao.HostDao;
+import java.util.List;
-import com.cloud.storage.Storage.ProvisioningType;
+import java.util.Map;
-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 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 com.cloud.agent.api.Command;
-
+import com.cloud.agent.api.MigrateVmToPoolCommand;
-import java.util.ArrayList;
+import com.cloud.dc.ClusterDetailsDao;
-import java.util.HashMap;
+import com.cloud.host.HostVO;
-import java.util.List;
+import com.cloud.host.dao.HostDao;
-import java.util.Map;
+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<Long, Long> clusterAndHost = new Pair<>(1L, 1L);
 
-        Mockito.when(vmManager.findClusterAndHostIdForVm(1L)).thenReturn(clusterAndHost);
+        Mockito.when(vmManager.findClusterAndHostIdForVm(vm, true)).thenReturn(clusterAndHost);
 
         List<StoragePoolHostVO> storagePoolHostVOS = new ArrayList<>();
         storagePoolHostVOS.add(storagePoolHostVO);