Fix: API Thread held forever during force deleting across MS

Author: nvazquez

core/src/main/java/com/cloud/agent/api/PropagateResourceEventCommand.java

@@ -24,6 +24,8 @@
 public class PropagateResourceEventCommand extends Command {
     long hostId;
     ResourceState.Event event;
+    boolean forced;
+    boolean forceDeleteStorage;
 
     protected PropagateResourceEventCommand() {
 
@@ -34,6 +36,13 @@ public PropagateResourceEventCommand(long hostId, ResourceState.Event event) {
         this.event = event;
     }
 
+    public PropagateResourceEventCommand(long hostId, ResourceState.Event event, boolean forced, boolean forceDeleteStorage) {
+        this.hostId = hostId;
+        this.event = event;
+        this.forced = forced;
+        this.forceDeleteStorage = forceDeleteStorage;
+    }
+
     public long getHostId() {
         return hostId;
     }
@@ -42,6 +51,14 @@ public ResourceState.Event getEvent() {
         return event;
     }
 
+    public boolean isForced() {
+        return forced;
+    }
+
+    public boolean isForceDeleteStorage() {
+        return forceDeleteStorage;
+    }
+
     @Override
     public boolean executeInSequence() {
         // TODO Auto-generated method stub

engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java

@@ -122,6 +122,8 @@ public interface ResourceManager extends ResourceService, Configurable {
 
     public boolean executeUserRequest(long hostId, ResourceState.Event event) throws AgentUnavailableException;
 
+    boolean executeUserRequest(long hostId, ResourceState.Event event, boolean isForced, boolean isForceDeleteStorage) throws AgentUnavailableException;
+
     boolean resourceStateTransitTo(Host host, Event event, long msId) throws NoTransitionException;
 
     boolean umanageHost(long hostId);

engine/orchestration/src/main/java/com/cloud/agent/manager/ClusteredAgentManagerImpl.java

@@ -1307,11 +1307,20 @@ public String dispatch(final ClusterServicePdu pdu) {
 
                 boolean result;
                 try {
-                    result = _resourceMgr.executeUserRequest(cmd.getHostId(), cmd.getEvent());
+                    result = _resourceMgr.executeUserRequest(cmd.getHostId(), cmd.getEvent(), cmd.isForced(), cmd.isForceDeleteStorage());
                     logger.debug("Result is {}", result);
                 } catch (final AgentUnavailableException ex) {
                     logger.warn("Agent is unavailable", ex);
                     return null;
+                } catch (final RuntimeException ex) {
+                    s_logger.error(String.format("Failed to execute propagated event %s for host %d", cmd.getEvent().name(), cmd.getHostId()), ex);
+                    final Answer[] answers = new Answer[1];
+                    String details = ex.getMessage();
+                    if (details == null || details.isEmpty()) {
+                        details = ex.toString();
+                    }
+                    answers[0] = new Answer(cmd, false, details);
+                    return _gson.toJson(answers);
                 }
 
                 final Answer[] answers = new Answer[1];

server/src/main/java/com/cloud/resource/ResourceManagerImpl.java

@@ -1169,7 +1169,7 @@ protected boolean canDeleteHost(HostVO host) {
     @Override
     public boolean deleteHost(final long hostId, final boolean isForced, final boolean isForceDeleteStorage) {
         try {
-            final Boolean result = propagateResourceEvent(hostId, ResourceState.Event.DeleteHost);
+            final Boolean result = propagateResourceEvent(hostId, ResourceState.Event.DeleteHost, isForced, isForceDeleteStorage);
             if (result != null) {
                 return result;
             }
@@ -3902,13 +3902,18 @@ public boolean cancelMaintenance(final long hostId) {
     }
 
     @Override
-    public boolean executeUserRequest(final long hostId, final ResourceState.Event event) {
+    public boolean executeUserRequest(final long hostId, final ResourceState.Event event) throws AgentUnavailableException {
+        return executeUserRequest(hostId, event, false, false);
+    }
+
+    @Override
+    public boolean executeUserRequest(final long hostId, final ResourceState.Event event, final boolean isForced, final boolean isForceDeleteStorage) throws AgentUnavailableException {
         if (event == ResourceState.Event.AdminAskMaintenance) {
             return doMaintain(hostId);
         } else if (event == ResourceState.Event.AdminCancelMaintenance) {
             return doCancelMaintenance(hostId);
         } else if (event == ResourceState.Event.DeleteHost) {
-            return doDeleteHost(hostId, false, false);
+            return doDeleteHost(hostId, isForced, isForceDeleteStorage);
         } else if (event == ResourceState.Event.Unmanaged) {
             return doUmanageHost(hostId);
         } else if (event == ResourceState.Event.UpdatePassword) {
@@ -4028,14 +4033,18 @@ public String getPeerName(final long agentHostId) {
     }
 
     public Boolean propagateResourceEvent(final long agentId, final ResourceState.Event event) throws AgentUnavailableException {
+        return propagateResourceEvent(agentId, event, false, false);
+    }
+
+    public Boolean propagateResourceEvent(final long agentId, final ResourceState.Event event, final boolean isForced, final boolean isForceDeleteStorage) throws AgentUnavailableException {
         final String msPeer = getPeerName(agentId);
         if (msPeer == null) {
             return null;
         }
 
         logger.debug("Propagating resource request event:" + event.toString() + " to agent:" + agentId);
         final Command[] cmds = new Command[1];
-        cmds[0] = new PropagateResourceEventCommand(agentId, event);
+        cmds[0] = new PropagateResourceEventCommand(agentId, event, isForced, isForceDeleteStorage);
 
         final String AnsStr = _clusterMgr.execute(msPeer, agentId, _gson.toJson(cmds), true);
         if (AnsStr == null) {
@@ -4048,6 +4057,13 @@ public Boolean propagateResourceEvent(final long agentId, final ResourceState.Ev
             logger.debug("Result for agent change is " + answers[0].getResult());
         }
 
+        if (!answers[0].getResult()) {
+            final String details = answers[0].getDetails();
+            if (details != null && !details.isEmpty()) {
+                throw new CloudRuntimeException(String.format("Failed to propagate resource event %s for host %d on peer %s: %s", event, agentId, msPeer, details));
+            }
+        }
+
         return answers[0].getResult();
     }
 

server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java

@@ -318,6 +318,11 @@ public boolean executeUserRequest(final long hostId, final Event event) throws A
         return false;
     }
 
+    @Override
+    public boolean executeUserRequest(long hostId, Event event, boolean isForced, boolean isForceDeleteStorage) throws AgentUnavailableException {
+        return false;
+    }
+
     /* (non-Javadoc)
      * @see com.cloud.resource.ResourceManager#resourceStateTransitTo(com.cloud.host.Host, com.cloud.resource.ResourceState.Event, long)
      */

server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

@@ -1184,4 +1184,31 @@ public void testUpdateHostStorageAccessGroups() {
         Mockito.verify(host).setStorageAccessGroups("group1,group2");
         Mockito.verify(hostDao).update(hostId, host);
     }
+
+    @Test
+    public void executeUserRequestDeleteHostPassesForcedFlags() throws Exception {
+        Mockito.doReturn(true).when(resourceManager).doDeleteHost(anyLong(), anyBoolean(), anyBoolean());
+
+        resourceManager.executeUserRequest(hostId, ResourceState.Event.DeleteHost, true, true);
+
+        Mockito.verify(resourceManager).doDeleteHost(hostId, true, true);
+    }
+
+    @Test
+    public void executeUserRequestDeleteHostPassesNonForcedFlags() throws Exception {
+        Mockito.doReturn(true).when(resourceManager).doDeleteHost(anyLong(), anyBoolean(), anyBoolean());
+
+        resourceManager.executeUserRequest(hostId, ResourceState.Event.DeleteHost, false, false);
+
+        Mockito.verify(resourceManager).doDeleteHost(hostId, false, false);
+    }
+
+    @Test
+    public void executeUserRequestDefaultOverloadPassesFalseForDeleteHost() throws Exception {
+        Mockito.doReturn(true).when(resourceManager).doDeleteHost(anyLong(), anyBoolean(), anyBoolean());
+
+        resourceManager.executeUserRequest(hostId, ResourceState.Event.DeleteHost);
+
+        Mockito.verify(resourceManager).doDeleteHost(hostId, false, false);
+    }
 }