Skip to content

Failed to scale between Service Offerings with the same root disk size #5095

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
rohityadavcloud merged 7 commits into from
Jun 14, 2021
Merged

Failed to scale between Service Offerings with the same root disk size #5095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d96008e
Cover a case where resizing root disk failed; add isNotPossibleToResi…
GabrielBrascher Jun 9, 2021
63da01c
remove format from resize validation
GabrielBrascher Jun 10, 2021
19e19c6
Revert if-conditional changes that removed ImageFormat.ISO validation
GabrielBrascher Jun 11, 2021
442196e
Add JUnit tests for VolumeApiServiceImpl.isNotPossibleToResize
GabrielBrascher Jun 11, 2021
1a18be5
Fix checkstyle of test Class
GabrielBrascher Jun 11, 2021
06a1fe5
Use _templateDao.findByIdIncludingRemoved instead of _templateDao.fin…
GabrielBrascher Jun 11, 2021
c3b412c
Prevent null serviceOfferingView and Mock findByIdIncludingRemoved in…
GabrielBrascher Jun 11, 2021
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
45 changes: 32 additions & 13 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 @@ -31,6 +31,8 @@

import javax.inject.Inject;

import com.cloud.api.query.dao.ServiceOfferingJoinDao;
import com.cloud.api.query.vo.ServiceOfferingJoinVO;
import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd;
Expand Down Expand Up @@ -218,6 +220,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
@Inject
private ServiceOfferingDetailsDao _serviceOfferingDetailsDao;
@Inject
private ServiceOfferingJoinDao serviceOfferingJoinDao;
@Inject
private UserVmDao _userVmDao;
@Inject
private UserVmDetailsDao userVmDetailsDao;
Expand Down Expand Up @@ -920,20 +924,7 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
}

// if we are to use the existing disk offering
ImageFormat format = null;
if (newDiskOffering == null) {
Long templateId = volume.getTemplateId();
if (templateId != null) {
VMTemplateVO template = _templateDao.findById(templateId);
format = template.getFormat();
}

if (volume.getVolumeType().equals(Volume.Type.ROOT) && diskOffering.getDiskSize() > 0 && format != null && format != ImageFormat.ISO) {
Copy link
Member

@rohityadavcloud rohityadavcloud Jun 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pearl1594 can also regression test for multi-disk ova and deploy-as-is (with/without)

throw new InvalidParameterValueException(
"Failed to resize Root volume. The service offering of this Volume has been configured with a root disk size; "
+ "on such case a Root Volume can only be resized when changing to another Service Offering with a Root disk size. "
+ "For more details please check out the Official Resizing Volumes documentation.");
}
newSize = cmd.getSize();
newHypervisorSnapshotReserve = volume.getHypervisorSnapshotReserve();

Expand All @@ -944,6 +935,13 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
+ "customizable or it must be a root volume (if providing a disk offering, make sure it is different from the current disk offering).");
}

if (isNotPossibleToResize(volume, diskOffering)) {
throw new InvalidParameterValueException(
"Failed to resize Root volume. The service offering of this Volume has been configured with a root disk size; "
+ "on such case a Root Volume can only be resized when changing to another Service Offering with a Root disk size. "
+ "For more details please check out the Official Resizing Volumes documentation.");
}

// convert from bytes to GiB
newSize = newSize << 30;
} else {
Expand Down Expand Up @@ -1167,6 +1165,27 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
shrinkOk);
}

/**
* A volume should not be resized if it covers ALL the following scenarios: <br>
* 1 - Root volume <br>
* 2 - && Current Disk Offering enforces a root disk size (in this case one can resize only by changing the Service Offering)
*/
protected boolean isNotPossibleToResize(VolumeVO volume, DiskOfferingVO diskOffering) {
Long templateId = volume.getTemplateId();
ImageFormat format = null;
if (templateId != null) {
VMTemplateVO template = _templateDao.findByIdIncludingRemoved(templateId);
format = template.getFormat();
}
boolean isNotIso = format != null && format != ImageFormat.ISO;
boolean isRoot = Volume.Type.ROOT.equals(volume.getVolumeType());

ServiceOfferingJoinVO serviceOfferingView = serviceOfferingJoinDao.findById(diskOffering.getId());
boolean isOfferingEnforcingRootDiskSize = serviceOfferingView != null && serviceOfferingView.getRootDiskSize() > 0;

return isOfferingEnforcingRootDiskSize && isRoot && isNotIso;
}

