Skip to content

Commit 69def68

Browse files
author
Daan Hoogland
committed
tests
1 parent 63010c9 commit 69def68

2 files changed

Lines changed: 226 additions & 12 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2317,10 +2317,10 @@ private VolumeVO resizeVolumeInternal(VolumeVO volume, DiskOfferingVO newDiskOff
23172317

23182318
if (userVm != null) {
23192319
if (Volume.Type.ROOT.equals(volume.getVolumeType())
2320-
&& ! State.Stopped.equals(userVm.getState())
2320+
&& !State.Stopped.equals(userVm.getState())
23212321
&& HypervisorType.VMware.equals(hypervisorType)) {
23222322
logger.error("For ROOT volume resize VM should be in Stopped state.");
2323-
throw new InvalidParameterValueException("VM current state is : " + userVm.getState() + ". But VM should be in " + State.Stopped + " state.");
2323+
throw new InvalidParameterValueException("The current VM state is '" + userVm.getState() + "'. But the VM should be in " + State.Stopped + " state.");
23242324
}
23252325
// serialize VM operation
23262326
AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
@@ -2402,7 +2402,7 @@ private void validateVolumeReadyStateAndHypervisorChecks(VolumeVO volume, long c
24022402
&& !State.Stopped.equals(userVm.getState())
24032403
&& hypervisorType == HypervisorType.VMware) {
24042404
logger.error("For ROOT volume resize VM should be in Stopped state.");
2405-
throw new InvalidParameterValueException("VM current state is : " + userVm.getState() + ". But VM should be in " + State.Stopped + " state.");
2405+
throw new InvalidParameterValueException("The current VM state is '" + userVm.getState() + "'. But VM should be in " + State.Stopped + " state.");
24062406
}
24072407
}
24082408
}

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

Lines changed: 223 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,22 +33,15 @@
3333
import static org.mockito.Mockito.when;
3434

3535
import java.lang.reflect.Field;
36+
import java.lang.reflect.InvocationTargetException;
37+
import java.lang.reflect.Method;
3638
import java.util.ArrayList;
3739
import java.util.Arrays;
3840
import java.util.Collections;
3941
import java.util.List;
4042
import java.util.UUID;
4143
import java.util.concurrent.ExecutionException;
4244

43-
import com.cloud.event.EventTypes;
44-
import com.cloud.event.UsageEventUtils;
45-
import com.cloud.host.HostVO;
46-
import com.cloud.resourcelimit.CheckedReservation;
47-
import com.cloud.service.ServiceOfferingVO;
48-
import com.cloud.service.dao.ServiceOfferingDao;
49-
import com.cloud.vm.snapshot.VMSnapshot;
50-
import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
51-
import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
5245
import org.apache.cloudstack.acl.ControlledEntity;
5346
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
5447
import org.apache.cloudstack.api.command.user.volume.CheckAndRepairVolumeCmd;
@@ -107,17 +100,23 @@
107100
import com.cloud.dc.dao.ClusterDao;
108101
import com.cloud.dc.dao.DataCenterDao;
109102
import com.cloud.dc.dao.HostPodDao;
103+
import com.cloud.event.EventTypes;
104+
import com.cloud.event.UsageEventUtils;
110105
import com.cloud.exception.InvalidParameterValueException;
111106
import com.cloud.exception.PermissionDeniedException;
112107
import com.cloud.exception.ResourceAllocationException;
108+
import com.cloud.host.HostVO;
113109
import com.cloud.host.dao.HostDao;
114110
import com.cloud.hypervisor.Hypervisor.HypervisorType;
115111
import com.cloud.org.Grouping;
116112
import com.cloud.projects.Project;
117113
import com.cloud.projects.ProjectManager;
114+
import com.cloud.resourcelimit.CheckedReservation;
118115
import com.cloud.serializer.GsonHelper;
119116
import com.cloud.server.ManagementService;
120117
import com.cloud.server.TaggedResourceService;
118+
import com.cloud.service.ServiceOfferingVO;
119+
import com.cloud.service.dao.ServiceOfferingDao;
121120
import com.cloud.storage.Storage.ProvisioningType;
122121
import com.cloud.storage.Volume.Type;
123122
import com.cloud.storage.dao.DiskOfferingDao;
@@ -145,8 +144,11 @@
145144
import com.cloud.vm.VirtualMachineManager;
146145
import com.cloud.vm.dao.UserVmDao;
147146
import com.cloud.vm.dao.VMInstanceDao;
147+
import com.cloud.vm.snapshot.VMSnapshot;
148+
import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
148149
import com.cloud.vm.snapshot.VMSnapshotVO;
149150
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
151+
import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
150152

