hardened ssh; fixed logging; fixed runscript test

compute/src/test/java/org/jclouds/compute/BaseComputeServiceLiveTest.java

@@ -232,7 +232,7 @@ public abstract class BaseComputeServiceLiveTest {
             Map<NodeMetadata, ExecResponse> responses = runScriptWithCreds(tag, simpleTemplate
                      .getImage().getOsFamily(), new Credentials(good.account, "romeo"));
             assert false : "shouldn't pass with a bad password\n" + responses;
-         } catch (SshException e) {
+         } catch (RunScriptOnNodesException e) {
             assert Throwables.getRootCause(e).getMessage().contains("Auth fail") : e;
          }
 

extensions/ssh/jsch/src/main/java/org/jclouds/ssh/jsch/JschSshClient.java

@@ -31,10 +31,12 @@ import java.net.InetSocketAddress;
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
 import javax.annotation.Resource;
-import javax.inject.Inject;
+import javax.inject.Named;
 
 import org.apache.commons.io.input.ProxyInputStream;
 import org.apache.commons.io.output.ByteArrayOutputStream;
+import org.jclouds.Constants;
+import org.jclouds.http.handlers.BackoffLimitedRetryHandler;
 import org.jclouds.logging.Logger;
 import org.jclouds.ssh.ExecResponse;
 import org.jclouds.ssh.SshClient;
@@ -44,6 +46,7 @@ import org.jclouds.util.Utils;
 import com.google.common.base.Throwables;
 import com.google.common.collect.Iterables;
 import com.google.common.io.Closeables;
+import com.google.inject.Inject;
 import com.jcraft.jsch.ChannelExec;
 import com.jcraft.jsch.ChannelSftp;
 import com.jcraft.jsch.JSch;
