This closes #1150

apache · Jan 15, 2016 · 848f750 · 848f750
2 parents 289eb5b + e07f0ee
commit 848f750
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 3 deletions.
diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
@@ -89,6 +89,7 @@
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.exceptions.RuntimeInterruptedException;
 import org.apache.brooklyn.util.guava.KeyTransformingLoadingCache.KeyTransformingSameTypeLoadingCache;
+import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.pool.BasicPool;
 import org.apache.brooklyn.util.pool.Pool;
 import org.apache.brooklyn.util.ssh.BashCommands;
@@ -389,7 +390,7 @@ public Pool<SshTool> load(Map<String, ?> properties) {
     private BasicPool<SshTool> buildPool(final Map<String, ?> properties) {
         return BasicPool.<SshTool>builder()
                 .name(getDisplayName()+"@"+address+":"+getPort()+
-                        (config().getRaw(SSH_HOST).isPresent() ? "("+getConfig(SSH_HOST)+":"+getConfig(SSH_PORT)+")" : "")+
+                        (config().getRaw(SSH_HOST).isPresent() ? "("+getConfig(SSH_HOST)+":"+getPort()+")" : "")+
                         ":hash"+System.identityHashCode(this))
                 .supplier(new Supplier<SshTool>() {
                         @Override public SshTool get() {
@@ -550,7 +551,21 @@ public String getUser() {
 
     /** port for SSHing */
     public int getPort() {
-        return getConfig(SshTool.PROP_PORT);
+        // Prefer PROP_PORT (i.e. "port"). However if that is explicitly null or is not set, then see if
+        // SSH_PORT (i.e. "brooklyn.ssh.config.port") has been set and use that.
+        // If neither is set (or is explicitly set to null), then use the default PROP_PORT value.
+        //
+        // Note we don't just rely on config().get(PROP_PORT) returning the default, because we hit a rebind
+        // error where the location's port configuration had been explicitly set to null.
+        // See https://issues.apache.org/jira/browse/BROOKLYN-215
+
+        Maybe<Object> raw = config().getRaw(SshTool.PROP_PORT);
+        if (raw.orNull() == null && config().getRaw(SSH_PORT).orNull() != null) {
+            return config().get(SSH_PORT);
+        } else {
+            Integer result = config().get(SshTool.PROP_PORT);
+            return (result != null) ? result : SshTool.PROP_PORT.getDefaultValue();
+        }
     }
 
     protected <T> T execSsh(final Map<String, ?> props, final Function<ShellTool, T> task) {

diff --git a/...core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindSshMachineLocationTest.java b/...core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindSshMachineLocationTest.java
@@ -80,5 +80,23 @@ public void testMachineUsableAfterRebind() throws Exception {
 
         assertEquals(newChildLoc.execScript(Collections.<String,Object>emptyMap(), "mysummary", ImmutableList.of("true")), 0);
     }
-
+
+    // See https://issues.apache.org/jira/browse/BROOKLYN-215
+    @Test(groups="Integration")
+    public void testRebindWhenPortNull() throws Exception {
+        SshMachineLocation machine = origManagementContext.getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
+                .configure("address", "localhost")
+                .configure("port", null));
+        FixedListMachineProvisioningLocation<?> byon = origManagementContext.getLocationManager().createLocation(LocationSpec.create(FixedListMachineProvisioningLocation.class)
+                .configure("machines", ImmutableList.of(machine)));
+        origApp.start(ImmutableList.of(byon));
+        LOG.info("Before rebind, machine="+machine.toString());
+
+        newApp = (TestApplication) rebind();
+
+        FixedListMachineProvisioningLocation<?> newByon = (FixedListMachineProvisioningLocation<?>) Iterables.getOnlyElement(newApp.getLocations(), 0);
+        SshMachineLocation newMachine = (SshMachineLocation) Iterables.get(newByon.getChildren(), 0);
+
+        LOG.info("After rebind, machine="+newMachine.toString());
+    }
 }