Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1082#discussion_r46521913
--- Diff:
locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsSshMachineLocation.java
---
@@ -146,24 +264,48 @@ public JcloudsLocation getParent() {
@Override
public String getHostname() {
- return node.getHostname();
+ return hostname;
}
- /** In most clouds, the public hostname is the only way to ensure VMs
in different zones can access each other. */
+ /** In clouds like AWS, the public hostname is the only way to ensure
VMs in different zones can access each other. */
@Override
public Set<String> getPublicAddresses() {
- return node.getPublicAddresses();
+ return (publicAddresses == null) ? ImmutableSet.<String>of() :
publicAddresses;
}
@Override
public Set<String> getPrivateAddresses() {
- return node.getPrivateAddresses();
+ return (privateAddresses == null) ? ImmutableSet.<String>of() :
privateAddresses;
}
@Override
public String getSubnetHostname() {
- String privateHostname = jcloudsParent.getPrivateHostname(node,
Optional.<HostAndPort>absent(), config().getBag());
- return privateHostname;
+ if (_privateHostname == null) {
+ Optional<NodeMetadata> node = getOptionalNode();
+ if (node.isPresent()) {
+ // TODO Could infer LoginCredentials in the same way as
super.connectSsh().
+ // But the code below falls back to extracting it from
privateAddress/publicAddress anyway.
+ _privateHostname =
jcloudsParent.getPrivateHostname(node.get(), Optional.of(getSshHostAndPort()),
config().getBag());
+ } else {
+ // TODO Duplication of some of
jcloudsParent.getPrivateHostnameGeneric(NodeMetadata, ConfigBag).
+ //
+ //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)
+ for (String p : getPrivateAddresses()) {
+ if (Networking.isLocalOnly(p)) continue;
+ _privateHostname = p;
+ }
+ if (groovyTruth(getPublicAddresses())) {
+ _privateHostname =
getPublicAddresses().iterator().next();
+ } else if (groovyTruth(getHostname())) {
+ _privateHostname = getHostname();
+ } else {
+ return null;
+ }
+ }
+ }
--- End diff --
I think having a single code path might be better here, i.e. have only the
fallback? Otherwise could be surprised by different behaviour in some edge
cases.
---
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.
---
