diff --git a/api/src/main/java/com/cloud/vm/VirtualMachine.java b/api/src/main/java/com/cloud/vm/VirtualMachine.java index d244de7115e8..41c9a864c9d0 100644 --- a/api/src/main/java/com/cloud/vm/VirtualMachine.java +++ b/api/src/main/java/com/cloud/vm/VirtualMachine.java @@ -124,6 +124,9 @@ public static StateMachine2 getStat s_fsm.addTransition(new Transition(State.Stopping, VirtualMachine.Event.StopRequested, State.Stopping, null)); s_fsm.addTransition(new Transition(State.Stopping, VirtualMachine.Event.AgentReportShutdowned, State.Stopped, Arrays.asList(new Impact[]{Impact.USAGE}))); s_fsm.addTransition(new Transition(State.Expunging, VirtualMachine.Event.OperationFailed, State.Expunging,null)); + // Note: In addition to the Stopped -> Error transition for failed VM creation, + // a VM can also transition from Expunging to Error on OperationFailedToError. + s_fsm.addTransition(new Transition(State.Expunging, VirtualMachine.Event.OperationFailedToError, State.Error, null)); 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)); diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index f36c851e5bb3..b3d1681b4f30 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2578,6 +2578,22 @@ public boolean expunge(UserVmVO vm) { } } + private void transitionExpungingToError(long vmId) { + try { + UserVmVO vm = _vmDao.findById(vmId); + if (vm != null && vm.getState() == State.Expunging) { + boolean transitioned = _itMgr.stateTransitTo(vm, VirtualMachine.Event.OperationFailedToError, null); + if (transitioned) { + logger.info("Transitioned VM [{}] from Expunging to Error after failed expunge", vm.getUuid()); + } else { + logger.warn("Failed to persist transition of VM [{}] from Expunging to Error after failed expunge, possibly due to concurrent update", vm.getUuid()); + } + } + } catch (NoTransitionException e) { + logger.warn("Failed to transition VM to Error state: {}", e.getMessage()); + } + } + /** * Release network resources, it was done on vm stop previously. * @param id vm id @@ -3561,8 +3577,19 @@ public UserVm destroyVm(DestroyVMCmd cmd) throws ResourceUnavailableException, C detachVolumesFromVm(vm, dataVols); UserVm destroyedVm = destroyVm(vmId, expunge); - if (expunge && !expunge(vm)) { - throw new CloudRuntimeException("Failed to expunge vm " + destroyedVm); + if (expunge) { + boolean expunged; + try { + expunged = expunge(vm); + } catch (RuntimeException e) { + logger.error("Failed to expunge VM [{}] due to: {}", vm, e.getMessage(), e); + transitionExpungingToError(vm.getId()); + throw new CloudRuntimeException("Failed to expunge VM " + vm.getUuid() + " due to: " + e.getMessage(), e); + } + if (!expunged) { + transitionExpungingToError(vm.getId()); + throw new CloudRuntimeException("Failed to expunge VM " + destroyedVm); + } } autoScaleManager.removeVmFromVmGroup(vmId); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 1ea78bff3db9..21bcec59a5d3 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -60,6 +60,7 @@ import java.util.UUID; import com.cloud.storage.dao.SnapshotPolicyDao; +import com.cloud.utils.fsm.NoTransitionException; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.api.ApiCommandResourceType; @@ -4177,4 +4178,55 @@ public void testUnmanageUserVMSuccess() { verify(userVmDao, times(1)).releaseFromLockTable(vmId); } + @Test + public void testTransitionExpungingToErrorVmInExpungingState() throws Exception { + UserVmVO vm = mock(UserVmVO.class); + when(vm.getState()).thenReturn(VirtualMachine.State.Expunging); + when(vm.getUuid()).thenReturn("test-uuid"); + when(userVmDao.findById(vmId)).thenReturn(vm); + when(virtualMachineManager.stateTransitTo(eq(vm), eq(VirtualMachine.Event.OperationFailedToError), eq(null))).thenReturn(true); + + java.lang.reflect.Method method = UserVmManagerImpl.class.getDeclaredMethod("transitionExpungingToError", long.class); + method.setAccessible(true); + method.invoke(userVmManagerImpl, vmId); + + Mockito.verify(virtualMachineManager).stateTransitTo(vm, VirtualMachine.Event.OperationFailedToError, null); + } + + @Test + public void testTransitionExpungingToErrorVmNotInExpungingState() throws Exception { + UserVmVO vm = mock(UserVmVO.class); + when(vm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(userVmDao.findById(vmId)).thenReturn(vm); + + java.lang.reflect.Method method = UserVmManagerImpl.class.getDeclaredMethod("transitionExpungingToError", long.class); + method.setAccessible(true); + method.invoke(userVmManagerImpl, vmId); + + Mockito.verify(virtualMachineManager, Mockito.never()).stateTransitTo(any(VirtualMachine.class), any(VirtualMachine.Event.class), any()); + } + + @Test + public void testTransitionExpungingToErrorVmNotFound() throws Exception { + when(userVmDao.findById(vmId)).thenReturn(null); + + java.lang.reflect.Method method = UserVmManagerImpl.class.getDeclaredMethod("transitionExpungingToError", long.class); + method.setAccessible(true); + method.invoke(userVmManagerImpl, vmId); + + Mockito.verify(virtualMachineManager, Mockito.never()).stateTransitTo(any(VirtualMachine.class), any(VirtualMachine.Event.class), any()); + } + + @Test + public void testTransitionExpungingToErrorHandlesNoTransitionException() throws Exception { + UserVmVO vm = mock(UserVmVO.class); + when(vm.getState()).thenReturn(VirtualMachine.State.Expunging); + when(userVmDao.findById(vmId)).thenReturn(vm); + when(virtualMachineManager.stateTransitTo(eq(vm), eq(VirtualMachine.Event.OperationFailedToError), eq(null))) + .thenThrow(new NoTransitionException("no transition")); + + java.lang.reflect.Method method = UserVmManagerImpl.class.getDeclaredMethod("transitionExpungingToError", long.class); + method.setAccessible(true); + method.invoke(userVmManagerImpl, vmId); + } }