diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 6ee712bdba5..020cd5b26de 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 5050a628246..6598e509605 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 hwVersions, HypervisorType hypervisorType, String expected) { when(_hostDao.listOrderedHostsHypervisorVersionsInDatacenter(anyLong(), any(HypervisorType.class)))