From 681582366afc77f00e56cfcd0c5217be02807f20 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 4 May 2010 16:49:44 -0700 Subject: [PATCH] hardened ssh; fixed logging; fixed runscript test --- .../compute/BaseComputeServiceLiveTest.java | 2 +- .../org/jclouds/ssh/jsch/JschSshClient.java | 82 ++++++++++--------- .../ssh/jsch/config/JschSshClientModule.java | 26 +++++- .../org/jclouds/gogrid/GoGridLiveTest.java | 5 +- 4 files changed, 68 insertions(+), 47 deletions(-) diff --git a/compute/src/test/java/org/jclouds/compute/BaseComputeServiceLiveTest.java b/compute/src/test/java/org/jclouds/compute/BaseComputeServiceLiveTest.java index 09a9438c09..fb0061e163 100755 --- a/compute/src/test/java/org/jclouds/compute/BaseComputeServiceLiveTest.java +++ b/compute/src/test/java/org/jclouds/compute/BaseComputeServiceLiveTest.java @@ -232,7 +232,7 @@ public abstract class BaseComputeServiceLiveTest { Map 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; } diff --git a/extensions/ssh/jsch/src/main/java/org/jclouds/ssh/jsch/JschSshClient.java b/extensions/ssh/jsch/src/main/java/org/jclouds/ssh/jsch/JschSshClient.java index dfb7a7b776..9eefa92127 100644 --- a/extensions/ssh/jsch/src/main/java/org/jclouds/ssh/jsch/JschSshClient.java +++ b/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(InetSocketAddress socket, int timeout, String username, String password) { + public JschSshClient(BackoffLimitedRetryHandler backoffLimitedRetryHandler, + 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; - } - - @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"); + this.password = password; + this.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++) { - 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); - } - } + session.connect(); logger.debug("%s@%s:%d: Session connected.", username, host.getHostAddress(), port); } diff --git a/extensions/ssh/jsch/src/main/java/org/jclouds/ssh/jsch/config/JschSshClientModule.java b/extensions/ssh/jsch/src/main/java/org/jclouds/ssh/jsch/config/JschSshClientModule.java index 9fb3307baf..a96b145fb0 100755 --- a/extensions/ssh/jsch/src/main/java/org/jclouds/ssh/jsch/config/JschSshClientModule.java +++ b/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 diff --git a/gogrid/src/test/java/org/jclouds/gogrid/GoGridLiveTest.java b/gogrid/src/test/java/org/jclouds/gogrid/GoGridLiveTest.java index dfcf6acf0a..d4c6b79673 100644 --- a/gogrid/src/test/java/org/jclouds/gogrid/GoGridLiveTest.java +++ b/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, - instanceCredentials.key); + SshClient sshClient = new JschSshClient(new BackoffLimitedRetryHandler(), socket, 60000, + instanceCredentials.account, instanceCredentials.key, null); sshClient.connect(); String output = sshClient.exec("df").getOutput(); assertTrue(output.contains("Filesystem"),