KVM: Fix agents dont reconnect post maintenance

agent/src/com/cloud/agent/Agent.java

@@ -606,9 +606,7 @@ protected void processRequest(final Request request, final Link link) {
                         System.exit(1);
                         return;
                     } else if (cmd instanceof MaintainCommand) {
-                        s_logger.debug("Received maintainCommand");
-                        cancelTasks();
-                        _reconnectAllowed = false;
+                        s_logger.debug("Received maintainCommand, do not cancel current tasks");
                         answer = new MaintainAnswer((MaintainCommand)cmd);
                     } else if (cmd instanceof AgentControlCommand) {
                         answer = null;

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

@@ -52,6 +52,11 @@ public interface ResourceManager extends ResourceService, Configurable {
             "Number of retries when preparing a host into Maintenance Mode is faulty before failing",
             true, ConfigKey.Scope.Cluster);
 
+    ConfigKey<Boolean> KvmSshToAgentEnabled = new ConfigKey<>("Advanced", Boolean.class,
+            "kvm.ssh.to.agent","true",
+            "Number of retries when preparing a host into Maintenance Mode is faulty before failing",
+            false);
+
     /**
      * Register a listener for different types of resource life cycle events.
      * There can only be one type of listener per type of host.

engine/orchestration/src/com/cloud/agent/manager/AgentAttache.java

@@ -32,6 +32,7 @@
 import java.util.concurrent.TimeUnit;
 
 import com.cloud.agent.api.ModifySshKeysCommand;
+import com.cloud.agent.api.ModifyStoragePoolCommand;
 import org.apache.cloudstack.managed.context.ManagedContextRunnable;
 import org.apache.log4j.Logger;
 
@@ -115,7 +116,7 @@ public int compare(final Object o1, final Object o2) {
         StopCommand.class.toString(), CheckVirtualMachineCommand.class.toString(), PingTestCommand.class.toString(), CheckHealthCommand.class.toString(),
         ReadyCommand.class.toString(), ShutdownCommand.class.toString(), SetupCommand.class.toString(),
         CleanupNetworkRulesCmd.class.toString(), CheckNetworkCommand.class.toString(), PvlanSetupCommand.class.toString(), CheckOnHostCommand.class.toString(),
-        ModifyTargetsCommand.class.toString(), ModifySshKeysCommand.class.toString()};
+        ModifyTargetsCommand.class.toString(), ModifySshKeysCommand.class.toString(), ModifyStoragePoolCommand.class.toString()};
     protected final static String[] s_commandsNotAllowedInConnectingMode = new String[] { StartCommand.class.toString(), CreateCommand.class.toString() };
     static {
         Arrays.sort(s_commandsAllowedInMaintenanceMode);

server/src/com/cloud/configuration/Config.java

@@ -1211,14 +1211,6 @@ public enum Config {
     KvmPublicNetwork("Hidden", ManagementServer.class, String.class, "kvm.public.network.device", null, "Specify the public bridge on host for public network", null),
     KvmPrivateNetwork("Hidden", ManagementServer.class, String.class, "kvm.private.network.device", null, "Specify the private bridge on host for private network", null),
     KvmGuestNetwork("Hidden", ManagementServer.class, String.class, "kvm.guest.network.device", null, "Specify the private bridge on host for private network", null),
-    KvmSshToAgentEnabled(
-            "Advanced",
-            ManagementServer.class,
-            Boolean.class,
-            "kvm.ssh.to.agent",
-            "true",
-            "Specify whether or not the management server is allowed to SSH into KVM Agents",
-            null),
 
     // Hyperv
     HypervPublicNetwork(

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

@@ -31,6 +31,7 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.utils.Pair;
 import com.cloud.vm.dao.UserVmDetailsDao;
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.commons.lang.ObjectUtils;
@@ -2344,45 +2345,76 @@ private boolean doCancelMaintenance(final long hostId) {
             }
         }
 
+        handleAgentIfNotConnected(host, vms_migrating);
+
         try {
             resourceStateTransitTo(host, ResourceState.Event.AdminCancelMaintenance, _nodeId);
             _agentMgr.pullAgentOutMaintenance(hostId);
             retryHostMaintenance.remove(hostId);
+        } catch (final NoTransitionException e) {
+            s_logger.debug("Cannot transmit host " + host.getId() + "to Enabled state", e);
+            return false;
+        }
 
-            // for kvm, need to log into kvm host, restart cloudstack-agent
-            if ((host.getHypervisorType() == HypervisorType.KVM && !vms_migrating) || host.getHypervisorType() == HypervisorType.LXC) {
+        return true;
 
-                final boolean sshToAgent = Boolean.parseBoolean(_configDao.getValue(Config.KvmSshToAgentEnabled.key()));
-                if (!sshToAgent) {
-                    s_logger.info("Configuration tells us not to SSH into Agents. Please restart the Agent (" + hostId + ")  manually");
-                    return true;
-                }
+    }
 
-                _hostDao.loadDetails(host);
-                final String password = host.getDetail("password");
-                final String username = host.getDetail("username");
-                if (password == null || username == null) {
-                    s_logger.debug("Can't find password/username");
-                    return false;
-                }
-                final com.trilead.ssh2.Connection connection = SSHCmdHelper.acquireAuthorizedConnection(host.getPrivateIpAddress(), 22, username, password);
-                if (connection == null) {
-                    s_logger.debug("Failed to connect to host: " + host.getPrivateIpAddress());
-                    return false;
-                }
+    /**
+     * Handle agent (if available) if its not connected before cancelling maintenance.
+     * Agent must be connected before cancelling maintenance.
+     * If the host status is not Up:
+     * - If kvm.ssh.to.agent is true, then SSH into the host and restart the agent.
+     * - If kvm.shh.to.agent is false, then fail cancelling maintenance
+     */
+    protected void handleAgentIfNotConnected(HostVO host, boolean vmsMigrating) {
+        final boolean isAgentOnHost = host.getHypervisorType() == HypervisorType.KVM ||
+                host.getHypervisorType() == HypervisorType.LXC;
+        if (!isAgentOnHost || vmsMigrating || host.getStatus() == Status.Up) {
+            return;
+        }
+        final boolean sshToAgent = Boolean.parseBoolean(_configDao.getValue(KvmSshToAgentEnabled.key()));
+        if (sshToAgent) {
+            Pair<String, String> credentials = getHostCredentials(host);
+            connectAndRestartAgentOnHost(host, credentials.first(), credentials.second());
+        } else {
+            throw new CloudRuntimeException("SSH access is disabled, cannot cancel maintenance mode as " +
+                    "host agent is not connected");
+        }
+    }
 
-                try {
-                    SSHCmdHelper.SSHCmdResult result = SSHCmdHelper.sshExecuteCmdOneShot(connection, "service cloudstack-agent restart");
-                    s_logger.debug("cloudstack-agent restart result: " + result.toString());
-                } catch (final SshException e) {
-                    return false;
-                }
-            }
+    /**
+     * Get host credentials
+     * @throws CloudRuntimeException if username or password are not found
+     */
+    protected Pair<String, String> getHostCredentials(HostVO host) {
+        _hostDao.loadDetails(host);
+        final String password = host.getDetail("password");
+        final String username = host.getDetail("username");
+        if (password == null || username == null) {
+            throw new CloudRuntimeException("SSH to agent is enabled, but username/password credentials are not found");
+        }
+        return new Pair<>(username, password);
+    }
 
