You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2015/10/21 14:11:59 UTC

[2/3] incubator-brooklyn git commit: BROOKLYN-186 workaround in JcloudsLocation

BROOKLYN-186 workaround in JcloudsLocation

If we get back a node with both a password and ssh key, and/or
if that node has a different user from the overridden loginUser,
then try all combinations of those credentials to try to login.


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/6ca7e267
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/6ca7e267
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/6ca7e267

Branch: refs/heads/master
Commit: 6ca7e26748e69365fe2d4d2d5231a3406197f877
Parents: ddfb6e2
Author: Aled Sage <al...@gmail.com>
Authored: Wed Oct 14 12:19:07 2015 +0200
Committer: Aled Sage <al...@gmail.com>
Committed: Wed Oct 21 00:51:15 2015 +0100

----------------------------------------------------------------------
 .../location/jclouds/JcloudsLocation.java       | 224 +++++++++++--------
 .../JcloudsLocationSecurityGroupCustomizer.java |  23 +-
 2 files changed, 154 insertions(+), 93 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6ca7e267/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
index 9a812b2..0705d26 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
@@ -23,8 +23,6 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.brooklyn.util.JavaGroovyEquivalents.elvis;
 import static org.apache.brooklyn.util.JavaGroovyEquivalents.groovyTruth;
-import static org.jclouds.compute.options.RunScriptOptions.Builder.overrideLoginCredentials;
-import static org.jclouds.scriptbuilder.domain.Statements.exec;
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
@@ -45,6 +43,7 @@ import java.util.concurrent.Callable;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -75,15 +74,52 @@ import org.apache.brooklyn.core.location.cloud.names.CloudMachineNamer;
 import org.apache.brooklyn.core.mgmt.persist.LocationWithObjectStore;
 import org.apache.brooklyn.core.mgmt.persist.PersistenceObjectStore;
 import org.apache.brooklyn.core.mgmt.persist.jclouds.JcloudsBlobStoreBasedObjectStore;
+import org.apache.brooklyn.location.jclouds.JcloudsPredicates.NodeInLocation;
 import org.apache.brooklyn.location.jclouds.networking.JcloudsPortForwarderExtension;
+import org.apache.brooklyn.location.jclouds.templates.PortableTemplateBuilder;
 import org.apache.brooklyn.location.jclouds.zone.AwsAvailabilityZoneExtension;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.ResourceUtils;
+import org.apache.brooklyn.util.core.config.ConfigBag;
+import org.apache.brooklyn.util.core.crypto.SecureKeys;
+import org.apache.brooklyn.util.core.flags.MethodCoercions;
+import org.apache.brooklyn.util.core.flags.SetFromFlag;
+import org.apache.brooklyn.util.core.flags.TypeCoercions;
+import org.apache.brooklyn.util.core.internal.ssh.ShellTool;
+import org.apache.brooklyn.util.core.internal.ssh.SshTool;
+import org.apache.brooklyn.util.core.text.TemplateProcessor;
+import org.apache.brooklyn.util.exceptions.CompoundRuntimeException;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.exceptions.ReferenceWithError;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.javalang.Enums;
+import org.apache.brooklyn.util.javalang.Reflections;
+import org.apache.brooklyn.util.net.Cidr;
+import org.apache.brooklyn.util.net.Networking;
+import org.apache.brooklyn.util.net.Protocol;
+import org.apache.brooklyn.util.os.Os;
+import org.apache.brooklyn.util.repeat.Repeater;
+import org.apache.brooklyn.util.ssh.BashCommands;
+import org.apache.brooklyn.util.ssh.IptablesCommands;
+import org.apache.brooklyn.util.ssh.IptablesCommands.Chain;
+import org.apache.brooklyn.util.ssh.IptablesCommands.Policy;
+import org.apache.brooklyn.util.stream.Streams;
+import org.apache.brooklyn.util.text.ByteSizeStrings;
+import org.apache.brooklyn.util.text.Identifiers;
+import org.apache.brooklyn.util.text.KeyValueParser;
+import org.apache.brooklyn.util.text.Strings;
+import org.apache.brooklyn.util.time.Duration;
+import org.apache.brooklyn.util.time.Time;
 import org.jclouds.aws.ec2.compute.AWSEC2TemplateOptions;
 import org.jclouds.cloudstack.compute.options.CloudStackTemplateOptions;
 import org.jclouds.compute.ComputeService;
 import org.jclouds.compute.RunNodesException;
 import org.jclouds.compute.config.AdminAccessConfiguration;
 import org.jclouds.compute.domain.ComputeMetadata;
