From 2df68021761adbd93eecda20114894c8f2edb8bd Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Mon, 5 Feb 2024 13:28:28 +0530 Subject: [PATCH] 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 --- .../java/com/cloud/vm/UserVmManagerImpl.java | 100 ++++++---- .../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 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 ( !Volume.State.Allocated.equals(root.getState()) || newTemplateId != null ) { + final UserVmVO userVm = vm; + Pair vmAndNewVol = Transaction.execute(new TransactionCallbackWithException, CloudRuntimeException>() { + @Override + public Pair 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); - vm.setIsoId(newTemplateId); - vm.setGuestOSId(template.getGuestOSId()); - vm.setTemplateId(newTemplateId); - } else { - newVol = volumeMgr.allocateDuplicateVolume(root, newTemplateId); - vm.setGuestOSId(template.getGuestOSId()); - vm.setTemplateId(newTemplateId); + /* 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); + + return new Pair<>(userVm, newVol); } - // 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 (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); - } - } - - // 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 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 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()); + + 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 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 volumes = new ArrayList<>(); + VolumeVO rootVolumeOfVm = Mockito.mock(VolumeVO.class); + volumes.add(rootVolumeOfVm); + when(volumeDaoMock.findByInstanceAndType(vmId, Volume.Type.ROOT)).thenReturn(volumes); + List vmSnapshots = new ArrayList<>(); + VMSnapshotVO vmSnapshot = Mockito.mock(VMSnapshotVO.class); + vmSnapshots.add(vmSnapshot); + when(vmSnapshotDaoMock.findByVm(vmId)).thenReturn(vmSnapshots); + + userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId); + } }