This is an automated email from the ASF dual-hosted git repository. dahn pushed a commit to tag 4.22.0.1 in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit ae184ed66951aa87c51ce39aaa011c413ab3a520 Author: Fabricio Duarte <[email protected]> AuthorDate: Fri Mar 27 01:55:16 2026 -0300 Fix NPE on external/unmanaged instance import using custom offerings (#12884) * Fix NPE on external/unmanaged instance import using custom offerings --- .../cloudstack/vm/UnmanagedVMsManagerImpl.java | 262 +++++++++++++-------- .../cloudstack/vm/UnmanagedVMsManagerImplTest.java | 123 +++++++++- 2 files changed, 288 insertions(+), 97 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 75f87931591..69e5a19eecb 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -1387,10 +1387,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { List<String> managedVms = new ArrayList<>(additionalNameFilters); managedVms.addAll(getHostsManagedVms(hosts)); - List<String> resourceLimitHostTags = resourceLimitService.getResourceLimitHostTags(serviceOffering, template); - try (CheckedReservation vmReservation = new CheckedReservation(owner, Resource.ResourceType.user_vm, resourceLimitHostTags, 1L, reservationDao, resourceLimitService); - CheckedReservation cpuReservation = new CheckedReservation(owner, Resource.ResourceType.cpu, resourceLimitHostTags, Long.valueOf(serviceOffering.getCpu()), reservationDao, resourceLimitService); - CheckedReservation memReservation = new CheckedReservation(owner, Resource.ResourceType.memory, resourceLimitHostTags, Long.valueOf(serviceOffering.getRamSize()), reservationDao, resourceLimitService)) { + try { ActionEventUtils.onStartedActionEvent(userId, owner.getId(), EventTypes.EVENT_VM_IMPORT, cmd.getEventDescription(), null, null, true, 0); @@ -1560,7 +1557,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { String hostName, Account caller, Account owner, long userId, ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap, Map<String, Long> nicNetworkMap, Map<String, Network.IpAddresses> nicIpAddressMap, - Map<String, String> details, Boolean migrateAllowed, List<String> managedVms, boolean forced) { + Map<String, String> details, Boolean migrateAllowed, List<String> managedVms, boolean forced) throws ResourceAllocationException { UserVm userVm = null; for (HostVO host : hosts) { HashMap<String, UnmanagedInstanceTO> unmanagedInstances = getUnmanagedInstancesForHost(host, instanceName, managedVms); @@ -1604,11 +1601,18 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { template.setGuestOSId(guestOSHypervisor.getGuestOsId()); } - userVm = importVirtualMachineInternal(unmanagedInstance, instanceName, zone, cluster, host, - template, displayName, hostName, CallContext.current().getCallingAccount(), owner, userId, - serviceOffering, dataDiskOfferingMap, - nicNetworkMap, nicIpAddressMap, - details, migrateAllowed, forced, true); + + List<Reserver> reservations = new ArrayList<>(); + try { + checkVmResourceLimitsForUnmanagedInstanceImport(owner, unmanagedInstance, serviceOffering, template, reservations); + userVm = importVirtualMachineInternal(unmanagedInstance, instanceName, zone, cluster, host, + template, displayName, hostName, CallContext.current().getCallingAccount(), owner, userId, + serviceOffering, dataDiskOfferingMap, + nicNetworkMap, nicIpAddressMap, + details, migrateAllowed, forced, true); + } finally { + ReservationHelper.closeAll(reservations); + } break; } if (userVm != null) { @@ -1618,6 +1622,36 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { return userVm; } + protected void checkVmResourceLimitsForUnmanagedInstanceImport(Account owner, UnmanagedInstanceTO unmanagedInstance, ServiceOfferingVO serviceOffering, VMTemplateVO template, List<Reserver> reservations) throws ResourceAllocationException { + // When importing an unmanaged instance, the amount of CPUs and memory is obtained from the hypervisor unless powered off + // and not using a dynamic offering, unlike the external VM import that always obtains it from the compute offering + Integer cpu = serviceOffering.getCpu(); + Integer memory = serviceOffering.getRamSize(); + + if (serviceOffering.isDynamic() || !UnmanagedInstanceTO.PowerState.PowerOff.equals(unmanagedInstance.getPowerState())) { + cpu = unmanagedInstance.getCpuCores(); + memory = unmanagedInstance.getMemory(); + } + + if (cpu == null || cpu == 0) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("CPU cores [%s] is not valid for importing VM [%s].", cpu, unmanagedInstance.getName())); + } + if (memory == null || memory == 0) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Memory [%s] is not valid for importing VM [%s].", memory, unmanagedInstance.getName())); + } + + List<String> resourceLimitHostTags = resourceLimitService.getResourceLimitHostTags(serviceOffering, template); + + CheckedReservation vmReservation = new CheckedReservation(owner, Resource.ResourceType.user_vm, resourceLimitHostTags, 1L, reservationDao, resourceLimitService); + reservations.add(vmReservation); + + CheckedReservation cpuReservation = new CheckedReservation(owner, Resource.ResourceType.cpu, resourceLimitHostTags, cpu.longValue(), reservationDao, resourceLimitService); + reservations.add(cpuReservation); + + CheckedReservation memReservation = new CheckedReservation(owner, Resource.ResourceType.memory, resourceLimitHostTags, memory.longValue(), reservationDao, resourceLimitService); + reservations.add(memReservation); + } + private Pair<UnmanagedInstanceTO, Boolean> getSourceVmwareUnmanagedInstance(String vcenter, String datacenterName, String username, String password, String clusterName, String sourceHostName, String sourceVM, ServiceOfferingVO serviceOffering) { @@ -1674,7 +1708,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { Account caller, Account owner, long userId, ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap, Map<String, Long> nicNetworkMap, Map<String, Network.IpAddresses> nicIpAddressMap, - Map<String, String> details, ImportVmCmd cmd, boolean forced) { + Map<String, String> details, ImportVmCmd cmd, boolean forced) throws ResourceAllocationException { Long existingVcenterId = cmd.getExistingVcenterId(); String vcenter = cmd.getVcenter(); String datacenterName = cmd.getDatacenterName(); @@ -1719,6 +1753,8 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { DataStoreTO temporaryConvertLocation = null; String ovfTemplateOnConvertLocation = null; ImportVmTask importVMTask = null; + List<Reserver> reservations = new ArrayList<>(); + try { HostVO convertHost = selectKVMHostForConversionInCluster(destinationCluster, convertInstanceHostId); HostVO importHost = selectKVMHostForImportingInCluster(destinationCluster, importInstanceHostId); @@ -1741,6 +1777,10 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { Pair<UnmanagedInstanceTO, Boolean> sourceInstanceDetails = getSourceVmwareUnmanagedInstance(vcenter, datacenterName, username, password, clusterName, sourceHostName, sourceVMName, serviceOffering); sourceVMwareInstance = sourceInstanceDetails.first(); isClonedInstance = sourceInstanceDetails.second(); + + // Ensure that the configured resource limits will not be exceeded before beginning the conversion process + checkVmResourceLimitsForUnmanagedInstanceImport(owner, sourceVMwareInstance, serviceOffering, template, reservations); + boolean isWindowsVm = sourceVMwareInstance.getOperatingSystem().toLowerCase().contains("windows"); if (isWindowsVm) { checkConversionSupportOnHost(convertHost, sourceVMName, true); @@ -1793,6 +1833,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { if (temporaryConvertLocation != null && StringUtils.isNotBlank(ovfTemplateOnConvertLocation)) { removeTemplate(temporaryConvertLocation, ovfTemplateOnConvertLocation); } + ReservationHelper.closeAll(reservations); } } @@ -2581,10 +2622,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { UserVm userVm = null; - List<String> resourceLimitHostTags = resourceLimitService.getResourceLimitHostTags(serviceOffering, template); - try (CheckedReservation vmReservation = new CheckedReservation(owner, Resource.ResourceType.user_vm, resourceLimitHostTags, 1L, reservationDao, resourceLimitService); - CheckedReservation cpuReservation = new CheckedReservation(owner, Resource.ResourceType.cpu, resourceLimitHostTags, Long.valueOf(serviceOffering.getCpu()), reservationDao, resourceLimitService); - CheckedReservation memReservation = new CheckedReservation(owner, Resource.ResourceType.memory, resourceLimitHostTags, Long.valueOf(serviceOffering.getRamSize()), reservationDao, resourceLimitService)) { + try { if (ImportSource.EXTERNAL == importSource) { String username = cmd.getUsername(); @@ -2647,6 +2685,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { List<Reserver> reservations = new ArrayList<>(); try { + checkVmResourceLimitsForExternalKvmVmImport(owner, serviceOffering, (VMTemplateVO) template, details, reservations); checkVolumeResourceLimitsForExternalKvmVmImport(owner, rootDisk, dataDisks, diskOffering, dataDiskOfferingMap, reservations); // Check NICs and supplied networks @@ -2811,98 +2850,137 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { profiles.add(nicProfile); networkNicMap.put(network.getUuid(), profiles); + List<Reserver> reservations = new ArrayList<>(); try { + checkVmResourceLimitsForExternalKvmVmImport(owner, serviceOffering, (VMTemplateVO) template, details, reservations); userVm = userVmManager.importVM(zone, null, template, null, displayName, owner, null, caller, true, null, owner.getAccountId(), userId, serviceOffering, null, hostName, Hypervisor.HypervisorType.KVM, allDetails, powerState, networkNicMap); - } catch (InsufficientCapacityException ice) { - logger.error(String.format("Failed to import vm name: %s", instanceName), ice); - throw new ServerApiException(ApiErrorCode.INSUFFICIENT_CAPACITY_ERROR, ice.getMessage()); - } - if (userVm == null) { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import vm name: %s", instanceName)); - } - DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId()); + if (userVm == null) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import vm name: %s", instanceName)); + } - List<Reserver> reservations = new ArrayList<>(); - List<String> resourceLimitStorageTags = resourceLimitService.getResourceLimitStorageTagsForResourceCountOperation(true, diskOffering); - try { - CheckedReservation volumeReservation = new CheckedReservation(owner, Resource.ResourceType.volume, resourceLimitStorageTags, - CollectionUtils.isNotEmpty(resourceLimitStorageTags) ? 1L : 0L, reservationDao, resourceLimitService); - reservations.add(volumeReservation); + try { + DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId()); + List<String> resourceLimitStorageTags = resourceLimitService.getResourceLimitStorageTagsForResourceCountOperation(true, diskOffering); + CheckedReservation volumeReservation = new CheckedReservation(owner, Resource.ResourceType.volume, resourceLimitStorageTags, + CollectionUtils.isNotEmpty(resourceLimitStorageTags) ? 1L : 0L, reservationDao, resourceLimitService); + reservations.add(volumeReservation); + + String rootVolumeName = String.format("ROOT-%s", userVm.getId()); + DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null, false); + + final VirtualMachineProfile profile = new VirtualMachineProfileImpl(userVm, template, serviceOffering, owner, null); + ServiceOfferingVO dummyOffering = serviceOfferingDao.findById(userVm.getId(), serviceOffering.getId()); + profile.setServiceOffering(dummyOffering); + DeploymentPlanner.ExcludeList excludeList = new DeploymentPlanner.ExcludeList(); + final DataCenterDeployment plan = new DataCenterDeployment(zone.getId(), null, null, hostId, poolId, null); + DeployDestination dest = null; + try { + dest = deploymentPlanningManager.planDeployment(profile, plan, excludeList, null); + } catch (Exception e) { + logger.warn("Import failed for Vm: {} while finding deployment destination", userVm, e); + cleanupFailedImportVM(userVm); + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Import failed for Vm: %s while finding deployment destination", userVm.getInstanceName())); + } + if(dest == null) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Import failed for Vm: %s. Suitable deployment destination not found", userVm.getInstanceName())); + } - String rootVolumeName = String.format("ROOT-%s", userVm.getId()); - DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null, false); + Map<Volume, StoragePool> storage = dest.getStorageForDisks(); + Volume volume = volumeDao.findById(diskProfile.getVolumeId()); + StoragePool storagePool = storage.get(volume); + CheckVolumeCommand checkVolumeCommand = new CheckVolumeCommand(); + checkVolumeCommand.setSrcFile(diskPath); + StorageFilerTO storageTO = new StorageFilerTO(storagePool); + checkVolumeCommand.setStorageFilerTO(storageTO); + Answer answer = agentManager.easySend(dest.getHost().getId(), checkVolumeCommand); + if (!(answer instanceof CheckVolumeAnswer)) { + cleanupFailedImportVM(userVm); + throw new CloudRuntimeException("Disk not found or is invalid"); + } + CheckVolumeAnswer checkVolumeAnswer = (CheckVolumeAnswer) answer; + try { + checkVolume(checkVolumeAnswer.getVolumeDetails()); + } catch (CloudRuntimeException e) { + cleanupFailedImportVM(userVm); + throw e; + } + if (!checkVolumeAnswer.getResult()) { + cleanupFailedImportVM(userVm); + throw new CloudRuntimeException("Disk not found or is invalid"); + } + diskProfile.setSize(checkVolumeAnswer.getSize()); - final VirtualMachineProfile profile = new VirtualMachineProfileImpl(userVm, template, serviceOffering, owner, null); - ServiceOfferingVO dummyOffering = serviceOfferingDao.findById(userVm.getId(), serviceOffering.getId()); - profile.setServiceOffering(dummyOffering); - DeploymentPlanner.ExcludeList excludeList = new DeploymentPlanner.ExcludeList(); - final DataCenterDeployment plan = new DataCenterDeployment(zone.getId(), null, null, hostId, poolId, null); - DeployDestination dest = null; - try { - dest = deploymentPlanningManager.planDeployment(profile, plan, excludeList, null); - } catch (Exception e) { - logger.warn("Import failed for Vm: {} while finding deployment destination", userVm, e); - cleanupFailedImportVM(userVm); - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Import failed for Vm: %s while finding deployment destination", userVm.getInstanceName())); - } - if(dest == null) { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Import failed for Vm: %s. Suitable deployment destination not found", userVm.getInstanceName())); - } + CheckedReservation primaryStorageReservation = new CheckedReservation(owner, Resource.ResourceType.primary_storage, resourceLimitStorageTags, + CollectionUtils.isNotEmpty(resourceLimitStorageTags) ? diskProfile.getSize() : 0L, reservationDao, resourceLimitService); + reservations.add(primaryStorageReservation); - Map<Volume, StoragePool> storage = dest.getStorageForDisks(); - Volume volume = volumeDao.findById(diskProfile.getVolumeId()); - StoragePool storagePool = storage.get(volume); - CheckVolumeCommand checkVolumeCommand = new CheckVolumeCommand(); - checkVolumeCommand.setSrcFile(diskPath); - StorageFilerTO storageTO = new StorageFilerTO(storagePool); - checkVolumeCommand.setStorageFilerTO(storageTO); - Answer answer = agentManager.easySend(dest.getHost().getId(), checkVolumeCommand); - if (!(answer instanceof CheckVolumeAnswer)) { - cleanupFailedImportVM(userVm); - throw new CloudRuntimeException("Disk not found or is invalid"); - } - CheckVolumeAnswer checkVolumeAnswer = (CheckVolumeAnswer) answer; - try { - checkVolume(checkVolumeAnswer.getVolumeDetails()); - } catch (CloudRuntimeException e) { - cleanupFailedImportVM(userVm); - throw e; - } - if (!checkVolumeAnswer.getResult()) { - cleanupFailedImportVM(userVm); - throw new CloudRuntimeException("Disk not found or is invalid"); + List<Pair<DiskProfile, StoragePool>> diskProfileStoragePoolList = new ArrayList<>(); + try { + long deviceId = 1L; + if(ImportSource.SHARED == importSource) { + diskProfileStoragePoolList.add(importKVMSharedDisk(userVm, diskOffering, Volume.Type.ROOT, + template, deviceId, poolId, diskPath, diskProfile)); + } else if(ImportSource.LOCAL == importSource) { + diskProfileStoragePoolList.add(importKVMLocalDisk(userVm, diskOffering, Volume.Type.ROOT, + template, deviceId, hostId, diskPath, diskProfile)); + } + } catch (Exception e) { + logger.error(String.format("Failed to import volumes while importing vm: %s", instanceName), e); + cleanupFailedImportVM(userVm); + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import volumes while importing vm: %s. %s", instanceName, StringUtils.defaultString(e.getMessage()))); + } + networkOrchestrationService.importNic(macAddress, 0, network, true, userVm, requestedIpPair, zone, true); + publishVMUsageUpdateResourceCount(userVm, dummyOffering, template); + return userVm; + + } catch (InsufficientCapacityException ice) { // This will be thrown by com.cloud.vm.UserVmService.importVM + logger.error(String.format("Failed to import vm name: %s", instanceName), ice); + throw new ServerApiException(ApiErrorCode.INSUFFICIENT_CAPACITY_ERROR, ice.getMessage()); + } catch (ResourceAllocationException e) { + cleanupFailedImportVM(userVm); + throw e; + } + } finally { + ReservationHelper.closeAll(reservations); } - diskProfile.setSize(checkVolumeAnswer.getSize()); + } - CheckedReservation primaryStorageReservation = new CheckedReservation(owner, Resource.ResourceType.primary_storage, resourceLimitStorageTags, - CollectionUtils.isNotEmpty(resourceLimitStorageTags) ? diskProfile.getSize() : 0L, reservationDao, resourceLimitService); - reservations.add(primaryStorageReservation); + protected void checkVmResourceLimitsForExternalKvmVmImport(Account owner, ServiceOfferingVO serviceOffering, VMTemplateVO template, Map<String, String> details, List<Reserver> reservations) throws ResourceAllocationException { + // When importing an external VM, the amount of CPUs and memory is always obtained from the compute offering, + // unlike the unmanaged instance import that obtains it from the hypervisor unless the VM is powered off and the offering is fixed + Integer cpu = serviceOffering.getCpu(); + Integer memory = serviceOffering.getRamSize(); - List<Pair<DiskProfile, StoragePool>> diskProfileStoragePoolList = new ArrayList<>(); - try { - long deviceId = 1L; - if(ImportSource.SHARED == importSource) { - diskProfileStoragePoolList.add(importKVMSharedDisk(userVm, diskOffering, Volume.Type.ROOT, - template, deviceId, poolId, diskPath, diskProfile)); - } else if(ImportSource.LOCAL == importSource) { - diskProfileStoragePoolList.add(importKVMLocalDisk(userVm, diskOffering, Volume.Type.ROOT, - template, deviceId, hostId, diskPath, diskProfile)); - } - } catch (Exception e) { - logger.error(String.format("Failed to import volumes while importing vm: %s", instanceName), e); - cleanupFailedImportVM(userVm); - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import volumes while importing vm: %s. %s", instanceName, StringUtils.defaultString(e.getMessage()))); + if (serviceOffering.isDynamic()) { + cpu = getDetailAsInteger(VmDetailConstants.CPU_NUMBER, details); + memory = getDetailAsInteger(VmDetailConstants.MEMORY, details); } - networkOrchestrationService.importNic(macAddress, 0, network, true, userVm, requestedIpPair, zone, true); - publishVMUsageUpdateResourceCount(userVm, dummyOffering, template); - return userVm; - } finally { - ReservationHelper.closeAll(reservations); + List<String> resourceLimitHostTags = resourceLimitService.getResourceLimitHostTags(serviceOffering, template); + + CheckedReservation vmReservation = new CheckedReservation(owner, Resource.ResourceType.user_vm, resourceLimitHostTags, 1L, reservationDao, resourceLimitService); + reservations.add(vmReservation); + + CheckedReservation cpuReservation = new CheckedReservation(owner, Resource.ResourceType.cpu, resourceLimitHostTags, cpu.longValue(), reservationDao, resourceLimitService); + reservations.add(cpuReservation); + + CheckedReservation memReservation = new CheckedReservation(owner, Resource.ResourceType.memory, resourceLimitHostTags, memory.longValue(), reservationDao, resourceLimitService); + reservations.add(memReservation); + } + + protected Integer getDetailAsInteger(String key, Map<String, String> details) { + String detail = details.get(key); + if (detail == null) { + throw new InvalidParameterValueException(String.format("Detail '%s' must be provided.", key)); + } + try { + return Integer.valueOf(detail); + } catch (NumberFormatException e) { + throw new InvalidParameterValueException(String.format("Please provide a valid integer value for detail '%s'.", key)); } } diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index 9935a228c5b..d27605269b1 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -38,9 +38,6 @@ import java.util.List; import java.util.Map; import java.util.UUID; -import com.cloud.offering.DiskOffering; -import com.cloud.resourcelimit.CheckedReservation; -import com.cloud.vm.ImportVMTaskVO; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ResponseGenerator; import org.apache.cloudstack.api.ResponseObject; @@ -60,6 +57,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.reservation.dao.ReservationDao; +import org.apache.cloudstack.resourcelimit.Reserver; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; @@ -113,6 +111,7 @@ import com.cloud.exception.InsufficientServerCapacityException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.PermissionDeniedException; +import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.UnsupportedServiceException; import com.cloud.host.Host; import com.cloud.host.HostVO; @@ -125,11 +124,13 @@ import com.cloud.network.Network; import com.cloud.network.NetworkModel; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; +import com.cloud.offering.DiskOffering; import com.cloud.offering.NetworkOffering; import com.cloud.offering.ServiceOffering; import com.cloud.org.Grouping; import com.cloud.resource.ResourceManager; import com.cloud.resource.ResourceState; +import com.cloud.resourcelimit.CheckedReservation; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.DataStoreRole; @@ -161,6 +162,7 @@ import com.cloud.utils.Pair; import com.cloud.utils.db.EntityManager; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.DiskProfile; +import com.cloud.vm.ImportVMTaskVO; import com.cloud.vm.NicProfile; import com.cloud.vm.UserVmManager; import com.cloud.vm.UserVmVO; @@ -176,9 +178,9 @@ import com.cloud.vm.dao.VMInstanceDetailsDao; @RunWith(MockitoJUnitRunner.class) public class UnmanagedVMsManagerImplTest { - @Spy @InjectMocks - private UnmanagedVMsManagerImpl unmanagedVMsManager = new UnmanagedVMsManagerImpl(); + @Spy + private UnmanagedVMsManagerImpl unmanagedVMsManager; @Mock private UserVmManager userVmManager; @@ -260,6 +262,14 @@ public class UnmanagedVMsManagerImplTest { private ConfigKey<Boolean> configKeyMockParamsAllowed; @Mock private ConfigKey<String> configKeyMockParamsAllowedList; + @Mock + private Account accountMock; + @Mock + private ServiceOfferingVO serviceOfferingMock; + @Mock + private VMTemplateVO templateMock; + @Mock + private UnmanagedInstanceTO unmanagedInstanceMock; private static final long virtualMachineId = 1L; @@ -386,6 +396,11 @@ public class UnmanagedVMsManagerImplTest { when(vmDao.findById(virtualMachineId)).thenReturn(virtualMachine); when(virtualMachine.getState()).thenReturn(VirtualMachine.State.Running); + + when(unmanagedInstanceMock.getCpuCores()).thenReturn(8); + when(unmanagedInstanceMock.getMemory()).thenReturn(4096); + when(serviceOfferingMock.getCpu()).thenReturn(4); + when(serviceOfferingMock.getRamSize()).thenReturn(2048); } @NotNull @@ -1321,4 +1336,102 @@ public class UnmanagedVMsManagerImplTest { Assert.assertFalse(params.containsKey(VmDetailConstants.CPU_SPEED)); Assert.assertFalse(params.containsKey(VmDetailConstants.MEMORY)); } + + @Test + public void checkVmResourceLimitsForUnmanagedInstanceImportTestUsesInformationFromHypervisorWhenOfferingIsDynamic() throws Exception { + when(serviceOfferingMock.isDynamic()).thenReturn(true); + List<Reserver> reservations = new ArrayList<>(); + + try (MockedConstruction<CheckedReservation> mockedConstruction = Mockito.mockConstruction(CheckedReservation.class)) { + unmanagedVMsManager.checkVmResourceLimitsForUnmanagedInstanceImport(accountMock, unmanagedInstanceMock, serviceOfferingMock, templateMock, reservations); + + Assert.assertEquals(3, mockedConstruction.constructed().size()); + Assert.assertEquals(3, reservations.size()); + verify(unmanagedInstanceMock).getCpuCores(); + verify(unmanagedInstanceMock).getMemory(); + } + } + + @Test + public void checkVmResourceLimitsForUnmanagedInstanceImportTestUsesInformationFromHypervisorWhenVmIsPoweredOn() throws Exception { + when(unmanagedInstanceMock.getPowerState()).thenReturn(UnmanagedInstanceTO.PowerState.PowerOn); + when(serviceOfferingMock.isDynamic()).thenReturn(false); + List<Reserver> reservations = new ArrayList<>(); + + try (MockedConstruction<CheckedReservation> mockedConstruction = Mockito.mockConstruction(CheckedReservation.class)) { + unmanagedVMsManager.checkVmResourceLimitsForUnmanagedInstanceImport(accountMock, unmanagedInstanceMock, serviceOfferingMock, templateMock, reservations); + + Assert.assertEquals(3, mockedConstruction.constructed().size()); + Assert.assertEquals(3, reservations.size()); + verify(unmanagedInstanceMock).getCpuCores(); + verify(unmanagedInstanceMock).getMemory(); + } + } + + @Test + public void checkVmResourceLimitsForUnmanagedInstanceImportTestUsesInformationFromOfferingWhenOfferingIsNotDynamicAndVmIsPoweredOff() throws Exception { + when(unmanagedInstanceMock.getPowerState()).thenReturn(UnmanagedInstanceTO.PowerState.PowerOff); + when(serviceOfferingMock.isDynamic()).thenReturn(false); + List<Reserver> reservations = new ArrayList<>(); + + try (MockedConstruction<CheckedReservation> mockedConstruction = Mockito.mockConstruction(CheckedReservation.class)) { + unmanagedVMsManager.checkVmResourceLimitsForUnmanagedInstanceImport(accountMock, unmanagedInstanceMock, serviceOfferingMock, templateMock, reservations); + + Assert.assertEquals(3, mockedConstruction.constructed().size()); + Assert.assertEquals(3, reservations.size()); + verify(serviceOfferingMock).getCpu(); + verify(serviceOfferingMock).getRamSize(); + verify(unmanagedInstanceMock, Mockito.never()).getCpuCores(); + verify(unmanagedInstanceMock, Mockito.never()).getMemory(); + } + } + + @Test + public void checkVmResourceLimitsForExternalKvmVmImportTestUsesInformationFromOfferingWhenOfferingIsNotDynamic() throws ResourceAllocationException { + when(serviceOfferingMock.isDynamic()).thenReturn(false); + Map<String, String> details = new HashMap<>(); + List<Reserver> reservations = new ArrayList<>(); + + try (MockedConstruction<CheckedReservation> mockedConstruction = Mockito.mockConstruction(CheckedReservation.class)) { + unmanagedVMsManager.checkVmResourceLimitsForExternalKvmVmImport(accountMock, serviceOfferingMock, templateMock, details, reservations); + + Assert.assertEquals(3, mockedConstruction.constructed().size()); + Assert.assertEquals(3, reservations.size()); + verify(serviceOfferingMock).getCpu(); + verify(serviceOfferingMock).getRamSize(); + verify(unmanagedVMsManager, Mockito.never()).getDetailAsInteger(VmDetailConstants.CPU_NUMBER, details); + verify(unmanagedVMsManager, Mockito.never()).getDetailAsInteger(VmDetailConstants.MEMORY, details); + } + } + + @Test + public void checkVmResourceLimitsForExternalKvmVmImportTestUsesInformationFromDetailsWhenOfferingIsDynamic() throws ResourceAllocationException { + when(serviceOfferingMock.isDynamic()).thenReturn(true); + Map<String, String> details = new HashMap<>(); + details.put(VmDetailConstants.CPU_NUMBER, "8"); + details.put(VmDetailConstants.MEMORY, "4096"); + List<Reserver> reservations = new ArrayList<>(); + + try (MockedConstruction<CheckedReservation> mockedConstruction = Mockito.mockConstruction(CheckedReservation.class)) { + unmanagedVMsManager.checkVmResourceLimitsForExternalKvmVmImport(accountMock, serviceOfferingMock, templateMock, details, reservations); + + Assert.assertEquals(3, mockedConstruction.constructed().size()); + Assert.assertEquals(3, reservations.size()); + verify(unmanagedVMsManager).getDetailAsInteger(VmDetailConstants.CPU_NUMBER, details); + verify(unmanagedVMsManager).getDetailAsInteger(VmDetailConstants.MEMORY, details); + } + } + + @Test(expected = InvalidParameterValueException.class) + public void getDetailAsIntegerTestThrowsInvalidParameterValueExceptionWhenDetailIsNull() { + Map<String, String> details = new HashMap<>(); + unmanagedVMsManager.getDetailAsInteger("non-existent", details); + } + + @Test(expected = InvalidParameterValueException.class) + public void getDetailAsIntegerTestThrowsInvalidParameterValueExceptionWhenValueIsInvalid() { + Map<String, String> details = new HashMap<>(); + details.put("key", "not-a-number"); + unmanagedVMsManager.getDetailAsInteger("key", details); + } }
