Allow altering only either CPU or memory during VM live scale (apache#8234)

* allow change only one parameter during live scale

* Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

Co-authored-by: sato03 <henriquesato2003@gmail.com>

* apply change method name

* Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com>

---------

Co-authored-by: Gabriel <gabriel.fernandes@scclouds.com.br>
Co-authored-by: sato03 <henriquesato2003@gmail.com>
Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com>

Author: 3 people

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -407,7 +407,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
     @Inject
     private VMTemplateZoneDao _templateZoneDao;
     @Inject
-    private TemplateDataStoreDao _templateStoreDao;
+    protected TemplateDataStoreDao _templateStoreDao;
     @Inject
     private DomainDao _domainDao;
     @Inject
@@ -1239,6 +1239,39 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE
         return userVm;
     }
 
+    /**
+     Updates the instance details map with the current values of the instance for the CPU speed, memory, and CPU number if they have not been specified.
+     @param details Map containing the instance details.
+     @param vmInstance The virtual machine instance.
+     @param newServiceOfferingId The ID of the new service offering.
+     */
+
+    protected void updateInstanceDetails (Map<String, String> details, VirtualMachine vmInstance, Long newServiceOfferingId) {
+        ServiceOfferingVO currentServiceOffering = serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId());
+        ServiceOfferingVO newServiceOffering = serviceOfferingDao.findById(newServiceOfferingId);
+        updateInstanceDetailsKeepCurrentValueIfNull(newServiceOffering.getSpeed(), details, VmDetailConstants.CPU_SPEED, currentServiceOffering.getSpeed());
+        updateInstanceDetailsKeepCurrentValueIfNull(newServiceOffering.getRamSize(), details, VmDetailConstants.MEMORY, currentServiceOffering.getRamSize());
+        updateInstanceDetailsKeepCurrentValueIfNull(newServiceOffering.getCpu(), details, VmDetailConstants.CPU_NUMBER, currentServiceOffering.getCpu());
+    }
+
+    /**
+     * Updates a specific instance detail with the current instance value if the new value is null.
+     *
+     * @param newValue         the new value to be set
+     * @param details          a map of instance details
+     * @param detailsConstant  the name of the detail constant to be updated
+     * @param currentValue     the current value of the detail constant
+     */
+
+    protected void updateInstanceDetailsKeepCurrentValueIfNull(Integer newValue, Map<String, String> details, String detailsConstant, Integer currentValue) {
+        if (newValue == null && details.get(detailsConstant) == null) {
+            String currentValueString = String.valueOf(currentValue);
+            logger.debug("{} was not specified, keeping the current value: {}.", detailsConstant, currentValueString);
+            details.put(detailsConstant, currentValueString);
+        }
+    }
+
+
     private void validateOfferingMaxResource(ServiceOfferingVO offering) {
         Integer maxCPUCores = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value() == 0 ? Integer.MAX_VALUE: ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value();
         if (offering.getCpu() > maxCPUCores) {
@@ -1904,7 +1937,11 @@ public UserVm upgradeVirtualMachine(ScaleVMCmd cmd) throws ResourceUnavailableEx
         }
         CallContext.current().setEventDetails("Vm Id: " + vm.getUuid());
 
-        boolean result = upgradeVirtualMachine(vmId, newServiceOfferingId, cmd.getDetails());
+        Map<String, String> cmdDetails = cmd.getDetails();
+
+        updateInstanceDetails(cmdDetails, vm, newServiceOfferingId);
+
+        boolean result = upgradeVirtualMachine(vmId, newServiceOfferingId, cmdDetails);
         if (result) {
             UserVmVO vmInstance = _vmDao.findById(vmId);
             if (vmInstance.getState().equals(State.Stopped)) {

server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

@@ -268,6 +268,9 @@ public class UserVmManagerImplTest {
     @Mock
     ServiceOfferingJoinDao serviceOfferingJoinDao;
 
+    @Mock
+    private VMInstanceVO vmInstanceMock;
+
     private static final long vmId = 1l;
     private static final long zoneId = 2L;
     private static final long accountId = 3L;
@@ -278,6 +281,8 @@ public class UserVmManagerImplTest {
 
     private Map<String, String> customParameters = new HashMap<>();
 
+    String[] detailsConstants = {VmDetailConstants.MEMORY, VmDetailConstants.CPU_NUMBER, VmDetailConstants.CPU_SPEED};
+
     private DiskOfferingVO smallerDisdkOffering = prepareDiskOffering(5l * GiB_TO_BYTES, 1l, 1L, 2L);
     private DiskOfferingVO largerDisdkOffering = prepareDiskOffering(10l * GiB_TO_BYTES, 2l, 10L, 20L);
 
@@ -294,6 +299,10 @@ public void beforeTest() {
         CallContext.register(callerUser, callerAccount);
 
         customParameters.put(VmDetailConstants.ROOT_DISK_SIZE, "123");
+        customParameters.put(VmDetailConstants.MEMORY, "2048");
+        customParameters.put(VmDetailConstants.CPU_NUMBER, "4");
+        customParameters.put(VmDetailConstants.CPU_SPEED, "1000");
+
         lenient().doNothing().when(resourceLimitMgr).incrementResourceCount(anyLong(), any(Resource.ResourceType.class));
         lenient().doNothing().when(resourceLimitMgr).decrementResourceCount(anyLong(), any(Resource.ResourceType.class), anyLong());
 
@@ -1444,4 +1453,71 @@ public void testRestoreVirtualMachineWithVMSnapshots() throws ResourceUnavailabl
 
         userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId);
     }
+
+    @Test
+    public void updateInstanceDetailsKeepCurrentValueIfNullTestDetailsConstantIsNotNullDoNothing() {
+        int currentValue = 123;
+
+        for (String detailsConstant : detailsConstants) {
+            userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(null, customParameters, detailsConstant, currentValue);
+        }
+
+        Assert.assertEquals(customParameters.get(VmDetailConstants.MEMORY), "2048");
+        Assert.assertEquals(customParameters.get(VmDetailConstants.CPU_NUMBER), "4");
+        Assert.assertEquals(customParameters.get(VmDetailConstants.CPU_SPEED), "1000");
+    }
+
+    @Test
+    public void updateInstanceDetailsKeepCurrentValueIfNullTestNewValueIsNotNullDoNothing() {
+        Map<String, String> details = new HashMap<>();
+        int currentValue = 123;
+
+        for (String detailsConstant : detailsConstants) {
+            userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(321, details, detailsConstant, currentValue);
+        }
+
+        Assert.assertNull(details.get(VmDetailConstants.MEMORY));
+        Assert.assertNull(details.get(VmDetailConstants.CPU_NUMBER));
+        Assert.assertNull(details.get(VmDetailConstants.CPU_SPEED));
+    }
+
+    @Test
+    public void updateInstanceDetailsKeepCurrentValueIfNullTestBothValuesAreNullKeepCurrentValue() {
+        Map<String, String> details = new HashMap<>();
+        int currentValue = 123;
+
+        for (String detailsConstant : detailsConstants) {
+            userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(null, details, detailsConstant, currentValue);
+        }
+
+        Assert.assertEquals(details.get(VmDetailConstants.MEMORY), String.valueOf(currentValue));
+        Assert.assertEquals(details.get(VmDetailConstants.CPU_NUMBER), String.valueOf(currentValue));
+        Assert.assertEquals(details.get(VmDetailConstants.CPU_SPEED),String.valueOf(currentValue));
+    }
+
+    @Test
+    public void updateInstanceDetailsKeepCurrentValueIfNullTestNeitherValueIsNullDoNothing() {
+        int currentValue = 123;
+
+        for (String detailsConstant : detailsConstants) {
+            userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(321, customParameters, detailsConstant, currentValue);
+        }
+
+        Assert.assertEquals(customParameters.get(VmDetailConstants.MEMORY), "2048");
+        Assert.assertEquals(customParameters.get(VmDetailConstants.CPU_NUMBER), "4");
+        Assert.assertEquals(customParameters.get(VmDetailConstants.CPU_SPEED),"1000");
+    }
+
+    @Test
+    public void updateInstanceDetailsTestAllConstantsAreUpdated() {
+        Mockito.doReturn(serviceOffering).when(_serviceOfferingDao).findById(Mockito.anyLong());
+        Mockito.doReturn(1L).when(vmInstanceMock).getId();
+        Mockito.doReturn(1L).when(vmInstanceMock).getServiceOfferingId();
+        Mockito.doReturn(serviceOffering).when(_serviceOfferingDao).findByIdIncludingRemoved(Mockito.anyLong(), Mockito.anyLong());
+        userVmManagerImpl.updateInstanceDetails(null, vmInstanceMock, 0l);
+
+        Mockito.verify(userVmManagerImpl).updateInstanceDetailsKeepCurrentValueIfNull(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.CPU_SPEED), Mockito.any());
+        Mockito.verify(userVmManagerImpl).updateInstanceDetailsKeepCurrentValueIfNull(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.MEMORY), Mockito.any());
+        Mockito.verify(userVmManagerImpl).updateInstanceDetailsKeepCurrentValueIfNull(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.CPU_NUMBER), Mockito.any());
+    }
 }