[Draft] vm: add Expunged as final VM state and enable expunge VMs in Expunging state

api/src/main/java/com/cloud/vm/VirtualMachine.java

@@ -53,7 +53,8 @@ public enum State {
         Stopping(true, "VM is being stopped.  host id has the host that it is being stopped on."),
         Stopped(false, "VM is stopped.  host id should be null."),
         Destroyed(false, "VM is marked for destroy."),
-        Expunging(true, "VM is being   expunged."),
+        Expunging(true, "VM is being expunged."),
+        Expunged(false, "VM is expunged"),
         Migrating(true, "VM is being migrated.  host id holds to from host"),
         Error(false, "VM is in error"),
         Unknown(false, "VM state is unknown."),
@@ -122,7 +123,8 @@ public static StateMachine2<State, VirtualMachine.Event, VirtualMachine> getStat
             s_fsm.addTransition(new Transition<State, Event>(State.Stopping, VirtualMachine.Event.AgentReportStopped, State.Stopped, Arrays.asList(new Impact[]{Impact.USAGE})));
             s_fsm.addTransition(new Transition<State, Event>(State.Stopping, VirtualMachine.Event.StopRequested, State.Stopping, null));
             s_fsm.addTransition(new Transition<State, Event>(State.Stopping, VirtualMachine.Event.AgentReportShutdowned, State.Stopped, Arrays.asList(new Impact[]{Impact.USAGE})));
-            s_fsm.addTransition(new Transition<State, Event>(State.Expunging, VirtualMachine.Event.OperationFailed, State.Expunging,null));
+            s_fsm.addTransition(new Transition<>(State.Expunging, VirtualMachine.Event.OperationSucceeded, State.Expunged, null));
+            s_fsm.addTransition(new Transition<>(State.Expunging, VirtualMachine.Event.OperationFailed, State.Error,null));
             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));
@@ -186,6 +188,10 @@ public static boolean isVmDestroyed(State oldState, Event e, State newState) {
 
             return false;
         }
+
+        public static boolean isVmExpungingOrExpunged(State state) {
+            return State.Expunging.equals(state) || State.Expunged.equals(state);
+        }
     }
 
     static final Set<Type> systemVMs = new HashSet<>(Arrays.asList(VirtualMachine.Type.ConsoleProxy, VirtualMachine.Type.SecondaryStorageVm));

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -561,7 +561,20 @@ public void expunge(final String vmUuid) throws ResourceUnavailableException {
     @Override
     public void advanceExpunge(final String vmUuid) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException {
         final VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
-        advanceExpunge(vm);
+        boolean result = false;
+        try {
+            advanceExpunge(vm);
+            result = true;
+        } finally {
+            if (vm != null && !State.Expunged.equals(vm.getState())) {
+                Event event = result ? Event.OperationSucceeded : Event.OperationFailed;
+                try {
+                    stateTransitTo(_vmDao.findByUuid(vmUuid), event, null, null);
+                } catch (final NoTransitionException e) {
+                    s_logger.warn(e.getMessage());
+                }
+            }
+        }
     }
 
     private boolean isValidSystemVMType(VirtualMachine vm) {
@@ -576,6 +589,12 @@ protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableExcepti
             }
             return;
         }
+        if (State.Expunged.equals(vm.getState())) {
+            if (s_logger.isDebugEnabled()) {
+                s_logger.debug("vm has already been expunged: " + vm);
+            }
+            return;
+        }
 
         advanceStop(vm.getUuid(), VmDestroyForcestop.value());
         vm = _vmDao.findByUuid(vm.getUuid());
@@ -1948,7 +1967,7 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl
             return;
         }
 