151153
@RunWith(MockitoJUnitRunner.class)
152154
public class VolumeApiServiceImplTest {
@@ -2320,4 +2322,216 @@ private List<VMSnapshotVO> generateVmSnapshotVoList(VMSnapshot.Type t1, VMSnapsh
23202322
Mockito.doReturn(1L).when(mock2).getId();
23212323
return List.of(mock1, mock2);
23222324
}
2325+
2326+
// =====================================================================
2327+
// VMware ROOT-volume resize / offering-change: VM power_state-lag tests
2328+
//
2329+
// Both private guards that protect VMware ROOT-volume resize operations
2330+
// are covered here:
2331+
// • validateVolumeReadyStateAndHypervisorChecks (called by changeDiskOfferingForVolumeInternal)
2332+
// • resizeVolumeInternal (called by both resize and change-offering flows)
2333+
//
2334+
// The "power_state lag" scenario: when a VMware VM is stopped via CloudStack
2335+
// the VirtualMachine.State transitions to Stopped immediately, but the
2336+
// VMware-side VirtualMachine.PowerState (polled from ESX) may still read
2337+
// PoweredOn for some seconds. The production code must use only the
2338+
// authoritative CloudStack State field and must NOT additionally gate on
2339+
// PowerState.
2340+
// =====================================================================
2341+
2342+
/**
2343+
* Positive – validateVolumeReadyStateAndHypervisorChecks:
2344+
* The guard must allow a VMware ROOT-volume resize when the CloudStack VM
2345+
* state is {@code Stopped}, regardless of the VMware power_state value.
2346+
* getPowerState() is intentionally left un-stubbed so that any invocation
2347+
* of that method would cause a Mockito strict-stubbing error and surface a
2348+
* regression.
2349+
*/
2350+
@Test
2351+
public void testValidateVolumeReadyStateVMware_VMStopped_PowerStateLag_ShouldPass()
2352+
throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
2353+
2354+
long volumeId = 200L;
2355+
long vmId = 201L;
2356+
2357+
VolumeVO volume = Mockito.mock(VolumeVO.class);
2358+
when(volume.getId()).thenReturn(volumeId);
2359+
when(volume.getInstanceId()).thenReturn(vmId);
2360+
when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT);
2361+
when(volume.getState()).thenReturn(Volume.State.Ready);
2362+
2363+
// snapshotDaoMock returns an empty list by default (Mockito default behaviour)
2364+
when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware);
2365+
2366+
UserVmVO stoppedVm = Mockito.mock(UserVmVO.class);
2367+
// Authoritative cloud state: Stopped.
2368+
// getPowerState() is NOT stubbed – power_state lag scenario.
2369+
when(stoppedVm.getState()).thenReturn(State.Stopped);
2370+
when(userVmDaoMock.findById(vmId)).thenReturn(stoppedVm);
2371+
2372+
long currentSizeBytes = 10L << 30; // 10 GiB
2373+
Long newSizeBytes = 20L << 30; // 20 GiB (grow; VMware prohibits shrink)
2374+
2375+
Method method = VolumeApiServiceImpl.class.getDeclaredMethod(
2376+
"validateVolumeReadyStateAndHypervisorChecks",
2377+
VolumeVO.class, long.class, Long.class);
2378+
method.setAccessible(true);
2379+
2380+
// Must complete without throwing any exception
2381+
method.invoke(volumeApiServiceImpl, volume, currentSizeBytes, newSizeBytes);
2382+
}
2383+
2384+
/**
2385+
* Negative – validateVolumeReadyStateAndHypervisorChecks:
2386+
* The guard must reject a VMware ROOT-volume resize when the CloudStack VM
2387+
* state is {@code Running}.
2388+
*/
2389+
@Test
2390+
public void testValidateVolumeReadyStateVMware_VMRunning_ShouldThrowInvalidParameterValueException()
2391+
throws NoSuchMethodException, IllegalAccessException {
2392+
2393+
long volumeId = 200L;
2394+
long vmId = 201L;
2395+
2396+
VolumeVO volume = Mockito.mock(VolumeVO.class);
2397+
when(volume.getId()).thenReturn(volumeId);
2398+
when(volume.getInstanceId()).thenReturn(vmId);
2399+
when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT);
2400+
when(volume.getState()).thenReturn(Volume.State.Ready);
2401+
2402+
when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware);
2403+
2404+
UserVmVO runningVm = Mockito.mock(UserVmVO.class);
2405+
when(runningVm.getState()).thenReturn(State.Running);
2406+
when(userVmDaoMock.findById(vmId)).thenReturn(runningVm);
2407+
2408+
long currentSizeBytes = 10L << 30;
2409+
Long newSizeBytes = 20L << 30;
2410+
2411+
Method method = VolumeApiServiceImpl.class.getDeclaredMethod(
2412+
"validateVolumeReadyStateAndHypervisorChecks",
2413+
VolumeVO.class, long.class, Long.class);
2414+
method.setAccessible(true);
2415+
2416+
try {
2417+
method.invoke(volumeApiServiceImpl, volume, currentSizeBytes, newSizeBytes);
2418+
Assert.fail("Expected InvalidParameterValueException for VMware ROOT-volume resize "
2419+
+ "when VM state is Running");
2420+
} catch (InvocationTargetException e) {
2421+
Assert.assertNotNull("InvocationTargetException must carry a cause", e.getCause());
2422+
Assert.assertTrue(
2423+
"Cause must be InvalidParameterValueException, was: " + e.getCause().getClass(),
2424+
e.getCause() instanceof InvalidParameterValueException);
2425+
Assert.assertTrue(
2426+
"Exception message must reference Stopped-state requirement, was: " + e.getCause().getMessage(),
2427+
e.getCause().getMessage() != null
2428+
&& e.getCause().getMessage().contains("VM should be in"));
2429+
}
2430+
}
2431+
2432+
/**
2433+
* Positive – resizeVolumeInternal:
2434+
* The VMware stopped-state guard inside {@code resizeVolumeInternal} must NOT
2435+
* fire when the CloudStack VM state is {@code Stopped}, even when the VMware
2436+
* power_state has not yet transitioned to PowerOff.
2437+
* Any exception originating from deeper plumbing (job queue, storage layer)
2438+
* is acceptable; only the state-guard exception is a failure.
2439+
*/
2440+
@Test
2441+
public void testResizeVolumeInternal_VMware_VMStopped_PowerStateLag_ShouldNotThrowStateGuardError()
2442+
throws NoSuchMethodException, IllegalAccessException {
2443+
2444+
long volumeId = 300L;
2445+
long vmId = 301L;
2446+
2447+
VolumeVO volume = Mockito.mock(VolumeVO.class);
2448+
when(volume.getId()).thenReturn(volumeId);
2449+
when(volume.getInstanceId()).thenReturn(vmId);
2450+
when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT);
2451+
2452+
UserVmVO stoppedVm = Mockito.mock(UserVmVO.class);
2453+
when(stoppedVm.getState()).thenReturn(State.Stopped); // authoritative cloud state
2454+
// getPowerState() deliberately NOT stubbed – power_state lag scenario
2455+
when(userVmDaoMock.findById(vmId)).thenReturn(stoppedVm);
2456+
2457+
when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware);
2458+
2459+
// resizeVolumeInternal(VolumeVO, DiskOfferingVO, Long, Long, Long, Long, Integer, boolean)
2460+
Method method = VolumeApiServiceImpl.class.getDeclaredMethod(
2461+
"resizeVolumeInternal",
2462+
VolumeVO.class, DiskOfferingVO.class,
2463+
Long.class, Long.class, Long.class, Long.class, Integer.class, boolean.class);
2464+
method.setAccessible(true);
2465+
2466+
try {
2467+
method.invoke(volumeApiServiceImpl,
2468+
volume,
2469+
/* newDiskOffering */ (DiskOfferingVO) null,
2470+
/* currentSize */ 0L,
2471+
/* newSize */ 1L,
2472+
/* newMinIops */ (Long) null,
2473+
/* newMaxIops */ (Long) null,
2474+
/* snapshotReserve */ (Integer) null,
2475+
/* shrinkOk */ false);
2476+
} catch (InvocationTargetException e) {
2477+
// If the state guard triggered it is a test failure; all other deeper
2478+
// exceptions (NullPointerException from job-queue plumbing, etc.) are
2479+
// acceptable because they are outside the scope of this test.
2480+
if (e.getCause() instanceof InvalidParameterValueException
2481+
&& e.getCause().getMessage() != null
2482+
&& e.getCause().getMessage().contains("VM should be in")) {
2483+
Assert.fail("VMware ROOT-volume resize must be allowed when CloudStack VM state is "
2484+
+ "Stopped, even under a power_state lag. Unexpected exception: "
2485+
+ e.getCause().getMessage());
2486+
}
2487+
}
2488+
}
2489+
2490+
/**
2491+
* Negative – resizeVolumeInternal:
2492+
* The VMware stopped-state guard inside {@code resizeVolumeInternal} must
2493+
* reject the operation when the CloudStack VM state is {@code Running}.
2494+
*/
2495+
@Test
2496+
public void testResizeVolumeInternal_VMware_VMRunning_ShouldThrowStateGuardError()
2497+
throws NoSuchMethodException, IllegalAccessException {
2498+
2499+
long volumeId = 300L;
2500+
long vmId = 301L;
2501+
2502+
VolumeVO volume = Mockito.mock(VolumeVO.class);
2503+
when(volume.getId()).thenReturn(volumeId);
2504+
when(volume.getInstanceId()).thenReturn(vmId);
2505+
when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT);
2506+
2507+
UserVmVO runningVm = Mockito.mock(UserVmVO.class);
2508+
when(runningVm.getState()).thenReturn(State.Running);
2509+
when(userVmDaoMock.findById(vmId)).thenReturn(runningVm);
2510+
2511+
when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware);
2512+
2513+
Method method = VolumeApiServiceImpl.class.getDeclaredMethod(
2514+
"resizeVolumeInternal",
2515+
VolumeVO.class, DiskOfferingVO.class,
2516+
Long.class, Long.class, Long.class, Long.class, Integer.class, boolean.class);
2517+
method.setAccessible(true);
2518+
2519+
try {
2520+
method.invoke(volumeApiServiceImpl,
2521+
volume,
2522+
(DiskOfferingVO) null,
2523+
0L, 1L, (Long) null, (Long) null, (Integer) null, false);
2524+
Assert.fail("Expected InvalidParameterValueException for VMware ROOT-volume resize "
2525+
+ "when VM state is Running");
2526+
} catch (InvocationTargetException e) {
2527+
Assert.assertNotNull("InvocationTargetException must carry a cause", e.getCause());
2528+
Assert.assertTrue(
2529+
"Cause must be InvalidParameterValueException, was: " + e.getCause().getClass(),
2530+
e.getCause() instanceof InvalidParameterValueException);
2531+
Assert.assertTrue(
2532+
"Exception message must reference Stopped-state requirement, was: " + e.getCause().getMessage(),
2533+
e.getCause().getMessage() != null
2534+
&& e.getCause().getMessage().contains("VM should be in"));
2535+
}
2536+
}
23232537
}

0 commit comments

Comments
 (0)