server: Failed to scale between Service Offerings with the same root disk size (#5095)

* Cover a case where resizing root disk failed; add isNotPossibleToResize method.

* remove format from resize validation

* Revert if-conditional changes that removed ImageFormat.ISO validation

* Add JUnit tests for VolumeApiServiceImpl.isNotPossibleToResize

* Fix checkstyle of test Class

* Use _templateDao.findByIdIncludingRemoved instead of _templateDao.findById

* Prevent null serviceOfferingView and Mock findByIdIncludingRemoved instead of findById

Author: GabrielBrascher

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

@@ -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;
@@ -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;
@@ -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) {
-                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();
 
@@ -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 {
@@ -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

server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java

@@ -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;
@@ -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;
@@ -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();
@@ -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);
+    }
 }