-        if (state == State.Destroyed || state == State.Expunging || state == State.Error) {
+        if (state == State.Destroyed || State.isVmExpungingOrExpunged(state) || state == State.Error) {
             if (s_logger.isDebugEnabled()) {
                 s_logger.debug("Stopped called on " + vm + " but the state is " + state);
             }
@@ -2160,7 +2179,7 @@ public boolean stateTransitTo(final VirtualMachine vm1, final VirtualMachine.Eve
     @Override
     public void destroy(final String vmUuid, final boolean expunge) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException {
         VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
-        if (vm == null || vm.getState() == State.Destroyed || vm.getState() == State.Expunging || vm.getRemoved() != null) {
+        if (vm == null || vm.getState() == State.Destroyed || State.isVmExpungingOrExpunged(vm.getState()) || vm.getRemoved() != null) {
             if (s_logger.isDebugEnabled()) {
                 s_logger.debug("Unable to find vm or vm is destroyed: " + vm);
             }
@@ -4735,6 +4754,7 @@ private void handlePowerOnReportWithNoPendingJobsOnVM(final VMInstanceVO vm) {
 
         case Destroyed:
         case Expunging:
+        case Expunged:
             s_logger.info("Receive power on report when VM is in destroyed or expunging state. vm: "
                     + vm.getId() + ", state: " + vm.getState());
             break;
@@ -4813,6 +4833,7 @@ private void handlePowerOffReportWithNoPendingJobsOnVM(final VMInstanceVO vm) {
 
         case Destroyed:
         case Expunging:
+        case Expunged:
             break;
 
         case Error:

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java

@@ -3189,7 +3189,7 @@ public boolean destroyNetwork(final long networkId, final ReservationContext con
         final List<UserVmVO> userVms = _userVmDao.listByNetworkIdAndStates(networkId);
 
         for (final UserVmVO vm : userVms) {
-            if (!(vm.getState() == VirtualMachine.State.Expunging && vm.getRemoved() != null)) {
+            if (!(VirtualMachine.State.isVmExpungingOrExpunged(vm.getState()) && vm.getRemoved() != null)) {
                 s_logger.warn("Can't delete the network, not all user vms are expunged. Vm " + vm + " is in " + vm.getState() + " state");
                 return false;
             }

engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java

@@ -33,6 +33,9 @@
 import java.util.List;
 import java.util.Map;
 
+import org.apache.cloudstack.annotation.dao.AnnotationDao;
+import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
+import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
 import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
@@ -58,11 +61,16 @@
 import com.cloud.deploy.DeploymentPlan;
 import com.cloud.deploy.DeploymentPlanner;
 import com.cloud.deploy.DeploymentPlanner.ExcludeList;
+import com.cloud.deployasis.dao.UserVmDeployAsIsDetailsDao;
 import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.host.Host;
 import com.cloud.host.HostVO;
 import com.cloud.host.dao.HostDao;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.hypervisor.HypervisorGuru;
+import com.cloud.hypervisor.HypervisorGuruManager;
 import com.cloud.service.ServiceOfferingVO;
 import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.storage.DiskOfferingVO;
@@ -75,11 +83,15 @@
 import com.cloud.storage.dao.DiskOfferingDao;
 import com.cloud.storage.dao.StoragePoolHostDao;
 import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.Pair;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.VirtualMachine.State;
 import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.dao.UserVmDetailsDao;
 import com.cloud.vm.dao.VMInstanceDao;
 
+import org.springframework.test.util.ReflectionTestUtils;
+
 @RunWith(MockitoJUnitRunner.class)
 public class VirtualMachineManagerImplTest {
 
@@ -134,6 +146,26 @@ public class VirtualMachineManagerImplTest {
     @Mock
     private DiskOfferingDao diskOfferingDaoMock;
 
+    private String vmUuid = "5f515c66-86c6-11ed-81a3-43fdbfd10c20";
+    private long vmId = 1000L;
+    @Mock
+    private HypervisorGuruManager hvGuruMgrMock;
+    @Mock
+    private HypervisorGuru hvGuruMock;
+    @Mock
+    private NetworkOrchestrationService networkMgrMock;
+    @Mock
+    private VolumeOrchestrationService volumeMgrMock;
+    @Mock
+    private VirtualMachineGuru vmGuruMock;
+    @Mock
+    private UserVmDetailsDao userVmDetailsDao;
+    @Mock
+    private UserVmDeployAsIsDetailsDao userVmDeployAsIsDetailsDao;
+    @Mock
+    private AnnotationDao annotationDao;
+
+
     @Mock
     private HostDao hostDaoMock;
     @Mock
@@ -768,4 +800,122 @@ private void prepareAndRunCheckIfNewOfferingStorageScopeMatchesStoragePool(boole
         Mockito.doReturn(isOfferingUsingLocal).when(diskOfferingMock).isUseLocalStorage();
         virtualMachineManagerImpl.checkIfNewOfferingStorageScopeMatchesStoragePool(vmInstanceMock, diskOfferingMock);
     }
+
+    @Test
+    public void expungeVmSucceed() throws OperationTimedoutException, ResourceUnavailableException, NoSuchFieldException, IllegalAccessException {
+        when(vmInstanceDaoMock.findByUuid(vmUuid)).thenReturn(vmInstanceMock);
+        when(vmInstanceMock.getId()).thenReturn(vmId);
+        when(vmInstanceMock.getUuid()).thenReturn(vmUuid);
+        when(vmInstanceMock.getRemoved()).thenReturn(null);
+        when(vmInstanceMock.getState()).thenReturn(State.Running).thenReturn(State.Stopped).thenReturn(State.Expunging);
+        when(vmInstanceMock.getHostId()).thenReturn(null);
+
+        final Field f = ConfigKey.class.getDeclaredField("_defaultValue");
+        f.setAccessible(true);
+        f.set(VirtualMachineManagerImpl.VmDestroyForcestop, "false");
+        Mockito.doNothing().when(virtualMachineManagerImpl).advanceStop(vmUuid, false);
+
+        when(vmInstanceMock.getHypervisorType()).thenReturn(HypervisorType.Any);
+        when(hvGuruMgrMock.getGuru(any())).thenReturn(hvGuruMock);
+        when(hvGuruMock.finalizeExpungeNics(any(), any())).thenReturn(new ArrayList<>());
+        Mockito.doNothing().when(networkMgrMock).cleanupNics(any());
+        when(hvGuruMock.finalizeExpungeVolumes(any())).thenReturn(new ArrayList<>());
+        Mockito.doNothing().when(volumeMgrMock).cleanupVolumes(vmId);
+
+        Map<VirtualMachine.Type, VirtualMachineGuru> vmGurus = new HashMap<>();
+        vmGurus.put(VirtualMachine.Type.User, vmGuruMock);
+        ReflectionTestUtils.setField(virtualMachineManagerImpl, "_vmGurus", vmGurus);
+        when(vmInstanceMock.getType()).thenReturn(VirtualMachine.Type.User);
+        Mockito.doNothing().when(vmGuruMock).finalizeExpunge(any());
+
+        when(vmInstanceDaoMock.updateState(any(), any(), any(), any(), any())).thenReturn(true);
+
+        virtualMachineManagerImpl.expunge(vmUuid);
+
+        Mockito.verify(vmInstanceDaoMock).updateState(State.Expunging, VirtualMachine.Event.OperationSucceeded, State.Expunged, vmInstanceMock, new Pair<>(null, null));
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void expungeVmFail1() throws OperationTimedoutException, ResourceUnavailableException, NoSuchFieldException, IllegalAccessException {
+        when(vmInstanceDaoMock.findByUuid(vmUuid)).thenReturn(vmInstanceMock);
+        when(vmInstanceMock.getUuid()).thenReturn(vmUuid);
+        when(vmInstanceMock.getRemoved()).thenReturn(null);
+        when(vmInstanceMock.getState()).thenReturn(State.Running).thenReturn(State.Stopped).thenReturn(State.Expunging);
+
+        final Field f = ConfigKey.class.getDeclaredField("_defaultValue");
+        f.setAccessible(true);
+        f.set(VirtualMachineManagerImpl.VmDestroyForcestop, "false");
+        Mockito.doNothing().when(virtualMachineManagerImpl).advanceStop(vmUuid, false);
+
+        when(vmInstanceMock.getHypervisorType()).thenReturn(HypervisorType.Any);
+        when(hvGuruMgrMock.getGuru(any())).thenReturn(hvGuruMock);
+        when(hvGuruMock.finalizeExpungeNics(any(), any())).thenReturn(new ArrayList<>());
+        Mockito.doThrow(CloudRuntimeException.class).when(networkMgrMock).cleanupNics(any());
+
+        when(vmInstanceDaoMock.updateState(any(), any(), any(), any(), any())).thenReturn(true);
+
+        virtualMachineManagerImpl.expunge(vmUuid);
+
+        Mockito.verify(vmInstanceDaoMock).updateState(State.Expunging, VirtualMachine.Event.OperationFailed, State.Error, vmInstanceMock, null);
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void expungeVmFail2() throws OperationTimedoutException, ResourceUnavailableException, NoSuchFieldException, IllegalAccessException {
+        when(vmInstanceDaoMock.findByUuid(vmUuid)).thenReturn(vmInstanceMock);
+        when(vmInstanceMock.getId()).thenReturn(vmId);
+        when(vmInstanceMock.getUuid()).thenReturn(vmUuid);
+        when(vmInstanceMock.getRemoved()).thenReturn(null);
+        when(vmInstanceMock.getState()).thenReturn(State.Running).thenReturn(State.Stopped).thenReturn(State.Expunging);
+
+        final Field f = ConfigKey.class.getDeclaredField("_defaultValue");
+        f.setAccessible(true);
+        f.set(VirtualMachineManagerImpl.VmDestroyForcestop, "false");
+        Mockito.doNothing().when(virtualMachineManagerImpl).advanceStop(vmUuid, false);
+
+        when(vmInstanceMock.getHypervisorType()).thenReturn(HypervisorType.Any);
+        when(hvGuruMgrMock.getGuru(any())).thenReturn(hvGuruMock);
+        when(hvGuruMock.finalizeExpungeNics(any(), any())).thenReturn(new ArrayList<>());
+        Mockito.doNothing().when(networkMgrMock).cleanupNics(any());
+        when(hvGuruMock.finalizeExpungeVolumes(any())).thenReturn(new ArrayList<>());
+        Mockito.doThrow(CloudRuntimeException.class).when(volumeMgrMock).cleanupVolumes(vmId);
+
+        when(vmInstanceDaoMock.updateState(any(), any(), any(), any(), any())).thenReturn(true);
+
+        virtualMachineManagerImpl.expunge(vmUuid);
+
+        Mockito.verify(vmInstanceDaoMock).updateState(State.Expunging, VirtualMachine.Event.OperationFailed, State.Error, vmInstanceMock, null);
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void expungeVmFail3() throws OperationTimedoutException, ResourceUnavailableException, NoSuchFieldException, IllegalAccessException {
+        when(vmInstanceDaoMock.findByUuid(vmUuid)).thenReturn(vmInstanceMock);
+        when(vmInstanceMock.getId()).thenReturn(vmId);
+        when(vmInstanceMock.getUuid()).thenReturn(vmUuid);
+        when(vmInstanceMock.getRemoved()).thenReturn(null);
+        when(vmInstanceMock.getState()).thenReturn(State.Running).thenReturn(State.Stopped).thenReturn(State.Expunging);
+
+        final Field f = ConfigKey.class.getDeclaredField("_defaultValue");
+        f.setAccessible(true);
+        f.set(VirtualMachineManagerImpl.VmDestroyForcestop, "false");
+        Mockito.doNothing().when(virtualMachineManagerImpl).advanceStop(vmUuid, false);
+
+        when(vmInstanceMock.getHypervisorType()).thenReturn(HypervisorType.Any);
+        when(hvGuruMgrMock.getGuru(any())).thenReturn(hvGuruMock);
+        when(hvGuruMock.finalizeExpungeNics(any(), any())).thenReturn(new ArrayList<>());
+        Mockito.doNothing().when(networkMgrMock).cleanupNics(any());
+        when(hvGuruMock.finalizeExpungeVolumes(any())).thenReturn(new ArrayList<>());
+        Mockito.doNothing().when(volumeMgrMock).cleanupVolumes(vmId);
+
+        Map<VirtualMachine.Type, VirtualMachineGuru> vmGurus = new HashMap<>();
+        vmGurus.put(VirtualMachine.Type.User, vmGuruMock);
+        ReflectionTestUtils.setField(virtualMachineManagerImpl, "_vmGurus", vmGurus);
+        when(vmInstanceMock.getType()).thenReturn(VirtualMachine.Type.User);
+        Mockito.doThrow(CloudRuntimeException.class).when(vmGuruMock).finalizeExpunge(any());
+
+        when(vmInstanceDaoMock.updateState(any(), any(), any(), any(), any())).thenReturn(true);
+
+        virtualMachineManagerImpl.expunge(vmUuid);
+
+        Mockito.verify(vmInstanceDaoMock).updateState(State.Expunging, VirtualMachine.Event.OperationFailed, State.Error, vmInstanceMock, null);
+    }
 }

engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java

@@ -502,7 +502,7 @@ public void setRemoved(Date removed) {
 
     @Override
     public String toString() {
-        return String.format("VM instance %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "id", "instanceName", "uuid", "type"));
+        return String.format("VM instance %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "id", "instanceName", "uuid", "type", "state"));
     }
 
     @Override

engine/schema/src/main/java/com/cloud/vm/dao/ConsoleProxyDaoImpl.java

@@ -147,7 +147,7 @@ public boolean remove(Long id) {
         proxy.setPrivateIpAddress(null);
 
         UpdateBuilder ub = getUpdateBuilder(proxy);
-        ub.set(proxy, "state", State.Destroyed);
+        ub.set(proxy, "state", State.Expunged);
         ub.set(proxy, "privateIpAddress", null);
         update(id, ub, proxy);
 
@@ -182,7 +182,7 @@ public List<ConsoleProxyVO> listByHostId(long hostId) {
     public List<ConsoleProxyVO> listUpByHostId(long hostId) {
         SearchCriteria<ConsoleProxyVO> sc = HostUpSearch.create();
         sc.setParameters("host", hostId);
-        sc.setParameters("states", new Object[] {State.Destroyed, State.Stopped, State.Expunging});
+        sc.setParameters("states", new Object[] {State.Destroyed, State.Stopped, State.Expunging, State.Expunged});
         return listBy(sc);
     }
 

engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java

@@ -171,7 +171,7 @@ public boolean remove(final Long id) {
         final DomainRouterVO router = createForUpdate();
         router.setPublicIpAddress(null);
         final UpdateBuilder ub = getUpdateBuilder(router);
-        ub.set(router, "state", State.Destroyed);
+        ub.set(router, "state", State.Expunged);
         update(id, ub, router);
 
         final boolean result = super.remove(id);

engine/schema/src/main/java/com/cloud/vm/dao/SecondaryStorageVmDaoImpl.java

@@ -110,7 +110,7 @@ public boolean remove(Long id) {
         proxy.setPrivateIpAddress(null);
 
         UpdateBuilder ub = getUpdateBuilder(proxy);
-        ub.set(proxy, "state", State.Destroyed);
+        ub.set(proxy, "state", State.Expunged);
         ub.set(proxy, "privateIpAddress", null);
         update(id, ub, proxy);
 
@@ -155,7 +155,7 @@ public List<SecondaryStorageVmVO> listByHostId(SecondaryStorageVm.Role role, lon
     public List<SecondaryStorageVmVO> listUpByHostId(SecondaryStorageVm.Role role, long hostId) {
         SearchCriteria<SecondaryStorageVmVO> sc = HostUpSearch.create();
         sc.setParameters("host", hostId);
-        sc.setParameters("states", new Object[] {State.Destroyed, State.Stopped, State.Expunging});
+        sc.setParameters("states", new Object[] {State.Destroyed, State.Stopped, State.Expunging, State.Expunged});
         if (role != null) {
             sc.setParameters("role", role);
         }