-import org.jclouds.compute.domain.ExecResponse;
 import org.jclouds.compute.domain.Hardware;
 import org.jclouds.compute.domain.Image;
 import org.jclouds.compute.domain.NodeMetadata;
@@ -106,7 +142,6 @@ import org.jclouds.rest.AuthorizationException;
 import org.jclouds.scriptbuilder.domain.LiteralStatement;
 import org.jclouds.scriptbuilder.domain.Statement;
 import org.jclouds.scriptbuilder.domain.StatementList;
-import org.jclouds.scriptbuilder.domain.Statements;
 import org.jclouds.scriptbuilder.functions.InitAdminAccess;
 import org.jclouds.scriptbuilder.statements.login.AdminAccess;
 import org.jclouds.scriptbuilder.statements.login.ReplaceShadowPasswordEntry;
@@ -140,45 +175,6 @@ import com.google.common.io.Files;
 import com.google.common.net.HostAndPort;
 import com.google.common.primitives.Ints;
 
-import org.apache.brooklyn.location.jclouds.JcloudsPredicates.NodeInLocation;
-import org.apache.brooklyn.location.jclouds.templates.PortableTemplateBuilder;
-import org.apache.brooklyn.location.ssh.SshMachineLocation;
-import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
-import org.apache.brooklyn.util.collections.MutableList;
-import org.apache.brooklyn.util.collections.MutableMap;
-import org.apache.brooklyn.util.collections.MutableSet;
-import org.apache.brooklyn.util.core.ResourceUtils;
-import org.apache.brooklyn.util.core.config.ConfigBag;
-import org.apache.brooklyn.util.core.crypto.SecureKeys;
-import org.apache.brooklyn.util.core.flags.MethodCoercions;
-import org.apache.brooklyn.util.core.flags.SetFromFlag;
-import org.apache.brooklyn.util.core.flags.TypeCoercions;
-import org.apache.brooklyn.util.core.internal.ssh.ShellTool;
-import org.apache.brooklyn.util.core.internal.ssh.SshTool;
-import org.apache.brooklyn.util.core.text.TemplateProcessor;
-import org.apache.brooklyn.util.exceptions.CompoundRuntimeException;
-import org.apache.brooklyn.util.exceptions.Exceptions;
-import org.apache.brooklyn.util.exceptions.ReferenceWithError;
-import org.apache.brooklyn.util.guava.Maybe;
-import org.apache.brooklyn.util.javalang.Enums;
-import org.apache.brooklyn.util.javalang.Reflections;
-import org.apache.brooklyn.util.net.Cidr;
-import org.apache.brooklyn.util.net.Networking;
-import org.apache.brooklyn.util.net.Protocol;
-import org.apache.brooklyn.util.os.Os;
-import org.apache.brooklyn.util.repeat.Repeater;
-import org.apache.brooklyn.util.ssh.BashCommands;
-import org.apache.brooklyn.util.ssh.IptablesCommands;
-import org.apache.brooklyn.util.ssh.IptablesCommands.Chain;
-import org.apache.brooklyn.util.ssh.IptablesCommands.Policy;
-import org.apache.brooklyn.util.stream.Streams;
-import org.apache.brooklyn.util.text.ByteSizeStrings;
-import org.apache.brooklyn.util.text.Identifiers;
-import org.apache.brooklyn.util.text.KeyValueParser;
-import org.apache.brooklyn.util.text.Strings;
-import org.apache.brooklyn.util.time.Duration;
-import org.apache.brooklyn.util.time.Time;
-
 import io.cloudsoft.winrm4j.pywinrm.Session;
 import io.cloudsoft.winrm4j.pywinrm.WinRMFactory;
 