private void checkIfVolumeIsRootAndVmIsRunning(Long newSize, VolumeVO volume, VMInstanceVO vmInstanceVO) {
if (!volume.getSize().equals(newSize) && volume.getVolumeType().equals(Volume.Type.ROOT) && !State.Stopped.equals(vmInstanceVO.getState())) {
throw new InvalidParameterValueException(String.format("Cannot resize ROOT volume [%s] when VM is not on Stopped State. VM %s is in state %s", volume.getName(), vmInstanceVO
Expand Down
56 changes: 54 additions & 2 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 @@ -35,6 +35,9 @@
import java.util.UUID;
import java.util.concurrent.ExecutionException;

import com.cloud.api.query.dao.ServiceOfferingJoinDao;
import com.cloud.api.query.vo.ServiceOfferingJoinVO;
import com.cloud.storage.dao.VMTemplateDao;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
Expand Down Expand Up @@ -78,7 +81,6 @@
import com.cloud.exception.ResourceAllocationException;
import com.cloud.host.dao.HostDao;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao;
import com.cloud.org.Grouping;
import com.cloud.serializer.GsonHelper;
import com.cloud.server.TaggedResourceService;
Expand Down Expand Up @@ -153,7 +155,9 @@ public class VolumeApiServiceImplTest {
@Mock
private StoragePoolTagsDao storagePoolTagsDao;
@Mock
private HypervisorCapabilitiesDao hypervisorCapabilitiesDao;
private VMTemplateDao templateDao;
@Mock
private ServiceOfferingJoinDao serviceOfferingJoinDao;

private DetachVolumeCmd detachCmd = new DetachVolumeCmd();
private Class<?> _detachCmdClass = detachCmd.getClass();
Expand Down Expand Up @@ -1079,4 +1083,52 @@ public void doesTargetStorageSupportDiskOfferingTestDiskOfferingTagsEqualsStorag

Assert.assertTrue(result);
}

@Test
public void isNotPossibleToResizeTestAllFormats() {
Storage.ImageFormat[] imageFormat = Storage.ImageFormat.values();
for (int i = 0; i < imageFormat.length - 1; i++) {
if (imageFormat[i] != Storage.ImageFormat.ISO) {
prepareAndRunTestOfIsNotPossibleToResize(Type.ROOT, 10l, imageFormat[i], true);
} else {
prepareAndRunTestOfIsNotPossibleToResize(Type.ROOT, 10l, imageFormat[i], false);
}
}
}

@Test
public void isNotPossibleToResizeTestAllTypes() {
Type[] types = Type.values();
for (int i = 0; i < types.length - 1; i++) {
if (types[i] != Type.ROOT) {
prepareAndRunTestOfIsNotPossibleToResize(types[i], 10l, Storage.ImageFormat.QCOW2, false);
} else {
prepareAndRunTestOfIsNotPossibleToResize(types[i], 10l, Storage.ImageFormat.QCOW2, true);
}
}
}

@Test
public void isNotPossibleToResizeTestNoRootDiskSize() {
prepareAndRunTestOfIsNotPossibleToResize(Type.ROOT, 0l, Storage.ImageFormat.QCOW2, false);
}

private void prepareAndRunTestOfIsNotPossibleToResize(Type volumeType, Long rootDisk, Storage.ImageFormat imageFormat, boolean expectedIsNotPossibleToResize) {
VolumeVO volume = Mockito.mock(VolumeVO.class);
when(volume.getVolumeType()).thenReturn(volumeType);

when(volume.getTemplateId()).thenReturn(1l);
DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class);

ServiceOfferingJoinVO serviceOfferingJoinVO = Mockito.mock(ServiceOfferingJoinVO.class);
when(serviceOfferingJoinVO.getRootDiskSize()).thenReturn(rootDisk);
when(serviceOfferingJoinDao.findById(Mockito.anyLong())).thenReturn(serviceOfferingJoinVO);

VMTemplateVO template = Mockito.mock(VMTemplateVO.class);
when(template.getFormat()).thenReturn(imageFormat);
when(templateDao.findByIdIncludingRemoved(Mockito.anyLong())).thenReturn(template);

boolean result = volumeApiServiceImpl.isNotPossibleToResize(volume, diskOffering);
Assert.assertEquals(expectedIsNotPossibleToResize, result);
}
}