-            return true;
-        } catch (final NoTransitionException e) {
-            s_logger.debug("Cannot transmit host " + host.getId() + "to Enabled state", e);
-            return false;
+    /**
+     * True if agent is restarted via SSH. Assumes kvm.ssh.to.agent = true and host status is not Up
+     */
+    protected void connectAndRestartAgentOnHost(HostVO host, String username, String password) {
+        final com.trilead.ssh2.Connection connection = SSHCmdHelper.acquireAuthorizedConnection(
+                host.getPrivateIpAddress(), 22, username, password);
+        if (connection == null) {
+            throw new CloudRuntimeException("SSH to agent is enabled, but failed to connect to host: " + host.getPrivateIpAddress());
+        }
+        try {
+            SSHCmdHelper.SSHCmdResult result = SSHCmdHelper.sshExecuteCmdOneShot(
+                    connection, "service cloudstack-agent restart");
+            if (result.getReturnCode() != 0) {
+                throw new CloudRuntimeException("Could not restart agent on host " + host.getId() + " due to: " + result.getStdErr());
+            }
+            s_logger.debug("cloudstack-agent restart result: " + result.toString());
+        } catch (final SshException e) {
+            throw new CloudRuntimeException("SSH to agent is enabled, but agent restart failed", e);
         }
     }
 

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

@@ -25,13 +25,20 @@
 import com.cloud.ha.HighAvailabilityManager;
 import com.cloud.host.Host;
 import com.cloud.host.HostVO;
+import com.cloud.host.Status;
 import com.cloud.host.dao.HostDao;
 import com.cloud.hypervisor.Hypervisor;
 import com.cloud.storage.StorageManager;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.fsm.NoTransitionException;
+import com.cloud.utils.ssh.SSHCmdHelper;
+import com.cloud.utils.ssh.SshException;
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.dao.UserVmDetailsDao;
 import com.cloud.vm.dao.VMInstanceDao;
+import com.trilead.ssh2.Connection;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -56,12 +63,13 @@
 import static org.mockito.Matchers.anyLong;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @RunWith(PowerMockRunner.class)
-@PrepareForTest({ActionEventUtils.class, ResourceManagerImpl.class})
+@PrepareForTest({ActionEventUtils.class, ResourceManagerImpl.class, SSHCmdHelper.class})
 public class ResourceManagerImplTest {
 
     @Mock
@@ -78,6 +86,8 @@ public class ResourceManagerImplTest {
     private HostDao hostDao;
     @Mock
     private VMInstanceDao vmInstanceDao;
+    @Mock
+    private ConfigurationDao configurationDao;
 
     @Spy
     @InjectMocks
@@ -99,7 +109,13 @@ public class ResourceManagerImplTest {
     @Mock
     private GetVncPortCommand getVncPortCommandVm2;
 
+    @Mock
+    private Connection sshConnection;
+
     private static long hostId = 1L;
+    private static final String hostUsername = "user";
+    private static final String hostPassword = "password";
+    private static final String hostPrivateIp = "192.168.1.10";
 
     private static long vm1Id = 1L;
     private static String vm1InstanceName = "i-1-VM";
@@ -117,9 +133,13 @@ public void setup() throws Exception {
         when(host.getType()).thenReturn(Host.Type.Routing);
         when(host.getId()).thenReturn(hostId);
         when(host.getResourceState()).thenReturn(ResourceState.Enabled);
-        when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.VMware);
+        when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM);
         when(host.getClusterId()).thenReturn(1L);
         when(hostDao.findById(hostId)).thenReturn(host);
+        when(host.getDetail("username")).thenReturn(hostUsername);
+        when(host.getDetail("password")).thenReturn(hostPassword);
+        when(host.getStatus()).thenReturn(Status.Up);
+        when(host.getPrivateIpAddress()).thenReturn(hostPrivateIp);
         when(vm1.getId()).thenReturn(vm1Id);
         when(vm2.getId()).thenReturn(vm2Id);
         when(vm1.getInstanceName()).thenReturn(vm1InstanceName);
@@ -138,6 +158,15 @@ public void setup() throws Exception {
         PowerMockito.whenNew(GetVncPortCommand.class).withArguments(vm2Id, vm2InstanceName).thenReturn(getVncPortCommandVm2);
         when(agentManager.easySend(eq(hostId), eq(getVncPortCommandVm1))).thenReturn(getVncPortAnswerVm1);
         when(agentManager.easySend(eq(hostId), eq(getVncPortCommandVm2))).thenReturn(getVncPortAnswerVm2);
+
+        PowerMockito.mockStatic(SSHCmdHelper.class);
+        BDDMockito.given(SSHCmdHelper.acquireAuthorizedConnection(eq(hostPrivateIp), eq(22),
+                eq(hostUsername), eq(hostPassword))).willReturn(sshConnection);
+        BDDMockito.given(SSHCmdHelper.sshExecuteCmdOneShot(eq(sshConnection),
+                eq("service cloudstack-agent restart"))).
+                willReturn(new SSHCmdHelper.SSHCmdResult(0,"",""));
+
+        when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("true");
     }
 
     @Test
@@ -206,4 +235,76 @@ public void testCheckAndMaintainErrorInMaintenanceRetries() throws NoTransitionE
         verify(resourceManager, times(retries + 1)).isHostInMaintenance(host, failedMigrations, new ArrayList<>(), failedMigrations);
         verify(resourceManager).setHostIntoErrorInMaintenance(host, failedMigrations);
     }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void testGetHostCredentialsMissingParameter() {
+        when(host.getDetail("password")).thenReturn(null);
+        resourceManager.getHostCredentials(host);
+    }
+
+    @Test
+    public void testGetHostCredentials() {
+        Pair<String, String> credentials = resourceManager.getHostCredentials(host);
+        Assert.assertNotNull(credentials);
+        Assert.assertEquals(hostUsername, credentials.first());
+        Assert.assertEquals(hostPassword, credentials.second());
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void testConnectAndRestartAgentOnHostCannotConnect() {
+        BDDMockito.given(SSHCmdHelper.acquireAuthorizedConnection(eq(hostPrivateIp), eq(22),
+                eq(hostUsername), eq(hostPassword))).willReturn(null);
+        resourceManager.connectAndRestartAgentOnHost(host, hostUsername, hostPassword);
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void testConnectAndRestartAgentOnHostCannotRestart() throws Exception {
+        BDDMockito.given(SSHCmdHelper.sshExecuteCmdOneShot(eq(sshConnection),
+                eq("service cloudstack-agent restart"))).willThrow(new SshException("exception"));
+        resourceManager.connectAndRestartAgentOnHost(host, hostUsername, hostPassword);
+    }
+
+    @Test
+    public void testConnectAndRestartAgentOnHost() {
+        resourceManager.connectAndRestartAgentOnHost(host, hostUsername, hostPassword);
+    }
+
+    @Test
+    public void testHandleAgentSSHEnabledNotConnectedAgent() {
+        when(host.getStatus()).thenReturn(Status.Disconnected);
+        resourceManager.handleAgentIfNotConnected(host, false);
+        verify(resourceManager).getHostCredentials(eq(host));
+        verify(resourceManager).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword));
+    }
+
+    @Test
+    public void testHandleAgentSSHEnabledConnectedAgent() {
+        when(host.getStatus()).thenReturn(Status.Up);
+        resourceManager.handleAgentIfNotConnected(host, false);
+        verify(resourceManager, never()).getHostCredentials(eq(host));
+        verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword));
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void testHandleAgentSSHDisabledNotConnectedAgent() {
+        when(host.getStatus()).thenReturn(Status.Disconnected);
+        when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("false");
+        resourceManager.handleAgentIfNotConnected(host, false);
+    }
+
+    @Test
+    public void testHandleAgentSSHDisabledConnectedAgent() {
+        when(host.getStatus()).thenReturn(Status.Up);
+        when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("false");
+        resourceManager.handleAgentIfNotConnected(host, false);
+        verify(resourceManager, never()).getHostCredentials(eq(host));
+        verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword));
+    }
+
+    @Test
+    public void testHandleAgentVMsMigrating() {
+        resourceManager.handleAgentIfNotConnected(host, true);
+        verify(resourceManager, never()).getHostCredentials(eq(host));
+        verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword));
+    }
 }