This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push:
new 2df68021761 Allocate new ROOT volume (on restore virtual machine
operation) only when resource count increment succeeds (#8555)
2df68021761 is described below
commit 2df68021761adbd93eecda20114894c8f2edb8bd
Author: Suresh Kumar Anaparti <[email protected]>
AuthorDate: Mon Feb 5 13:28:28 2024 +0530
Allocate new ROOT volume (on restore virtual machine operation) only when
resource count increment succeeds (#8555)
* Allocate new volume on restore virtual machine operation when resource
count increment succeeds
- keep them in transaction, and fail operation if resource count increment
fails
* Added some (negative) unit tests for restore vm
---
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 100 +++++++-----
.../java/com/cloud/vm/UserVmManagerImplTest.java | 174 +++++++++++++++++++++
2 files changed, 234 insertions(+), 40 deletions(-)
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index 2927db638ab..0d3f047809a 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -354,6 +354,7 @@ import com.cloud.utils.db.GlobalLock;
import com.cloud.utils.db.SearchCriteria;
import com.cloud.utils.db.Transaction;
import com.cloud.utils.db.TransactionCallbackNoReturn;
+import com.cloud.utils.db.TransactionCallbackWithException;
import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn;
import com.cloud.utils.db.TransactionStatus;
import com.cloud.utils.db.UUIDManager;
@@ -7765,49 +7766,68 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
List<Volume> newVols = new ArrayList<>();
for (VolumeVO root : rootVols) {
- if ( !Volume.State.Allocated.equals(root.getState()) ||
newTemplateId != null ){
- Long templateId = root.getTemplateId();
- boolean isISO = false;
- if (templateId == null) {
- // Assuming that for a vm deployed using ISO, template ID
is set to NULL
- isISO = true;
- templateId = vm.getIsoId();
- }
-
- /* If new template/ISO is provided allocate a new volume from
new template/ISO otherwise allocate new volume from original template/ISO */
- Volume newVol = null;
- if (newTemplateId != null) {
- if (isISO) {
- newVol = volumeMgr.allocateDuplicateVolume(root, null);
- vm.setIsoId(newTemplateId);
- vm.setGuestOSId(template.getGuestOSId());
- vm.setTemplateId(newTemplateId);
- } else {
- newVol = volumeMgr.allocateDuplicateVolume(root,
newTemplateId);
- vm.setGuestOSId(template.getGuestOSId());
- vm.setTemplateId(newTemplateId);
- }
- // check and update VM if it can be dynamically scalable
with the new template
- updateVMDynamicallyScalabilityUsingTemplate(vm,
newTemplateId);
- } else {
- newVol = volumeMgr.allocateDuplicateVolume(root, null);
- }
- newVols.add(newVol);
+ if ( !Volume.State.Allocated.equals(root.getState()) ||
newTemplateId != null ) {
+ final UserVmVO userVm = vm;
+ Pair<UserVmVO, Volume> vmAndNewVol = Transaction.execute(new
TransactionCallbackWithException<Pair<UserVmVO, Volume>,
CloudRuntimeException>() {
+ @Override
+ public Pair<UserVmVO, Volume> doInTransaction(final
TransactionStatus status) throws CloudRuntimeException {
+ Long templateId = root.getTemplateId();
+ boolean isISO = false;
+ if (templateId == null) {
+ // Assuming that for a vm deployed using ISO,
template ID is set to NULL
+ isISO = true;
+ templateId = userVm.getIsoId();
+ }
+
+ /* If new template/ISO is provided allocate a new
volume from new template/ISO otherwise allocate new volume from original
template/ISO */
+ Volume newVol = null;
+ if (newTemplateId != null) {
+ if (isISO) {
+ newVol =
volumeMgr.allocateDuplicateVolume(root, null);
+ userVm.setIsoId(newTemplateId);
+ userVm.setGuestOSId(template.getGuestOSId());
+ userVm.setTemplateId(newTemplateId);
+ } else {
+ newVol =
volumeMgr.allocateDuplicateVolume(root, newTemplateId);
+ userVm.setGuestOSId(template.getGuestOSId());
+ userVm.setTemplateId(newTemplateId);
+ }
+ // check and update VM if it can be dynamically
scalable with the new template
+
updateVMDynamicallyScalabilityUsingTemplate(userVm, newTemplateId);
+ } else {
+ newVol = volumeMgr.allocateDuplicateVolume(root,
null);
+ }
+ newVols.add(newVol);
+
+ if (userVmDetailsDao.findDetail(userVm.getId(),
VmDetailConstants.ROOT_DISK_SIZE) == null &&
!newVol.getSize().equals(template.getSize())) {
+ VolumeVO resizedVolume = (VolumeVO) newVol;
+ if (template.getSize() != null) {
+ resizedVolume.setSize(template.getSize());
+ _volsDao.update(resizedVolume.getId(),
resizedVolume);
+ }
+ }
+
+ // 1. Save usage event and update resource count for
user vm volumes
+ try {
+
_resourceLimitMgr.incrementResourceCount(newVol.getAccountId(),
ResourceType.volume, newVol.isDisplay());
+
_resourceLimitMgr.incrementResourceCount(newVol.getAccountId(),
ResourceType.primary_storage, newVol.isDisplay(), new Long(newVol.getSize()));
+ } catch (final CloudRuntimeException e) {
+ throw e;
+ } catch (final Exception e) {
+ s_logger.error("Unable to restore VM " +
userVm.getUuid(), e);
+ throw new CloudRuntimeException(e);
+ }
+
+ // 2. Create Usage event for the newly created volume
+ UsageEventVO usageEvent = new
UsageEventVO(EventTypes.EVENT_VOLUME_CREATE, newVol.getAccountId(),
newVol.getDataCenterId(), newVol.getId(), newVol.getName(),
newVol.getDiskOfferingId(), template.getId(), newVol.getSize());
+ _usageEventDao.persist(usageEvent);
- if (userVmDetailsDao.findDetail(vm.getId(),
VmDetailConstants.ROOT_DISK_SIZE) == null &&
!newVol.getSize().equals(template.getSize())) {
- VolumeVO resizedVolume = (VolumeVO) newVol;
- if (template.getSize() != null) {
- resizedVolume.setSize(template.getSize());
- _volsDao.update(resizedVolume.getId(), resizedVolume);
+ return new Pair<>(userVm, newVol);
}
- }
+ });
- // 1. Save usage event and update resource count for user vm
volumes
-
_resourceLimitMgr.incrementResourceCount(newVol.getAccountId(),
ResourceType.volume, newVol.isDisplay());
-
_resourceLimitMgr.incrementResourceCount(newVol.getAccountId(),
ResourceType.primary_storage, newVol.isDisplay(), new Long(newVol.getSize()));
- // 2. Create Usage event for the newly created volume
- UsageEventVO usageEvent = new
UsageEventVO(EventTypes.EVENT_VOLUME_CREATE, newVol.getAccountId(),
newVol.getDataCenterId(), newVol.getId(), newVol.getName(),
newVol.getDiskOfferingId(), template.getId(), newVol.getSize());
- _usageEventDao.persist(usageEvent);
+ vm = vmAndNewVol.first();
+ Volume newVol = vmAndNewVol.second();
handleManagedStorage(vm, root);
diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
index 16607df7a9f..e2c2b8ef9e2 100644
--- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
+++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
@@ -30,6 +30,7 @@ import
com.cloud.exception.InsufficientAddressCapacityException;
import com.cloud.exception.InsufficientCapacityException;
import com.cloud.exception.InsufficientServerCapacityException;
import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.PermissionDeniedException;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.host.Host;
@@ -47,6 +48,8 @@ import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.storage.DiskOfferingVO;
import com.cloud.storage.GuestOSVO;
import com.cloud.storage.ScopeType;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
import com.cloud.storage.Storage;
import com.cloud.storage.VMTemplateVO;
import com.cloud.storage.Volume;
@@ -54,6 +57,7 @@ import com.cloud.storage.VolumeApiService;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.storage.dao.GuestOSDao;
+import com.cloud.storage.dao.SnapshotDao;
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.template.VirtualMachineTemplate;
@@ -66,6 +70,7 @@ import com.cloud.user.UserData;
import com.cloud.user.UserDataVO;
import com.cloud.user.UserVO;
import com.cloud.user.dao.AccountDao;
+import com.cloud.user.dao.UserDao;
import com.cloud.user.dao.UserDataDao;
import com.cloud.uservm.UserVm;
import com.cloud.utils.Pair;
@@ -74,10 +79,14 @@ import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.dao.NicDao;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.UserVmDetailsDao;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+
import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
import org.apache.cloudstack.api.command.user.vm.DeployVMCmd;
import org.apache.cloudstack.api.command.user.vm.DeployVnfApplianceCmd;
import org.apache.cloudstack.api.command.user.vm.ResetVMUserDataCmd;
+import org.apache.cloudstack.api.command.user.vm.RestoreVMCmd;
import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd;
import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
import org.apache.cloudstack.context.CallContext;
@@ -191,6 +200,9 @@ public class UserVmManagerImplTest {
@Mock
private AccountDao accountDao;
+ @Mock
+ private UserDao userDao;
+
@Mock
ResourceLimitService resourceLimitMgr;
@@ -218,6 +230,12 @@ public class UserVmManagerImplTest {
@Mock
private VolumeDao volumeDaoMock;
+ @Mock
+ private SnapshotDao snapshotDaoMock;
+
+ @Mock
+ private VMSnapshotDao vmSnapshotDaoMock;
+
@Mock
AccountVO account;
@@ -1221,4 +1239,160 @@ public class UserVmManagerImplTest {
Mockito.verify(userVmVoMock).setLastHostId(2L);
Mockito.verify(userVmVoMock).setState(VirtualMachine.State.Running);
}
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void testRestoreVMNoVM() throws ResourceUnavailableException,
InsufficientCapacityException {
+ CallContext callContextMock = Mockito.mock(CallContext.class);
+
Mockito.lenient().doReturn(accountMock).when(callContextMock).getCallingAccount();
+
+ RestoreVMCmd cmd = Mockito.mock(RestoreVMCmd.class);
+ when(cmd.getVmId()).thenReturn(vmId);
+ when(cmd.getTemplateId()).thenReturn(2L);
+ when(userVmDao.findById(vmId)).thenReturn(null);
+
+ userVmManagerImpl.restoreVM(cmd);
+ }
+
+ @Test(expected = CloudRuntimeException.class)
+ public void testRestoreVMWithVolumeSnapshots() throws
ResourceUnavailableException, InsufficientCapacityException {
+ CallContext callContextMock = Mockito.mock(CallContext.class);
+
Mockito.lenient().doReturn(accountMock).when(callContextMock).getCallingAccount();
+
Mockito.lenient().doNothing().when(accountManager).checkAccess(accountMock,
null, true, userVmVoMock);
+
+ RestoreVMCmd cmd = Mockito.mock(RestoreVMCmd.class);
+ when(cmd.getVmId()).thenReturn(vmId);
+ when(cmd.getTemplateId()).thenReturn(2L);
+ when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
+
+ List<VolumeVO> volumes = new ArrayList<>();
+ long rootVolumeId = 1l;
+ VolumeVO rootVolumeOfVm = Mockito.mock(VolumeVO.class);
+ Mockito.when(rootVolumeOfVm.getId()).thenReturn(rootVolumeId);
+ volumes.add(rootVolumeOfVm);
+ when(volumeDaoMock.findByInstanceAndType(vmId,
Volume.Type.ROOT)).thenReturn(volumes);
+
+ List<SnapshotVO> snapshots = new ArrayList<>();
+ SnapshotVO snapshot = Mockito.mock(SnapshotVO.class);
+ snapshots.add(snapshot);
+ when(snapshotDaoMock.listByStatus(rootVolumeId,
Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary,
Snapshot.State.BackingUp)).thenReturn(snapshots);
+
+ userVmManagerImpl.restoreVM(cmd);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void testRestoreVirtualMachineNoOwner() throws
ResourceUnavailableException, InsufficientCapacityException {
+ long userId = 1l;
+ long accountId = 2l;
+ long newTemplateId = 2l;
+ when(accountMock.getId()).thenReturn(userId);
+ when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
+ when(userVmVoMock.getAccountId()).thenReturn(accountId);
+ when(accountDao.findById(accountId)).thenReturn(null);
+
+ userVmManagerImpl.restoreVirtualMachine(accountMock, vmId,
newTemplateId);
+ }
+
+ @Test(expected = PermissionDeniedException.class)
+ public void testRestoreVirtualMachineOwnerDisabled() throws
ResourceUnavailableException, InsufficientCapacityException {
+ long userId = 1l;
+ long accountId = 2l;
+ long newTemplateId = 2l;
+ when(accountMock.getId()).thenReturn(userId);
+ when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
+ when(userVmVoMock.getAccountId()).thenReturn(accountId);
+ when(accountDao.findById(accountId)).thenReturn(callerAccount);
+ when(callerAccount.getState()).thenReturn(Account.State.DISABLED);
+
+ userVmManagerImpl.restoreVirtualMachine(accountMock, vmId,
newTemplateId);
+ }
+
+ @Test(expected = CloudRuntimeException.class)
+ public void testRestoreVirtualMachineNotInRightState() throws
ResourceUnavailableException, InsufficientCapacityException {
+ long userId = 1l;
+ long accountId = 2l;
+ long newTemplateId = 2l;
+ when(accountMock.getId()).thenReturn(userId);
+ when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
+ when(userVmVoMock.getAccountId()).thenReturn(accountId);
+
when(userVmVoMock.getUuid()).thenReturn("a967643d-7633-4ab4-ac26-9c0b63f50cc1");
+ when(accountDao.findById(accountId)).thenReturn(callerAccount);
+
when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Starting);
+
+ userVmManagerImpl.restoreVirtualMachine(accountMock, vmId,
newTemplateId);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void testRestoreVirtualMachineNoRootVolume() throws
ResourceUnavailableException, InsufficientCapacityException {
+ long userId = 1l;
+ long accountId = 2l;
+ long currentTemplateId = 1l;
+ long newTemplateId = 2l;
+ when(accountMock.getId()).thenReturn(userId);
+ when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
+ when(userVmVoMock.getAccountId()).thenReturn(accountId);
+
when(userVmVoMock.getUuid()).thenReturn("a967643d-7633-4ab4-ac26-9c0b63f50cc1");
+ when(accountDao.findById(accountId)).thenReturn(callerAccount);
+ when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running);
+ when(userVmVoMock.getTemplateId()).thenReturn(currentTemplateId);
+
+ VMTemplateVO currentTemplate = Mockito.mock(VMTemplateVO.class);
+
when(templateDao.findById(currentTemplateId)).thenReturn(currentTemplate);
+ when(volumeDaoMock.findByInstanceAndType(vmId,
Volume.Type.ROOT)).thenReturn(new ArrayList<VolumeVO>());
+
+ userVmManagerImpl.restoreVirtualMachine(accountMock, vmId,
newTemplateId);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void testRestoreVirtualMachineMoreThanOneRootVolume() throws
ResourceUnavailableException, InsufficientCapacityException {
+ long userId = 1l;
+ long accountId = 2l;
+ long currentTemplateId = 1l;
+ long newTemplateId = 2l;
+ when(accountMock.getId()).thenReturn(userId);
+ when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
+ when(userVmVoMock.getAccountId()).thenReturn(accountId);
+
when(userVmVoMock.getUuid()).thenReturn("a967643d-7633-4ab4-ac26-9c0b63f50cc1");
+ when(accountDao.findById(accountId)).thenReturn(callerAccount);
+ when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running);
+ when(userVmVoMock.getTemplateId()).thenReturn(currentTemplateId);
+
+ VMTemplateVO currentTemplate = Mockito.mock(VMTemplateVO.class);
+ when(currentTemplate.isDeployAsIs()).thenReturn(false);
+
when(templateDao.findById(currentTemplateId)).thenReturn(currentTemplate);
+ List<VolumeVO> volumes = new ArrayList<>();
+ VolumeVO rootVolume1 = Mockito.mock(VolumeVO.class);
+ volumes.add(rootVolume1);
+ VolumeVO rootVolume2 = Mockito.mock(VolumeVO.class);
+ volumes.add(rootVolume2);
+ when(volumeDaoMock.findByInstanceAndType(vmId,
Volume.Type.ROOT)).thenReturn(volumes);
+
+ userVmManagerImpl.restoreVirtualMachine(accountMock, vmId,
newTemplateId);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void testRestoreVirtualMachineWithVMSnapshots() throws
ResourceUnavailableException, InsufficientCapacityException {
+ long userId = 1l;
+ long accountId = 2l;
+ long currentTemplateId = 1l;
+ long newTemplateId = 2l;
+ when(accountMock.getId()).thenReturn(userId);
+ when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
+ when(userVmVoMock.getAccountId()).thenReturn(accountId);
+ when(accountDao.findById(accountId)).thenReturn(callerAccount);
+ when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running);
+ when(userVmVoMock.getTemplateId()).thenReturn(currentTemplateId);
+
+ VMTemplateVO currentTemplate = Mockito.mock(VMTemplateVO.class);
+
when(templateDao.findById(currentTemplateId)).thenReturn(currentTemplate);
+ List<VolumeVO> volumes = new ArrayList<>();
+ VolumeVO rootVolumeOfVm = Mockito.mock(VolumeVO.class);
+ volumes.add(rootVolumeOfVm);
+ when(volumeDaoMock.findByInstanceAndType(vmId,
Volume.Type.ROOT)).thenReturn(volumes);
+ List<VMSnapshotVO> vmSnapshots = new ArrayList<>();
+ VMSnapshotVO vmSnapshot = Mockito.mock(VMSnapshotVO.class);
+ vmSnapshots.add(vmSnapshot);
+ when(vmSnapshotDaoMock.findByVm(vmId)).thenReturn(vmSnapshots);
+
+ userVmManagerImpl.restoreVirtualMachine(accountMock, vmId,
newTemplateId);
+ }
}
