Update MS state after executing cluster cmd in the target MS, and some code improvements

Author: sureshanaparti

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

@@ -273,8 +273,6 @@ public boolean configure(final String name, final Map<String, Object> params) th
 
         _executor = new ThreadPoolExecutor(agentTaskThreads, agentTaskThreads, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), new NamedThreadFactory("AgentTaskPool"));
 
-        initConnectExecutor();
-
         maxConcurrentNewAgentConnections = RemoteAgentMaxConcurrentNewConnections.value();
 
         _connection = new NioServer("AgentManager", Port.value(), Workers.value() + 10,
@@ -828,6 +826,7 @@ public boolean start() {
             return true;
         }
 
+        initConnectExecutor();
         startDirectlyConnectedHosts(false);
 
         if (_connection != null) {

plugins/maintenance/src/main/java/org/apache/cloudstack/maintenance/ManagementServerMaintenanceManager.java

@@ -85,7 +85,7 @@ public interface ManagementServerMaintenanceManager {
     // Indicates whether the current management server is preparing to maintenance
     boolean isPreparingForMaintenance();
 
-    void resetPreparingForMaintenance();
+    void resetMaintenanceParams();
 
     long getMaintenanceStartTime();
 

plugins/maintenance/src/main/java/org/apache/cloudstack/maintenance/ManagementServerMaintenanceManagerImpl.java

@@ -198,13 +198,20 @@ public boolean isPreparingForShutdown() {
         return preparingForShutdown;
     }
 
+    private void resetShutdownParams() {
+        logger.debug("Resetting shutdown params");
+        preparingForShutdown = false;
+        shutdownTriggered = false;
+    }
+
     @Override
     public boolean isPreparingForMaintenance() {
         return preparingForMaintenance;
     }
 
     @Override
-    public void resetPreparingForMaintenance() {
+    public void resetMaintenanceParams() {
+        logger.debug("Resetting maintenance params");
         preparingForMaintenance = false;
         maintenanceStartTime = 0;
         lbAlgorithm = null;
@@ -237,6 +244,8 @@ public void triggerShutdown() {
         }
         this.shutdownTriggered = true;
         prepareForShutdown(true);
+        ManagementServerHostVO msHost = msHostDao.findByMsid(ManagementServerNode.getManagementServerId());
+        msHostDao.updateState(msHost.getId(), State.ShuttingDown);
     }
 
     private void prepareForShutdown(boolean postTrigger) {
@@ -259,19 +268,22 @@ private void prepareForShutdown(boolean postTrigger) {
     @Override
     public void prepareForShutdown() {
         prepareForShutdown(false);
+        ManagementServerHostVO msHost = msHostDao.findByMsid(ManagementServerNode.getManagementServerId());
+        msHostDao.updateState(msHost.getId(), State.PreparingForShutDown);
     }
 
     @Override
     public void cancelShutdown() {
-        if (!this.preparingForShutdown) {
+        ManagementServerHostVO msHost = msHostDao.findByMsid(ManagementServerNode.getManagementServerId());
+        if (!this.preparingForShutdown && !(State.PreparingForShutDown.equals(msHost.getState()) || State.ReadyToShutDown.equals(msHost.getState()))) {
             throw new CloudRuntimeException("Shutdown has not been triggered");
         }
 
-        this.preparingForShutdown = false;
-        this.shutdownTriggered = false;
-        resetPreparingForMaintenance();
+        resetShutdownParams();
+        resetMaintenanceParams();
         jobManager.enableAsyncJobs();
         cancelWaitForPendingJobs();
+        msHostDao.updateState(msHost.getId(), State.Up);
     }
 
     @Override
@@ -289,19 +301,20 @@ public void prepareForMaintenance(String lbAlorithm, boolean forced) {
         jobManager.disableAsyncJobs();
         onPreparingForMaintenance();
         waitForPendingJobs(forced);
+        ManagementServerHostVO msHost = msHostDao.findByMsid(ManagementServerNode.getManagementServerId());
+        msHostDao.updateState(msHost.getId(), State.PreparingForMaintenance);
     }
 
     @Override
     public void cancelMaintenance() {
-        if (!this.preparingForMaintenance) {
+        ManagementServerHostVO msHost = msHostDao.findByMsid(ManagementServerNode.getManagementServerId());
+        if (!this.preparingForMaintenance && !(State.Maintenance.equals(msHost.getState()) || State.PreparingForMaintenance.equals(msHost.getState()))) {
             throw new CloudRuntimeException("Maintenance has not been initiated");
         }
-        resetPreparingForMaintenance();
-        this.preparingForShutdown = false;
-        this.shutdownTriggered = false;
+        resetMaintenanceParams();
+        resetShutdownParams();
         jobManager.enableAsyncJobs();
         cancelWaitForPendingJobs();
-        ManagementServerHostVO msHost = msHostDao.findByMsid(ManagementServerNode.getManagementServerId());
         if (msHost != null) {
             if (State.PreparingForMaintenance.equals(msHost.getState())) {
                 onCancelPreparingForMaintenance();
@@ -310,6 +323,8 @@ public void cancelMaintenance() {
                 onCancelMaintenance();
             }
         }
+
+        msHostDao.updateState(msHost.getId(), State.Up);
     }
 
     private void waitForPendingJobs(boolean forceMaintenance) {
@@ -352,7 +367,6 @@ public ManagementServerMaintenanceResponse prepareForShutdown(PrepareForShutdown
         cmds[0] = new PrepareForShutdownManagementServerHostCommand(msHost.getMsid());
         executeCmd(msHost, cmds);
 
-        msHostDao.updateState(msHost.getId(), State.PreparingForShutDown);
         return prepareMaintenanceResponse(cmd.getManagementServerId());
     }
 
@@ -378,7 +392,6 @@ public ManagementServerMaintenanceResponse triggerShutdown(TriggerShutdownCmd cm
         cmds[0] = new TriggerShutdownManagementServerHostCommand(msHost.getMsid());
         executeCmd(msHost, cmds);
 
-        msHostDao.updateState(msHost.getId(), State.ShuttingDown);
         return prepareMaintenanceResponse(cmd.getManagementServerId());
     }
 
@@ -398,7 +411,6 @@ public ManagementServerMaintenanceResponse cancelShutdown(CancelShutdownCmd cmd)
         cmds[0] = new CancelShutdownManagementServerHostCommand(msHost.getMsid());
         executeCmd(msHost, cmds);
 
-        msHostDao.updateState(msHost.getId(), State.Up);
         return prepareMaintenanceResponse(cmd.getManagementServerId());
     }
 
@@ -444,7 +456,6 @@ public ManagementServerMaintenanceResponse prepareForMaintenance(PrepareForMaint
         cmds[0] = new PrepareForMaintenanceManagementServerHostCommand(msHost.getMsid(), cmd.getAlgorithm(), cmd.isForced());
         executeCmd(msHost, cmds);
 
-        msHostDao.updateState(msHost.getId(), State.PreparingForMaintenance);
         return prepareMaintenanceResponse(cmd.getManagementServerId());
     }
 
@@ -464,12 +475,11 @@ public ManagementServerMaintenanceResponse cancelMaintenance(CancelMaintenanceCm
         cmds[0] = new CancelMaintenanceManagementServerHostCommand(msHost.getMsid());
         executeCmd(msHost, cmds);
 
-        msHostDao.updateState(msHost.getId(), State.Up);
-
         if (cmd.getRebalance()) {
             logger.info("Propagate MS list and rebalance indirect agents");
             indirectAgentLB.propagateMSListToAgents(true);
         }
+
         return prepareMaintenanceResponse(cmd.getManagementServerId());
     }
 
@@ -494,9 +504,8 @@ private void executeCmd(ManagementServerHostVO msHost, Command[] cmds) {
 
     @Override
     public void cancelPreparingForMaintenance(ManagementServerHostVO msHost) {
-        resetPreparingForMaintenance();
-        this.preparingForShutdown = false;
-        this.shutdownTriggered = false;
+        resetMaintenanceParams();
+        resetShutdownParams();
         jobManager.enableAsyncJobs();
         if (msHost == null) {
             msHost = msHostDao.findByMsid(ManagementServerNode.getManagementServerId());
@@ -602,8 +611,8 @@ protected void runInContext() {
 
                 long totalAgents = totalAgentsInMs();
 
-                String msg = String.format("Checking for triggered maintenance or shutdown... shutdownTriggered [%b] AllowAsyncJobs [%b] PendingJobCount [%d] AgentsCount [%d]",
-                        managementServerMaintenanceManager.isShutdownTriggered(), managementServerMaintenanceManager.isAsyncJobsEnabled(), totalPendingJobs, totalAgents);
+                String msg = String.format("Checking for triggered maintenance or shutdown... shutdownTriggered [%b] preparingForShutdown[%b] preparingForMaintenance[%b] AllowAsyncJobs [%b] PendingJobCount [%d] AgentsCount [%d]",
+                        managementServerMaintenanceManager.isShutdownTriggered(), managementServerMaintenanceManager.isPreparingForShutdown(), managementServerMaintenanceManager.isPreparingForMaintenance(), managementServerMaintenanceManager.isAsyncJobsEnabled(), totalPendingJobs, totalAgents);
                 logger.debug(msg);
 
                 if (totalPendingJobs > 0) {

plugins/maintenance/src/test/java/org/apache/cloudstack/maintenance/ManagementServerMaintenanceManagerImplTest.java

@@ -92,6 +92,8 @@ public void countPendingJobs() {
     @Test
     public void prepareForShutdown() {
         Mockito.doNothing().when(jobManagerMock).disableAsyncJobs();
+        ManagementServerHostVO msHost = mock(ManagementServerHostVO.class);
+        Mockito.when(msHostDao.findByMsid(anyLong())).thenReturn(msHost);
         spy.prepareForShutdown();
         Mockito.verify(jobManagerMock).disableAsyncJobs();
 
@@ -106,6 +108,9 @@ public void prepareForShutdown() {
 
     @Test
     public void cancelShutdown() {
+        ManagementServerHostVO msHost = mock(ManagementServerHostVO.class);
+        Mockito.when(msHost.getState()).thenReturn(ManagementServerHost.State.Up);
+        Mockito.when(msHostDao.findByMsid(anyLong())).thenReturn(msHost);
         Assert.assertThrows(CloudRuntimeException.class, () -> {
             spy.cancelShutdown();
         });
@@ -115,6 +120,8 @@ public void cancelShutdown() {
     public void triggerShutdown() {
         Mockito.doNothing().when(jobManagerMock).disableAsyncJobs();
         Mockito.lenient().when(spy.isShutdownTriggered()).thenReturn(false);
+        ManagementServerHostVO msHost = mock(ManagementServerHostVO.class);
+        Mockito.when(msHostDao.findByMsid(anyLong())).thenReturn(msHost);
         spy.triggerShutdown();
         Mockito.verify(jobManagerMock).disableAsyncJobs();
 
@@ -305,16 +312,16 @@ public void triggerShutdownCmd() {
     @Test
     public void prepareForMaintenanceAndCancelFromMaintenanceState() {
         Mockito.doNothing().when(jobManagerMock).disableAsyncJobs();
+        ManagementServerHostVO msHost = mock(ManagementServerHostVO.class);
+        Mockito.when(msHostDao.findByMsid(anyLong())).thenReturn(msHost);
         spy.prepareForMaintenance("static", false);
         Mockito.verify(jobManagerMock).disableAsyncJobs();
 
         Assert.assertThrows(CloudRuntimeException.class, () -> {
             spy.prepareForMaintenance("static", false);
         });
 
-        ManagementServerHostVO msHost = mock(ManagementServerHostVO.class);
         Mockito.when(msHost.getState()).thenReturn(ManagementServerHost.State.Maintenance);
-        Mockito.when(msHostDao.findByMsid(anyLong())).thenReturn(msHost);
         Mockito.doNothing().when(jobManagerMock).enableAsyncJobs();
         spy.cancelMaintenance();
         Mockito.verify(jobManagerMock).enableAsyncJobs();
@@ -324,16 +331,16 @@ public void prepareForMaintenanceAndCancelFromMaintenanceState() {
     @Test
     public void prepareForMaintenanceAndCancelFromPreparingForMaintenanceState() {
         Mockito.doNothing().when(jobManagerMock).disableAsyncJobs();
+        ManagementServerHostVO msHost = mock(ManagementServerHostVO.class);
+        Mockito.when(msHostDao.findByMsid(anyLong())).thenReturn(msHost);
         spy.prepareForMaintenance("static", false);
         Mockito.verify(jobManagerMock).disableAsyncJobs();
 
         Assert.assertThrows(CloudRuntimeException.class, () -> {
             spy.prepareForMaintenance("static", false);
         });
 
-        ManagementServerHostVO msHost = mock(ManagementServerHostVO.class);
         Mockito.when(msHost.getState()).thenReturn(ManagementServerHost.State.PreparingForMaintenance);
-        Mockito.when(msHostDao.findByMsid(anyLong())).thenReturn(msHost);
         Mockito.doNothing().when(jobManagerMock).enableAsyncJobs();
         spy.cancelMaintenance();
         Mockito.verify(jobManagerMock).enableAsyncJobs();
@@ -342,6 +349,9 @@ public void prepareForMaintenanceAndCancelFromPreparingForMaintenanceState() {
 
     @Test
     public void cancelMaintenance() {
+        ManagementServerHostVO msHost = mock(ManagementServerHostVO.class);
+        Mockito.when(msHost.getState()).thenReturn(ManagementServerHost.State.Up);
+        Mockito.when(msHostDao.findByMsid(anyLong())).thenReturn(msHost);
         Assert.assertThrows(CloudRuntimeException.class, () -> {
             spy.cancelMaintenance();
         });