This is an automated email from the ASF dual-hosted git repository.

dahn pushed a commit to branch 4.17
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/4.17 by this push:
     new b2f1965ccb Fix ScaleVM to consider resize volume in any type of 
service offering (#7359)
b2f1965ccb is described below

commit b2f1965ccbda001b7acc44c43b8670125fe43572
Author: Harikrishna <[email protected]>
AuthorDate: Wed Apr 5 19:51:24 2023 +0530

    Fix ScaleVM to consider resize volume in any type of service offering 
(#7359)
---
 .../com/cloud/storage/VolumeApiServiceImpl.java    | 77 +++++++++-------------
 .../cloud/storage/VolumeApiServiceImplTest.java    | 44 -------------
 2 files changed, 32 insertions(+), 89 deletions(-)

diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java 
b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
index 6ee712bdba..020cd5b26d 100644
--- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
@@ -26,6 +26,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
@@ -1741,12 +1742,6 @@ 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};
@@ -1853,19 +1848,6 @@ public class VolumeApiServiceImpl extends ManagerBase 
implements VolumeApiServic
         return a.equals(b);
     }
 
-    /**
-     * 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());
@@ -1965,30 +1947,8 @@ public class VolumeApiServiceImpl extends ManagerBase 
implements VolumeApiServic
             throw new InvalidParameterValueException("Requested disk offering 
has been removed.");
         }
 
-        if (newDiskOffering.getId() == existingDiskOffering.getId()) {
-            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.");
-        }
-
-        if 
(MatchStoragePoolTagsWithDiskOffering.valueIn(volume.getDataCenterId())) {
-            if 
(!doesNewDiskOfferingHasTagsAsOldDiskOffering(existingDiskOffering, 
newDiskOffering)) {
-                throw new 
InvalidParameterValueException(String.format("Selected disk offering %s does 
not have tags as in existing disk offering of volume %s", 
existingDiskOffering.getUuid(), volume.getUuid()));
-            }
-        }
-
-        Long instanceId = volume.getInstanceId();
-        VMInstanceVO vmInstanceVO = _vmInstanceDao.findById(instanceId);
-        if (volume.getVolumeType().equals(Volume.Type.ROOT)) {
-            ServiceOfferingVO serviceOffering = 
_serviceOfferingDao.findById(vmInstanceVO.getServiceOfferingId());
-            if (serviceOffering != null && 
serviceOffering.getDiskOfferingStrictness()) {
-                throw new InvalidParameterValueException(String.format("Cannot 
resize ROOT volume [%s] with new disk offering since existing disk offering is 
strictly assigned to the ROOT volume.", volume.getName()));
-            }
-        }
-
         
_configMgr.checkDiskOfferingAccess(_accountMgr.getActiveAccountById(volume.getAccountId()),
 newDiskOffering, _dcDao.findById(volume.getDataCenterId()));
+
         if (newDiskOffering.getDiskSize() > 0 && 
!newDiskOffering.isComputeOnly()) {
             newSize[0] = (Long) newDiskOffering.getDiskSize();
         } else if (newDiskOffering.isCustomized() && 
!newDiskOffering.isComputeOnly()) {
@@ -2022,13 +1982,40 @@ public class VolumeApiServiceImpl extends ManagerBase 
implements VolumeApiServic
             newHypervisorSnapshotReserve[0] = 
volume.getHypervisorSnapshotReserve() != null ? 
newDiskOffering.getHypervisorSnapshotReserve() : null;
         }
 
-        if (existingDiskOffering.getDiskSizeStrictness() && 
!(volume.getSize().equals(newSize[0]))) {
-            throw new InvalidParameterValueException(String.format("Resize 
volume for %s is not allowed since disk offering's size is fixed", 
volume.getName()));
-        }
+        Long instanceId = volume.getInstanceId();
+        VMInstanceVO vmInstanceVO = _vmInstanceDao.findById(instanceId);
+
+        checkIfVolumeCanResizeWithNewDiskOffering(volume, 
existingDiskOffering, newDiskOffering, newSize[0], vmInstanceVO);
         checkIfVolumeIsRootAndVmIsRunning(newSize[0], volume, vmInstanceVO);
 
     }
 
+    private void checkIfVolumeCanResizeWithNewDiskOffering(VolumeVO volume, 
DiskOfferingVO existingDiskOffering, DiskOfferingVO newDiskOffering, Long 
newSize, VMInstanceVO vmInstanceVO) {
+        if (existingDiskOffering.getId() == newDiskOffering.getId() &&
+                (!newDiskOffering.isCustomized() || 
(newDiskOffering.isCustomized() && Objects.equals(volume.getSize(), newSize << 
30)))) {
+            throw new InvalidParameterValueException(String.format("Volume %s 
is already having disk offering %s", volume, newDiskOffering.getUuid()));
+        }
+
+        if (existingDiskOffering.getDiskSizeStrictness() != 
newDiskOffering.getDiskSizeStrictness()) {
+            throw new InvalidParameterValueException("Disk offering size 
strictness does not match with new disk offering.");
+        }
+
+        if 
(MatchStoragePoolTagsWithDiskOffering.valueIn(volume.getDataCenterId()) && 
!doesNewDiskOfferingHasTagsAsOldDiskOffering(existingDiskOffering, 
newDiskOffering)) {
+            throw new InvalidParameterValueException(String.format("Selected 
disk offering %s does not have tags as in existing disk offering of volume %s", 
existingDiskOffering.getUuid(), volume.getUuid()));
+        }
+
+        if (volume.getVolumeType().equals(Volume.Type.ROOT)) {
+            ServiceOfferingVO serviceOffering = 
_serviceOfferingDao.findById(vmInstanceVO.getServiceOfferingId());
+            if (serviceOffering != null && 
serviceOffering.getDiskOfferingStrictness()) {
+                throw new InvalidParameterValueException(String.format("Cannot 
resize ROOT volume [%s] with new disk offering since existing disk offering is 
strictly assigned to the ROOT volume.", volume.getName()));
+            }
+        }
+
+        if (existingDiskOffering.getDiskSizeStrictness() && 
!(volume.getSize().equals(newSize))) {
+            throw new InvalidParameterValueException(String.format("Resize 
volume for %s is not allowed since disk offering's size is fixed", 
volume.getName()));
+        }
+    }
+
     private void validateVolumeResizeWithSize(VolumeVO volume, long 
currentSize, Long newSize, boolean shrinkOk) throws ResourceAllocationException 
{
 
         // if the caller is looking to change the size of the volume
diff --git 
a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java 
b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
index 5050a62824..6598e50960 100644
--- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
+++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
@@ -1154,50 +1154,6 @@ public class VolumeApiServiceImplTest {
         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);
-    }
-
     private void testBaseListOrderedHostsHypervisorVersionInDc(List<String> 
hwVersions, HypervisorType hypervisorType,
                                                                String 
expected) {
         
when(_hostDao.listOrderedHostsHypervisorVersionsInDatacenter(anyLong(), 
any(HypervisorType.class)))

Reply via email to