Skip to content

Fix ScaleVM to consider resize volume in any type of service offering #7359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 5, 2023
Merged

Fix ScaleVM to consider resize volume in any type of service offering #7359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b5cafa
Fix ScaleVM to consider resize volume in any type of compute offering
harikrishna-patnala Mar 23, 2023
99a54e9
Reformat code
harikrishna-patnala Mar 24, 2023
e1c8b79
Addressed review comment
harikrishna-patnala Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 32 additions & 45 deletions server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
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;
Expand Down Expand Up @@ -1741,12 +1742,6 @@ private Volume changeDiskOfferingForVolumeInternal(VolumeVO volume, Long newDisk
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};
Expand Down Expand Up @@ -1853,19 +1848,6 @@ private boolean compareEqualsIncludingNullOrZero(Long a, Long b) {
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());
Expand Down Expand Up @@ -1965,30 +1947,8 @@ private void validateVolumeResizeWithNewDiskOfferingAndLoad(VolumeVO volume, Dis
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()) {
Expand Down Expand Up @@ -2022,13 +1982,40 @@ private void validateVolumeResizeWithNewDiskOfferingAndLoad(VolumeVO volume, Dis
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
Expand Down
44 changes: 0 additions & 44 deletions server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1154,50 +1154,6 @@ private void prepareAndRunTestOfIsNotPossibleToResize(Type volumeType, Long root
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)))
Expand Down