brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aleds...@apache.org
Subject [2/5] brooklyn-server git commit: Fix loc.getHostname when DSL for credentials
Date Fri, 30 Sep 2016 13:01:36 GMT
Fix loc.getHostname when DSL for credentials

If using DSL $brooklyn:config(“password”) for machine’s creds, then
if call machine.getHostname() outside of the entity’s context it will
fail to resolve the password so will throw an exception.

Instead, we now defer calling getLoginCredentials(), to only do it for
AWS (where we ask it for the hostname), and then if there is an 
exception we fall back to the defaults.

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

Branch: refs/heads/master
Commit: eab2dfa291b18ecfb490b89c7dd96bb33710311e
Parents: 6604f7a
Author: Aled Sage <aled.sage@gmail.com>
Authored: Tue Sep 27 17:24:39 2016 +0100
Committer: Aled Sage <aled.sage@gmail.com>
Committed: Fri Sep 30 12:44:55 2016 +0100

----------------------------------------------------------------------
 .../location/jclouds/JcloudsLocation.java       | 181 +++++++++----------
 .../jclouds/JcloudsSshMachineLocation.java      |  13 +-
 .../jclouds/JcloudsAddressesLiveTest.java       |   3 +-
 3 files changed, 94 insertions(+), 103 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/eab2dfa2/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 4681ba7..64e43b0 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
