You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2026/04/30 11:58:27 UTC
(cloudstack) 01/03: Fix NPE on external/unmanaged instance import using custom offerings (#12884)
This is an automated email from the ASF dual-hosted git repository.
DaanHoogland pushed a commit to tag 4.22.0.1
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 89d915493fb83c5e0abe8db0fc1800bedc5be42b
Author: Fabricio Duarte <fa...@gmail.com>
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 | 254 +++++++++++++--------
.../cloudstack/vm/UnmanagedVMsManagerImplTest.java | 118 +++++++++-
2 files changed, 280 insertions(+), 92 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..18ba2d17179 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,135 @@ 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,
+ 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);
+ 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);
+ 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()));
- }
+ 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()));
+ }
- 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) {
+ 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());
+
+ CheckedReservation primaryStorageReservation = new CheckedReservation(owner, Resource.ResourceType.primary_storage, resourceLimitStorageTags,
+ CollectionUtils.isNotEmpty(resourceLimitStorageTags) ? diskProfile.getSize() : 0L, reservationDao, resourceLimitService);
+ reservations.add(primaryStorageReservation);
+
+ 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);
}
- if (!checkVolumeAnswer.getResult()) {
- cleanupFailedImportVM(userVm);
- throw new CloudRuntimeException("Disk not found or is invalid");
- }
- 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..544c37d57e9 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,11 @@ import java.util.List;
import java.util.Map;
import java.util.UUID;
+import com.cloud.exception.ResourceAllocationException;
import com.cloud.offering.DiskOffering;
import com.cloud.resourcelimit.CheckedReservation;
import com.cloud.vm.ImportVMTaskVO;
+import com.cloud.vm.VmDetailConstants;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.ResponseGenerator;
import org.apache.cloudstack.api.ResponseObject;
@@ -60,6 +62,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;
@@ -167,7 +170,6 @@ import com.cloud.vm.UserVmVO;
import com.cloud.vm.VMInstanceDetailVO;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine;
-import com.cloud.vm.VmDetailConstants;
import com.cloud.vm.dao.NicDao;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.VMInstanceDao;
@@ -178,7 +180,7 @@ public class UnmanagedVMsManagerImplTest {
@Spy
@InjectMocks
- private UnmanagedVMsManagerImpl unmanagedVMsManager = new UnmanagedVMsManagerImpl();
+ private UnmanagedVMsManagerImpl unmanagedVMsManager;
@Mock
private UserVmManager userVmManager;
@@ -255,11 +257,18 @@ public class UnmanagedVMsManagerImplTest {
ImportVMTaskVO importVMTaskVO;
@Mock
private VMInstanceDetailsDao vmInstanceDetailsDao;
-
@Mock
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 +395,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 +1335,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);
+ }
}