brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aledsage <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #355: Public Hostname is first public address o...
Date Wed, 28 Sep 2016 10:54:28 GMT
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/355#discussion_r80890063
  
    --- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
---
    @@ -3314,15 +3314,12 @@ private String getPublicHostnameGeneric(NodeMetadata node, @Nullable
ConfigBag s
             // public IP? It is sometimes wrong/abbreviated, resolving to the wrong IP, also
e.g. on
             // rackspace, the hostname lacks the domain.
             //
    -        // TODO If POLL_FOR_FIRST_REACHABLE_ADDRESS=false, then won't have checked if
any node is reachable.
    -        // TODO Some of the private addresses might not be reachable, should check connectivity
before
    -        // making a choice.
             // TODO Choose an IP once and stick to it - multiple places call JcloudsUtil.getFirstReachableAddress(),
             // could even get different IP on each call.
             if (groovyTruth(node.getPublicAddresses())) {
                 return node.getPublicAddresses().iterator().next();
             } else if (groovyTruth(node.getPrivateAddresses())) {
    -            return node.getPrivateAddresses().iterator().next();
    +            return getFirstReachableAddress(node, setup);
    --- End diff --
    
    TL;DR: Tricky. I don't think we should solve the problem like this. I need to think more
about this!
    
    The comment above says `JcloudsUtil.getFirstReachableAddress() (probably) already succeeded
so at least one of the provided public and private IPs is reachable`.
    
    The "probably" is particularly scary. Also, we may need to worry about this being called
some time after provisioning, where the VM might have been terminated behind Brooklyn's back.
In that case, none of the addresses would be reachable. The `getFirstReachableAddress` throws
an exception (and might take quite some time to do that, depending on the firewalls used within
a given cloud). We really don't want `getPublicHostname()` to throw an exception.
    
    Also, the code now looks strange: why is it calling `getFirstReachableAddress` for the
private addresses but not the public addresses?
    
    The comment you deleted is also very worrying: if `POLL_FOR_FIRST_REACHABLE_ADDRESS=false`
then we shouldn't be calling `getFirstReachableAddress` here. Reasons one might set that include
if the VM being provisioned requires special DNAT access (so none of the IP addresses are
directly accessible), or if the VM being provisioned is locked down so Brooklyn won't be able
to reach it ever.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message