@@ -180,6 +180,7 @@ import com.google.common.base.Predicates;
 import com.google.common.base.Splitter;
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -3266,47 +3267,51 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation
im
      * For some clouds (e.g. aws-ec2), it will attempt to find the public hostname.
      */
     protected String getPublicHostname(NodeMetadata node, Optional<HostAndPort> sshHostAndPort,
LoginCredentials userCredentials, ConfigBag setup) {
+        return getPublicHostname(node, sshHostAndPort, Suppliers.ofInstance(userCredentials),
setup);
+    }
+    
+    protected String getPublicHostname(NodeMetadata node, Optional<HostAndPort> sshHostAndPort,
Supplier<? extends LoginCredentials> userCredentials, ConfigBag setup) {
         String provider = (setup != null) ? setup.get(CLOUD_PROVIDER) : null;
         Boolean lookupAwsHostname = (setup != null) ? setup.get(LOOKUP_AWS_HOSTNAME) : null;
         if (provider == null) provider= getProvider();
 
         if ("aws-ec2".equals(provider) && Boolean.TRUE.equals(lookupAwsHostname))
{
-            HostAndPort inferredHostAndPort = null;
-            if (!sshHostAndPort.isPresent()) {
-                try {
-                    String vmIp = getFirstReachableAddress(node, setup);
-                    int port = node.getLoginPort();
-                    inferredHostAndPort = HostAndPort.fromParts(vmIp, port);
-                } catch (Exception e) {
-                    LOG.warn("Error reaching aws-ec2 instance "+node.getId()+"@"+node.getLocation()+"
on port "+node.getLoginPort()+"; falling back to jclouds metadata for address", e);
-                }
-            }
-            if (sshHostAndPort.isPresent() || inferredHostAndPort != null) {
-                if (isWindows(node, setup)) {
-                    if (inferredHostAndPort != null) {
-                        LOG.warn("Cannot querying aws-ec2 Windows instance "+node.getId()+"@"+node.getLocation()+"
over ssh for its hostname; falling back to first reachable IP");
-                        return inferredHostAndPort.getHostText();
-                    }
-                } else {
-                    HostAndPort hostAndPortToUse = sshHostAndPort.isPresent() ? sshHostAndPort.get()
: inferredHostAndPort;
-                    try {
-                        return getPublicHostnameAws(hostAndPortToUse, userCredentials, setup);
-                    } catch (Exception e) {
-                        if (inferredHostAndPort != null) { 
-                            LOG.warn("Error querying aws-ec2 instance "+node.getId()+"@"+node.getLocation()+"
over ssh for its hostname; falling back to first reachable IP", e);
-                            // We've already found a reachable address so settle for that,
rather than doing it again
-                            return inferredHostAndPort.getHostText();
-                        } else {
-                            LOG.warn("Error querying aws-ec2 instance "+node.getId()+"@"+node.getLocation()+"
over ssh for its hostname; falling back to jclouds metadata for address", e);
-                        }
-                    }
-                }
-            }
+            Maybe<String> result = getHostnameAws(node, sshHostAndPort, userCredentials,
setup);
+            if (result.isPresent()) return result.get();
         }
 
         return getPublicHostnameGeneric(node, setup);
     }
 
+    /**
+     * Attempts to obtain the private hostname or IP of the node, as advertised by the cloud
provider.
+     * 
+     * For some clouds (e.g. aws-ec2), it will attempt to find the fully qualified hostname
(as that works in public+private).
+     */
+    protected String getPrivateHostname(NodeMetadata node, Optional<HostAndPort> sshHostAndPort,
ConfigBag setup) {
+        return getPrivateHostname(node, sshHostAndPort, node.getCredentials(), setup);
+    }
+
+    protected String getPrivateHostname(NodeMetadata node, Optional<HostAndPort> sshHostAndPort,
LoginCredentials userCredentials, ConfigBag setup) {
+        return getPrivateHostname(node, sshHostAndPort, Suppliers.ofInstance(userCredentials),
setup);
+    }
+    
+    protected String getPrivateHostname(NodeMetadata node, Optional<HostAndPort> sshHostAndPort,
Supplier<? extends LoginCredentials> userCredentials, ConfigBag setup) {
+        String provider = (setup != null) ? setup.get(CLOUD_PROVIDER) : null;
+        Boolean lookupAwsHostname = (setup != null) ? setup.get(LOOKUP_AWS_HOSTNAME) : null;
+        if (provider == null) provider = getProvider();
+
+        // TODO Discouraged to do cloud-specific things; think of this code for aws as an
+        // exceptional situation rather than a pattern to follow. We need a better way to
+        // do cloud-specific things.
+        if ("aws-ec2".equals(provider) && Boolean.TRUE.equals(lookupAwsHostname))
{
+            Maybe<String> result = getHostnameAws(node, sshHostAndPort, userCredentials,
setup);
+            if (result.isPresent()) return result.get();
+        }
+
+        return getPrivateHostnameGeneric(node, setup);
+    }
+
     private String getPublicHostnameGeneric(NodeMetadata node, @Nullable ConfigBag setup)
{
         // JcloudsUtil.getFirstReachableAddress() (probably) already succeeded so at least
one of the provided
         // public and private IPs is reachable. Prefer the public IP. Don't use hostname
as a fallback
@@ -3328,60 +3333,26 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation
im
         }
     }
 
-    private String getPublicHostnameAws(HostAndPort hostAndPort, LoginCredentials userCredentials,
ConfigBag setup) {
-        SshMachineLocation sshLocByIp = null;
-        try {
-            // TODO messy way to get an SSH session
-            sshLocByIp = createTemporarySshMachineLocation(hostAndPort, userCredentials,
setup);
-
-            ByteArrayOutputStream outStream = new ByteArrayOutputStream();
-            ByteArrayOutputStream errStream = new ByteArrayOutputStream();
-            int exitcode = sshLocByIp.execCommands(
-                    MutableMap.of("out", outStream, "err", errStream),
-                    "get public AWS hostname",
-                    ImmutableList.of(
-                            BashCommands.INSTALL_CURL,
-                            "echo `curl --silent --retry 20 http://169.254.169.254/latest/meta-data/public-hostname`;
exit"));
-            String outString = new String(outStream.toByteArray());
-            String[] outLines = outString.split("\n");
-            for (String line : outLines) {
-                if (line.startsWith("ec2-")) return line.trim();
+    private String getPrivateHostnameGeneric(NodeMetadata node, @Nullable ConfigBag setup)
{
+        //prefer the private address to the hostname because hostname is sometimes wrong/abbreviated
+        //(see that javadoc; also e.g. on rackspace/cloudstack, the hostname is not registered
with any DNS).
+        //Don't return local-only address (e.g. never 127.0.0.1)
+        if (groovyTruth(node.getPrivateAddresses())) {
+            for (String p : node.getPrivateAddresses()) {
+                if (Networking.isLocalOnly(p)) continue;
+                return p;
             }
-            throw new IllegalStateException("Could not obtain aws-ec2 hostname for vm "+hostAndPort+";
exitcode="+exitcode+"; stdout="+outString+"; stderr="+new String(errStream.toByteArray()));
-        } finally {
-            Streams.closeQuietly(sshLocByIp);
         }
-    }
-
-    /**
-     * Attempts to obtain the private hostname or IP of the node, as advertised by the cloud
provider.
-     * 
-     * For some clouds (e.g. aws-ec2), it will attempt to find the fully qualified hostname
(as that works in public+private).
-     */
-    protected String getPrivateHostname(NodeMetadata node, Optional<HostAndPort> sshHostAndPort,
ConfigBag setup) {
-        return getPrivateHostname(node, sshHostAndPort, node.getCredentials(), setup);
-    }
-    
-    protected String getPrivateHostname(NodeMetadata node, Optional<HostAndPort> sshHostAndPort,
LoginCredentials userCredentials, ConfigBag setup) {
-        String provider = (setup != null) ? setup.get(CLOUD_PROVIDER) : null;
-        Boolean lookupAwsHostname = (setup != null) ? setup.get(LOOKUP_AWS_HOSTNAME) : null;
-        
-        if (provider == null) provider = getProvider();
-
-        // TODO Discouraged to do cloud-specific things; think of this code for aws as an
-        // exceptional situation rather than a pattern to follow. We need a better way to
-        // do cloud-specific things.
-        if ("aws-ec2".equals(provider) && Boolean.TRUE.equals(lookupAwsHostname))
{
-            Maybe<String> result = getPrivateHostnameAws(node, sshHostAndPort, userCredentials,
setup);
-            if (result.isPresent()) return result.get();
+        if (groovyTruth(node.getPublicAddresses())) {
+            return node.getPublicAddresses().iterator().next();
+        } else if (groovyTruth(node.getHostname())) {
+            return node.getHostname();
+        } else {
+            return null;
         }
-
-        return getPrivateHostnameGeneric(node, setup);
     }
 
-    private Maybe<String> getPrivateHostnameAws(NodeMetadata node, Optional<HostAndPort>
sshHostAndPort, LoginCredentials userCredentials, ConfigBag setup) {
-        // TODO Remove duplication from getPublicHostname.
-        // TODO Don't like 
+    private Maybe<String> getHostnameAws(NodeMetadata node, Optional<HostAndPort>
sshHostAndPort, Supplier<? extends LoginCredentials> userCredentials, ConfigBag setup)
{
         HostAndPort inferredHostAndPort = null;
         if (!sshHostAndPort.isPresent()) {
             try {
@@ -3393,32 +3364,42 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation
im
             }
         }
         if (sshHostAndPort.isPresent() || inferredHostAndPort != null) {
-            HostAndPort hostAndPortToUse = sshHostAndPort.isPresent() ? sshHostAndPort.get()
: inferredHostAndPort;
-            try {
-                return Maybe.of(getPublicHostnameAws(hostAndPortToUse, userCredentials, setup));
-            } catch (Exception e) {
-                LOG.warn("Error querying aws-ec2 instance instance "+node.getId()+"@"+node.getLocation()+"
over ssh for its hostname; falling back to jclouds metadata for address", e);
+            if (isWindows(node, setup)) {
+                LOG.warn("Cannpy query aws-ec2 Windows instance "+node.getId()+"@"+node.getLocation()+"
over ssh for its hostname; falling back to jclouds metadata for address");
+            } else {
+                HostAndPort hostAndPortToUse = sshHostAndPort.isPresent() ? sshHostAndPort.get()
: inferredHostAndPort;
+                try {
+                    return Maybe.of(getHostnameAws(hostAndPortToUse, userCredentials.get(),
setup));
+                } catch (Exception e) {
+                    LOG.warn("Error querying aws-ec2 instance "+node.getId()+"@"+node.getLocation()+"
over ssh for its hostname; falling back to jclouds metadata for address", e);
+                }
             }
         }
         return Maybe.absent();
     }
 
-    private String getPrivateHostnameGeneric(NodeMetadata node, @Nullable ConfigBag setup)
{
-        //prefer the private address to the hostname because hostname is sometimes wrong/abbreviated
-        //(see that javadoc; also e.g. on rackspace/cloudstack, the hostname is not registered
with any DNS).
-        //Don't return local-only address (e.g. never 127.0.0.1)
-        if (groovyTruth(node.getPrivateAddresses())) {
-            for (String p : node.getPrivateAddresses()) {
-                if (Networking.isLocalOnly(p)) continue;
-                return p;
+    private String getHostnameAws(HostAndPort hostAndPort, LoginCredentials userCredentials,
ConfigBag setup) {
+        SshMachineLocation sshLocByIp = null;
+        try {
+            // TODO messy way to get an SSH session
+            sshLocByIp = createTemporarySshMachineLocation(hostAndPort, userCredentials,
setup);
+
+            ByteArrayOutputStream outStream = new ByteArrayOutputStream();
+            ByteArrayOutputStream errStream = new ByteArrayOutputStream();
+            int exitcode = sshLocByIp.execCommands(
+                    MutableMap.of("out", outStream, "err", errStream),
+                    "get public AWS hostname",
+                    ImmutableList.of(
+                            BashCommands.INSTALL_CURL,
+                            "echo `curl --silent --retry 20 http://169.254.169.254/latest/meta-data/public-hostname`;
exit"));
+            String outString = new String(outStream.toByteArray());
+            String[] outLines = outString.split("\n");
+            for (String line : outLines) {
+                if (line.startsWith("ec2-")) return line.trim();
             }
-        }
-        if (groovyTruth(node.getPublicAddresses())) {
-            return node.getPublicAddresses().iterator().next();
-        } else if (groovyTruth(node.getHostname())) {
-            return node.getHostname();
-        } else {
-            return null;
+            throw new IllegalStateException("Could not obtain aws-ec2 hostname for vm "+hostAndPort+";
exitcode="+exitcode+"; stdout="+outString+"; stderr="+new String(errStream.toByteArray()));
+        } finally {
+            Streams.closeQuietly(sshLocByIp);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/eab2dfa2/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsSshMachineLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsSshMachineLocation.java
b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsSshMachineLocation.java
index 714fda4..3ef596a 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsSshMachineLocation.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsSshMachineLocation.java
@@ -63,6 +63,7 @@ import org.slf4j.LoggerFactory;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Objects;
 import com.google.common.base.Optional;
+import com.google.common.base.Supplier;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -312,7 +313,7 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements
Jcl
             Optional<NodeMetadata> node = getOptionalNode();
             if (node.isPresent()) {
                 HostAndPort sshHostAndPort = getSshHostAndPort();
-                LoginCredentials creds = getLoginCredentials();
+                Supplier<LoginCredentials> creds = getLoginCredentialsSupplier();
                 hostname = jcloudsParent.getPublicHostname(node.get(), Optional.of(sshHostAndPort),
creds, config().getBag());
                 requestPersist();
 
@@ -355,7 +356,7 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements
Jcl
                 // If we can't get the node (i.e. the cloud provider doesn't know that id,
because it has
                 // been terminated), then we don't care as much about getting the right id!
                 HostAndPort sshHostAndPort = getSshHostAndPort();
-                LoginCredentials creds = getLoginCredentials();
+                Supplier<LoginCredentials> creds = getLoginCredentialsSupplier();
                 privateHostname = jcloudsParent.getPrivateHostname(node.get(), Optional.of(sshHostAndPort),
creds, config().getBag());
 
             } else {
@@ -496,6 +497,14 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements
Jcl
         }
     }
 
+    private Supplier<LoginCredentials> getLoginCredentialsSupplier() {
+        return new Supplier<LoginCredentials>() {
+            @Override public LoginCredentials get() {
+                return getLoginCredentials();
+            }
+        };
+    }
+    
     private LoginCredentials getLoginCredentials() {
         OsCredential creds = LocationConfigUtils.getOsCredential(new ResolvingConfigBag(getManagementContext(),
config().getBag()));
         

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/eab2dfa2/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsAddressesLiveTest.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsAddressesLiveTest.java
b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsAddressesLiveTest.java
index 12e064d..35a5e6a 100644
--- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsAddressesLiveTest.java
+++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsAddressesLiveTest.java
@@ -107,11 +107,12 @@ public class JcloudsAddressesLiveTest extends AbstractJcloudsLiveTest
{
         assertNotNull(subnetIp, msg);
         assertReachableFromMachine(machine, subnetIp, msg);
 
-        // hostname is reachable from inside; not necessarily reachable from outside
+        // hostname is reachable from inside; for AWS machines, it is also reachable from
outside
         assertNotNull(hostname, msg);
         assertReachableFromMachine(machine, hostname, msg);
         
         assertNotNull(subnetHostname, msg);
+        assertReachable(machine, subnetHostname, msg);
         assertReachableFromMachine(machine, subnetHostname, msg);
     }
 


Mime
View raw message