This is an automated email from the ASF dual-hosted git repository.
harikrishna-patnala pushed a commit to branch 4.22
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.22 by this push:
new 7c2a8768e67 check state instead of power state on change offering for
ROOT volume (#13271)
7c2a8768e67 is described below
commit 7c2a8768e67409247cc592d043044d165d5dd506
Author: dahn <[email protected]>
AuthorDate: Mon Jul 6 11:11:25 2026 +0200
check state instead of power state on change offering for ROOT volume
(#13271)
* check state instead of power state on change offering for ROOT volume
* tests
* Apply suggestions from code review
Co-authored-by: Fabricio Duarte <[email protected]>
Co-authored-by: dahn <[email protected]>
* Apply test by package private
* fixing tests
---------
Co-authored-by: Daan Hoogland <[email protected]>
Co-authored-by: Fabricio Duarte <[email protected]>
---
.../com/cloud/storage/VolumeApiServiceImpl.java | 22 ++-
.../cloud/storage/VolumeApiServiceImplTest.java | 202 ++++++++++++++++++++-
2 files changed, 206 insertions(+), 18 deletions(-)
diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
index 687e432f166..7be9408308b 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 @@ public class VolumeApiServiceImpl extends ManagerBase
implements VolumeApiServic
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 @@ public class VolumeApiServiceImpl extends ManagerBase
implements VolumeApiServic
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<SnapshotVO> ongoingSnapshots =
_snapshotDao.listByStatus(volume.getId(), Snapshot.State.Creating,
Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp);
if (ongoingSnapshots.size() > 0) {
@@ -2396,9 +2398,11 @@ public class VolumeApiServiceImpl extends ManagerBase
implements VolumeApiServic
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 @@ public class VolumeApiServiceImpl extends ManagerBase
implements VolumeApiServic
}
}
- 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 77e321ab859..b806eb4afe5 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.verify;
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.List;
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.engine.subsystem.api.storage.VolumeService.VolumeAp
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.HostPodVO;
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.VirtualMachine.State;
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 @@ public class VolumeApiServiceImplTest {
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<Volume> 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"));
+ }
+ }
}
