Github user alasdairhodge commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/790#discussion_r36396507
  
    --- Diff: 
locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java 
---
    @@ -2664,6 +2664,59 @@ private String getPublicHostnameAws(HostAndPort 
sshHostAndPort, ConfigBag 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 
hostname (as that works in public+private).
    +     */
    +    protected String getPrivateHostname(NodeMetadata node, 
Optional<HostAndPort> sshHostAndPort, ConfigBag setup) {
    +        String provider = (setup != null) ? setup.get(CLOUD_PROVIDER) : 
null;
    +        if (provider == null) provider= getProvider();
    +
    +        if ("aws-ec2".equals(provider)) {
    +            // TODO Remove duplication from getPublicHostname
    +            HostAndPort inferredHostAndPort = null;
    +            if (!sshHostAndPort.isPresent()) {
    +                try {
    +                    String vmIp = 
JcloudsUtil.getFirstReachableAddress(this.getComputeService().getContext(), 
node);
    +                    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) 
{
    +                HostAndPort hostAndPortToUse = sshHostAndPort.isPresent() 
? sshHostAndPort.get() : inferredHostAndPort;
    +                try {
    +                    return getPublicHostnameAws(hostAndPortToUse, 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);
    +                }
    +            }
    +        }
    +
    +        return getPrivateHostnameGeneric(node, setup);
    +    }
    +
    +    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 lacks the domain).
    --- End diff --
    
    One shouldn't expect a hostname to contain a domain part!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to