Merge pull request #2 from jenkinsci/fix/fix_connection_loss

Fix/fix connection loss

src/main/java/fr/edf/jenkins/plugins/mac/MacHost.groovy

@@ -1,25 +1,21 @@
 package fr.edf.jenkins.plugins.mac
 
-import javax.annotation.Nullable
-
 import org.apache.commons.lang.StringUtils
 import org.kohsuke.stapler.AncestorInPath
 import org.kohsuke.stapler.DataBoundConstructor
 import org.kohsuke.stapler.DataBoundSetter
 import org.kohsuke.stapler.QueryParameter
 
-import fr.edf.jenkins.plugins.mac.Messages
 import fr.edf.jenkins.plugins.mac.util.FormUtils
 import hudson.Extension
 import hudson.model.Describable
 import hudson.model.Descriptor
 import hudson.model.Item
-import hudson.model.ItemGroup
 import hudson.model.Label
 import hudson.model.labels.LabelAtom
 import hudson.util.FormValidation
-import hudson.util.FormValidation.Kind
 import hudson.util.ListBoxModel
+import hudson.util.FormValidation.Kind
 import jenkins.model.Jenkins
 
 /**

src/main/java/fr/edf/jenkins/plugins/mac/connector/MacComputerConnector.groovy

@@ -1,12 +1,9 @@
 package fr.edf.jenkins.plugins.mac.connector
 
 
-import fr.edf.jenkins.plugins.mac.MacCloud
 import fr.edf.jenkins.plugins.mac.MacHost
 import fr.edf.jenkins.plugins.mac.MacUser
-import fr.edf.jenkins.plugins.mac.slave.MacSlave
 import hudson.model.AbstractDescribableImpl
-import hudson.model.TaskListener
 import hudson.slaves.ComputerLauncher
 
 abstract class MacComputerConnector extends AbstractDescribableImpl<MacComputerConnector> {

src/main/java/fr/edf/jenkins/plugins/mac/connector/MacComputerJNLPConnector.groovy

@@ -3,6 +3,7 @@ package fr.edf.jenkins.plugins.mac.connector
 
 import java.time.Instant
 
+import org.apache.commons.lang.exception.ExceptionUtils
 import org.jenkinsci.Symbol
 import org.kohsuke.stapler.DataBoundConstructor
 import org.kohsuke.stapler.DataBoundSetter
@@ -11,7 +12,6 @@ import fr.edf.jenkins.plugins.mac.MacHost
 import fr.edf.jenkins.plugins.mac.MacUser
 import fr.edf.jenkins.plugins.mac.slave.MacComputer
 import fr.edf.jenkins.plugins.mac.ssh.SSHCommand
-import fr.edf.jenkins.plugins.mac.ssh.SSHCommandException
 import hudson.Extension
 import hudson.model.Descriptor
 import hudson.model.TaskListener
@@ -83,9 +83,10 @@ class MacComputerJNLPConnector extends MacComputerConnector {
                 SSHCommand.jnlpConnect(host, user, jenkinsUrl, computer.getJnlpMac())
             }catch(Exception e) {
                 launched = false
-                String message = String.format("Error while connecting computer %s due to exception %s", computer.name, e.message)
+                String message = String.format("Error while connecting computer %s due to error %s ",
+                    computer.name, ExceptionUtils.getStackTrace(e))
                 listener.error(message)
-                throw new InterruptedException(message, e)
+                throw new InterruptedException(message)
             }
             long currentTimestamp = Instant.now().toEpochMilli()
             while(!macComputer.isOnline()) {
@@ -102,7 +103,7 @@ class MacComputerJNLPConnector extends MacComputerConnector {
                     launched = false
                     String message = toString().format("Connection timeout for the computer %s", computer.name)
                     listener.error(message)
-                    throw new InterruptedException(message, e)
+                    throw new InterruptedException(message)
                 }
             }
         }

src/main/java/fr/edf/jenkins/plugins/mac/provisioning/InProvisioning.groovy

@@ -1,7 +1,5 @@
 package fr.edf.jenkins.plugins.mac.provisioning
 
-import static java.util.stream.Collectors.toSet
-
 import javax.annotation.CheckForNull
 import javax.annotation.Nonnull
 

src/main/java/fr/edf/jenkins/plugins/mac/slave/MacComputerListener.groovy

@@ -1,6 +1,5 @@
 package fr.edf.jenkins.plugins.mac.slave
 
-import java.io.IOException
 import java.util.logging.Level
 import java.util.logging.Logger
 

src/main/java/fr/edf/jenkins/plugins/mac/slave/MacSlave.groovy

@@ -1,6 +1,5 @@
 package fr.edf.jenkins.plugins.mac.slave;
 
-import java.nio.channels.ClosedChannelException
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.logging.Level
 import java.util.logging.Logger
@@ -20,7 +19,6 @@ import hudson.model.Slave
 import hudson.model.TaskListener
 import hudson.model.Node.Mode
 import hudson.model.Slave.SlaveDescriptor
-import hudson.remoting.VirtualChannel
 import hudson.slaves.AbstractCloudSlave
 import hudson.slaves.Cloud
 import hudson.slaves.ComputerLauncher

src/main/java/fr/edf/jenkins/plugins/mac/ssh/SSHCommand.groovy

@@ -1,5 +1,6 @@
 package fr.edf.jenkins.plugins.mac.ssh
 
+import java.util.concurrent.TimeUnit
 import java.util.logging.Level
 import java.util.logging.Logger
 
@@ -8,12 +9,11 @@ import org.apache.commons.lang.StringUtils
 import org.kohsuke.accmod.Restricted
 import org.kohsuke.accmod.restrictions.NoExternalUse
 
-import com.trilead.ssh2.Connection
-
 import fr.edf.jenkins.plugins.mac.MacHost
 import fr.edf.jenkins.plugins.mac.MacUser
-import fr.edf.jenkins.plugins.mac.ssh.connection.SSHClientFactory
-import fr.edf.jenkins.plugins.mac.ssh.connection.SSHClientFactoryConfiguration
+import fr.edf.jenkins.plugins.mac.ssh.connection.SSHConnectionConfiguration
+import fr.edf.jenkins.plugins.mac.ssh.connection.SSHGlobalConnectionConfiguration
+import fr.edf.jenkins.plugins.mac.ssh.connection.SSHUserConnectionConfiguration
 import fr.edf.jenkins.plugins.mac.util.Constants
 import hudson.util.Secret
 import jenkins.model.Jenkins
@@ -33,8 +33,8 @@ class SSHCommand {
      * @return the user connected, or null if error
      */
     @Restricted(NoExternalUse)
