diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java index 731cb67aa83..549d02b4579 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java @@ -83,6 +83,12 @@ public class MigrateVirtualMachineWithVolumeCmd extends BaseAsyncCmd { "<1b331390-59f2-4796-9993-bf11c6e76225>&migrateto[2].pool=<41fdb564-9d3b-447d-88ed-7628f7640cbc>") private Map migrateVolumeTo; + @Parameter(name = ApiConstants.AUTO_SELECT, + since = "4.19.0", + type = CommandType.BOOLEAN, + description = "Automatically select a destination host for a running instance, if hostId is not specified. false by default") + private Boolean autoSelect; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -144,31 +150,39 @@ public class MigrateVirtualMachineWithVolumeCmd extends BaseAsyncCmd { return ApiCommandResourceType.VirtualMachine; } + private Host getDestinationHost() { + if (getHostId() == null) { + return null; + } + Host destinationHost = _resourceService.getHost(getHostId()); + // OfflineVmwareMigration: destination host would have to not be a required parameter for stopped VMs + if (destinationHost == null) { + s_logger.error(String.format("Unable to find the host with ID [%s].", getHostId())); + throw new InvalidParameterValueException("Unable to find the specified host to migrate the VM."); + } + return destinationHost; + } + @Override public void execute() { - if (hostId == null && MapUtils.isEmpty(migrateVolumeTo)) { - throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO)); + if (hostId == null && MapUtils.isEmpty(migrateVolumeTo) && !Boolean.TRUE.equals(autoSelect)) { + throw new InvalidParameterValueException(String.format("Either %s or %s must be passed or %s must be true for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO, ApiConstants.AUTO_SELECT)); } VirtualMachine virtualMachine = _userVmService.getVm(getVirtualMachineId()); - if (!VirtualMachine.State.Running.equals(virtualMachine.getState()) && hostId != null) { + if (!VirtualMachine.State.Running.equals(virtualMachine.getState()) && (hostId != null || Boolean.TRUE.equals(autoSelect))) { throw new InvalidParameterValueException(String.format("%s is not in the Running state to migrate it to the new host.", virtualMachine)); } - if (!VirtualMachine.State.Stopped.equals(virtualMachine.getState()) && hostId == null) { - throw new InvalidParameterValueException(String.format("%s is not in the Stopped state to migrate, use the %s parameter to migrate it to a new host.", - virtualMachine, ApiConstants.HOST_ID)); + if (!VirtualMachine.State.Stopped.equals(virtualMachine.getState()) && hostId == null && Boolean.FALSE.equals(autoSelect)) { + throw new InvalidParameterValueException(String.format("%s is not in the Stopped state to migrate, use the %s or %s parameter to migrate it to a new host.", + virtualMachine, ApiConstants.HOST_ID,ApiConstants.AUTO_SELECT)); } try { VirtualMachine migratedVm = null; - if (hostId != null) { - Host destinationHost = _resourceService.getHost(getHostId()); - // OfflineVmwareMigration: destination host would have to not be a required parameter for stopped VMs - if (destinationHost == null) { - s_logger.error(String.format("Unable to find the host with ID [%s].", getHostId())); - throw new InvalidParameterValueException("Unable to find the specified host to migrate the VM."); - } + if (getHostId() != null || Boolean.TRUE.equals(autoSelect)) { + Host destinationHost = getDestinationHost(); migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, getVolumeToPool()); } else if (MapUtils.isNotEmpty(migrateVolumeTo)) { migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), getVolumeToPool()); diff --git a/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmdTest.java index 7a98626ca45..61a3c8fb9e6 100644 --- a/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmdTest.java +++ b/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmdTest.java @@ -16,16 +16,10 @@ // under the License. package org.apache.cloudstack.api.command.admin.vm; -import com.cloud.exception.InvalidParameterValueException; -import com.cloud.exception.ManagementServerException; -import com.cloud.exception.ResourceUnavailableException; -import com.cloud.exception.VirtualMachineMigrationException; -import com.cloud.host.Host; -import com.cloud.resource.ResourceService; -import com.cloud.uservm.UserVm; -import com.cloud.utils.db.UUIDManager; -import com.cloud.vm.UserVmService; -import com.cloud.vm.VirtualMachine; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ResponseGenerator; import org.apache.cloudstack.api.ResponseObject; @@ -40,13 +34,21 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.Spy; -import org.powermock.modules.junit4.PowerMockRunner; +import org.mockito.junit.MockitoJUnitRunner; import org.springframework.test.util.ReflectionTestUtils; -import java.util.List; -import java.util.Map; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.ManagementServerException; +import com.cloud.exception.ResourceUnavailableException; +import com.cloud.exception.VirtualMachineMigrationException; +import com.cloud.host.Host; +import com.cloud.resource.ResourceService; +import com.cloud.uservm.UserVm; +import com.cloud.utils.db.UUIDManager; +import com.cloud.vm.UserVmService; +import com.cloud.vm.VirtualMachine; -@RunWith(PowerMockRunner.class) +@RunWith(MockitoJUnitRunner.class) public class MigrateVirtualMachineWithVolumeCmdTest { @Mock UserVmService userVmServiceMock; @@ -68,44 +70,46 @@ public class MigrateVirtualMachineWithVolumeCmdTest { @Spy @InjectMocks - MigrateVirtualMachineWithVolumeCmd cmdSpy = new MigrateVirtualMachineWithVolumeCmd(); + MigrateVirtualMachineWithVolumeCmd cmdSpy; private Long hostId = 1L; - private Long virtualMachineUuid = 1L; + private Long virtualMachineId = 1L; private String virtualMachineName = "VM-name"; - private Map migrateVolumeTo = Map.of("key","value"); + private Map migrateVolumeTo = null; private SystemVmResponse systemVmResponse = new SystemVmResponse(); private UserVmResponse userVmResponse = new UserVmResponse(); @Before - public void setup() { - Mockito.when(cmdSpy.getVirtualMachineId()).thenReturn(virtualMachineUuid); - Mockito.when(cmdSpy.getHostId()).thenReturn(hostId); - Mockito.when(cmdSpy.getVolumeToPool()).thenReturn(migrateVolumeTo); + public void setUp() throws Exception { + ReflectionTestUtils.setField(cmdSpy, "virtualMachineId", virtualMachineId); + migrateVolumeTo = new HashMap<>(); + migrateVolumeTo.put("volume", "abc"); + migrateVolumeTo.put("pool", "xyz"); } @Test - public void executeTestHostIdIsNullAndMigrateVolumeToIsNullThrowsInvalidParameterValueException(){ + public void executeTestRequiredArgsNullThrowsInvalidParameterValueException() { ReflectionTestUtils.setField(cmdSpy, "hostId", null); ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", null); + ReflectionTestUtils.setField(cmdSpy, "autoSelect", null); try { cmdSpy.execute(); } catch (Exception e) { Assert.assertEquals(InvalidParameterValueException.class, e.getClass()); - String expected = String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO); + String expected = String.format("Either %s or %s must be passed or %s must be true for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO, ApiConstants.AUTO_SELECT); Assert.assertEquals(expected , e.getMessage()); } } @Test - public void executeTestVMIsStoppedAndHostIdIsNotNullThrowsInvalidParameterValueException(){ + public void executeTestVMIsStoppedAndHostIdIsNotNullThrowsInvalidParameterValueException() { ReflectionTestUtils.setField(cmdSpy, "hostId", hostId); ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo); Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock); Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Stopped); - Mockito.when(virtualMachineMock.toString()).thenReturn(String.format("VM [uuid: %s, name: %s]", virtualMachineUuid, virtualMachineName)); + Mockito.when(virtualMachineMock.toString()).thenReturn(String.format("VM [uuid: %s, name: %s]", virtualMachineId, virtualMachineName)); try { cmdSpy.execute(); @@ -117,33 +121,35 @@ public class MigrateVirtualMachineWithVolumeCmdTest { } @Test - public void executeTestVMIsRunningAndHostIdIsNullThrowsInvalidParameterValueException(){ + public void executeTestVMIsRunningHostIdIsNullAndAutoSelectIsFalseThrowsInvalidParameterValueException() { ReflectionTestUtils.setField(cmdSpy, "hostId", null); + ReflectionTestUtils.setField(cmdSpy, "autoSelect", false); ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo); Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock); Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Running); - Mockito.when(virtualMachineMock.toString()).thenReturn(String.format("VM [uuid: %s, name: %s]", virtualMachineUuid, virtualMachineName)); + Mockito.when(virtualMachineMock.toString()).thenReturn(String.format("VM [uuid: %s, name: %s]", virtualMachineId, virtualMachineName)); try { cmdSpy.execute(); } catch (Exception e) { Assert.assertEquals(InvalidParameterValueException.class, e.getClass()); - String expected = String.format("%s is not in the Stopped state to migrate, use the %s parameter to migrate it to a new host.", virtualMachineMock, - ApiConstants.HOST_ID); + String expected = String.format("%s is not in the Stopped state to migrate, use the %s or %s parameter to migrate it to a new host.", virtualMachineMock, + ApiConstants.HOST_ID, ApiConstants.AUTO_SELECT); Assert.assertEquals(expected , e.getMessage()); } } @Test - public void executeTestHostIdIsNullThrowsInvalidParameterValueException(){ + public void executeTestHostIdIsNullThrowsInvalidParameterValueException() { + ReflectionTestUtils.setField(cmdSpy, "virtualMachineId", virtualMachineId); ReflectionTestUtils.setField(cmdSpy, "hostId", hostId); ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo); + ReflectionTestUtils.setField(cmdSpy, "autoSelect", false); Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock); Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Running); Mockito.when(resourceServiceMock.getHost(Mockito.anyLong())).thenReturn(null); - Mockito.when(uuidManagerMock.getUuid(Host.class, virtualMachineUuid)).thenReturn(virtualMachineUuid.toString()); try { cmdSpy.execute(); @@ -154,15 +160,22 @@ public class MigrateVirtualMachineWithVolumeCmdTest { } } + private Map getMockedMigrateVolumeToApiCmdParam() { + Map migrateVolumeTo = new HashMap<>(); + migrateVolumeTo.put("volume", "abc"); + migrateVolumeTo.put("pool", "xyz"); + return Map.of("", migrateVolumeTo); + } + @Test public void executeTestHostIsNotNullMigratedVMIsNullThrowsServerApiException() throws ManagementServerException, ResourceUnavailableException, VirtualMachineMigrationException { ReflectionTestUtils.setField(cmdSpy, "hostId", hostId); - ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo); + ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", getMockedMigrateVolumeToApiCmdParam()); Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock); Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Running); - Mockito.when(resourceServiceMock.getHost(Mockito.anyLong())).thenReturn(hostMock); - Mockito.when(userVmServiceMock.migrateVirtualMachineWithVolume(virtualMachineUuid, hostMock, migrateVolumeTo)).thenReturn(null); + Mockito.when(resourceServiceMock.getHost(hostId)).thenReturn(hostMock); + Mockito.when(userVmServiceMock.migrateVirtualMachineWithVolume(Mockito.anyLong(), Mockito.any(), Mockito.anyMap())).thenReturn(null); try { cmdSpy.execute(); @@ -176,11 +189,11 @@ public class MigrateVirtualMachineWithVolumeCmdTest { @Test public void executeTestHostIsNullMigratedVMIsNullThrowsServerApiException() { ReflectionTestUtils.setField(cmdSpy, "hostId", null); - ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo); + ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", getMockedMigrateVolumeToApiCmdParam()); Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock); Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Stopped); - Mockito.when(userVmServiceMock.vmStorageMigration(virtualMachineUuid, migrateVolumeTo)).thenReturn(null); + Mockito.when(userVmServiceMock.vmStorageMigration(Mockito.anyLong(), Mockito.anyMap())).thenReturn(null); try { cmdSpy.execute(); @@ -194,11 +207,11 @@ public class MigrateVirtualMachineWithVolumeCmdTest { @Test public void executeTestSystemVMMigratedWithSuccess() { ReflectionTestUtils.setField(cmdSpy, "hostId", null); - ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo); + ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", getMockedMigrateVolumeToApiCmdParam()); Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock); Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Stopped); - Mockito.when(userVmServiceMock.vmStorageMigration(virtualMachineUuid, migrateVolumeTo)).thenReturn(virtualMachineMock); + Mockito.when(userVmServiceMock.vmStorageMigration(Mockito.anyLong(), Mockito.anyMap())).thenReturn(virtualMachineMock); Mockito.when(virtualMachineMock.getType()).thenReturn(VirtualMachine.Type.ConsoleProxy); Mockito.when(responseGeneratorMock.createSystemVmResponse(virtualMachineMock)).thenReturn(systemVmResponse); @@ -211,11 +224,11 @@ public class MigrateVirtualMachineWithVolumeCmdTest { public void executeTestUserVMMigratedWithSuccess() { UserVm userVmMock = Mockito.mock(UserVm.class); ReflectionTestUtils.setField(cmdSpy, "hostId", null); - ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo); + ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", getMockedMigrateVolumeToApiCmdParam()); Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(userVmMock); Mockito.when(userVmMock.getState()).thenReturn(VirtualMachine.State.Stopped); - Mockito.when(userVmServiceMock.vmStorageMigration(virtualMachineUuid, migrateVolumeTo)).thenReturn(userVmMock); + Mockito.when(userVmServiceMock.vmStorageMigration(Mockito.anyLong(), Mockito.anyMap())).thenReturn(userVmMock); Mockito.when(userVmMock.getType()).thenReturn(VirtualMachine.Type.User); Mockito.when(responseGeneratorMock.createUserVmResponse(ResponseObject.ResponseView.Full, "virtualmachine", userVmMock)).thenReturn(List.of(userVmResponse)); diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index f0aed627f15..65aecb1cbc3 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -6347,22 +6347,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } public boolean isVMUsingLocalStorage(VMInstanceVO vm) { - boolean usesLocalStorage = false; - List volumes = _volsDao.findByInstance(vm.getId()); - for (VolumeVO vol : volumes) { - DiskOfferingVO diskOffering = _diskOfferingDao.findById(vol.getDiskOfferingId()); - if (diskOffering.isUseLocalStorage()) { - usesLocalStorage = true; - break; - } - StoragePoolVO storagePool = _storagePoolDao.findById(vol.getPoolId()); - if (storagePool.isLocal()) { - usesLocalStorage = true; - break; - } - } - return usesLocalStorage; + return isAnyVmVolumeUsingLocalStorage(volumes); } @Override @@ -6421,7 +6407,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir DeployDestination dest = null; if (destinationHost == null) { - dest = chooseVmMigrationDestination(vm, srcHost); + dest = chooseVmMigrationDestination(vm, srcHost, null); } else { dest = checkVmMigrationDestination(vm, srcHost, destinationHost); } @@ -6436,7 +6422,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return findMigratedVm(vm.getId(), vm.getType()); } - private DeployDestination chooseVmMigrationDestination(VMInstanceVO vm, Host srcHost) { + private DeployDestination chooseVmMigrationDestination(VMInstanceVO vm, Host srcHost, Long poolId) { vm.setLastHostId(null); // Last host does not have higher priority in vm migration final ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offering, null, null); @@ -6444,7 +6430,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir final Host host = _hostDao.findById(srcHostId); ExcludeList excludes = new ExcludeList(); excludes.addHost(srcHostId); - final DataCenterDeployment plan = _itMgr.getMigrationDeployment(vm, host, null, excludes); + final DataCenterDeployment plan = _itMgr.getMigrationDeployment(vm, host, poolId, excludes); try { return _planningMgr.planDeployment(profile, plan, excludes, null); } catch (final AffinityConflictException e2) { @@ -6744,8 +6730,21 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return implicitPlannerUsed; } - private boolean isVmVolumesOnZoneWideStore(VMInstanceVO vm) { - final List volumes = _volsDao.findCreatedByInstance(vm.getId()); + protected boolean isAnyVmVolumeUsingLocalStorage(final List volumes) { + for (VolumeVO vol : volumes) { + DiskOfferingVO diskOffering = _diskOfferingDao.findById(vol.getDiskOfferingId()); + if (diskOffering.isUseLocalStorage()) { + return true; + } + StoragePoolVO storagePool = _storagePoolDao.findById(vol.getPoolId()); + if (storagePool.isLocal()) { + return true; + } + } + return false; + } + + protected boolean isAllVmVolumesOnZoneWideStore(final List volumes) { if (CollectionUtils.isEmpty(volumes)) { return false; } @@ -6769,6 +6768,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir throw new InvalidParameterValueException("Cannot migrate VM, host with ID: " + srcHostId + " for VM not found"); } + if (destinationHost == null) { + return new Pair<>(srcHost, null); + } + // Check if source and destination hosts are valid and migrating to same host if (destinationHost.getId() == srcHostId) { throw new InvalidParameterValueException(String.format("Cannot migrate VM as it is already present on host %s (ID: %s), please specify valid destination host to migrate the VM", @@ -6888,6 +6891,25 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return volToPoolObjectMap; } + protected boolean isVmCanBeMigratedWithoutStorage(Host srcHost, Host destinationHost, List volumes, + Map volumeToPool) { + return !isAnyVmVolumeUsingLocalStorage(volumes) && + MapUtils.isEmpty(volumeToPool) && destinationHost != null + && (destinationHost.getClusterId().equals(srcHost.getClusterId()) || isAllVmVolumesOnZoneWideStore(volumes)); + } + + protected Host chooseVmMigrationDestinationUsingVolumePoolMap(VMInstanceVO vm, Host srcHost, Map volToPoolObjectMap) { + Long poolId = null; + if (MapUtils.isNotEmpty(volToPoolObjectMap)) { + poolId = new ArrayList<>(volToPoolObjectMap.values()).get(0); + } + DeployDestination deployDestination = chooseVmMigrationDestination(vm, srcHost, poolId); + if (deployDestination == null || deployDestination.getHost() == null) { + throw new CloudRuntimeException("Unable to find suitable destination to migrate VM " + vm.getInstanceName()); + } + return deployDestination.getHost(); + } + @Override @ActionEvent(eventType = EventTypes.EVENT_VM_MIGRATE, eventDescription = "migrating VM", async = true) public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinationHost, Map volumeToPool) throws ResourceUnavailableException, @@ -6932,8 +6954,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir Pair sourceDestinationHosts = getHostsForMigrateVmWithStorage(vm, destinationHost); Host srcHost = sourceDestinationHosts.first(); - if (!isVMUsingLocalStorage(vm) && MapUtils.isEmpty(volumeToPool) - && (destinationHost.getClusterId().equals(srcHost.getClusterId()) || isVmVolumesOnZoneWideStore(vm))){ + final List volumes = _volsDao.findCreatedByInstance(vm.getId()); + if (isVmCanBeMigratedWithoutStorage(srcHost, destinationHost, volumes, volumeToPool)) { // If volumes do not have to be migrated // call migrateVirtualMachine for non-user VMs else throw exception if (!VirtualMachine.Type.User.equals(vm.getType())) { @@ -6945,6 +6967,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir Map volToPoolObjectMap = getVolumePoolMappingForMigrateVmWithStorage(vm, volumeToPool); + if (destinationHost == null) { + destinationHost = chooseVmMigrationDestinationUsingVolumePoolMap(vm, srcHost, volToPoolObjectMap); + } + checkHostsDedication(vm, srcHost.getId(), destinationHost.getId()); _itMgr.migrateWithStorage(vm.getUuid(), srcHost.getId(), destinationHost.getId(), volToPoolObjectMap); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 18b4a891917..93995bc860a 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -16,8 +16,6 @@ // under the License. package com.cloud.vm; -import com.cloud.storage.Volume; -import com.cloud.storage.dao.VolumeDao; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -38,18 +36,15 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import com.cloud.template.VirtualMachineTemplate; -import com.cloud.user.UserData; -import com.cloud.user.UserDataVO; -import com.cloud.user.dao.UserDataDao; -import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.api.BaseCmd.HTTPMethod; -import org.apache.cloudstack.api.command.user.vm.ResetVMUserDataCmd; import org.apache.cloudstack.api.command.user.vm.DeployVMCmd; +import org.apache.cloudstack.api.command.user.vm.ResetVMUserDataCmd; import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd; import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -67,11 +62,19 @@ import com.cloud.configuration.Resource; import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.deploy.DataCenterDeployment; +import com.cloud.deploy.DeployDestination; +import com.cloud.deploy.DeploymentPlanner; +import com.cloud.deploy.DeploymentPlanningManager; import com.cloud.exception.InsufficientAddressCapacityException; import com.cloud.exception.InsufficientCapacityException; +import com.cloud.exception.InsufficientServerCapacityException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; +import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.network.NetworkModel; import com.cloud.network.dao.NetworkDao; @@ -81,21 +84,30 @@ import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.GuestOSVO; +import com.cloud.storage.ScopeType; import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.Volume; import com.cloud.storage.VolumeApiService; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.GuestOSDao; import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountService; import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; +import com.cloud.user.UserData; +import com.cloud.user.UserDataVO; import com.cloud.user.UserVO; import com.cloud.user.dao.AccountDao; +import com.cloud.user.dao.UserDataDao; import com.cloud.uservm.UserVm; +import com.cloud.utils.Pair; import com.cloud.utils.db.EntityManager; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.UserVmDetailsDao; @@ -181,6 +193,18 @@ public class UserVmManagerImplTest { @Mock UserDataDao userDataDao; + @Mock + PrimaryDataStoreDao primaryDataStoreDao; + + @Mock + VirtualMachineManager virtualMachineManager; + + @Mock + DeploymentPlanningManager planningManager; + + @Mock + HostDao hostDao; + @Mock private VolumeVO volumeVOMock; @@ -912,4 +936,122 @@ public class UserVmManagerImplTest { userVmManagerImpl.createVirtualMachine(deployVMCmd); } + + private List mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(int localVolumes, int nonLocalVolumes) { + List volumes = new ArrayList<>(); + for (int i=0; i< localVolumes + nonLocalVolumes; ++i) { + VolumeVO vol = Mockito.mock(VolumeVO.class); + long index = i + 1; + Mockito.when(vol.getDiskOfferingId()).thenReturn(index); + Mockito.when(vol.getPoolId()).thenReturn(index); + DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class); + Mockito.when(diskOfferingDao.findById(index)).thenReturn(diskOffering); + StoragePoolVO storagePool = Mockito.mock(StoragePoolVO.class); + Mockito.when(primaryDataStoreDao.findById(index)).thenReturn(storagePool); + if (i < localVolumes) { + if ((localVolumes + nonLocalVolumes) % 2 == 0) { + Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(true); + } else { + + Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(false); + Mockito.when(storagePool.isLocal()).thenReturn(true); + } + } else { + Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(false); + Mockito.when(storagePool.isLocal()).thenReturn(false); + } + volumes.add(vol); + } + return volumes; + } + + @Test + public void testIsAnyVmVolumeUsingLocalStorage() { + Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(1, 0))); + Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(2, 0))); + Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(1, 1))); + Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(0, 2))); + Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(0, 0))); + } + + private List mockVolumesForIsAllVmVolumesOnZoneWideStore(int nullPoolIdVolumes, int nullPoolVolumes, int zoneVolumes, int nonZoneVolumes) { + List volumes = new ArrayList<>(); + for (int i=0; i< nullPoolIdVolumes + nullPoolVolumes + zoneVolumes + nonZoneVolumes; ++i) { + VolumeVO vol = Mockito.mock(VolumeVO.class); + volumes.add(vol); + if (i < nullPoolIdVolumes) { + Mockito.when(vol.getPoolId()).thenReturn(null); + continue; + } + long index = i + 1; + Mockito.when(vol.getPoolId()).thenReturn(index); + if (i < nullPoolVolumes) { + Mockito.when(primaryDataStoreDao.findById(index)).thenReturn(null); + continue; + } + StoragePoolVO storagePool = Mockito.mock(StoragePoolVO.class); + Mockito.when(primaryDataStoreDao.findById(index)).thenReturn(storagePool); + if (i < zoneVolumes) { + Mockito.when(storagePool.getScope()).thenReturn(ScopeType.ZONE); + } else { + Mockito.when(storagePool.getScope()).thenReturn(ScopeType.CLUSTER); + } + } + return volumes; + } + + @Test + public void testIsAllVmVolumesOnZoneWideStoreCombinations() { + Assert.assertTrue(userVmManagerImpl.isAllVmVolumesOnZoneWideStore(mockVolumesForIsAllVmVolumesOnZoneWideStore(0, 0, 1, 0))); + Assert.assertTrue(userVmManagerImpl.isAllVmVolumesOnZoneWideStore(mockVolumesForIsAllVmVolumesOnZoneWideStore(0, 0, 2, 0))); + Assert.assertFalse(userVmManagerImpl.isAllVmVolumesOnZoneWideStore(mockVolumesForIsAllVmVolumesOnZoneWideStore(0, 0, 1, 1))); + Assert.assertFalse(userVmManagerImpl.isAllVmVolumesOnZoneWideStore(mockVolumesForIsAllVmVolumesOnZoneWideStore(0, 0, 0, 0))); + Assert.assertFalse(userVmManagerImpl.isAllVmVolumesOnZoneWideStore(mockVolumesForIsAllVmVolumesOnZoneWideStore(1, 0, 1, 1))); + Assert.assertFalse(userVmManagerImpl.isAllVmVolumesOnZoneWideStore(mockVolumesForIsAllVmVolumesOnZoneWideStore(0, 1, 1, 1))); + } + + private Pair mockObjectsForChooseVmMigrationDestinationUsingVolumePoolMapTest(boolean nullPlan, Host destinationHost) { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(vm.getServiceOfferingId()).thenReturn(1L); + Host host = Mockito.mock(Host.class); + Mockito.when(host.getId()).thenReturn(1L); + Mockito.when(hostDao.findById(1L)).thenReturn(Mockito.mock(HostVO.class)); + Mockito.when(virtualMachineManager.getMigrationDeployment(Mockito.any(VirtualMachine.class), + Mockito.any(Host.class), Mockito.nullable(Long.class), + Mockito.any(DeploymentPlanner.ExcludeList.class))) + .thenReturn(Mockito.mock(DataCenterDeployment.class)); + if (!nullPlan) { + try { + DeployDestination destination = Mockito.mock(DeployDestination.class); + Mockito.when(destination.getHost()).thenReturn(destinationHost); + Mockito.when(planningManager.planDeployment(Mockito.any(VirtualMachineProfile.class), + Mockito.any(DataCenterDeployment.class), Mockito.any(DeploymentPlanner.ExcludeList.class), + Mockito.nullable(DeploymentPlanner.class))) + .thenReturn(destination); + } catch (InsufficientServerCapacityException e) { + Assert.fail("Failed to mock DeployDestination"); + } + } + return new Pair<>(vm, host); + } + + @Test(expected = CloudRuntimeException.class) + public void testChooseVmMigrationDestinationUsingVolumePoolMapNullDestination() { + Pair pair = mockObjectsForChooseVmMigrationDestinationUsingVolumePoolMapTest(true, null); + userVmManagerImpl.chooseVmMigrationDestinationUsingVolumePoolMap(pair.first(), pair.second(), null); + } + + @Test(expected = CloudRuntimeException.class) + public void testChooseVmMigrationDestinationUsingVolumePoolMapNullHost() { + Pair pair = mockObjectsForChooseVmMigrationDestinationUsingVolumePoolMapTest(false, null); + userVmManagerImpl.chooseVmMigrationDestinationUsingVolumePoolMap(pair.first(), pair.second(), null); + } + + @Test + public void testChooseVmMigrationDestinationUsingVolumePoolMapValid() { + Host destinationHost = Mockito.mock(Host.class); + Pair pair = mockObjectsForChooseVmMigrationDestinationUsingVolumePoolMapTest(false, destinationHost); + Assert.assertEquals(destinationHost, userVmManagerImpl.chooseVmMigrationDestinationUsingVolumePoolMap(pair.first(), pair.second(), null)); + } } diff --git a/ui/src/views/compute/MigrateWizard.vue b/ui/src/views/compute/MigrateWizard.vue index 0819a5a3351..8aad50d4fc0 100644 --- a/ui/src/views/compute/MigrateWizard.vue +++ b/ui/src/views/compute/MigrateWizard.vue @@ -188,7 +188,8 @@ export default { } ], migrateWithStorage: false, - volumeToPoolSelection: [] + volumeToPoolSelection: [], + volumes: [] } }, created () { @@ -248,6 +249,7 @@ export default { handleSelectedHostChange (host) { if (host.id === -1) { this.migrateWithStorage = false + this.fetchVolumes() } this.selectedHost = host this.selectedVolumeForStoragePoolSelection = {} @@ -259,6 +261,31 @@ export default { handleVolumeToPoolChange (volumeToPool) { this.volumeToPoolSelection = volumeToPool }, + fetchVolumes () { + this.loading = true + this.volumes = [] + api('listVolumes', { + listAll: true, + virtualmachineid: this.resource.id + }).then(response => { + this.volumes = response.listvolumesresponse.volume + }).finally(() => { + this.loading = false + }) + }, + requiresStorageMigration () { + if (this.selectedHost.requiresStorageMotion || this.volumeToPoolSelection.length > 0) { + return true + } + if (this.selectedHost.id === -1 && this.volumes && this.volumes.length > 0) { + for (var volume of this.volumes) { + if (volume.storagetype === 'local') { + return true + } + } + } + return false + }, handleKeyboardSubmit () { if (this.selectedHost.id) { this.submitForm() @@ -271,7 +298,7 @@ export default { if (this.loading) return this.loading = true const migrateApi = this.isUserVm - ? (this.selectedHost.requiresStorageMotion || this.volumeToPoolSelection.length > 0) + ? this.requiresStorageMigration() ? 'migrateVirtualMachineWithVolume' : 'migrateVirtualMachine' : 'migrateSystemVm'