diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 687e432f166b..7be9408308b6 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2311,14 +2311,16 @@ private boolean compareEqualsIncludingNullOrZero(Long a, Long b) { return a.equals(b); } - private VolumeVO resizeVolumeInternal(VolumeVO volume, DiskOfferingVO newDiskOffering, Long currentSize, Long newSize, Long newMinIops, Long newMaxIops, Integer newHypervisorSnapshotReserve, boolean shrinkOk) throws ResourceAllocationException { + 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()); if (userVm != null) { - if (volume.getVolumeType().equals(Volume.Type.ROOT) && userVm.getPowerState() != VirtualMachine.PowerState.PowerOff && hypervisorType == HypervisorType.VMware) { - logger.error(" For ROOT volume resize VM should be in Power Off state."); - throw new InvalidParameterValueException("VM current state is : " + userVm.getPowerState() + ". But VM should be in " + VirtualMachine.PowerState.PowerOff + " state."); + if (Volume.Type.ROOT.equals(volume.getVolumeType()) + && !State.Stopped.equals(userVm.getState()) + && HypervisorType.VMware.equals(hypervisorType)) { + logger.error("For ROOT volume resize VM should be in Stopped state."); + throw new InvalidParameterValueException("The current VM state is '" + userVm.getState() + "'. But the VM should be in " + State.Stopped + " state."); } // serialize VM operation AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext(); @@ -2372,7 +2374,7 @@ private VolumeVO resizeVolumeInternal(VolumeVO volume, DiskOfferingVO newDiskOff shrinkOk); } - private void validateVolumeReadyStateAndHypervisorChecks(VolumeVO volume, long currentSize, Long newSize) { + void validateVolumeReadyStateAndHypervisorChecks(VolumeVO volume, long currentSize, Long newSize) { // checking if there are any ongoing snapshots on the volume which is to be resized List ongoingSnapshots = _snapshotDao.listByStatus(volume.getId(), Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp); if (ongoingSnapshots.size() > 0) { @@ -2396,9 +2398,11 @@ private void validateVolumeReadyStateAndHypervisorChecks(VolumeVO volume, long c UserVmVO userVm = _userVmDao.findById(volume.getInstanceId()); if (userVm != null) { - if (volume.getVolumeType().equals(Volume.Type.ROOT) && userVm.getPowerState() != VirtualMachine.PowerState.PowerOff && hypervisorType == HypervisorType.VMware) { - logger.error(" For ROOT volume resize VM should be in Power Off state."); - throw new InvalidParameterValueException("VM current state is : " + userVm.getPowerState() + ". But VM should be in " + VirtualMachine.PowerState.PowerOff + " state."); + if (Volume.Type.ROOT.equals(volume.getVolumeType()) + && !State.Stopped.equals(userVm.getState()) + && HypervisorType.VMware.equals(hypervisorType)) { + logger.error("For ROOT volume resize VM should be in Stopped state."); + throw new InvalidParameterValueException("The current VM state is '" + userVm.getState() + "'. But VM should be in " + State.Stopped + " state."); } } } @@ -2415,7 +2419,7 @@ private void setNewIopsLimits(VolumeVO volume, DiskOfferingVO newDiskOffering, L } } - private void validateVolumeResizeWithNewDiskOfferingAndLoad(VolumeVO volume, DiskOfferingVO existingDiskOffering, DiskOfferingVO newDiskOffering, Long[] newSize, Long[] newMinIops, Long[] newMaxIops, Integer[] newHypervisorSnapshotReserve) { + void validateVolumeResizeWithNewDiskOfferingAndLoad(VolumeVO volume, DiskOfferingVO existingDiskOffering, DiskOfferingVO newDiskOffering, Long[] newSize, Long[] newMinIops, Long[] newMaxIops, Integer[] newHypervisorSnapshotReserve) { if (newDiskOffering.getRemoved() != null) { throw new InvalidParameterValueException("Requested disk offering has been removed."); } diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 77e321ab8591..b806eb4afe5c 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -33,6 +33,7 @@ import static org.mockito.Mockito.when; import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -40,15 +41,6 @@ import java.util.UUID; import java.util.concurrent.ExecutionException; -import com.cloud.event.EventTypes; -import com.cloud.event.UsageEventUtils; -import com.cloud.host.HostVO; -import com.cloud.resourcelimit.CheckedReservation; -import com.cloud.service.ServiceOfferingVO; -import com.cloud.service.dao.ServiceOfferingDao; -import com.cloud.vm.snapshot.VMSnapshot; -import com.cloud.vm.snapshot.VMSnapshotDetailsVO; -import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.user.volume.CheckAndRepairVolumeCmd; @@ -70,6 +62,7 @@ import org.apache.cloudstack.framework.async.AsyncCallFuture; import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext; import org.apache.cloudstack.framework.jobs.AsyncJobManager; +import org.apache.cloudstack.framework.jobs.Outcome; import org.apache.cloudstack.framework.jobs.dao.AsyncJobJoinMapDao; import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; import org.apache.cloudstack.resourcedetail.dao.SnapshotPolicyDetailsDao; @@ -107,17 +100,23 @@ import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; import com.cloud.dc.dao.HostPodDao; +import com.cloud.event.EventTypes; +import com.cloud.event.UsageEventUtils; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceAllocationException; +import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.org.Grouping; import com.cloud.projects.Project; import com.cloud.projects.ProjectManager; +import com.cloud.resourcelimit.CheckedReservation; import com.cloud.serializer.GsonHelper; import com.cloud.server.ManagementService; import com.cloud.server.TaggedResourceService; +import com.cloud.service.ServiceOfferingVO; +import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.Storage.ProvisioningType; import com.cloud.storage.Volume.Type; import com.cloud.storage.dao.DiskOfferingDao; @@ -145,8 +144,11 @@ import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; +import com.cloud.vm.snapshot.VMSnapshot; +import com.cloud.vm.snapshot.VMSnapshotDetailsVO; import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; +import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao; @RunWith(MockitoJUnitRunner.class) public class VolumeApiServiceImplTest { @@ -2320,4 +2322,186 @@ private List generateVmSnapshotVoList(VMSnapshot.Type t1, VMSnapsh Mockito.doReturn(1L).when(mock2).getId(); return List.of(mock1, mock2); } + +// ===================================================================== +// VMware ROOT-volume resize / offering-change: VM power_state-lag tests +// +// Both private guards that protect VMware ROOT-volume resize operations +// are covered here: +// • validateVolumeReadyStateAndHypervisorChecks (called by changeDiskOfferingForVolumeInternal) +// • resizeVolumeInternal (called by both resize and change-offering flows) +// +// The "power_state lag" scenario: when a VMware VM is stopped via CloudStack +// the VirtualMachine.State transitions to Stopped immediately, but the +// VMware-side VirtualMachine.PowerState (polled from ESX) may still read +// PoweredOn for some seconds. The production code must use only the +// authoritative CloudStack State field and must NOT additionally gate on +// PowerState. +// ===================================================================== + + /** + * Positive – validateVolumeReadyStateAndHypervisorChecks: + * The guard must allow a VMware ROOT-volume resize when the CloudStack VM + * state is {@code Stopped}, regardless of the VMware power_state value. + * getPowerState() is intentionally left un-stubbed so that any invocation + * of that method would cause a Mockito strict-stubbing error and surface a + * regression. + */ + @Test + public void testValidateVolumeReadyStateVMware_VMStopped_PowerStateLag_ShouldPass() + throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + + long volumeId = 200L; + long vmId = 201L; + + VolumeVO volume = Mockito.mock(VolumeVO.class); + when(volume.getId()).thenReturn(volumeId); + when(volume.getInstanceId()).thenReturn(vmId); + when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(volume.getState()).thenReturn(Volume.State.Ready); + + // snapshotDaoMock returns an empty list by default (Mockito default behaviour) + when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware); + + UserVmVO stoppedVm = Mockito.mock(UserVmVO.class); + // Authoritative cloud state: Stopped. + // getPowerState() is NOT stubbed – power_state lag scenario. + when(stoppedVm.getState()).thenReturn(State.Stopped); + when(userVmDaoMock.findById(vmId)).thenReturn(stoppedVm); + + long currentSizeBytes = 10L << 30; // 10 GiB + Long newSizeBytes = 20L << 30; // 20 GiB (grow; VMware prohibits shrink) + + // Must complete without throwing any exception + volumeApiServiceImpl.validateVolumeReadyStateAndHypervisorChecks(volume, currentSizeBytes, newSizeBytes); + } + + /** + * Negative – validateVolumeReadyStateAndHypervisorChecks: + * The guard must reject a VMware ROOT-volume resize when the CloudStack VM + * state is {@code Running}. + */ + @Test(expected = InvalidParameterValueException.class) + public void testValidateVolumeReadyStateVMware_VMRunning_ShouldThrowInvalidParameterValueException() + throws NoSuchMethodException, IllegalAccessException { + + long volumeId = 200L; + long vmId = 201L; + + VolumeVO volume = Mockito.mock(VolumeVO.class); + when(volume.getId()).thenReturn(volumeId); + when(volume.getInstanceId()).thenReturn(vmId); + when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(volume.getState()).thenReturn(Volume.State.Ready); + + when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware); + + UserVmVO runningVm = Mockito.mock(UserVmVO.class); + when(runningVm.getState()).thenReturn(State.Running); + when(userVmDaoMock.findById(vmId)).thenReturn(runningVm); + + long currentSizeBytes = 10L << 30; + Long newSizeBytes = 20L << 30; + + volumeApiServiceImpl.validateVolumeReadyStateAndHypervisorChecks(volume, currentSizeBytes, newSizeBytes); + Assert.fail("Expected InvalidParameterValueException for VMware ROOT-volume resize " + + "when VM state is Running"); + } + + /** + * Positive – resizeVolumeInternal: + * The VMware stopped-state guard inside {@code resizeVolumeInternal} must NOT + * fire when the CloudStack VM state is {@code Stopped}, even when the VMware + * power_state has not yet transitioned to PowerOff. + * Any exception originating from deeper plumbing (job queue, storage layer) + * is acceptable; only the state-guard exception is a failure. + */ + @Test + public void testResizeVolumeInternal_VMware_VMStopped_PowerStateLag_ShouldNotThrowStateGuardError() + throws NoSuchMethodException, IllegalAccessException { + + long volumeId = 300L; + long vmId = 301L; + + VolumeVO volume = Mockito.mock(VolumeVO.class); + when(volume.getId()).thenReturn(volumeId); + when(volume.getInstanceId()).thenReturn(vmId); + when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT); + + UserVmVO stoppedVm = Mockito.mock(UserVmVO.class); + when(stoppedVm.getState()).thenReturn(State.Stopped); // authoritative cloud state + // getPowerState() deliberately NOT stubbed – power_state lag scenario + when(userVmDaoMock.findById(vmId)).thenReturn(stoppedVm); + + when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware); + + // Stub the job-queue call so OutcomeImpl.s_jobMgr (static, uninitialized in tests) + // is never reached. The Outcome mock returns null from get() and a bare AsyncJobVO + // from getJob(), which is enough for the unmarshallResultObject path to return null. + @SuppressWarnings("unchecked") + Outcome outcomemock = Mockito.mock(Outcome.class); + AsyncJobVO stubJob = new AsyncJobVO(); + when(outcomemock.getJob()).thenReturn(stubJob); + doReturn(outcomemock).when(volumeApiServiceImpl).resizeVolumeThroughJobQueue( + anyLong(), anyLong(), anyLong(), anyLong(), + nullable(Long.class), nullable(Long.class), + nullable(Integer.class), nullable(Long.class), anyBoolean()); + + // resizeVolumeInternal(VolumeVO, DiskOfferingVO, Long, Long, Long, Long, Integer, boolean) + try { + volumeApiServiceImpl.resizeVolumeInternal( + volume, + /* newDiskOffering */ (DiskOfferingVO) null, + /* currentSize */ 0L, + /* newSize */ 1L, + /* newMinIops */ (Long) null, + /* newMaxIops */ (Long) null, + /* snapshotReserve */ (Integer) null, + /* shrinkOk */ false); + } catch (ResourceAllocationException e) { + Assert.fail("Unexpected ResourceAllocationException"); + } catch (InvalidParameterValueException e) { + Assert.fail("VMware ROOT-volume resize must be allowed when CloudStack VM state is " + + "Stopped, even under a power_state lag. Unexpected exception: " + + e.getMessage()); + } + } + + /** + * Negative – resizeVolumeInternal: + * The VMware stopped-state guard inside {@code resizeVolumeInternal} must + * reject the operation when the CloudStack VM state is {@code Running}. + */ + @Test + public void testResizeVolumeInternal_VMware_VMRunning_ShouldThrowStateGuardError() + throws NoSuchMethodException, IllegalAccessException { + + long volumeId = 300L; + long vmId = 301L; + + VolumeVO volume = Mockito.mock(VolumeVO.class); + when(volume.getId()).thenReturn(volumeId); + when(volume.getInstanceId()).thenReturn(vmId); + when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT); + + UserVmVO runningVm = Mockito.mock(UserVmVO.class); + when(runningVm.getState()).thenReturn(State.Running); + when(userVmDaoMock.findById(vmId)).thenReturn(runningVm); + + when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware); + + try { + volumeApiServiceImpl.resizeVolumeInternal( + volume, + (DiskOfferingVO) null, + 0L, 1L, (Long) null, (Long) null, (Integer) null, false); + Assert.fail("Expected an InvalidParameterValueException for VMware ROOT-volume resize when VM state is Running"); + } catch (ResourceAllocationException e) { + Assert.fail("Cause must be InvalidParameterValueException, was: " + e.getClass()); + } catch (InvalidParameterValueException e) { + Assert.assertTrue( + "Exception message must reference Stopped-state requirement, was: " + e.getMessage(), + e.getMessage() != null && e.getMessage().contains("VM should be in")); + } + } }