mirror of https://github.com/apache/cloudstack.git
Fix Revert Instance to Snapshot with custom service offering (#12885)
* Fix revertVM with custom svc offering
This commit is contained in:
parent
89d915493f
commit
a127a26ebd
|
|
@ -751,11 +751,17 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
|
|||
"VM Snapshot reverting failed due to vm is not in the state of Running or Stopped.");
|
||||
}
|
||||
|
||||
if (userVm.getState() == VirtualMachine.State.Running && vmSnapshotVo.getType() == VMSnapshot.Type.Disk || userVm.getState() == VirtualMachine.State.Stopped
|
||||
&& vmSnapshotVo.getType() == VMSnapshot.Type.DiskAndMemory) {
|
||||
if (userVm.getState() == VirtualMachine.State.Running && vmSnapshotVo.getType() == VMSnapshot.Type.Disk) {
|
||||
throw new InvalidParameterValueException(
|
||||
"VM Snapshot revert not allowed. This will result in VM state change. You can revert running VM to disk and memory type snapshot and stopped VM to disk type"
|
||||
+ " snapshot");
|
||||
"Reverting to the Instance Snapshot is not allowed for running Instances as this would result in an Instance state change. " +
|
||||
"For running Instances only Snapshots with memory can be reverted. " +
|
||||
"In order to revert to a Snapshot without memory you need to first stop the Instance.");
|
||||
}
|
||||
|
||||
if (userVm.getState() == VirtualMachine.State.Stopped && vmSnapshotVo.getType() == VMSnapshot.Type.DiskAndMemory) {
|
||||
throw new InvalidParameterValueException(
|
||||
"Reverting to the Instance Snapshot is not allowed for stopped Instances when the Snapshot contains memory as this would result in an Instance state change. " +
|
||||
"In order to revert to a Snapshot with memory you need to first start the Instance.");
|
||||
}
|
||||
|
||||
// if snapshot is not created, error out
|
||||
|
|
@ -808,20 +814,36 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
|
|||
}
|
||||
|
||||
/**
|
||||
* If snapshot was taken with a different service offering than actual used in vm, should change it back to it.
|
||||
* We also call <code>changeUserVmServiceOffering</code> in case the service offering is dynamic in order to
|
||||
* perform resource limit validation, as the amount of CPUs or memory may have been changed.
|
||||
* @param vmSnapshotVo vm snapshot
|
||||
* Check if service offering change is needed for user vm when reverting to vm snapshot.
|
||||
* Service offering change is needed when snapshot was taken with a different service offering than actual used in vm.
|
||||
* Service offering change is also needed when service offering is dynamic and the amount of cpu, memory or cpu speed
|
||||
* has been changed since snapshot was taken.
|
||||
* @param userVm
|
||||
* @param vmSnapshotVo
|
||||
* @return true if service offering change is needed; false otherwise
|
||||
*/
|
||||
protected void updateUserVmServiceOffering(UserVm userVm, VMSnapshotVO vmSnapshotVo) {
|
||||
protected boolean userVmServiceOfferingNeedsChange(UserVm userVm, VMSnapshotVO vmSnapshotVo) {
|
||||
if (vmSnapshotVo.getServiceOfferingId() != userVm.getServiceOfferingId()) {
|
||||
changeUserVmServiceOffering(userVm, vmSnapshotVo);
|
||||
return;
|
||||
return true;
|
||||
}
|
||||
ServiceOfferingVO serviceOffering = _serviceOfferingDao.findById(userVm.getServiceOfferingId());
|
||||
if (serviceOffering.isDynamic()) {
|
||||
changeUserVmServiceOffering(userVm, vmSnapshotVo);
|
||||
|
||||
ServiceOfferingVO currentServiceOffering = _serviceOfferingDao.findByIdIncludingRemoved(userVm.getId(), userVm.getServiceOfferingId());
|
||||
if (currentServiceOffering.isDynamic()) {
|
||||
Map<String, String> vmDetails = getVmMapDetails(vmSnapshotVo);
|
||||
ServiceOfferingVO newServiceOffering = _serviceOfferingDao.getComputeOffering(currentServiceOffering, vmDetails);
|
||||
|
||||
int newCpu = newServiceOffering.getCpu();
|
||||
int newMemory = newServiceOffering.getRamSize();
|
||||
int newSpeed = newServiceOffering.getSpeed();
|
||||
int currentCpu = currentServiceOffering.getCpu();
|
||||
int currentMemory = currentServiceOffering.getRamSize();
|
||||
int currentSpeed = currentServiceOffering.getSpeed();
|
||||
|
||||
if (newCpu != currentCpu || newMemory != currentMemory || newSpeed != currentSpeed) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -941,7 +963,9 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
|
|||
Transaction.execute(new TransactionCallbackWithExceptionNoReturn<CloudRuntimeException>() {
|
||||
@Override
|
||||
public void doInTransactionWithoutResult(TransactionStatus status) throws CloudRuntimeException {
|
||||
updateUserVmServiceOffering(userVm, vmSnapshotVo);
|
||||
if (userVmServiceOfferingNeedsChange(userVm, vmSnapshotVo)) {
|
||||
changeUserVmServiceOffering(userVm, vmSnapshotVo);
|
||||
}
|
||||
revertUserVmDetailsFromVmSnapshot(userVm, vmSnapshotVo);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -67,7 +67,6 @@ import org.junit.Before;
|
|||
import org.junit.Test;
|
||||
import org.mockito.ArgumentCaptor;
|
||||
import org.mockito.Captor;
|
||||
import org.mockito.ArgumentMatchers;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.MockitoAnnotations;
|
||||
import org.mockito.Spy;
|
||||
|
|
@ -79,9 +78,12 @@ import java.util.List;
|
|||
import java.util.Map;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.anyLong;
|
||||
import static org.mockito.ArgumentMatchers.anyString;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.Mockito.doNothing;
|
||||
import static org.mockito.Mockito.doReturn;
|
||||
import static org.mockito.Mockito.mock;
|
||||
|
|
@ -223,6 +225,7 @@ public class VMSnapshotManagerTest {
|
|||
when(vmSnapshotVO.getId()).thenReturn(VM_SNAPSHOT_ID);
|
||||
when(serviceOffering.isDynamic()).thenReturn(false);
|
||||
when(_serviceOfferingDao.findById(SERVICE_OFFERING_ID)).thenReturn(serviceOffering);
|
||||
when(_serviceOfferingDao.findByIdIncludingRemoved(TEST_VM_ID, SERVICE_OFFERING_ID)).thenReturn(serviceOffering);
|
||||
|
||||
for (ResourceDetail detail : Arrays.asList(userVmDetailCpuNumber, vmSnapshotDetailCpuNumber)) {
|
||||
when(detail.getName()).thenReturn("cpuNumber");
|
||||
|
|
@ -340,20 +343,51 @@ public class VMSnapshotManagerTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
public void testUpdateUserVmServiceOfferingSameServiceOffering() {
|
||||
_vmSnapshotMgr.updateUserVmServiceOffering(userVm, vmSnapshotVO);
|
||||
verify(_vmSnapshotMgr, never()).changeUserVmServiceOffering(userVm, vmSnapshotVO);
|
||||
public void testUserVmServiceOfferingNeedsChangeWhenSnapshotOfferingDiffers() {
|
||||
when(userVm.getServiceOfferingId()).thenReturn(SERVICE_OFFERING_DIFFERENT_ID);
|
||||
when(vmSnapshotVO.getServiceOfferingId()).thenReturn(SERVICE_OFFERING_ID);
|
||||
|
||||
assertTrue(_vmSnapshotMgr.userVmServiceOfferingNeedsChange(userVm, vmSnapshotVO));
|
||||
|
||||
verify(_serviceOfferingDao, never()).findByIdIncludingRemoved(anyLong(), anyLong());
|
||||
verify(_serviceOfferingDao, never()).getComputeOffering(any(ServiceOfferingVO.class), any());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testUpdateUserVmServiceOfferingDifferentServiceOffering() throws ConcurrentOperationException, ResourceUnavailableException, ManagementServerException, VirtualMachineMigrationException {
|
||||
when(userVm.getServiceOfferingId()).thenReturn(SERVICE_OFFERING_DIFFERENT_ID);
|
||||
when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true);
|
||||
_vmSnapshotMgr.updateUserVmServiceOffering(userVm, vmSnapshotVO);
|
||||
public void testUserVmServiceOfferingNeedsChangeWhenSameNonDynamicOffering() {
|
||||
assertFalse(_vmSnapshotMgr.userVmServiceOfferingNeedsChange(userVm, vmSnapshotVO));
|
||||
|
||||
verify(_vmSnapshotMgr).changeUserVmServiceOffering(userVm, vmSnapshotVO);
|
||||
verify(_serviceOfferingDao).findByIdIncludingRemoved(TEST_VM_ID, SERVICE_OFFERING_ID);
|
||||
verify(_serviceOfferingDao, never()).getComputeOffering(any(ServiceOfferingVO.class), any());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testUserVmServiceOfferingNeedsChangeWhenDynamicOfferingMatchesSnapshot() {
|
||||
when(serviceOffering.isDynamic()).thenReturn(true);
|
||||
when(serviceOffering.getCpu()).thenReturn(2);
|
||||
when(serviceOffering.getRamSize()).thenReturn(2048);
|
||||
when(serviceOffering.getSpeed()).thenReturn(1000);
|
||||
when(_serviceOfferingDao.getComputeOffering(eq(serviceOffering), any())).thenReturn(serviceOffering);
|
||||
|
||||
assertFalse(_vmSnapshotMgr.userVmServiceOfferingNeedsChange(userVm, vmSnapshotVO));
|
||||
|
||||
verify(_serviceOfferingDao).getComputeOffering(eq(serviceOffering), any());
|
||||
verify(_vmSnapshotMgr).getVmMapDetails(vmSnapshotVO);
|
||||
verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testUserVmServiceOfferingNeedsChangeWhenDynamicCpuDiffersFromSnapshot() {
|
||||
when(serviceOffering.isDynamic()).thenReturn(true);
|
||||
when(serviceOffering.getCpu()).thenReturn(2);
|
||||
when(serviceOffering.getRamSize()).thenReturn(2048);
|
||||
when(serviceOffering.getSpeed()).thenReturn(1000);
|
||||
ServiceOfferingVO fromSnapshot = mock(ServiceOfferingVO.class);
|
||||
when(fromSnapshot.getCpu()).thenReturn(4);
|
||||
when(fromSnapshot.getRamSize()).thenReturn(2048);
|
||||
when(fromSnapshot.getSpeed()).thenReturn(1000);
|
||||
when(_serviceOfferingDao.getComputeOffering(eq(serviceOffering), any())).thenReturn(fromSnapshot);
|
||||
|
||||
assertTrue(_vmSnapshotMgr.userVmServiceOfferingNeedsChange(userVm, vmSnapshotVO));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -368,18 +402,18 @@ public class VMSnapshotManagerTest {
|
|||
|
||||
@Test
|
||||
public void testChangeUserVmServiceOffering() throws ConcurrentOperationException, ResourceUnavailableException, ManagementServerException, VirtualMachineMigrationException {
|
||||
when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true);
|
||||
when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true);
|
||||
_vmSnapshotMgr.changeUserVmServiceOffering(userVm, vmSnapshotVO);
|
||||
verify(_vmSnapshotMgr).getVmMapDetails(vmSnapshotVO);
|
||||
verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
|
||||
verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
|
||||
}
|
||||
|
||||
@Test(expected=CloudRuntimeException.class)
|
||||
public void testChangeUserVmServiceOfferingFailOnUpgradeVMServiceOffering() throws ConcurrentOperationException, ResourceUnavailableException, ManagementServerException, VirtualMachineMigrationException {
|
||||
when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(false);
|
||||
when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(false);
|
||||
_vmSnapshotMgr.changeUserVmServiceOffering(userVm, vmSnapshotVO);
|
||||
verify(_vmSnapshotMgr).getVmMapDetails(vmSnapshotVO);
|
||||
verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
|
||||
verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
Loading…
Reference in New Issue