From 1e253401b0cb316764173da80164270f09570f2b Mon Sep 17 00:00:00 2001 From: SadiJr Date: Tue, 4 Apr 2023 03:49:21 -0300 Subject: [PATCH] [Veeam] Block operations in restoring VMs (#7238) Co-authored-by: SadiJr --- .../main/java/com/cloud/storage/Volume.java | 13 ++- .../com/cloud/storage/VolumeApiService.java | 3 + .../java/com/cloud/vm/VirtualMachine.java | 11 +- .../cloud/capacity/CapacityManagerImpl.java | 6 +- .../cloud/storage/VolumeApiServiceImpl.java | 2 +- .../cloudstack/backup/BackupManagerImpl.java | 107 +++++++++++++++++- .../cloudstack/backup/BackupManagerTest.java | 67 +++++++++++ 7 files changed, 195 insertions(+), 14 deletions(-) diff --git a/api/src/main/java/com/cloud/storage/Volume.java b/api/src/main/java/com/cloud/storage/Volume.java index 57db35f0c11..4a14197bd30 100644 --- a/api/src/main/java/com/cloud/storage/Volume.java +++ b/api/src/main/java/com/cloud/storage/Volume.java @@ -57,7 +57,8 @@ public interface Volume extends ControlledEntity, Identity, InternalIdentity, Ba UploadInProgress("Volume upload is in progress"), UploadError("Volume upload encountered some error"), UploadAbandoned("Volume upload is abandoned since the upload was never initiated within a specified time"), - Attaching("The volume is attaching to a VM from Ready state."); + Attaching("The volume is attaching to a VM from Ready state."), + Restoring("The volume is being restored from backup."); String _description; @@ -133,6 +134,11 @@ public interface Volume extends ControlledEntity, Identity, InternalIdentity, Ba s_fsm.addTransition(new StateMachine2.Transition(Attaching, Event.OperationSucceeded, Ready, null)); s_fsm.addTransition(new StateMachine2.Transition(Attaching, Event.OperationFailed, Ready, null)); s_fsm.addTransition(new StateMachine2.Transition(Destroy, Event.RecoverRequested, Ready, null)); + s_fsm.addTransition(new StateMachine2.Transition(Ready, Event.RestoreRequested, Restoring, null)); + s_fsm.addTransition(new StateMachine2.Transition(Expunged, Event.RestoreRequested, Restoring, null)); + s_fsm.addTransition(new StateMachine2.Transition(Destroy, Event.RestoreRequested, Restoring, null)); + s_fsm.addTransition(new StateMachine2.Transition(Restoring, Event.RestoreSucceeded, Ready, null)); + s_fsm.addTransition(new StateMachine2.Transition(Restoring, Event.RestoreFailed, Ready, null)); } } @@ -156,7 +162,10 @@ public interface Volume extends ControlledEntity, Identity, InternalIdentity, Ba ExpungingRequested, ResizeRequested, AttachRequested, - OperationTimeout; + OperationTimeout, + RestoreRequested, + RestoreSucceeded, + RestoreFailed; } /** diff --git a/api/src/main/java/com/cloud/storage/VolumeApiService.java b/api/src/main/java/com/cloud/storage/VolumeApiService.java index b85195dafae..8b7565d66ed 100644 --- a/api/src/main/java/com/cloud/storage/VolumeApiService.java +++ b/api/src/main/java/com/cloud/storage/VolumeApiService.java @@ -21,6 +21,7 @@ package com.cloud.storage; import java.net.MalformedURLException; import java.util.Map; +import com.cloud.utils.fsm.NoTransitionException; import org.apache.cloudstack.api.command.user.volume.AssignVolumeCmd; import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd; import org.apache.cloudstack.api.command.user.volume.ChangeOfferingForVolumeCmd; @@ -172,4 +173,6 @@ public interface VolumeApiService { Volume changeDiskOfferingForVolume(ChangeOfferingForVolumeCmd cmd) throws ResourceAllocationException; void publishVolumeCreationUsageEvent(Volume volume); + + boolean stateTransitTo(Volume vol, Volume.Event event) throws NoTransitionException; } diff --git a/api/src/main/java/com/cloud/vm/VirtualMachine.java b/api/src/main/java/com/cloud/vm/VirtualMachine.java index 7a0715f1ec9..34c7a42b9a2 100644 --- a/api/src/main/java/com/cloud/vm/VirtualMachine.java +++ b/api/src/main/java/com/cloud/vm/VirtualMachine.java @@ -57,7 +57,8 @@ public interface VirtualMachine extends RunningOn, ControlledEntity, Partition, Migrating(true, "VM is being migrated. host id holds to from host"), Error(false, "VM is in error"), Unknown(false, "VM state is unknown."), - Shutdown(false, "VM state is shutdown from inside"); + Shutdown(false, "VM state is shutdown from inside"), + Restoring(true, "VM is being restored from backup"); private final boolean _transitional; String _description; @@ -126,6 +127,11 @@ public interface VirtualMachine extends RunningOn, ControlledEntity, Partition, s_fsm.addTransition(new Transition(State.Expunging, VirtualMachine.Event.ExpungeOperation, State.Expunging,null)); s_fsm.addTransition(new Transition(State.Error, VirtualMachine.Event.DestroyRequested, State.Expunging, null)); s_fsm.addTransition(new Transition(State.Error, VirtualMachine.Event.ExpungeOperation, State.Expunging, null)); + s_fsm.addTransition(new Transition(State.Stopped, Event.RestoringRequested, State.Restoring, null)); + s_fsm.addTransition(new Transition(State.Expunging, Event.RestoringRequested, State.Restoring, null)); + s_fsm.addTransition(new Transition(State.Destroyed, Event.RestoringRequested, State.Restoring, null)); + s_fsm.addTransition(new Transition(State.Restoring, Event.RestoringSuccess, State.Stopped, null)); + s_fsm.addTransition(new Transition(State.Restoring, Event.RestoringFailed, State.Stopped, null)); s_fsm.addTransition(new Transition(State.Starting, VirtualMachine.Event.FollowAgentPowerOnReport, State.Running, Arrays.asList(new Impact[]{Impact.USAGE}))); s_fsm.addTransition(new Transition(State.Stopping, VirtualMachine.Event.FollowAgentPowerOnReport, State.Running, null)); @@ -210,6 +216,9 @@ public interface VirtualMachine extends RunningOn, ControlledEntity, Partition, AgentReportMigrated, RevertRequested, SnapshotRequested, + RestoringRequested, + RestoringFailed, + RestoringSuccess, // added for new VMSync logic FollowAgentPowerOnReport, diff --git a/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java b/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java index 9c5f3e0fd3b..6926f67daea 100644 --- a/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java +++ b/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java @@ -978,16 +978,12 @@ public class CapacityManagerImpl extends ManagerBase implements CapacityManager, allocateVmCapacity(vm, fromLastHost); } - if (newState == State.Stopped) { - if (vm.getType() == VirtualMachine.Type.User) { - + if (newState == State.Stopped && event != Event.RestoringFailed && event != Event.RestoringSuccess && vm.getType() == VirtualMachine.Type.User) { UserVmVO userVM = _userVMDao.findById(vm.getId()); _userVMDao.loadDetails(userVM); // free the message sent flag if it exists userVM.setDetail(VmDetailConstants.MESSAGE_RESERVED_CAPACITY_FREED_FLAG, "false"); _userVMDao.saveDetails(userVM); - - } } return true; diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 8919e0f5100..4d6861d42a8 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1655,7 +1655,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } } - protected boolean stateTransitTo(Volume vol, Volume.Event event) throws NoTransitionException { + public boolean stateTransitTo(Volume vol, Volume.Event event) throws NoTransitionException { return _volStateMachine.transitTo(vol, event, null, _volsDao); } diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index 314d263f75d..4b972dc1ebe 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -27,6 +27,9 @@ import java.util.TimeZone; import java.util.Timer; import java.util.TimerTask; +import com.cloud.storage.VolumeApiService; +import com.cloud.utils.fsm.NoTransitionException; +import com.cloud.vm.VirtualMachineManager; import javax.inject.Inject; import javax.naming.ConfigurationException; @@ -53,6 +56,7 @@ import org.apache.cloudstack.backup.dao.BackupDao; import org.apache.cloudstack.backup.dao.BackupOfferingDao; import org.apache.cloudstack.backup.dao.BackupScheduleDao; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.jobs.AsyncJobDispatcher; import org.apache.cloudstack.framework.jobs.AsyncJobManager; @@ -147,6 +151,12 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { private ApiDispatcher apiDispatcher; @Inject private AsyncJobManager asyncJobManager; + @Inject + private VirtualMachineManager virtualMachineManager; + @Inject + private VolumeApiService volumeApiService; + @Inject + private VolumeOrchestrationService volumeOrchestrationService; private AsyncJobDispatcher asyncJobDispatcher; private Timer backupTimer; @@ -585,17 +595,100 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { final BackupOffering offering = backupOfferingDao.findByIdIncludingRemoved(vm.getBackupOfferingId()); if (offering == null) { - throw new CloudRuntimeException("Failed to find backup offering of the VM backup"); + throw new CloudRuntimeException("Failed to find backup offering of the VM backup."); } - final BackupProvider backupProvider = getBackupProvider(offering.getProvider()); - if (!backupProvider.restoreVMFromBackup(vm, backup)) { - throw new CloudRuntimeException("Error restoring VM from backup ID " + backup.getId()); - } + String backupDetailsInMessage = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(backup, "uuid", "externalId", "vmId", "type", "status", "date"); + tryRestoreVM(backup, vm, offering, backupDetailsInMessage); + updateVolumeState(vm, Volume.Event.RestoreSucceeded, Volume.State.Ready); + updateVmState(vm, VirtualMachine.Event.RestoringSuccess, VirtualMachine.State.Stopped); + return importRestoredVM(vm.getDataCenterId(), vm.getDomainId(), vm.getAccountId(), vm.getUserId(), vm.getInstanceName(), vm.getHypervisorType(), backup); } + /** + * Tries to restore a VM from a backup.
+ * First update the VM state to {@link VirtualMachine.Event#RestoringRequested} and its volume states to {@link Volume.Event#RestoreRequested},
+ * and then try to restore the backup.
+ * + * If restore fails, then update the VM state to {@link VirtualMachine.Event#RestoringFailed}, and its volumes to {@link Volume.Event#RestoreFailed} and throw an {@link CloudRuntimeException}. + */ + protected void tryRestoreVM(BackupVO backup, VMInstanceVO vm, BackupOffering offering, String backupDetailsInMessage) { + try { + updateVmState(vm, VirtualMachine.Event.RestoringRequested, VirtualMachine.State.Restoring); + updateVolumeState(vm, Volume.Event.RestoreRequested, Volume.State.Restoring); + final BackupProvider backupProvider = getBackupProvider(offering.getProvider()); + if (!backupProvider.restoreVMFromBackup(vm, backup)) { + throw new CloudRuntimeException(String.format("Error restoring %s from backup [%s].", vm, backupDetailsInMessage)); + } + // The restore process is executed by a backup provider outside of ACS, I am using the catch-all (Exception) to + // ensure that no provider-side exception is missed. Therefore, we have a proper handling of exceptions, and rollbacks if needed. + } catch (Exception e) { + LOG.error(String.format("Failed to restore backup [%s] due to: [%s].", backupDetailsInMessage, e.getMessage()), e); + updateVolumeState(vm, Volume.Event.RestoreFailed, Volume.State.Ready); + updateVmState(vm, VirtualMachine.Event.RestoringFailed, VirtualMachine.State.Stopped); + throw new CloudRuntimeException(String.format("Error restoring VM from backup [%s].", backupDetailsInMessage)); + } + } + + /** + * Tries to update the state of given VM, given specified event + * @param vm The VM to update its state + * @param event The event to update the VM state + * @param next The desired state, just needed to add more context to the logs + */ + private void updateVmState(VMInstanceVO vm, VirtualMachine.Event event, VirtualMachine.State next) { + LOG.debug(String.format("Trying to update state of VM [%s] with event [%s].", vm, event)); + Transaction.execute(TransactionLegacy.CLOUD_DB, (TransactionCallback) status -> { + try { + if (!virtualMachineManager.stateTransitTo(vm, event, vm.getHostId())) { + throw new CloudRuntimeException(String.format("Unable to change state of VM [%s] to [%s].", vm, next)); + } + } catch (NoTransitionException e) { + String errMsg = String.format("Failed to update state of VM [%s] with event [%s] due to [%s].", vm, event, e.getMessage()); + LOG.error(errMsg, e); + throw new RuntimeException(errMsg); + } + return null; + }); + } + + /** + * Tries to update all volume states of given VM, given specified event + * @param vm The VM to which the volumes belong + * @param event The event to update the volume states + * @param next The desired state, just needed to add more context to the logs + */ + private void updateVolumeState(VMInstanceVO vm, Volume.Event event, Volume.State next) { + Transaction.execute(TransactionLegacy.CLOUD_DB, (TransactionCallback) status -> { + for (VolumeVO volume : volumeDao.findIncludingRemovedByInstanceAndType(vm.getId(), null)) { + tryToUpdateStateOfSpecifiedVolume(volume, event, next); + } + return null; + }); + } + + /** + * Tries to update the state of just one volume using any passed {@link Volume.Event}. Throws an {@link RuntimeException} when fails. + * @param volume The volume to update it state + * @param event The event to update the volume state + * @param next The desired state, just needed to add more context to the logs + * + */ + private void tryToUpdateStateOfSpecifiedVolume(VolumeVO volume, Volume.Event event, Volume.State next) { + LOG.debug(String.format("Trying to update state of volume [%s] with event [%s].", volume, event)); + try { + if (!volumeApiService.stateTransitTo(volume, event)) { + throw new CloudRuntimeException(String.format("Unable to change state of volume [%s] to [%s].", volume, next)); + } + } catch (NoTransitionException e) { + String errMsg = String.format("Failed to update state of volume [%s] with event [%s] due to [%s].", volume, event, e.getMessage()); + LOG.error(errMsg, e); + throw new RuntimeException(errMsg); + } + } + private Backup.VolumeInfo getVolumeInfo(List backedUpVolumes, String volumeUuid) { for (Backup.VolumeInfo volInfo : backedUpVolumes) { if (volInfo.getUuid().equals(volumeUuid)) { @@ -652,16 +745,20 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { String[] hostPossibleValues = {host.getPrivateIpAddress(), host.getName()}; String[] datastoresPossibleValues = {datastore.getUuid(), datastore.getName()}; + updateVmState(vm, VirtualMachine.Event.RestoringRequested, VirtualMachine.State.Restoring); Pair result = restoreBackedUpVolume(backedUpVolumeUuid, backup, backupProvider, hostPossibleValues, datastoresPossibleValues); if (BooleanUtils.isFalse(result.first())) { + updateVmState(vm, VirtualMachine.Event.RestoringFailed, VirtualMachine.State.Stopped); throw new CloudRuntimeException(String.format("Error restoring volume [%s] of VM [%s] to host [%s] using backup provider [%s] due to: [%s].", backedUpVolumeUuid, vm.getUuid(), host.getUuid(), backupProvider.getName(), result.second())); } if (!attachVolumeToVM(vm.getDataCenterId(), result.second(), vmFromBackup.getBackupVolumeList(), backedUpVolumeUuid, vm, datastore.getUuid(), backup)) { + updateVmState(vm, VirtualMachine.Event.RestoringFailed, VirtualMachine.State.Stopped); throw new CloudRuntimeException(String.format("Error attaching volume [%s] to VM [%s]." + backedUpVolumeUuid, vm.getUuid())); } + updateVmState(vm, VirtualMachine.Event.RestoringSuccess, VirtualMachine.State.Stopped); return true; } diff --git a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java index 077518c295e..ce95f4d4a3d 100644 --- a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java +++ b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java @@ -16,9 +16,19 @@ // under the License. package org.apache.cloudstack.backup; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; +import com.cloud.storage.Volume; +import com.cloud.storage.VolumeApiService; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.fsm.NoTransitionException; +import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VirtualMachineManager; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.command.admin.backup.UpdateBackupOfferingCmd; import org.apache.cloudstack.backup.dao.BackupOfferingDao; @@ -33,6 +43,8 @@ import org.mockito.Spy; import com.cloud.exception.InvalidParameterValueException; import com.cloud.utils.Pair; +import java.util.Collections; + public class BackupManagerTest { @Spy @InjectMocks @@ -44,6 +56,15 @@ public class BackupManagerTest { @Mock BackupProvider backupProvider; + @Mock + VirtualMachineManager virtualMachineManager; + + @Mock + VolumeApiService volumeApiService; + + @Mock + VolumeDao volumeDao; + private String[] hostPossibleValues = {"127.0.0.1", "hostname"}; private String[] datastoresPossibleValues = {"e9804933-8609-4de3-bccc-6278072a496c", "datastore-name"}; @@ -189,4 +210,50 @@ public class BackupManagerTest { Mockito.verify(backupProvider, times(4)).restoreBackedUpVolume(Mockito.any(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()); } + + @Test + public void tryRestoreVMTestRestoreSucceeded() throws NoTransitionException { + BackupOffering offering = Mockito.mock(BackupOffering.class); + VolumeVO volumeVO = Mockito.mock(VolumeVO.class); + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + BackupVO backup = Mockito.mock(BackupVO.class); + + Mockito.when(volumeDao.findIncludingRemovedByInstanceAndType(1L, null)).thenReturn(Collections.singletonList(volumeVO)); + Mockito.when(virtualMachineManager.stateTransitTo(Mockito.eq(vm), Mockito.eq(VirtualMachine.Event.RestoringRequested), Mockito.any())).thenReturn(true); + Mockito.when(volumeApiService.stateTransitTo(Mockito.eq(volumeVO), Mockito.eq(Volume.Event.RestoreRequested))).thenReturn(true); + + + + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(offering.getProvider()).thenReturn("veeam"); + Mockito.doReturn(backupProvider).when(backupManager).getBackupProvider("veeam"); + Mockito.when(backupProvider.restoreVMFromBackup(vm, backup)).thenReturn(true); + + backupManager.tryRestoreVM(backup, vm, offering, "Nothing to write here."); + } + + @Test + public void tryRestoreVMTestRestoreFails() throws NoTransitionException { + BackupOffering offering = Mockito.mock(BackupOffering.class); + VolumeVO volumeVO = Mockito.mock(VolumeVO.class); + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + BackupVO backup = Mockito.mock(BackupVO.class); + + Mockito.when(volumeDao.findIncludingRemovedByInstanceAndType(1L, null)).thenReturn(Collections.singletonList(volumeVO)); + Mockito.when(virtualMachineManager.stateTransitTo(Mockito.eq(vm), Mockito.eq(VirtualMachine.Event.RestoringRequested), Mockito.any())).thenReturn(true); + Mockito.when(volumeApiService.stateTransitTo(Mockito.eq(volumeVO), Mockito.eq(Volume.Event.RestoreRequested))).thenReturn(true); + Mockito.when(virtualMachineManager.stateTransitTo(Mockito.eq(vm), Mockito.eq(VirtualMachine.Event.RestoringFailed), Mockito.any())).thenReturn(true); + Mockito.when(volumeApiService.stateTransitTo(Mockito.eq(volumeVO), Mockito.eq(Volume.Event.RestoreFailed))).thenReturn(true); + + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(offering.getProvider()).thenReturn("veeam"); + Mockito.doReturn(backupProvider).when(backupManager).getBackupProvider("veeam"); + Mockito.when(backupProvider.restoreVMFromBackup(vm, backup)).thenReturn(false); + try { + backupManager.tryRestoreVM(backup, vm, offering, "Checking message error."); + fail("An exception is needed."); + } catch (CloudRuntimeException e) { + assertEquals("Error restoring VM from backup [Checking message error.].", e.getMessage()); + } + } }