From 50b2dc2789c9f443c5e9370dce89e3884739a20b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Beims=20Br=C3=A4scher?= Date: Fri, 15 Apr 2022 16:58:31 +0200 Subject: [PATCH] server: Fix #6263 Cannot scale VM with custom offering (#6267) * When scaling with custom offering, which changes only CPU/Memory and keeps same disk offering an exception is thrown. This commit fixes such cases by checking if the operation is happening on a custom service offering. * Improve the unit tests that cover null objects. --- .../cloud/storage/VolumeApiServiceImpl.java | 23 +++++++++- .../storage/VolumeApiServiceImplTest.java | 44 +++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 2c741a205c0..fb05e678948 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1705,6 +1705,12 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic boolean volumeMigrateRequired = false; boolean volumeResizeRequired = false; + // Skip the Disk offering change on Volume if Disk Offering is the same and is linked to a custom Service Offering + if(isNewDiskOfferingTheSameAndCustomServiceOffering(existingDiskOffering, newDiskOffering)) { + s_logger.debug(String.format("Scaling CPU and/or Memory of VM with custom service offering. New disk offering stills the same. Skipping the Disk offering change on Volume %s.", volume.getUuid())); + return volume; + } + // VALIDATIONS Long[] updateNewSize = {newSize}; Long[] updateNewMinIops = {newMinIops}; @@ -1796,6 +1802,19 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic return volume; } + /** + * Returns true if the new disk offering is the same than current offering, and the respective Service offering is a custom (constraint or unconstraint) offering. + */ + protected boolean isNewDiskOfferingTheSameAndCustomServiceOffering(DiskOfferingVO existingDiskOffering, DiskOfferingVO newDiskOffering) { + if (newDiskOffering.getId() == existingDiskOffering.getId()) { + ServiceOfferingVO serviceOffering = _serviceOfferingDao.findServiceOfferingByComputeOnlyDiskOffering(newDiskOffering.getId()); + if (serviceOffering != null && serviceOffering.isCustomized()) { + return true; + } + } + return false; + } + private VolumeVO resizeVolumeInternal(VolumeVO volume, DiskOfferingVO newDiskOffering, Long currentSize, Long newSize, Long newMinIops, Long newMaxIops, Integer newHypervisorSnapshotReserve, boolean shrinkOk) throws ResourceAllocationException { UserVmVO userVm = _userVmDao.findById(volume.getInstanceId()); HypervisorType hypervisorType = _volsDao.getHypervisorType(volume.getId()); @@ -1896,11 +1915,11 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } if (newDiskOffering.getId() == existingDiskOffering.getId()) { - throw new InvalidParameterValueException(String.format("Volume %s already have the new disk offering %s provided", volume.getUuid(), existingDiskOffering.getUuid())); + throw new InvalidParameterValueException(String.format("Volume %s already have the new disk offering %s provided.", volume.getUuid(), existingDiskOffering.getUuid())); } if (existingDiskOffering.getDiskSizeStrictness() != newDiskOffering.getDiskSizeStrictness()) { - throw new InvalidParameterValueException("Disk offering size strictness does not match with new disk offering"); + throw new InvalidParameterValueException("Disk offering size strictness does not match with new disk offering."); } if (MatchStoragePoolTagsWithDiskOffering.valueIn(volume.getDataCenterId())) { diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 7c741832de7..4320e5f6a88 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -1156,4 +1156,48 @@ public class VolumeApiServiceImplTest { boolean result = volumeApiServiceImpl.isNotPossibleToResize(volume, diskOffering); Assert.assertEquals(expectedIsNotPossibleToResize, result); } + + @Test + public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestDifferentOfferings() { + prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 2l, false, true, false); + } + + @Test + public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestDifferentOfferingsCustom() { + prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 2l, true, true, false); + } + + @Test + public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestSameOfferingsCustom() { + prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 1l, true, true, true); + } + + @Test + public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestSameOfferingsNotCustom() { + prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 1l, false, true, false); + } + + @Test + public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestDifferentOfferingsAndNullOffering() { + prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 2l, true, false, false); + } + @Test + public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestSameOfferingsNullOffering() { + prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 1l, false, false, false); + } + + private void prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(long existingDiskOfferingId, long newDiskOfferingId, boolean isCustomized, + boolean isNotNullServiceOffering, boolean expectedResult) { + DiskOfferingVO existingDiskOffering = Mockito.mock(DiskOfferingVO.class); + when(existingDiskOffering.getId()).thenReturn(existingDiskOfferingId); + DiskOfferingVO newDiskOffering = Mockito.mock(DiskOfferingVO.class); + when(newDiskOffering.getId()).thenReturn(newDiskOfferingId); + if(isNotNullServiceOffering) { + ServiceOfferingVO serviceOfferingVO = Mockito.mock(ServiceOfferingVO.class); + when(serviceOfferingVO.isCustomized()).thenReturn(isCustomized); + when(serviceOfferingDao.findServiceOfferingByComputeOnlyDiskOffering(anyLong())).thenReturn(serviceOfferingVO); + } + boolean result = volumeApiServiceImpl.isNewDiskOfferingTheSameAndCustomServiceOffering(existingDiskOffering, newDiskOffering); + Assert.assertEquals(expectedResult, result); + } }