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)))