brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sjcorb...@apache.org
Subject [1/7] brooklyn-server git commit: Deletions, deprecations and typos
Date Tue, 20 Dec 2016 16:11:47 GMT
Repository: brooklyn-server
Updated Branches:
  refs/heads/master 58b1de717 -> 770709475


Deletions, deprecations and typos

* Remove ComputeService arg from JcloudsLocation.resolveManagementHostAndPort
* Improve javadoc for Duration.of
* Deprecate Attributes.HOST_AND_PORT and delete Attributes.SERVICE_STATE
* Miscellaneous typos
* Readability of JcloudsLocation and related classes
* Delete method commented out for two years
* Remove ComputeService arg from JcloudsLocation.resolveManagementHostAndPort
* Delete unused variable from BasicJcloudsLocationCustomizer


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

Branch: refs/heads/master
Commit: 2cbe309e741fd92776e588eb14edf7700ac9db61
Parents: 58b1de7
Author: Sam Corbett <sam.corbett@cloudsoftcorp.com>
Authored: Thu Dec 1 15:11:24 2016 +0000
Committer: Sam Corbett <sam.corbett@cloudsoftcorp.com>
Committed: Tue Dec 20 14:59:12 2016 +0000

----------------------------------------------------------------------
 .../brooklyn/core/config/BasicConfigKey.java    |   2 +-
 .../apache/brooklyn/core/entity/Attributes.java |  18 +-
 .../location/cloud/CloudLocationConfig.java     |   2 +-
 .../core/sensor/DependentConfiguration.java     |   2 +-
 .../location/ssh/SshMachineLocation.java        |  11 -
 .../jclouds/BasicJcloudsLocationCustomizer.java |   5 +-
 .../location/jclouds/JcloudsLocation.java       | 248 ++++++++++---------
 .../location/jclouds/JcloudsLocationConfig.java |  16 +-
 .../jclouds/JcloudsSshMachineLocation.java      |   7 +-
 .../MachineLifecycleEffectorTasks.java          |   5 +-
 .../org/apache/brooklyn/util/time/Duration.java |  13 +-
 11 files changed, 162 insertions(+), 167 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2cbe309e/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java
index 2600ae7..95b4bac 100644
--- a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java
+++ b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java
@@ -141,7 +141,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
             this.reconfigurable = val; return self();
         }
         /**
-         * @deprecated since 0.10.0; use {@link #runtime2Inheritance(ConfigInheritance)}
+         * @deprecated since 0.10.0; use {@link #runtimeInheritance(ConfigInheritance)}
          */ 
         @Deprecated
         public B parentInheritance(ConfigInheritance val) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2cbe309e/core/src/main/java/org/apache/brooklyn/core/entity/Attributes.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/Attributes.java b/core/src/main/java/org/apache/brooklyn/core/entity/Attributes.java
index ba60a42..02437ef 100644
--- a/core/src/main/java/org/apache/brooklyn/core/entity/Attributes.java
+++ b/core/src/main/java/org/apache/brooklyn/core/entity/Attributes.java
@@ -95,11 +95,15 @@ public interface Attributes {
             UserAndHostAndPort.class, 
             "host.winrmAddress", 
             "user@host:port for WinRM'ing (or null if inappropriate)");
-    AttributeSensor<String> SUBNET_HOSTNAME = Sensors.newStringSensor( "host.subnet.hostname", "Host name as known internally in " +
-            "the subnet where it is running (if different to host.name)");
-    AttributeSensor<String> SUBNET_ADDRESS = Sensors.newStringSensor( "host.subnet.address", "Host address as known internally in " +
-            "the subnet where it is running (if different to host.name)");
-
+    AttributeSensor<String> SUBNET_HOSTNAME = Sensors.newStringSensor(
+            "host.subnet.hostname",
+            "Host name as known internally in the subnet where it is running (if different to host.name)");
+    AttributeSensor<String> SUBNET_ADDRESS = Sensors.newStringSensor(
+            "host.subnet.address",
+            "Host address as known internally in the subnet where it is running (if different to host.name)");
+
+    /** @deprecated since 0.11.0 without replacement */
+    @Deprecated
     AttributeSensor<String> HOST_AND_PORT = Sensors.newStringSensor( "hostandport", "host:port" );
 
     /*
@@ -134,10 +138,6 @@ public interface Attributes {
     AttributeSensor<Lifecycle.Transition> SERVICE_STATE_EXPECTED = Sensors.newSensor(Lifecycle.Transition.class,
             "service.state.expected", "Last controlled change to service state, indicating what the expected state should be");
     
-    /** @deprecated since 0.7.0 use {@link #SERVICE_STATE_ACTUAL} or {@link #SERVICE_STATE_EXPECTED} as appropriate. */
-    @Deprecated
-    AttributeSensor<Lifecycle> SERVICE_STATE = SERVICE_STATE_ACTUAL;
-
     /*
      * Other metadata (optional)
      */

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2cbe309e/core/src/main/java/org/apache/brooklyn/core/location/cloud/CloudLocationConfig.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/location/cloud/CloudLocationConfig.java b/core/src/main/java/org/apache/brooklyn/core/location/cloud/CloudLocationConfig.java
index cc3e222..c5f3773 100644
--- a/core/src/main/java/org/apache/brooklyn/core/location/cloud/CloudLocationConfig.java
+++ b/core/src/main/java/org/apache/brooklyn/core/location/cloud/CloudLocationConfig.java
@@ -79,7 +79,7 @@ public interface CloudLocationConfig {
 
     public static final ConfigKey<String> POLL_FOR_FIRST_REACHABLE_ADDRESS = ConfigKeys.newStringConfigKey("pollForFirstReachableAddress", 
             "Whether and how long to wait for reaching the VM's ip:port; "
-            + "if 'false', will default to the node's first public IP (or privae if no public IPs); "
+            + "if 'false', will default to the node's first public IP (or private if no public IPs); "
             + "if 'true' uses default duration; otherwise accepts a time string e.g. '5m' (the default) or a number of milliseconds", "5m");
 
     @SuppressWarnings("serial")

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2cbe309e/core/src/main/java/org/apache/brooklyn/core/sensor/DependentConfiguration.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/sensor/DependentConfiguration.java b/core/src/main/java/org/apache/brooklyn/core/sensor/DependentConfiguration.java
index 1263226..d6e0622 100644
--- a/core/src/main/java/org/apache/brooklyn/core/sensor/DependentConfiguration.java
+++ b/core/src/main/java/org/apache/brooklyn/core/sensor/DependentConfiguration.java
@@ -700,7 +700,7 @@ public class DependentConfiguration {
     public static class ProtoBuilder {
         /**
          * Will wait for the attribute on the given entity, with default behaviour:
-         * If that entity reports {@link Lifecycle#ON_FIRE} for its {@link Attributes#SERVICE_STATE} then it will abort;
+         * If that entity reports {@link Lifecycle#ON_FIRE} for its {@link Attributes#SERVICE_STATE_ACTUAL} then it will abort;
          * If that entity is stopping or destroyed (see {@link Builder#timeoutIfStoppingOrDestroyed(Duration)}),
          * then it will timeout after 1 minute.
          */

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2cbe309e/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java b/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
index b611c3c..d19f08e 100644
--- a/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
+++ b/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
@@ -165,7 +165,6 @@ public class SshMachineLocation extends AbstractLocation implements MachineLocat
      * brooklyn.location.named.myLocation.sshToolClass = com.acme.brooklyn.MyCustomSshTool
      * brooklyn.location.named.myLocation.sshToolClass.myparam = myvalue
      * }
-     * }
      * </pre>
      * <p>
      */
@@ -517,16 +516,6 @@ public class SshMachineLocation extends AbstractLocation implements MachineLocat
         sshPoolCacheOrNull = null;
     }
 
