[Veeam] Block operations in restoring VMs (#7238)

SadiJr · SadiJr · web-flow · commit 1e253401b0cb · 2023-04-04T08:49:21.000+02:00
Co-authored-by: SadiJr &lt;sadi@scclouds.com.br&gt;
diff --git a/api/src/main/java/com/cloud/storage/Volume.java b/api/src/main/java/com/cloud/storage/Volume.java
@@ -57,7 +57,8 @@ enum State {
         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 String getDescription() {
             s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Attaching, Event.OperationSucceeded, Ready, null));
             s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Attaching, Event.OperationFailed, Ready, null));
             s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Destroy, Event.RecoverRequested, Ready, null));
+            s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Ready, Event.RestoreRequested, Restoring, null));
+            s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Expunged, Event.RestoreRequested, Restoring, null));
+            s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Destroy, Event.RestoreRequested, Restoring, null));
+            s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Restoring, Event.RestoreSucceeded, Ready, null));
+            s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Restoring, Event.RestoreFailed, Ready, null));
         }
     }
 
@@ -156,7 +162,10 @@ enum Event {
         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
@@ -21,6 +21,7 @@
 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 @@ Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account acc
     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
@@ -57,7 +57,8 @@ public enum State {
         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 static StateMachine2<State, VirtualMachine.Event, VirtualMachine> getStat
             s_fsm.addTransition(new Transition<State, Event>(State.Expunging, VirtualMachine.Event.ExpungeOperation, State.Expunging,null));
             s_fsm.addTransition(new Transition<State, Event>(State.Error, VirtualMachine.Event.DestroyRequested, State.Expunging, null));
             s_fsm.addTransition(new Transition<State, Event>(State.Error, VirtualMachine.Event.ExpungeOperation, State.Expunging, null));
+            s_fsm.addTransition(new Transition<State, Event>(State.Stopped, Event.RestoringRequested, State.Restoring, null));
+            s_fsm.addTransition(new Transition<State, Event>(State.Expunging, Event.RestoringRequested, State.Restoring, null));
+            s_fsm.addTransition(new Transition<State, Event>(State.Destroyed, Event.RestoringRequested, State.Restoring, null));
+            s_fsm.addTransition(new Transition<State, Event>(State.Restoring, Event.RestoringSuccess, State.Stopped, null));
+            s_fsm.addTransition(new Transition<State, Event>(State.Restoring, Event.RestoringFailed, State.Stopped, null));
 
             s_fsm.addTransition(new Transition<State, Event>(State.Starting, VirtualMachine.Event.FollowAgentPowerOnReport, State.Running, Arrays.asList(new Impact[]{Impact.USAGE})));
             s_fsm.addTransition(new Transition<State, Event>(State.Stopping, VirtualMachine.Event.FollowAgentPowerOnReport, State.Running, null));
@@ -210,6 +216,9 @@ public enum Event {
         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
@@ -978,16 +978,12 @@ public boolean postStateTransitionEvent(StateMachine2.Transition<State, Event> t
         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
@@ -1655,7 +1655,7 @@ private void removeVolume(long volumeId) {
         }
     }
 
-    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
@@ -27,6 +27,9 @@
 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.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 boolean restoreBackup(final Long backupId) {
 
         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. <br/>
+     * First update the VM state to {@link VirtualMachine.Event#RestoringRequested} and its volume states to {@link Volume.Event#RestoreRequested}, <br/>
+     * and then try to restore the backup. <br/>
+     *
+     * 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<VMInstanceVO>) 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<VolumeVO>) 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<Backup.VolumeInfo> backedUpVolumes, String volumeUuid) {
         for (Backup.VolumeInfo volInfo : backedUpVolumes) {
             if (volInfo.getUuid().equals(volumeUuid)) {
@@ -652,16 +745,20 @@ public boolean restoreBackupVolumeAndAttachToVM(final String backedUpVolumeUuid,
         String[] hostPossibleValues = {host.getPrivateIpAddress(), host.getName()};
         String[] datastoresPossibleValues = {datastore.getUuid(), datastore.getName()};
 
+        updateVmState(vm, VirtualMachine.Event.RestoringRequested, VirtualMachine.State.Restoring);
         Pair<Boolean, String> 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
@@ -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 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 void restoreBackedUpVolumeTestHostAndDatastoreName() {
         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());
+        }
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -1655,7 +1655,7 @@ private void removeVolume(long volumeId) {`
`1655`	`1655`	`}`
`1656`	`1656`	`}`
`1657`	`1657`
`1658`		`- protected boolean stateTransitTo(Volume vol, Volume.Event event) throws NoTransitionException {`
	`1658`	`+ public boolean stateTransitTo(Volume vol, Volume.Event event) throws NoTransitionException {`
`1659`	`1659`	`return _volStateMachine.transitTo(vol, event, null, _volsDao);`
`1660`	`1660`	`}`
`1661`	`1661`