From 83af44796673fcdae1d3c0ec50d276deebb40f86 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Tue, 14 Nov 2023 12:07:09 -0300 Subject: [PATCH 1/4] allow change only one parameter during live scale --- .../java/com/cloud/vm/UserVmManagerImpl.java | 42 +++++++++- .../com/cloud/vm/UserVmManagerImplTest.java | 76 +++++++++++++++++++ 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 0d3f047809a2..5438f9dccd6d 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -400,7 +400,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 @@ -1227,6 +1227,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 details, VirtualMachine vmInstance, Long newServiceOfferingId) { + ServiceOfferingVO currentServiceOffering = serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); + ServiceOfferingVO newServiceOffering = serviceOfferingDao.findById(newServiceOfferingId); + updateInstanceDetailsWithCurrentValue(newServiceOffering.getSpeed(), details, VmDetailConstants.CPU_SPEED, currentServiceOffering.getSpeed()); + updateInstanceDetailsWithCurrentValue(newServiceOffering.getRamSize(), details, VmDetailConstants.MEMORY, currentServiceOffering.getRamSize()); + updateInstanceDetailsWithCurrentValue(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 updateInstanceDetailsWithCurrentValue(Integer newValue, Map details, String detailsConstant, Integer currentValue) { + if (newValue == null && details.get(detailsConstant) == null) { + String currentValueString = String.valueOf(currentValue); + s_logger.debug(String.format("[%s] was not specified, keeping the current value: [%s].", 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) { @@ -1892,7 +1925,12 @@ public UserVm upgradeVirtualMachine(ScaleVMCmd cmd) throws ResourceUnavailableEx } CallContext.current().setEventDetails("Vm Id: " + vm.getUuid()); - boolean result = upgradeVirtualMachine(vmId, newServiceOfferingId, cmd.getDetails()); +// boolean result = upgradeVirtualMachine(vmId, newServiceOfferingId, cmd.getDetails()); + Map 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)) { diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 1b353c8626ae..64ff6954877e 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -263,6 +263,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; @@ -273,6 +276,8 @@ public class UserVmManagerImplTest { private Map 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); @@ -289,6 +294,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()); @@ -1395,4 +1404,71 @@ public void testRestoreVirtualMachineWithVMSnapshots() throws ResourceUnavailabl userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId); } + + @Test + public void updateInstanceDetailsWithCurrentValueTestDetailsConstantIsNotNullDoNothing() { + int currentValue = 123; + + for (String detailsConstant : detailsConstants) { + userVmManagerImpl.updateInstanceDetailsWithCurrentValue(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 updateInstanceDetailsWithCurrentValueTestNewValueIsNotNullDoNothing() { + Map details = new HashMap<>(); + int currentValue = 123; + + for (String detailsConstant : detailsConstants) { + userVmManagerImpl.updateInstanceDetailsWithCurrentValue(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 updateInstanceDetailsWithCurrentValueTestBothValuesAreNullKeepCurrentValue() { + Map details = new HashMap<>(); + int currentValue = 123; + + for (String detailsConstant : detailsConstants) { + userVmManagerImpl.updateInstanceDetailsWithCurrentValue(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 updateInstanceDetailsWithCurrentValueTestNeitherValueIsNullDoNothing() { + int currentValue = 123; + + for (String detailsConstant : detailsConstants) { + userVmManagerImpl.updateInstanceDetailsWithCurrentValue(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).updateInstanceDetailsWithCurrentValue(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.CPU_SPEED), Mockito.any()); + Mockito.verify(userVmManagerImpl).updateInstanceDetailsWithCurrentValue(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.MEMORY), Mockito.any()); + Mockito.verify(userVmManagerImpl).updateInstanceDetailsWithCurrentValue(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.CPU_NUMBER), Mockito.any()); + } } From e817a59c1be74ed02290093026680181490f4950 Mon Sep 17 00:00:00 2001 From: GaOrtiga <49285692+GaOrtiga@users.noreply.github.com> Date: Thu, 16 Nov 2023 08:56:13 -0300 Subject: [PATCH 2/4] Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java Co-authored-by: sato03 --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 5438f9dccd6d..6072b2b2ad4a 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -1925,7 +1925,6 @@ public UserVm upgradeVirtualMachine(ScaleVMCmd cmd) throws ResourceUnavailableEx } CallContext.current().setEventDetails("Vm Id: " + vm.getUuid()); -// boolean result = upgradeVirtualMachine(vmId, newServiceOfferingId, cmd.getDetails()); Map cmdDetails = cmd.getDetails(); updateInstanceDetails(cmdDetails, vm, newServiceOfferingId); From d05f25f21d9a0998db2f182496f3d827042284f4 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 17 Nov 2023 09:08:35 -0300 Subject: [PATCH 3/4] apply change method name --- .../java/com/cloud/vm/UserVmManagerImpl.java | 8 +++---- .../com/cloud/vm/UserVmManagerImplTest.java | 22 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 6072b2b2ad4a..dfe54b705f70 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -1237,9 +1237,9 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE protected void updateInstanceDetails (Map details, VirtualMachine vmInstance, Long newServiceOfferingId) { ServiceOfferingVO currentServiceOffering = serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); ServiceOfferingVO newServiceOffering = serviceOfferingDao.findById(newServiceOfferingId); - updateInstanceDetailsWithCurrentValue(newServiceOffering.getSpeed(), details, VmDetailConstants.CPU_SPEED, currentServiceOffering.getSpeed()); - updateInstanceDetailsWithCurrentValue(newServiceOffering.getRamSize(), details, VmDetailConstants.MEMORY, currentServiceOffering.getRamSize()); - updateInstanceDetailsWithCurrentValue(newServiceOffering.getCpu(), details, VmDetailConstants.CPU_NUMBER, currentServiceOffering.getCpu()); + 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()); } /** @@ -1251,7 +1251,7 @@ protected void updateInstanceDetails (Map details, VirtualMachin * @param currentValue the current value of the detail constant */ - protected void updateInstanceDetailsWithCurrentValue(Integer newValue, Map details, String detailsConstant, Integer currentValue) { + protected void updateInstanceDetailsKeepCurrentValueIfNull(Integer newValue, Map details, String detailsConstant, Integer currentValue) { if (newValue == null && details.get(detailsConstant) == null) { String currentValueString = String.valueOf(currentValue); s_logger.debug(String.format("[%s] was not specified, keeping the current value: [%s].", detailsConstant, currentValueString)); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 64ff6954877e..860d3a21bf3d 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -1406,11 +1406,11 @@ public void testRestoreVirtualMachineWithVMSnapshots() throws ResourceUnavailabl } @Test - public void updateInstanceDetailsWithCurrentValueTestDetailsConstantIsNotNullDoNothing() { + public void updateInstanceDetailsKeepCurrentValueIfNullTestDetailsConstantIsNotNullDoNothing() { int currentValue = 123; for (String detailsConstant : detailsConstants) { - userVmManagerImpl.updateInstanceDetailsWithCurrentValue(null, customParameters, detailsConstant, currentValue); + userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(null, customParameters, detailsConstant, currentValue); } Assert.assertEquals(customParameters.get(VmDetailConstants.MEMORY), "2048"); @@ -1419,12 +1419,12 @@ public void updateInstanceDetailsWithCurrentValueTestDetailsConstantIsNotNullDoN } @Test - public void updateInstanceDetailsWithCurrentValueTestNewValueIsNotNullDoNothing() { + public void updateInstanceDetailsKeepCurrentValueIfNullTestNewValueIsNotNullDoNothing() { Map details = new HashMap<>(); int currentValue = 123; for (String detailsConstant : detailsConstants) { - userVmManagerImpl.updateInstanceDetailsWithCurrentValue(321, details, detailsConstant, currentValue); + userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(321, details, detailsConstant, currentValue); } Assert.assertNull(details.get(VmDetailConstants.MEMORY)); @@ -1433,12 +1433,12 @@ public void updateInstanceDetailsWithCurrentValueTestNewValueIsNotNullDoNothing( } @Test - public void updateInstanceDetailsWithCurrentValueTestBothValuesAreNullKeepCurrentValue() { + public void updateInstanceDetailsKeepCurrentValueIfNullTestBothValuesAreNullKeepCurrentValue() { Map details = new HashMap<>(); int currentValue = 123; for (String detailsConstant : detailsConstants) { - userVmManagerImpl.updateInstanceDetailsWithCurrentValue(null, details, detailsConstant, currentValue); + userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(null, details, detailsConstant, currentValue); } Assert.assertEquals(details.get(VmDetailConstants.MEMORY), String.valueOf(currentValue)); @@ -1447,11 +1447,11 @@ public void updateInstanceDetailsWithCurrentValueTestBothValuesAreNullKeepCurren } @Test - public void updateInstanceDetailsWithCurrentValueTestNeitherValueIsNullDoNothing() { + public void updateInstanceDetailsKeepCurrentValueIfNullTestNeitherValueIsNullDoNothing() { int currentValue = 123; for (String detailsConstant : detailsConstants) { - userVmManagerImpl.updateInstanceDetailsWithCurrentValue(321, customParameters, detailsConstant, currentValue); + userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(321, customParameters, detailsConstant, currentValue); } Assert.assertEquals(customParameters.get(VmDetailConstants.MEMORY), "2048"); @@ -1467,8 +1467,8 @@ public void updateInstanceDetailsTestAllConstantsAreUpdated() { Mockito.doReturn(serviceOffering).when(_serviceOfferingDao).findByIdIncludingRemoved(Mockito.anyLong(), Mockito.anyLong()); userVmManagerImpl.updateInstanceDetails(null, vmInstanceMock, 0l); - Mockito.verify(userVmManagerImpl).updateInstanceDetailsWithCurrentValue(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.CPU_SPEED), Mockito.any()); - Mockito.verify(userVmManagerImpl).updateInstanceDetailsWithCurrentValue(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.MEMORY), Mockito.any()); - Mockito.verify(userVmManagerImpl).updateInstanceDetailsWithCurrentValue(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.CPU_NUMBER), Mockito.any()); + 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()); } } From 17f64f460fb085986492d8b5a4249031dc0fc37f Mon Sep 17 00:00:00 2001 From: GaOrtiga <49285692+GaOrtiga@users.noreply.github.com> Date: Fri, 9 Feb 2024 13:45:06 -0300 Subject: [PATCH 4/4] Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com> --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index dfe54b705f70..d21b61e8857c 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -1254,7 +1254,7 @@ protected void updateInstanceDetails (Map details, VirtualMachin protected void updateInstanceDetailsKeepCurrentValueIfNull(Integer newValue, Map details, String detailsConstant, Integer currentValue) { if (newValue == null && details.get(detailsConstant) == null) { String currentValueString = String.valueOf(currentValue); - s_logger.debug(String.format("[%s] was not specified, keeping the current value: [%s].", detailsConstant, currentValueString)); + logger.debug("{} was not specified, keeping the current value: {}.", detailsConstant, currentValueString); details.put(detailsConstant, currentValueString); } }