-    // should not be necessary, and causes objects to be kept around a lot longer than desired
-//    @Override
-//    protected void finalize() throws Throwable {
-//        try {
-//            close();
-//        } finally {
-//            super.finalize();
-//        }
-//    }
-
     @Override
     public InetAddress getAddress() {
         return address;

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2cbe309e/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java
index 317f4e9..dc03447 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java
@@ -120,8 +120,6 @@ public class BasicJcloudsLocationCustomizer extends BasicConfigurableObject impl
      * @return the calling entity
      */
     protected Entity getCallerContext(JcloudsMachineLocation machine) {
-        SudoTtyFixingCustomizer s;
-
         Object context = config().get(LocationConfigKeys.CALLER_CONTEXT);
         if (context == null) {
             context = machine.config().get(LocationConfigKeys.CALLER_CONTEXT);
@@ -129,7 +127,6 @@ public class BasicJcloudsLocationCustomizer extends BasicConfigurableObject impl
         if (!(context instanceof Entity)) {
             throw new IllegalStateException("Invalid location context: " + context);
         }
-        Entity entity = (Entity) context;
-        return entity;
+        return (Entity) context;
     }
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2cbe309e/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 8e8d4d3..14bab2d 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
@@ -384,11 +384,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
     public boolean isLocationFirewalldEnabled(SshMachineLocation location) {
         int result = location.execCommands("checking if firewalld is active", 
                 ImmutableList.of(IptablesCommands.firewalldServiceIsActive()));
-        if (result == 0) {
-            return true;
-        }
-        
-        return false;
+        return result == 0;
     }
     
     protected Semaphore getMachineCreationSemaphore() {
@@ -732,11 +728,6 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 LOG.debug("jclouds using template {} / options {} to provision machine in {}",
                         new Object[] {template, template.getOptions(), setup.getDescription()});
 
-                // no longer supported because config is sealed, we use an underlying config map
-//                if (!setup.getUnusedConfig().isEmpty())
-//                    if (LOG.isDebugEnabled())
-//                        LOG.debug("NOTE: unused flags passed to obtain VM in "+setup.getDescription()+": "
-//                                + Sanitizer.sanitize(setup.getUnusedConfig()));
                 nodes = computeService.createNodesInGroup(groupId, 1, template);
                 provisionTimestamp = Duration.of(provisioningStopwatch);
             } finally {
@@ -752,11 +743,14 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             boolean waitForConnectable = (windows) ? waitForWinRmable : waitForSshable;
             
             if (windows) {
-                int newLoginPort = node.getLoginPort() == 22 ? (getConfig(WinRmMachineLocation.USE_HTTPS_WINRM) ? 5986 : 5985) : node.getLoginPort();
-                String newLoginUser = "root".equals(node.getCredentials().getUser()) ? "Administrator" : node.getCredentials().getUser();
-                LOG.debug("jclouds created Windows VM {}; transforming connection details: loginPort from {} to {}; loginUser from {} to {}", 
+                int newLoginPort = node.getLoginPort() == 22
+                        ? (getConfig(WinRmMachineLocation.USE_HTTPS_WINRM) ? 5986 : 5985)
+                        : node.getLoginPort();
+                String newLoginUser = "root".equals(node.getCredentials().getUser())
+                        ? "Administrator"
+                        : node.getCredentials().getUser();
+                LOG.debug("jclouds created Windows VM {}; transforming connection details: loginPort from {} to {}; loginUser from {} to {}",
                         new Object[] {node, node.getLoginPort(), newLoginPort, node.getCredentials().getUser(), newLoginUser});
-                
                 node = NodeMetadataBuilder.fromNodeMetadata(node)
                         .loginPort(newLoginPort)
                         .credentials(LoginCredentials.builder(node.getCredentials()).user(newLoginUser).build())
@@ -779,7 +773,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             LoginCredentials initialCredentials = node.getCredentials();
             
             final HostAndPort managementHostAndPort = resolveManagementHostAndPort(
-                    computeService, node, Optional.fromNullable(userCredentials), sshHostAndPortOverride, setup,
+                    node, Optional.fromNullable(userCredentials), sshHostAndPortOverride, setup,
                     new ResolveOptions()
                             .windows(windows)
                             .expectConnectable(waitForConnectable)
@@ -831,7 +825,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             if (waitForSshable && !windows) {
                 waitForSshable(computeService, node, managementHostAndPort, ImmutableList.of(userCredentials), setup);
             } else {
-                LOG.debug("Skipping ssh check for {} ({}) due to config waitForSshable=false", node, setup.getDescription());
+                LOG.debug("Skipping ssh check for {} ({}) due to config waitForSshable={}, windows={}",
+                        new Object[]{node, setup.getDescription(), waitForSshable, windows});
             }
 
             // Do not store the credentials on the node as this may leak the credentials if they
@@ -969,7 +964,10 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                                     iptablesRules.add(IptablesCommands.addFirewalldRule(Chain.INPUT, Protocol.TCP, port, Policy.ACCEPT));
                                  }
                             } else {
-                                iptablesRules = createIptablesRulesForNetworkInterface(inboundPorts);
+                                iptablesRules = Lists.newArrayList();
+                                for (Integer port : inboundPorts) {
+                                   iptablesRules.add(IptablesCommands.insertIptablesRule(Chain.INPUT, Protocol.TCP, port, Policy.ACCEPT));
+                                }
                                 iptablesRules.add(IptablesCommands.saveIptablesRules());
                             }
                             List<String> batch = Lists.newArrayList();
@@ -1250,11 +1248,11 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
     // ------------- constructing the template, etc ------------------------
 
-    private static interface CustomizeTemplateBuilder {
+    private interface CustomizeTemplateBuilder {
         void apply(TemplateBuilder tb, ConfigBag props, Object v);
     }
 
-    public static interface CustomizeTemplateOptions {
+    public interface CustomizeTemplateOptions {
         void apply(TemplateOptions tb, ConfigBag props, Object v);
     }
 
@@ -2003,8 +2001,10 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
      * return that fully qualified name (which we expect to be reachable from inside
      * and outside the AWS region).
      */
-    private HostAndPort resolveManagementHostAndPort(ComputeService computeService, NodeMetadata node, Optional<LoginCredentials> userCredentials, Optional<HostAndPort> hostAndPortOverride, ConfigBag config, ResolveOptions options) {
-        Boolean lookupAwsHostname = config.get(LOOKUP_AWS_HOSTNAME);
+    private HostAndPort resolveManagementHostAndPort(
+            NodeMetadata node, Optional<LoginCredentials> userCredentials,
+            Optional<HostAndPort> hostAndPortOverride, ConfigBag config, ResolveOptions options) {
+        boolean lookupAwsHostname = Boolean.TRUE.equals(config.get(LOOKUP_AWS_HOSTNAME));
         String provider = config.get(CLOUD_PROVIDER);
         if (provider == null) provider= getProvider();
         int defaultPort;
@@ -2019,7 +2019,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             int port = hostAndPortOverride.get().hasPort() ? hostAndPortOverride.get().getPort() : defaultPort;
             return HostAndPort.fromParts(hostAndPortOverride.get().getHostText(), port);
         }
-        if (options.expectConnectable && userCredentials.isPresent() && "aws-ec2".equals(provider) && Boolean.TRUE.equals(lookupAwsHostname)) {
+        if (options.expectConnectable && userCredentials.isPresent() && "aws-ec2".equals(provider) && lookupAwsHostname) {
             // Treat AWS as a special case because the DNS fully qualified hostname in AWS is 
             // (normally?!) a good way to refer to the VM from both inside and outside of the 
             // region.
@@ -2036,7 +2036,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 if (options.propagatePollForReachableFailure) {
                     throw Exceptions.propagate(e);
                 } else {
-                    LOG.warn("No reachable address ("+config.getDescription()+"/"+node+"); falling back to any advertised address; may cause future failures");
+                    LOG.warn("No reachable address ({}/{}); falling back to any advertised address; may cause future failures",
+                            config.getDescription(), node);
                 }
             }
         }
@@ -2047,8 +2048,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 return HostAndPort.fromParts(address, defaultPort);
             }
         }
-        
-        LOG.warn("No resolvable address in "+addresses+" ("+config.getDescription()+"/"+node+"); using first; may cause future failures");
+        LOG.warn("No resolvable address in {} ({}/{}); using first; may cause future failures",
+                new Object[]{addresses, config.getDescription(), node});
         String host = Iterables.get(addresses, 0);
         return HostAndPort.fromParts(host, defaultPort);
     }
@@ -2378,7 +2379,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         boolean windows = isWindows(node, setup);
         
         HostAndPort managementHostAndPort = resolveManagementHostAndPort(
-                computeService, node, Optional.<LoginCredentials>absent(), Optional.<HostAndPort>absent(), setup,
+                node, Optional.<LoginCredentials>absent(), Optional.<HostAndPort>absent(), setup,
                 new ResolveOptions()
                         .windows(windows)
                         .expectConnectable(true)
@@ -2519,9 +2520,9 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         }
     }
 
-    protected JcloudsSshMachineLocation registerJcloudsSshMachineLocation(ComputeService computeService, NodeMetadata node, 
-            Optional<Template> template, LoginCredentials credentials, HostAndPort managementHostAndPort, ConfigBag setup) 
-            throws IOException {
+    protected JcloudsSshMachineLocation registerJcloudsSshMachineLocation(
+            ComputeService computeService, NodeMetadata node, Optional<Template> template,
+            LoginCredentials credentials, HostAndPort managementHostAndPort, ConfigBag setup) throws IOException {
         JcloudsSshMachineLocation machine = createJcloudsSshMachineLocation(computeService, node, template, credentials, managementHostAndPort, setup);
         registerJcloudsMachineLocation(node.getId(), machine);
         return machine;
@@ -2533,9 +2534,9 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         vmInstanceIds.put(machine, nodeId);
     }
 
-    protected JcloudsSshMachineLocation createJcloudsSshMachineLocation(ComputeService computeService, NodeMetadata node, 
-            Optional<Template> template, LoginCredentials userCredentials, HostAndPort managementHostAndPort, ConfigBag setup) 
-            throws IOException {
+    protected JcloudsSshMachineLocation createJcloudsSshMachineLocation(
+            ComputeService computeService, NodeMetadata node, Optional<Template> template,
+            LoginCredentials userCredentials, HostAndPort managementHostAndPort, ConfigBag setup) throws IOException {
         Map<?,?> sshConfig = extractSshConfig(setup, node);
         String nodeAvailabilityZone = extractAvailabilityZone(setup, node);
         String nodeRegion = extractRegion(setup, node);
@@ -2544,15 +2545,16 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             nodeRegion = extractProvider(setup, node);
         }
 
-        if (LOG.isDebugEnabled())
+        if (LOG.isDebugEnabled()) {
             LOG.debug("creating JcloudsSshMachineLocation representation for {}@{} ({}) for {}/{}",
-                    new Object[] {
+                    new Object[]{
                             getUser(setup),
                             managementHostAndPort,
                             Sanitizer.sanitize(sshConfig),
                             setup.getDescription(),
                             node
                     });
+        }
 
         String address = managementHostAndPort.getHostText();
         int port = managementHostAndPort.hasPort() ? managementHostAndPort.getPort() : node.getLoginPort();
@@ -2563,19 +2565,22 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         // a shared IP address, for example).
         String displayName = getPublicHostnameGeneric(node, setup, Optional.of(address));
         
+        final Object password = sshConfig.get(SshMachineLocation.PASSWORD.getName()) != null
+                ? sshConfig.get(SshMachineLocation.PASSWORD.getName())
+                : userCredentials.getOptionalPassword().orNull();
+        final Object privateKeyData = sshConfig.get(SshMachineLocation.PRIVATE_KEY_DATA.getName()) != null
+                ? sshConfig.get(SshMachineLocation.PRIVATE_KEY_DATA.getName())
+                : userCredentials.getOptionalPrivateKey().orNull();
         if (isManaged()) {
-            return getManagementContext().getLocationManager().createLocation(LocationSpec.create(JcloudsSshMachineLocation.class)
+            final LocationSpec<JcloudsSshMachineLocation> spec = LocationSpec.create(JcloudsSshMachineLocation.class)
                     .configure(sshConfig)
                     .configure("displayName", displayName)
                     .configure("address", address)
                     .configure(JcloudsSshMachineLocation.SSH_PORT, port)
                     .configure("user", userCredentials.getUser())
-                    .configure(SshMachineLocation.PASSWORD.getName(), sshConfig.get(SshMachineLocation.PASSWORD.getName()) != null ?
-                            sshConfig.get(SshMachineLocation.PASSWORD.getName()) :
-                            userCredentials.getOptionalPassword().orNull())
-                    .configure(SshMachineLocation.PRIVATE_KEY_DATA.getName(), sshConfig.get(SshMachineLocation.PRIVATE_KEY_DATA.getName()) != null ?
-                            sshConfig.get(SshMachineLocation.PRIVATE_KEY_DATA.getName()) :
-                            userCredentials.getOptionalPrivateKey().orNull())
+                    // The use of `getName` is intentional. See 11741d85b9f54 for details.
+                    .configure(SshMachineLocation.PASSWORD.getName(), password)
+                    .configure(SshMachineLocation.PRIVATE_KEY_DATA.getName(), privateKeyData)
                     .configure("jcloudsParent", this)
                     .configure("node", node)
                     .configure("template", template.orNull())
@@ -2586,42 +2591,43 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                     .configureIfNotNull(SshMachineLocation.SCRIPT_DIR, setup.get(SshMachineLocation.SCRIPT_DIR))
                     .configureIfNotNull(USE_PORT_FORWARDING, setup.get(USE_PORT_FORWARDING))
                     .configureIfNotNull(PORT_FORWARDER, setup.get(PORT_FORWARDER))
-                    .configureIfNotNull(PORT_FORWARDING_MANAGER, setup.get(PORT_FORWARDING_MANAGER)));
+                    .configureIfNotNull(PORT_FORWARDING_MANAGER, setup.get(PORT_FORWARDING_MANAGER))
+                    .configureIfNotNull(SshMachineLocation.PRIVATE_ADDRESSES, node.getPrivateAddresses());
+            return getManagementContext().getLocationManager().createLocation(spec);
         } else {
-            LOG.warn("Using deprecated JcloudsSshMachineLocation constructor because "+this+" is not managed");
-            return new JcloudsSshMachineLocation(MutableMap.builder()
+            LOG.warn("Using deprecated JcloudsSshMachineLocation constructor because " + this + " is not managed");
+            final MutableMap<Object, Object> properties = MutableMap.builder()
                     .putAll(sshConfig)
                     .put("displayName", displayName)
                     .put("address", address)
                     .put("port", port)
                     .put("user", userCredentials.getUser())
-                    .putIfNotNull(SshMachineLocation.PASSWORD.getName(), sshConfig.get(SshMachineLocation.PASSWORD.getName()) != null ?
-                            SshMachineLocation.PASSWORD.getName() :
-                            userCredentials.getOptionalPassword().orNull())
-                    .putIfNotNull(SshMachineLocation.PRIVATE_KEY_DATA.getName(), sshConfig.get(SshMachineLocation.PRIVATE_KEY_DATA.getName()) != null ?
-                            SshMachineLocation.PRIVATE_KEY_DATA.getName() :
-                            userCredentials.getOptionalPrivateKey().orNull())
+                    // The use of `getName` is intentional. See 11741d85b9f54 for details.
+                    .putIfNotNull(SshMachineLocation.PASSWORD.getName(), password)
+                    .putIfNotNull(SshMachineLocation.PRIVATE_KEY_DATA.getName(), privateKeyData)
                     .put("callerContext", setup.get(CALLER_CONTEXT))
                     .putIfNotNull(CLOUD_AVAILABILITY_ZONE_ID.getName(), nodeAvailabilityZone)
                     .putIfNotNull(CLOUD_REGION_ID.getName(), nodeRegion)
                     .put(USE_PORT_FORWARDING, setup.get(USE_PORT_FORWARDING))
                     .put(PORT_FORWARDER, setup.get(PORT_FORWARDER))
                     .put(PORT_FORWARDING_MANAGER, setup.get(PORT_FORWARDING_MANAGER))
-                    .build(),
-                    this,
-                    node);
+                    .put(SshMachineLocation.PRIVATE_ADDRESSES, node.getPrivateAddresses())
+                    .build();
+            return new JcloudsSshMachineLocation(properties, this, node);
         }
     }
 
-    protected JcloudsWinRmMachineLocation registerWinRmMachineLocation(ComputeService computeService, NodeMetadata node, 
-            Optional<Template> template, LoginCredentials credentials, HostAndPort managementHostAndPort, ConfigBag setup) {
+    protected JcloudsWinRmMachineLocation registerWinRmMachineLocation(
+            ComputeService computeService, NodeMetadata node, Optional<Template> template,
+            LoginCredentials credentials, HostAndPort managementHostAndPort, ConfigBag setup) {
         JcloudsWinRmMachineLocation machine = createWinRmMachineLocation(computeService, node, template, credentials, managementHostAndPort, setup);
         registerJcloudsMachineLocation(node.getId(), machine);
         return machine;
     }
 
-    protected JcloudsWinRmMachineLocation createWinRmMachineLocation(ComputeService computeService, NodeMetadata node, 
-            Optional<Template> template, LoginCredentials userCredentials, HostAndPort winrmHostAndPort, ConfigBag setup) {
+    protected JcloudsWinRmMachineLocation createWinRmMachineLocation(
+            ComputeService computeService, NodeMetadata node, Optional<Template> template,
+            LoginCredentials userCredentials, HostAndPort winrmHostAndPort, ConfigBag setup) {
         Map<?,?> winrmConfig = extractWinrmConfig(setup, node);
         String nodeAvailabilityZone = extractAvailabilityZone(setup, node);
         String nodeRegion = extractRegion(setup, node);
@@ -2634,7 +2640,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         String displayName = getPublicHostnameGeneric(node, setup, Optional.of(address));
 
         if (isManaged()) {
-            return getManagementContext().getLocationManager().createLocation(LocationSpec.create(JcloudsWinRmMachineLocation.class)
+            final LocationSpec<JcloudsWinRmMachineLocation> spec = LocationSpec.create(JcloudsWinRmMachineLocation.class)
                     .configure(winrmConfig)
                     .configure("jcloudsParent", this)
                     .configure("displayName", displayName)
@@ -2653,14 +2659,13 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                     .configureIfNotNull(SshMachineLocation.SCRIPT_DIR, setup.get(SshMachineLocation.SCRIPT_DIR))
                     .configureIfNotNull(USE_PORT_FORWARDING, setup.get(USE_PORT_FORWARDING))
                     .configureIfNotNull(PORT_FORWARDER, setup.get(PORT_FORWARDER))
-                    .configureIfNotNull(PORT_FORWARDING_MANAGER, setup.get(PORT_FORWARDING_MANAGER)));
+                    .configureIfNotNull(PORT_FORWARDING_MANAGER, setup.get(PORT_FORWARDING_MANAGER));
+            return getManagementContext().getLocationManager().createLocation(spec);
         } else {
             throw new UnsupportedOperationException("Cannot create WinRmMachineLocation because " + this + " is not managed");
         }
     }
 
-    // -------------- give back the machines------------------
-
     protected Map<String,Object> extractSshConfig(ConfigBag setup, NodeMetadata node) {
         ConfigBag nodeConfig = new ConfigBag();
         if (node!=null && node.getCredentials() != null) {
@@ -2719,6 +2724,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         return null;
     }
 
+    // -------------- give back the machines------------------
 
     @Override
     public void release(MachineLocation rawMachine) {
@@ -2934,6 +2940,10 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
     /**
      * Extracts the user that jclouds tells us about (i.e. from the jclouds node).
+     * <p>
+     * Modifies <code>setup</code> to set {@link #USER} if it is unset when the method is called or
+     * if the value in the bag is {@link #ROOT_USERNAME} and the user on the node is contained in
+     * {@link #ROOT_ALIASES}.
      */
     protected LoginCredentials extractVmCredentials(ConfigBag setup, NodeMetadata node, LoginCredentials nodeCredentials) {
         boolean windows = isWindows(node, setup);
@@ -2952,18 +2962,21 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 setup.put(USER, user = nodeCredentials.getUser());
             }
 
-            String pkd = Strings.maybeNonBlank(localCredentials.getPrivateKeyData()).or(nodeCredentials.getOptionalPrivateKey().orNull());
-            String pwd = Strings.maybeNonBlank(localCredentials.getPassword()).or(nodeCredentials.getOptionalPassword().orNull());
+            String pkd = Strings.maybeNonBlank(localCredentials.getPrivateKeyData())
+                    .or(nodeCredentials.getOptionalPrivateKey().orNull());
+            String pwd = Strings.maybeNonBlank(localCredentials.getPassword())
+                    .or(nodeCredentials.getOptionalPassword().orNull());
             if (Strings.isBlank(user) || (Strings.isBlank(pkd) && pwd==null)) {
                 String missing = (user==null ? "user" : "credential");
                 LOG.warn("Not able to determine "+missing+" for "+this+" at "+node+"; will likely fail subsequently");
                 return null;
             } else {
                 LoginCredentials.Builder resultBuilder = LoginCredentials.builder().user(user);
-                if (pwd != null && (Strings.isBlank(pkd) || localCredentials.isUsingPassword() || windows))
+                if (pwd != null && (Strings.isBlank(pkd) || localCredentials.isUsingPassword() || windows)) {
                     resultBuilder.password(pwd);
-                else // pkd guaranteed non-blank due to above
+                } else { // pkd guaranteed non-blank due to above
                     resultBuilder.privateKey(pkd);
+                }
                 return resultBuilder.build();
             }
         }
@@ -2982,17 +2995,19 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
             Predicate<? super HostAndPort> pollForFirstReachableHostAndPortPredicate;
             if (setup.get(POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE) != null) {
-                LOG.debug("Using pollForFirstReachableAddress.predicate supplied from config for location " + this + " "
-                        + POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE.getName() + " : " + setup.get(POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE));
+                LOG.debug("{} polling for first reachable address with {}",
+                        this, setup.get(POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE));
                 pollForFirstReachableHostAndPortPredicate = setup.get(POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE);
             } else {
-                LOG.debug("Using pollForFirstReachableAddress.predicate.type supplied from config for location " + this + " " + POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE_TYPE.getName()
-                        + " : " + setup.get(POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE_TYPE));
+                LOG.debug("{} polling for first reachable address with instance of {}",
+                        this, POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE_TYPE.getName());
 
-                Class<? extends Predicate<? super HostAndPort>> predicateType = setup.get(POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE_TYPE);
+                Class<? extends Predicate<? super HostAndPort>> predicateType =
+                        setup.get(POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE_TYPE);
 
                 Map<String, Object> args = MutableMap.of();
-                ConfigUtils.addUnprefixedConfigKeyInConfigBack(POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE.getName() + ".", setup, args);
+                ConfigUtils.addUnprefixedConfigKeyInConfigBack(
+                        POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE.getName() + ".", setup, args);
                 try {
                     pollForFirstReachableHostAndPortPredicate = predicateType.getConstructor(Map.class).newInstance(args);
                 } catch (NoSuchMethodException|IllegalAccessException e) {
@@ -3006,12 +3021,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 }
             }
 
-            try {
-                result = JcloudsUtil.getFirstReachableAddress(node, timeout, pollForFirstReachableHostAndPortPredicate);
-            } catch (Exception e) {
-                throw Exceptions.propagate("Problem instantiating reachability checker for " + this
-                        + " pollForFirstReachableHostAndPortPredicate: " + pollForFirstReachableHostAndPortPredicate, e);
-            }
+            // Throws if no suitable address is found.
+            result = JcloudsUtil.getFirstReachableAddress(node, timeout, pollForFirstReachableHostAndPortPredicate);
             LOG.debug("Using first-reachable address "+result+" for node "+node+" in "+this);
         } else {
             result = Iterables.getFirst(Iterables.concat(node.getPublicAddresses(), node.getPrivateAddresses()), null);
@@ -3048,7 +3059,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         // The createTemporaryWinRmMachineLocation deals with removing that.
         ConfigBag winrmProps = ConfigBag.newInstanceCopying(setup);
 
-        final Pair<WinRmMachineLocation, LoginCredentials> machinesToTry = Pair.of(createTemporaryWinRmMachineLocation(managementHostAndPort, credentialsToTry, winrmProps), credentialsToTry);
+        final Pair<WinRmMachineLocation, LoginCredentials> machinesToTry = Pair.of(
+                createTemporaryWinRmMachineLocation(managementHostAndPort, credentialsToTry, winrmProps), credentialsToTry);
 
         try {
             Callable<Boolean> checker = new Callable<Boolean>() {
@@ -3059,46 +3071,46 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                             ImmutableList.of("echo testing"));
                     boolean success = (response.getStatusCode() == 0);
                     if (success) {
-                            credsSuccessful.set(machinesToTry.getRight());
+                        credsSuccessful.set(machinesToTry.getRight());
 
-                            String verifyWindowsUp = getConfig(WinRmMachineLocation.WAIT_WINDOWS_TO_START);
-                            if (Strings.isBlank(verifyWindowsUp) || verifyWindowsUp.equals("false")) {
-                                return true;
-                            }
+                        String verifyWindowsUp = getConfig(WinRmMachineLocation.WAIT_WINDOWS_TO_START);
+                        if (Strings.isBlank(verifyWindowsUp) || verifyWindowsUp.equals("false")) {
+                            return true;
+                        }
 
-                            Predicate<WinRmMachineLocation> machineReachable = new Predicate<WinRmMachineLocation>() {
-                                @Override
-                                public boolean apply(@Nullable WinRmMachineLocation machine) {
-                                    try {
-                                        WinRmToolResponse response = machine.executeCommand("echo testing");
-                                        int statusCode = response.getStatusCode();
-                                        return statusCode == 0;
-                                    } catch (RuntimeException e) {
-                                        if (getFirstThrowableOfType(e, IOException.class) != null || getFirstThrowableOfType(e, WebServiceException.class) != null) {
-                                            LOG.debug("WinRM Connectivity lost", e);
-                                            return false;
-                                        } else {
-                                            throw e;
-                                        }
+                        Predicate<WinRmMachineLocation> machineReachable = new Predicate<WinRmMachineLocation>() {
+                            @Override
+                            public boolean apply(@Nullable WinRmMachineLocation machine) {
+                                try {
+                                    WinRmToolResponse response = machine.executeCommand("echo testing");
+                                    int statusCode = response.getStatusCode();
+                                    return statusCode == 0;
+                                } catch (RuntimeException e) {
+                                    if (getFirstThrowableOfType(e, IOException.class) != null || getFirstThrowableOfType(e, WebServiceException.class) != null) {
+                                        LOG.debug("WinRM Connectivity lost", e);
+                                        return false;
+                                    } else {
+                                        throw e;
                                     }
                                 }
-                            };
-                            Duration verifyWindowsUpTime = Duration.of(verifyWindowsUp);
-                            boolean restartHappened = Predicates2.retry(Predicates.not(machineReachable),
+                            }
+                        };
+                        Duration verifyWindowsUpTime = Duration.of(verifyWindowsUp);
+                        boolean restartHappened = Predicates2.retry(Predicates.not(machineReachable),
+                                verifyWindowsUpTime.toMilliseconds(),
+                                Duration.FIVE_SECONDS.toMilliseconds(),
+                                Duration.THIRTY_SECONDS.toMilliseconds(),
+                                TimeUnit.MILLISECONDS).apply(machine);
+                        if (restartHappened) {
+                            LOG.info("Connectivity to the machine was lost. Probably Windows have restarted {} as part of the provisioning process.\nRetrying to connect...", machine);
+                            return Predicates2.retry(machineReachable,
                                     verifyWindowsUpTime.toMilliseconds(),
-                                    Duration.FIVE_SECONDS.toMilliseconds(),
-                                    Duration.THIRTY_SECONDS.toMilliseconds(),
+                                    Duration.of(5, TimeUnit.SECONDS).toMilliseconds(),
+                                    Duration.of(30, TimeUnit.SECONDS).toMilliseconds(),
                                     TimeUnit.MILLISECONDS).apply(machine);
-                            if (restartHappened) {
-                                LOG.info("Connectivity to the machine was lost. Probably Windows have restarted {} as part of the provisioning process.\nRetrying to connect...", machine);
-                                return Predicates2.retry(machineReachable,
-                                        verifyWindowsUpTime.toMilliseconds(),
-                                        Duration.of(5, TimeUnit.SECONDS).toMilliseconds(),
-                                        Duration.of(30, TimeUnit.SECONDS).toMilliseconds(),
-                                        TimeUnit.MILLISECONDS).apply(machine);
-                            } else {
-                                return true;
-                            }
+                        } else {
+                            return true;
+                        }
                     }
                     return false;
                 }};
@@ -3145,8 +3157,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
     protected LoginCredentials waitForSshable(final ComputeService computeService, final NodeMetadata node, HostAndPort hostAndPort, List<LoginCredentials> credentialsToTry, ConfigBag setup) {
         String waitForSshable = setup.get(WAIT_FOR_SSHABLE);
-        checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForReachable called despite waitForSshable=%s for %s", waitForSshable, node);
-        checkArgument(credentialsToTry.size() > 0, "waitForReachable called without credentials for %s", node);
+        checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForSshable called despite waitForSshable=%s for %s", waitForSshable, node);
+        checkArgument(credentialsToTry.size() > 0, "waitForSshable called without credentials for %s", node);
 
         Duration timeout = null;
         try {
@@ -3592,14 +3604,6 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         }
     }
 
-    private List<String> createIptablesRulesForNetworkInterface(Iterable<Integer> ports) {
-       List<String> iptablesRules = Lists.newArrayList();
-       for (Integer port : ports) {
-          iptablesRules.add(IptablesCommands.insertIptablesRule(Chain.INPUT, Protocol.TCP, port, Policy.ACCEPT));
-       }
-       return iptablesRules;
-    }
-
     @Override
     public PersistenceObjectStore newPersistenceObjectStore(String container) {
         return new JcloudsBlobStoreBasedObjectStore(this, container);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2cbe309e/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
index 7e72b2d..eb47a8f 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
@@ -257,8 +257,7 @@ public interface JcloudsLocationConfig extends CloudLocationConfig {
     public static final ConfigKey<JcloudsPortForwarderExtension> PORT_FORWARDER = ConfigKeys.newConfigKey(
             JcloudsPortForwarderExtension.class, "portforwarding.forwarder", "The port-forwarder to use");
     
-    public static final ConfigKey<PortForwardManager> PORT_FORWARDING_MANAGER = BrooklynAccessUtils
-            .PORT_FORWARDING_MANAGER;
+    public static final ConfigKey<PortForwardManager> PORT_FORWARDING_MANAGER = BrooklynAccessUtils.PORT_FORWARDING_MANAGER;
 
     public static final ConfigKey<Integer> MACHINE_CREATE_ATTEMPTS = ConfigKeys.newIntegerConfigKey(
             "machineCreateAttempts", "Number of times to retry if jclouds fails to create a VM", 2);
@@ -293,10 +292,10 @@ public interface JcloudsLocationConfig extends CloudLocationConfig {
     public static final ConfigKey<Map<String,Object>> TEMPLATE_OPTIONS = ConfigKeys.newConfigKey(
             new TypeToken<Map<String, Object>>() {}, "templateOptions", "Additional jclouds template options");
 
-    /*
-         The purpose of this config is to aid deployment of blueprints to clouds where it is difficult to get nodes to
-         communicate via private IPs. This config allows a user to overwrite private IPs as public IPs, thus ensuring
-         that any blueprints they wish to deploy which may use private IPs still work in these clouds.
+    /**
+     * The purpose of this config is to aid deployment of blueprints to clouds where it is difficult to get nodes to
+     * communicate via private IPs. This config allows a user to overwrite private IPs as public IPs, thus ensuring
+     * that any blueprints they wish to deploy which may use private IPs still work in these clouds.
      */
     @Beta
     public static final ConfigKey<Boolean> USE_MACHINE_PUBLIC_ADDRESS_AS_PRIVATE_ADDRESS = ConfigKeys.newBooleanConfigKey(
@@ -304,9 +303,4 @@ public interface JcloudsLocationConfig extends CloudLocationConfig {
             "When true we will use the public IP/Hostname of a JClouds Location as the private IP/Hostname",
             false);
 
-    // TODO
-    
-//  "noDefaultSshKeys" - hints that local ssh keys should not be read as defaults
-    // this would be useful when we need to indicate a password
-
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2cbe309e/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 dd096b1..e2bed51 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
@@ -420,9 +420,10 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements Jcl
     protected Optional<String> getPrivateAddress() {
         for (String p : getPrivateAddresses()) {
             // disallow local only addresses
-            if (Networking.isLocalOnly(p)) continue;
-            // other things may be public or private, but either way, return it
-            return Optional.of(p);
+            if (!Networking.isLocalOnly(p)) {
+                // other things may be public or private, but either way, return it
+                return Optional.of(p);
+            }
         }
         return Optional.absent();
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2cbe309e/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
index 83a34de..9a4d3b9 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
@@ -115,7 +115,7 @@ import com.google.common.reflect.TypeToken;
  *  <li> {@link #preStopCustom()}
  *  <li> {@link #postStopCustom()}
  * </ul>
- * Note methods at this level typically look after the {@link Attributes#SERVICE_STATE} sensor.
+ * Note methods at this level typically look after the {@link Attributes#SERVICE_STATE_ACTUAL} sensor.
  *
  * @since 0.6.0
  */
@@ -489,7 +489,8 @@ public abstract class MachineLifecycleEffectorTasks {
             entity().sensors().set(Attributes.ADDRESS, machine.getAddress().getHostAddress());
             if (machine instanceof SshMachineLocation) {
                 SshMachineLocation sshMachine = (SshMachineLocation) machine;
-                UserAndHostAndPort sshAddress = UserAndHostAndPort.fromParts(sshMachine.getUser(), sshMachine.getAddress().getHostName(), sshMachine.getPort());
+                UserAndHostAndPort sshAddress = UserAndHostAndPort.fromParts(
+                        sshMachine.getUser(), sshMachine.getAddress().getHostName(), sshMachine.getPort());
                 entity().sensors().set(Attributes.SSH_ADDRESS, sshAddress);
             }
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2cbe309e/utils/common/src/main/java/org/apache/brooklyn/util/time/Duration.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/time/Duration.java b/utils/common/src/main/java/org/apache/brooklyn/util/time/Duration.java
index 473aef3..f649bef 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/time/Duration.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/time/Duration.java
@@ -202,8 +202,17 @@ public class Duration implements Comparable<Duration>, Serializable {
             }
         };
 
-    /** tries to convert given object to a Duration, parsing strings, treating numbers as millis, etc;
-     * throws IAE if not convertible */
+    /**
+     * Converts the given object to a Duration by attempting the following in order:
+     * <ol>
+     *     <li>return null if the object is null</li>
+     *     <li>{@link #parse(String) parse} the object as a string</li>
+     *     <li>treat numbers as milliseconds</li>
+     *     <li>obtain the elapsed time of a {@link Stopwatch}</li>
+     *     <li>invoke the object's <code>toMilliseconds</code> method, if one exists.</li>
+     * </ol>
+     * If none of these cases work the method throws an {@link IllegalArgumentException}.
+     */
     public static Duration of(Object o) {
         if (o == null) return null;
         if (o instanceof Duration) return (Duration)o;


Mime
View raw message