@@ -78,7 +81,9 @@ public class JschSshClient implements SshClient {
    private final int port;
    private final String username;
    private final String password;
-   private int sshRetries = 3;
+   @Inject(optional = true)
+   @Named(Constants.PROPERTY_MAX_RETRIES)
+   private int sshRetries = 5;
 
    @Resource
    protected Logger logger = Logger.NULL;
@@ -86,27 +91,21 @@ public class JschSshClient implements SshClient {
    private final byte[] privateKey;
    final byte[] emptyPassPhrase = new byte[0];
    private final int timeout;
+   private final BackoffLimitedRetryHandler backoffLimitedRetryHandler;
 
-   @Inject
+   public JschSshClient(BackoffLimitedRetryHandler backoffLimitedRetryHandler,
-   public JschSshClient(InetSocketAddress socket, int timeout, String username, String password) {
+            InetSocketAddress socket, int timeout, String username, String password,
+            byte[] privateKey) {
       this.host = checkNotNull(socket, "socket").getAddress();
       checkArgument(socket.getPort() > 0, "ssh port must be greater then zero" + socket.getPort());
+      checkArgument(password != null || privateKey != null, "you must specify a password or a key");
       this.port = socket.getPort();
       this.username = checkNotNull(username, "username");
-      this.password = checkNotNull(password, "password");
+      this.backoffLimitedRetryHandler = checkNotNull(backoffLimitedRetryHandler,
+               "backoffLimitedRetryHandler");
       this.timeout = timeout;
-      this.privateKey = null;
+      this.password = password;
-   }
+      this.privateKey = privateKey;
-
-   @Inject
-   public JschSshClient(InetSocketAddress socket, int timeout, String username, byte[] privateKey) {
-      this.host = checkNotNull(socket, "socket").getAddress();
-      checkArgument(socket.getPort() > 0, "ssh port must be greater then zero" + socket.getPort());
-      this.port = socket.getPort();
-      this.username = checkNotNull(username, "username");
-      this.timeout = timeout;
-      this.password = null;
-      this.privateKey = checkNotNull(privateKey, "privateKey");
    }
 
    public InputStream get(String path) {
@@ -162,6 +161,31 @@ public class JschSshClient implements SshClient {
    @PostConstruct
    public void connect() {
       disconnect();
+      RETRY_LOOP: for (int i = 0; i < sshRetries; i++) {
+         try {
+            newSession();
+            break RETRY_LOOP;
+         } catch (Exception from) {
+            disconnect();
+            String rootMessage = Throwables.getRootCause(from).getMessage();
+            if (i + 1 == sshRetries)
+               throw propagate(from);
+            if (Iterables.size(Iterables.filter(Throwables.getCausalChain(from),
+                     ConnectException.class)) >= 1
+                     || rootMessage.indexOf("Auth fail") != -1// auth fail sometimes happens in EC2
+                     || rootMessage.indexOf("invalid data") != -1
+                     || rootMessage.indexOf("invalid privatekey") != -1) {
+               backoffLimitedRetryHandler.imposeBackoffExponentialDelay(i + 1, String.format(
+                        "%s@%s:%d: connection error: %s", username, host.getHostAddress(), port,
+                        from.getMessage()));
+               continue;
+            }
+            throw propagate(from);
+         }
+      }
+   }
+
+   private void newSession() throws JSchException {
       JSch jsch = new JSch();
       session = null;
       try {
@@ -181,29 +205,7 @@ public class JschSshClient implements SshClient {
       java.util.Properties config = new java.util.Properties();
       config.put("StrictHostKeyChecking", "no");
       session.setConfig(config);
-      RETRY_LOOP: for (int i = 0; i < sshRetries; i++) {
+      session.connect();
-         try {
-            session.connect();
-            break RETRY_LOOP;
-         } catch (Exception from) {
-            String rootMessage = Throwables.getRootCause(from).getMessage();
-            if (i + 1 == sshRetries)
-               throw propagate(from);
-            if (Iterables.size(Iterables.filter(Throwables.getCausalChain(from),
-                     ConnectException.class)) >= 1
-                     || rootMessage.indexOf("Auth fail") != -1// auth fail sometimes happens in EC2
-                     || rootMessage.indexOf("invalid data") != -1
-                     || rootMessage.indexOf("invalid privatekey") != -1) {
-               try {
-                  Thread.sleep(100);
-               } catch (InterruptedException e) {
-                  throw propagate(e);
-               }
-               continue;
-            }
-            throw propagate(from);
-         }
-      }
       logger.debug("%s@%s:%d: Session connected.", username, host.getHostAddress(), port);
    }
 

extensions/ssh/jsch/src/main/java/org/jclouds/ssh/jsch/config/JschSshClientModule.java

@@ -25,6 +25,7 @@ import java.util.Map;
 import javax.inject.Named;
 
 import org.jclouds.Constants;
+import org.jclouds.http.handlers.BackoffLimitedRetryHandler;
 import org.jclouds.ssh.ConfiguresSshClient;
 import org.jclouds.ssh.SshClient;
 import org.jclouds.ssh.jsch.JschSshClient;
@@ -33,6 +34,7 @@ import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableMap;
 import com.google.inject.AbstractModule;
 import com.google.inject.Inject;
+import com.google.inject.Injector;
 import com.google.inject.Scopes;
 import com.jcraft.jsch.JSch;
 import com.jcraft.jsch.JSchException;
@@ -51,15 +53,31 @@ public class JschSshClientModule extends AbstractModule {
 
    private static class Factory implements SshClient.Factory {
       @Named(Constants.PROPERTY_CONNECTION_TIMEOUT)
-      @Inject(optional=true)
+      @Inject(optional = true)
       int timeout = 60000;
-      
+
+      private final BackoffLimitedRetryHandler backoffLimitedRetryHandler;
+      private final Injector injector;
+
+      @SuppressWarnings("unused")
+      @Inject
+      public Factory(BackoffLimitedRetryHandler backoffLimitedRetryHandler, Injector injector) {
+         this.backoffLimitedRetryHandler = backoffLimitedRetryHandler;
+         this.injector = injector;
+      }
+
       public SshClient create(InetSocketAddress socket, String username, String password) {
-         return new JschSshClient(socket, timeout, username, password);
+         SshClient client = new JschSshClient(backoffLimitedRetryHandler, socket, timeout,
+                  username, password, null);
+         injector.injectMembers(client);// add logger
+         return client;
       }
 
       public SshClient create(InetSocketAddress socket, String username, byte[] privateKey) {
-         return new JschSshClient(socket, timeout, username, privateKey);
+         SshClient client = new JschSshClient(backoffLimitedRetryHandler, socket, timeout,
+                  username, null, privateKey);
+         injector.injectMembers(client);// add logger
+         return client;
       }
 
       @Override

gogrid/src/test/java/org/jclouds/gogrid/GoGridLiveTest.java

@@ -53,6 +53,7 @@ import org.jclouds.gogrid.options.AddServerOptions;
 import org.jclouds.gogrid.options.GetImageListOptions;
 import org.jclouds.gogrid.predicates.LoadBalancerLatestJobCompleted;
 import org.jclouds.gogrid.predicates.ServerLatestJobCompleted;
+import org.jclouds.http.handlers.BackoffLimitedRetryHandler;
 import org.jclouds.logging.log4j.config.Log4JLoggingModule;
 import org.jclouds.predicates.RetryablePredicate;
 import org.jclouds.predicates.SocketOpen;
@@ -351,8 +352,8 @@ public class GoGridLiveTest {
 
       socketOpen.apply(socket);
 
-      SshClient sshClient = new JschSshClient(socket, 60000, instanceCredentials.account,
+      SshClient sshClient = new JschSshClient(new BackoffLimitedRetryHandler(), socket, 60000,
-               instanceCredentials.key);
+               instanceCredentials.account, instanceCredentials.key, null);
       sshClient.connect();
       String output = sshClient.exec("df").getOutput();
       assertTrue(output.contains("Filesystem"),