-    static String checkConnection(Connection connection) {
-        return SSHCommandLauncher.executeCommand(connection, false,  Constants.WHOAMI)
+    static String checkConnection(SSHGlobalConnectionConfiguration config) {
+        return SSHCommandLauncher.executeCommand(config, false,  Constants.WHOAMI)
     }
 
     /**
@@ -44,23 +44,21 @@ class SSHCommand {
      */
     @Restricted(NoExternalUse)
     static MacUser createUserOnMac(MacHost macHost, MacUser user) throws Exception {
-        Connection connection = null
         try {
-            connection = SSHClientFactory.getSshClient(new SSHClientFactoryConfiguration(credentialsId: macHost.credentialsId, port: macHost.port,
+            SSHGlobalConnectionConfiguration connectionConfig = new SSHGlobalConnectionConfiguration(credentialsId: macHost.credentialsId, port: macHost.port,
             context: Jenkins.get(), host: macHost.host, connectionTimeout: macHost.connectionTimeout,
-            readTimeout: macHost.readTimeout, kexTimeout: macHost.kexTimeout))
-            LOGGER.log(Level.FINE, SSHCommandLauncher.executeCommand(connection, true, String.format(Constants.CREATE_USER, user.username, user.password.getPlainText())))
-            Thread.sleep(5000)
-            if(!isUserExist(connection, user.username)) {
+            readTimeout: macHost.readTimeout, kexTimeout: macHost.kexTimeout)
+            LOGGER.log(Level.FINE, SSHCommandLauncher.executeCommand(connectionConfig, true, String.format(Constants.CREATE_USER, user.username, user.password.getPlainText())))
+            TimeUnit.SECONDS.sleep(5)
+            if(!isUserExist(connectionConfig, user.username)) {
                 throw new Exception(String.format("The user %s wasn't created after verification", user.username))
             }
-            LOGGER.log(Level.FINE, SSHCommandLauncher.executeCommand(connection, true, String.format(Constants.CHANGE_RIGHTS_ON_USER, user.username)))
+            LOGGER.log(Level.FINE, SSHCommandLauncher.executeCommand(connectionConfig, true, String.format(Constants.CHANGE_RIGHTS_ON_USER, user.username)))
             LOGGER.log(Level.FINE, "The User {0} has been CREATED on Mac {1}", user.username, macHost.host)
-            connection.close()
             return user
         } catch(Exception e) {
-            if(null != connection) connection.close()
             final String message = String.format(SSHCommandException.CREATE_MAC_USER_ERROR_MESSAGE, macHost.host)
+            LOGGER.log(Level.SEVERE, message, e)
             throw new SSHCommandException(message, e)
         }
     }
@@ -73,22 +71,19 @@ class SSHCommand {
      */
     @Restricted(NoExternalUse)
     static void deleteUserOnMac(String username, MacHost macHost) throws Exception {
-        Connection connection = null
-        String groupname = username
         try {
-            connection = SSHClientFactory.getSshClient(new SSHClientFactoryConfiguration(credentialsId: macHost.credentialsId, port: macHost.port,
+            SSHGlobalConnectionConfiguration connectionConfig = new SSHGlobalConnectionConfiguration(credentialsId: macHost.credentialsId, port: macHost.port,
             context: Jenkins.get(), host: macHost.host, connectionTimeout: macHost.connectionTimeout,
-            readTimeout: macHost.readTimeout, kexTimeout: macHost.kexTimeout))
-            LOGGER.log(Level.FINE, SSHCommandLauncher.executeCommand(connection, true, String.format(Constants.DELETE_USER, username)))
-            Thread.sleep(5000)
-            if(isUserExist(connection, username)) {
+            readTimeout: macHost.readTimeout, kexTimeout: macHost.kexTimeout)
+            LOGGER.log(Level.FINE, SSHCommandLauncher.executeCommand(connectionConfig, true, String.format(Constants.DELETE_USER, username)))
+            TimeUnit.SECONDS.sleep(5)
+            if(isUserExist(connectionConfig, username)) {
                 throw new Exception(String.format("The user %s still exist after verification", username))
             }
             LOGGER.log(Level.FINE, "The User {0} has been DELETED from Mac {1}", username, macHost.host)
-            connection.close()
         } catch (Exception e) {
-            if(null != connection) connection.close()
             final String message = String.format(SSHCommandException.DELETE_MAC_USER_ERROR_MESSAGE, username, macHost.host)
+            LOGGER.log(Level.SEVERE, message, e)
             throw new SSHCommandException(message, e)
         }
     }
@@ -108,20 +103,15 @@ class SSHCommand {
             jenkinsUrl += "/"
         }
         String remotingUrl = jenkinsUrl + Constants.REMOTING_JAR_PATH
-        Connection connection = null
         try {
-            connection = SSHClientFactory.getUserClient(user.username, user.password, macHost.host,
-                    macHost.port, macHost.connectionTimeout, macHost.readTimeout, macHost.kexTimeout)
-            LOGGER.log(Level.FINE, SSHCommandLauncher.executeCommand(connection, false, String.format(Constants.GET_REMOTING_JAR, remotingUrl)))
-            String result = SSHCommandLauncher.executeCommand(connection, false, String.format(Constants.LAUNCH_JNLP, jenkinsUrl, user.username, slaveSecret))
-            LOGGER.log(Level.FINE, result)
-            connection.close()
+            SSHUserConnectionConfiguration connectionConfig = new SSHUserConnectionConfiguration(username: user.username, password: user.password, host: macHost.host,
+                    port: macHost.port, connectionTimeout: macHost.connectionTimeout, readTimeout: macHost.readTimeout, kexTimeout: macHost.kexTimeout)
+            LOGGER.log(Level.FINE, SSHCommandLauncher.executeCommand(connectionConfig, false, String.format(Constants.GET_REMOTING_JAR, remotingUrl)))
+            LOGGER.log(Level.FINE, SSHCommandLauncher.executeCommand(connectionConfig, false, String.format(Constants.LAUNCH_JNLP, jenkinsUrl, user.username, slaveSecret)))
             return true
         } catch(Exception e) {
-            if(null != connection) connection.close()
             final String message = String.format(SSHCommandException.JNLP_CONNECTION_ERROR_MESSAGE, macHost.host, user.username)
-            LOGGER.log(Level.WARNING, message)
-            LOGGER.log(Level.FINEST, "Exception : ", e)
+            LOGGER.log(Level.SEVERE, message, e)
             throw new SSHCommandException(message, e)
         }
     }
@@ -145,8 +135,8 @@ class SSHCommand {
      * @return true if exist, false if not
      */
     @Restricted(NoExternalUse)
-    private static boolean isUserExist(Connection connection, String username) {
-        String result = SSHCommandLauncher.executeCommand(connection, true, String.format(Constants.CHECK_USER_EXIST, username))
+    private static boolean isUserExist(SSHConnectionConfiguration connectionConfig, String username) throws Exception {
+        String result = SSHCommandLauncher.executeCommand(connectionConfig, true, String.format(Constants.CHECK_USER_EXIST, username))
         return result.trim() == username
     }
 
@@ -158,21 +148,17 @@ class SSHCommand {
      */
     @Restricted(NoExternalUse)
     static List<String> listUsers(MacHost macHost) throws SSHCommandException {
-        Connection connection = null
         try {
-            connection = SSHClientFactory.getSshClient(new SSHClientFactoryConfiguration(credentialsId: macHost.credentialsId, port: macHost.port,
+            SSHGlobalConnectionConfiguration connectionConfig = new SSHGlobalConnectionConfiguration(credentialsId: macHost.credentialsId, port: macHost.port,
             context: Jenkins.get(), host: macHost.host, connectionTimeout: macHost.connectionTimeout,
-            readTimeout: macHost.readTimeout, kexTimeout: macHost.kexTimeout))
-            String result = SSHCommandLauncher.executeCommand(connection, true, String.format(Constants.LIST_USERS, Constants.USERNAME_PATTERN.substring(0, Constants.USERNAME_PATTERN.lastIndexOf("%"))))
+            readTimeout: macHost.readTimeout, kexTimeout: macHost.kexTimeout)
+            String result = SSHCommandLauncher.executeCommand(connectionConfig, true, String.format(Constants.LIST_USERS, Constants.USERNAME_PATTERN.substring(0, Constants.USERNAME_PATTERN.lastIndexOf("%"))))
             LOGGER.log(Level.FINE, result)
-            connection.close()
             if(StringUtils.isEmpty(result)) return new ArrayList()
             return result.split(Constants.REGEX_NEW_LINE) as List
         } catch(Exception e) {
-            if(null != connection) connection.close()
             String message = String.format(SSHCommandException.LIST_USERS_ERROR_MESSAGE, macHost.host, e.getMessage())
-            LOGGER.log(Level.WARNING, message)
-            LOGGER.log(Level.FINEST, "Exception : ", e)
+            LOGGER.log(Level.SEVERE, message, e)
             throw new SSHCommandException(message, e)
         }
     }

src/main/java/fr/edf/jenkins/plugins/mac/ssh/SSHCommandLauncher.groovy

@@ -13,7 +13,8 @@ import com.trilead.ssh2.ChannelCondition
 import com.trilead.ssh2.Connection
 import com.trilead.ssh2.Session
 
-import groovy.util.logging.Slf4j
+import fr.edf.jenkins.plugins.mac.ssh.connection.SSHConnectionConfiguration
+import fr.edf.jenkins.plugins.mac.ssh.connection.SSHConnectionFactory
 
 /**
  * Runner of SSH command.
@@ -35,16 +36,19 @@ protected class SSHCommandLauncher {
      * @throws Exception if cannot execute the command or if the command return an error
      */
     @Restricted(NoExternalUse)
-    synchronized static String executeCommand(@NotNull Connection conn, @NotNull boolean ignoreError, @NotNull String command) throws Exception {
+    static String executeCommand(@NotNull SSHConnectionConfiguration connectionConfiguration, @NotNull boolean ignoreError, @NotNull String command) throws Exception {
+        Connection connection = null
         Session session = null
         try {
-            session = conn.openSession()
+            connection = SSHConnectionFactory.getSshConnection(connectionConfiguration)
+            session = connection.openSession()
             LOGGER.log(Level.FINE, "Executing command {0}", command)
             session.execCommand(command)
             session.waitForCondition(ChannelCondition.EXIT_STATUS | ChannelCondition.EXIT_SIGNAL, 5000)
             LOGGER.log(Level.FINEST, "Exit SIGNAL : {0}", session.getExitSignal())
             LOGGER.log(Level.FINEST,"Exit STATUS : {0}", null != session.getExitStatus() ? session.getExitStatus().intValue() : null)
             session.close()
+            connection.close()
             String out = convertInputStream(session.getStdout())
             String err = convertInputStream(session.getStderr())
             LOGGER.log(Level.FINEST, out)
@@ -58,6 +62,7 @@ protected class SSHCommandLauncher {
             return StringUtils.isNotEmpty(out) ? out : StringUtils.isNotEmpty(err) ? err : ""
         } catch(Exception e) {
             if(session != null) session.close()
+            if(connection != null) connection.close()
             throw e
         }
     }

src/main/java/fr/edf/jenkins/plugins/mac/ssh/connection/SSHClientFactoryConfiguration.groovy → src/main/java/fr/edf/jenkins/plugins/mac/ssh/connection/SSHConnectionConfiguration.groovy

@@ -1,18 +1,27 @@
 package fr.edf.jenkins.plugins.mac.ssh.connection
 
 import hudson.model.ModelObject
+import hudson.util.Secret
 
 /**
  * All properties needed for SshClientFactory
  * @author Mathieu DELROCQ
  *
  */
-class SSHClientFactoryConfiguration {
+abstract class SSHConnectionConfiguration {
     String host
-    String credentialsId
     Integer port
-    ModelObject context
     Integer connectionTimeout
     Integer readTimeout
     Integer kexTimeout
 }
+
+class SSHGlobalConnectionConfiguration extends SSHConnectionConfiguration {
+    String credentialsId
+    ModelObject context
+}
+
+class SSHUserConnectionConfiguration extends SSHConnectionConfiguration {
+    String username
+    Secret password
+}