@@ -749,16 +745,17 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 sshHostAndPortOverride = Optional.absent();
             }
 
+            LoginCredentials initialCredentials = node.getCredentials();
             if (skipJcloudsSshing) {
                 boolean waitForConnectable = (windows) ? waitForWinRmable : waitForSshable;
                 if (waitForConnectable) {
                     if (windows) {
                         // TODO Does jclouds support any windows user setup?
-                        waitForWinRmAvailable(computeService, node, sshHostAndPortOverride, node.getCredentials(), setup);
+                        initialCredentials = waitForWinRmAvailable(computeService, node, sshHostAndPortOverride, setup);
                     } else {
-                        waitForSshable(computeService, node, sshHostAndPortOverride, node.getCredentials(), setup);
+                        initialCredentials = waitForSshable(computeService, node, sshHostAndPortOverride, setup);
                     }
-                    userCredentials = createUser(computeService, node, sshHostAndPortOverride, setup);
+                    userCredentials = createUser(computeService, node, sshHostAndPortOverride, initialCredentials, setup);
                 }
             }
 
@@ -773,7 +770,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 if (customCredentials.getOptionalPrivateKey().isPresent()) setup.put(PRIVATE_KEY_DATA, customCredentials.getOptionalPrivateKey().get());
             }
             if (userCredentials == null) {
-                userCredentials = extractVmCredentials(setup, node);
+                // TODO See waitForSshable, which now handles if the node.getLoginCredentials has both a password+key
+                userCredentials = extractVmCredentials(setup, node, initialCredentials);
             }
             if (userCredentials != null) {
                 node = NodeMetadataBuilder.fromNodeMetadata(node).credentials(userCredentials).build();
@@ -787,7 +785,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
             // Wait for the VM to be reachable over SSH
             if (waitForSshable && !windows) {
-                waitForSshable(computeService, node, sshHostAndPortOverride, userCredentials, setup);
+                waitForSshable(computeService, node, sshHostAndPortOverride, ImmutableList.of(userCredentials), setup);
             } else {
                 LOG.debug("Skipping ssh check for {} ({}) due to config waitForSshable=false", node, setup.getDescription());
             }
@@ -1575,7 +1573,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
     /**
      * Create the user immediately - executing ssh commands as required.
      */
-    protected LoginCredentials createUser(ComputeService computeService, NodeMetadata node, Optional<HostAndPort> hostAndPortOverride, ConfigBag config) {
+    protected LoginCredentials createUser(ComputeService computeService, NodeMetadata node, Optional<HostAndPort> hostAndPortOverride, LoginCredentials initialCredentials, ConfigBag config) {
         Image image = (node.getImageId() != null) ? computeService.getImage(node.getImageId()) : null;
         UserCreation userCreation = createUserStatements(image, config);
 
@@ -1601,7 +1599,6 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                     commands.add(statement.render(scriptOsFamily));
                 }
 
-                LoginCredentials initialCredentials = node.getCredentials();
                 Optional<String> initialPassword = initialCredentials.getOptionalPassword();
                 Optional<String> initialPrivateKey = initialCredentials.getOptionalPrivateKey();
                 String initialUser = initialCredentials.getUser();
@@ -2433,10 +2430,9 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
     /**
      * Extracts the user that jclouds tells us about (i.e. from the jclouds node).
      */
-    protected LoginCredentials extractVmCredentials(ConfigBag setup, NodeMetadata node) {
+    protected LoginCredentials extractVmCredentials(ConfigBag setup, NodeMetadata node, LoginCredentials nodeCredentials) {
         String user = getUser(setup);
         OsCredential localCredentials = LocationConfigUtils.getOsCredential(setup).checkNoErrors();
-        LoginCredentials nodeCredentials = LoginCredentials.fromCredentials(node.getCredentials());
 
         LOG.debug("Credentials extracted for {}: {}/{} with {}/{}", new Object[] { node,
             user, nodeCredentials.getUser(), localCredentials, nodeCredentials });
@@ -2470,7 +2466,11 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         return null;
     }
 
-    protected void waitForWinRmAvailable(final ComputeService computeService, final NodeMetadata node, Optional<HostAndPort> hostAndPortOverride, final LoginCredentials expectedCredentials, ConfigBag setup) {
+    protected LoginCredentials waitForWinRmAvailable(final ComputeService computeService, final NodeMetadata node, Optional<HostAndPort> hostAndPortOverride, ConfigBag setup) {
+        return waitForWinRmAvailable(computeService, node, hostAndPortOverride, node.getCredentials(), setup);
+    }
+    
+    protected LoginCredentials waitForWinRmAvailable(final ComputeService computeService, final NodeMetadata node, Optional<HostAndPort> hostAndPortOverride, LoginCredentials expectedCredentials, ConfigBag setup) {
         String waitForWinrmAvailable = setup.get(WAIT_FOR_WINRM_AVAILABLE);
         checkArgument(!"false".equalsIgnoreCase(waitForWinrmAvailable), "waitForWinRmAvailable called despite waitForWinRmAvailable=%s", waitForWinrmAvailable);
         Duration timeout = null;
@@ -2506,12 +2506,37 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             }};
         String connectionDetails = user + "@" + vmIp + ":" + vmPort;
 
-        waitForReachable(checker, connectionDetails, expectedCredentials, setup, timeout);
+        waitForReachable(checker, connectionDetails, ImmutableList.of(expectedCredentials), setup, timeout);
+        
+        return expectedCredentials;
     }
 
-    protected void waitForSshable(final ComputeService computeService, final NodeMetadata node, Optional<HostAndPort> hostAndPortOverride, final LoginCredentials expectedCredentials, ConfigBag setup) {
+    protected LoginCredentials waitForSshable(final ComputeService computeService, final NodeMetadata node, Optional<HostAndPort> hostAndPortOverride, ConfigBag setup) {
+        LoginCredentials nodeCreds = node.getCredentials();
+        String nodeUser = nodeCreds.getUser();
+        String loginUserOverride = setup.get(LOGIN_USER);
+        Set<String> users = MutableSet.<String>builder().add(nodeUser).add(loginUserOverride).build();
+        
+        // See https://issues.apache.org/jira/browse/BROOKLYN-186
+        // Handle where jclouds gives us the wrong login user (!) and both a password + ssh key.
+        // Try all the permutations to find the one that works.
+        List<LoginCredentials> credentialsToTry = Lists.newArrayList();
+        for (String user : users) {
+            if (nodeCreds.getOptionalPassword().isPresent() && nodeCreds.getOptionalPrivateKey().isPresent()) {
+                credentialsToTry.add(LoginCredentials.builder(nodeCreds).noPassword().user(user).build());
+                credentialsToTry.add(LoginCredentials.builder(nodeCreds).noPrivateKey().user(user).build());
+            } else {
+                credentialsToTry.add(LoginCredentials.builder(nodeCreds).user(user).build());
+            }
+        }
+        
+        return waitForSshable(computeService, node, hostAndPortOverride, credentialsToTry, setup);
+    }
+    
+    protected LoginCredentials waitForSshable(final ComputeService computeService, final NodeMetadata node, Optional<HostAndPort> hostAndPortOverride, List<LoginCredentials> credentialsToTry, ConfigBag setup) {
         String waitForSshable = setup.get(WAIT_FOR_SSHABLE);
-        checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForReachable called despite waitForSshable=%s", waitForSshable);
+        checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForReachable called despite waitForSshable=%s for %s", waitForSshable, node);
+        checkArgument(credentialsToTry.size() > 0, "waitForReachable called without credentials for %s", node);
 
         Duration timeout = null;
         try {
@@ -2523,53 +2548,68 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             timeout = Duration.parse(WAIT_FOR_SSHABLE.getDefaultValue());
         }
         
-        String user = expectedCredentials.getUser();
+        Set<String> users = Sets.newLinkedHashSet();
+        for (LoginCredentials creds : credentialsToTry) {
+            users.add(creds.getUser());
+        }
+        String user = (users.size() == 1) ? Iterables.getOnlyElement(users) : "{" + Joiner.on(",").join(users) + "}";
         String vmIp = hostAndPortOverride.isPresent() ? hostAndPortOverride.get().getHostText() : JcloudsUtil.getFirstReachableAddress(computeService.getContext(), node);
         if (vmIp==null) LOG.warn("Unable to extract IP for "+node+" ("+setup.getDescription()+"): subsequent connection attempt will likely fail");
         int vmPort = hostAndPortOverride.isPresent() ? hostAndPortOverride.get().getPortOrDefault(22) : 22;
 
         String connectionDetails = user + "@" + vmIp + ":" + vmPort;
-
-        Callable<Boolean> checker;
-        if (hostAndPortOverride.isPresent()) {
-            final SshMachineLocation machine = createTemporarySshMachineLocation(hostAndPortOverride.get(), expectedCredentials, setup);
-            checker = new Callable<Boolean>() {
-                public Boolean call() {
-                    int exitstatus = machine.execScript("check-connectivity", ImmutableList.of("hostname"));
-                    return exitstatus == 0;
-                }};
-        } else {
-            checker = new Callable<Boolean>() {
-                public Boolean call() {
-                    Statement statement = Statements.newStatementList(exec("hostname"));
-                    ExecResponse response = computeService.runScriptOnNode(node.getId(), statement,
-                            overrideLoginCredentials(expectedCredentials).runAsRoot(false));
-                    return response.getExitStatus() == 0;
-                }};
+        final HostAndPort hostAndPort = hostAndPortOverride.isPresent() ? hostAndPortOverride.get() : HostAndPort.fromParts(vmIp, vmPort);
+        final AtomicReference<LoginCredentials> credsSuccessful = new AtomicReference<LoginCredentials>();
+        
+        final Map<SshMachineLocation, LoginCredentials> machinesToTry = Maps.newLinkedHashMap();
+        for (LoginCredentials creds : credentialsToTry) {
+            machinesToTry.put(createTemporarySshMachineLocation(hostAndPort, creds, setup), creds);
         }
+        Callable<Boolean> checker = new Callable<Boolean>() {
+            public Boolean call() {
+                for (Map.Entry<SshMachineLocation, LoginCredentials> entry : machinesToTry.entrySet()) {
+                    SshMachineLocation machine = entry.getKey();
+                    int exitstatus = machine.execScript(
+                            ImmutableMap.of(
+                                    SshTool.PROP_SSH_TRIES_TIMEOUT.getName(), Duration.THIRTY_SECONDS.toMilliseconds(),
+                                    SshTool.PROP_SSH_TRIES.getName(), 1), 
+                            "check-connectivity", 
+                            ImmutableList.of("true"));
+                    boolean success = (exitstatus == 0);
+                    if (success) {
+                        credsSuccessful.set(entry.getValue());
+                        return true;
+                    }
+                }
+                return false;
+            }};
 
-        waitForReachable(checker, connectionDetails, expectedCredentials, setup, timeout);
+        waitForReachable(checker, connectionDetails, credentialsToTry, setup, timeout);
+        return credsSuccessful.get();
     }
 
-    protected void waitForReachable(Callable<Boolean> checker, String connectionDetails, LoginCredentials expectedCredentials, ConfigBag setup, Duration timeout) {
-        String user = expectedCredentials.getUser();
+    protected void waitForReachable(Callable<Boolean> checker, String hostAndPort, List<LoginCredentials> credentialsToLog, ConfigBag setup, Duration timeout) {
         if (LOG.isDebugEnabled()) {
-            Optional<String> password;
-            Optional<String> key;
-            if (Boolean.TRUE.equals(setup.get(LOG_CREDENTIALS))) {
-                password = expectedCredentials.getOptionalPassword();
-                key = expectedCredentials.getOptionalPrivateKey();
-            } else {
-                password = expectedCredentials.getOptionalPassword().isPresent() ? Optional.of("******") : Optional.<String>absent();
-                key = expectedCredentials.getOptionalPrivateKey().isPresent() ? Optional.of("******") : Optional.<String>absent();
+            List<String> credsToString = Lists.newArrayList();
+            for (LoginCredentials creds : credentialsToLog) {
+                String user = creds.getUser();
+                String password;
+                String key;
+                if (Boolean.TRUE.equals(setup.get(LOG_CREDENTIALS))) {
+                    password = creds.getOptionalPassword().or("<absent>");
+                    key = creds.getOptionalPrivateKey().or("<absent>");
+                } else {
+                    password = creds.getOptionalPassword().isPresent() ? "******" : "<absent>";
+                    key = creds.getOptionalPrivateKey().isPresent() ? "******" : "<absent>";
+                }
+                credsToString.add("user="+user+", password="+password+", key="+key);
             }
-            LOG.debug("VM {}: reported online, now waiting {} for it to be contactable on {}{}; using credentials password={}; key={}",
+
+            LOG.debug("VM {}: reported online, now waiting {} for it to be contactable on {}; using credentials {}",
                     new Object[] {
                             setup.getDescription(), timeout,
-                            connectionDetails,
-                            Objects.equal(user, getUser(setup)) ? "" : " (setup user is different: "+getUser(setup)+")",
-                            password.or("<absent>"),
-                            key.or("<absent>")
+                            hostAndPort,
+                            (credsToString.size() == 1) ? credsToString.get(0) : "(multiple!):" + Joiner.on("\n\t").join(credsToString)
                     });
         }
 
@@ -2583,13 +2623,13 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
         if (!reachable.getWithoutError()) {
             throw new IllegalStateException("Connection failed for "
-                    +connectionDetails+" ("+setup.getDescription()+") after waiting "
+                    +hostAndPort+" ("+setup.getDescription()+") after waiting "
                     +Time.makeTimeStringRounded(timeout), reachable.getError());
         }
 
         LOG.debug("VM {}: connection succeeded after {} on {}",new Object[] {
                 setup.getDescription(), Time.makeTimeStringRounded(stopwatch),
-                connectionDetails});
+                hostAndPort});
     }
 
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6ca7e267/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
index 60a5271..ba5ce64 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
@@ -463,7 +463,28 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation
         Callable<SecurityGroup> callable = new Callable<SecurityGroup>() {
             @Override
             public SecurityGroup call() throws Exception {
-                return securityApi.addIpPermission(permission, group);
+                try {
+                    return securityApi.addIpPermission(permission, group);
+                } catch (AWSResponseException e) {
+                    if ("InvalidPermission.Duplicate".equals(e.getError().getCode())) {
+                        // already exists
+                        LOG.info("Permission already exists for security group; continuing (logging underlying exception at debug): permission="+permission+"; group="+group);
+                        LOG.debug("Permission already exists for security group; continuing: permission="+permission+"; group="+group, e);
+                        return null;
+                    } else {
+                        throw e;
+                    }
+                } catch (Exception e) {
+                    Exceptions.propagateIfFatal(e);
+                    if (e.toString().contains("InvalidPermission.Duplicate")) {
+                        // belt-and-braces, in case 
+                        // already exists
+                        LOG.info("Permission already exists for security group; continuing (but unexpected exception type): permission="+permission+"; group="+group, e);
+                        return null;
+                    } else {
+                        throw Exceptions.propagate(e);
+                    }
+                }
             }
         };
         return runOperationWithRetry(callable);