brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aleds...@apache.org
Subject [25/27] incubator-brooklyn git commit: WinRM support: incorporate review comments PR #650
Date Fri, 29 May 2015 17:22:18 GMT
WinRM support: incorporate review comments PR #650

See https://github.com/apache/incubator-brooklyn/pull/650


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/9c37f241
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/9c37f241
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/9c37f241

Branch: refs/heads/master
Commit: 9c37f2416b08c8f00123c60ba27f0d0ace10dff1
Parents: 7cd5a25
Author: Aled Sage <aled.sage@gmail.com>
Authored: Wed May 27 09:54:12 2015 +0100
Committer: Richard Downer <richard@apache.org>
Committed: Thu May 28 17:40:46 2015 +0100

----------------------------------------------------------------------
 .../entity/group/DynamicClusterImpl.java        |  40 +-
 .../windows/WindowsPerformanceCounterFeed.java  | 169 ++------
 .../location/basic/WinRmMachineLocation.java    | 119 ++++--
 .../java/brooklyn/util/flags/TypeCoercions.java |  40 +-
 .../entity/group/DynamicClusterTest.java        |  15 +
 .../basic/ByonLocationResolverTest.java         |  15 +-
 .../util/internal/TypeCoercionsTest.java        |  26 +-
 .../jclouds/BasicJcloudsLocationCustomizer.java |  30 ++
 .../location/jclouds/JcloudsLocation.java       | 383 +++++++++----------
 .../jclouds/JcloudsLocationCustomizer.java      |  26 ++
 .../jclouds/JcloudsMachineLocation.java         |  45 +++
 .../jclouds/JcloudsSshMachineLocation.java      |   6 +-
 .../jclouds/JcloudsWinRmMachineLocation.java    | 127 +++++-
 .../jclouds/SudoTtyFixingCustomizer.java        |   8 +-
 .../JcloudsLocationSecurityGroupCustomizer.java |   8 +-
 .../location/jclouds/JcloudsLocationTest.java   |   4 +-
 pom.xml                                         |   2 +-
 .../AbstractSoftwareProcessWinRmDriver.java     |  22 +-
 .../basic/AbstractVanillaProcessDriver.java     |  22 --
 .../brooklyn/entity/basic/SoftwareProcess.java  |   9 +
 .../entity/basic/SoftwareProcessImpl.java       |   8 +-
 .../basic/VanillaSoftwareProcessDriver.java     |   2 +-
 .../entity/basic/VanillaWindowsProcess.java     |  18 +-
 .../basic/VanillaWindowsProcessDriver.java      |   2 +-
 .../entity/basic/VanillaWindowsProcessImpl.java |   1 +
 .../brooklyn/entity/software/StaticSensor.java  |  10 +-
 .../winrm/WindowsPerformanceCounterSensors.java |  12 +-
 ...rWithAvailabilityZonesMultiLocationTest.java |   2 +-
 .../entity/software/StaticSensorTest.java       |  55 +++
 .../basic/WinRmMachineLocationLiveTest.java     |  91 +++++
 .../brooklyn/entity/database/mssql/mssql.yaml   |  13 +-
 .../database/mssql/MssqlBlueprintLiveTest.java  |  60 +++
 .../launcher/src/test/resources/mssql-test.yaml |  60 +++
 33 files changed, 985 insertions(+), 465 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java b/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java
index 5c168aa..e269880 100644
--- a/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java
+++ b/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java
@@ -438,7 +438,7 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus
     /**
      * {@inheritDoc}
      *
-     * <strong>Note</strong> for sub-clases; this method can be called while synchronized on {@link #mutex}.
+     * <strong>Note</strong> for sub-classes; this method can be called while synchronized on {@link #mutex}.
      */
     @Override
     public String replaceMember(String memberId) {
@@ -485,11 +485,10 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus
                     throw new IllegalStateException("Unexpected condition! cluster="+this+"; member="+member+"; actualMemberLocs="+actualMemberLocs);
                 }
             } else {
-                if (getMemberSpec() != null && getMemberSpec().getLocations().size() > 0) {
-                    memberLoc = getMemberSpec().getLocations().iterator().next();
-                } else {
-                    memberLoc = getLocation();
-                }
+                // Replacing member, so new member should be in the same location as that being replaced.
+                // Expect this to agree with `getMemberSpec().getLocations()` (if set). If not, then 
+                // presumably there was a reason this specific member was started somewhere else!
+                memberLoc = getLocation();
             }
 
             Entity replacement = replaceMember(member, memberLoc, ImmutableMap.of());
@@ -588,22 +587,25 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus
 
         // choose locations to be deployed to
         List<Location> chosenLocations;
-        chosenLocations = getMemberSpec() == null ? null : getMemberSpec().getLocations();
-        if (chosenLocations == null || chosenLocations.size() == 0) {
+        List<Location> memberLocations = getMemberSpec() == null ? null : getMemberSpec().getLocations();
+        if (memberLocations != null && memberLocations.size() > 0) {
+            // The memberSpec overrides the location passed to cluster.start(); use
+            // the location defined on the member.
             if (isAvailabilityZoneEnabled()) {
-                List<Location> subLocations = getNonFailedSubLocations();
-                Multimap<Location, Entity> membersByLocation = getMembersByLocation();
-                chosenLocations = getZonePlacementStrategy().locationsForAdditions(membersByLocation, subLocations, delta);
-                if (chosenLocations.size() != delta) {
-                    throw new IllegalStateException("Node placement strategy chose " + Iterables.size(chosenLocations)
-                            + ", when expected delta " + delta + " in " + this);
-                }
-            } else {
-                chosenLocations = Collections.nCopies(delta, getLocation());
+                LOG.warn("Cluster {} has availability-zone enabled, but memberSpec overrides location with {}; using "
+                        + "memberSpec's location; availability-zone behaviour will not apply", this, memberLocations);
+            }
+            chosenLocations = Collections.nCopies(delta, memberLocations.get(0));
+        } else if (isAvailabilityZoneEnabled()) {
+            List<Location> subLocations = getNonFailedSubLocations();
+            Multimap<Location, Entity> membersByLocation = getMembersByLocation();
+            chosenLocations = getZonePlacementStrategy().locationsForAdditions(membersByLocation, subLocations, delta);
+            if (chosenLocations.size() != delta) {
+                throw new IllegalStateException("Node placement strategy chose " + Iterables.size(chosenLocations)
+                        + ", when expected delta " + delta + " in " + this);
             }
         } else {
-            // FIXME: Tidy this up!
-            chosenLocations = Collections.nCopies(delta, chosenLocations.get(0));
+            chosenLocations = Collections.nCopies(delta, getLocation());
         }
 
         // create and start the entities

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/main/java/brooklyn/event/feed/windows/WindowsPerformanceCounterFeed.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/event/feed/windows/WindowsPerformanceCounterFeed.java b/core/src/main/java/brooklyn/event/feed/windows/WindowsPerformanceCounterFeed.java
index 35ffa93..a64d2e6 100644
--- a/core/src/main/java/brooklyn/event/feed/windows/WindowsPerformanceCounterFeed.java
+++ b/core/src/main/java/brooklyn/event/feed/windows/WindowsPerformanceCounterFeed.java
@@ -20,14 +20,13 @@ package brooklyn.event.feed.windows;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import io.cloudsoft.winrm4j.winrm.WinRmToolResponse;
 
-import java.math.BigInteger;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.SortedMap;
-import java.util.SortedSet;
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Matcher;
@@ -49,23 +48,16 @@ import brooklyn.event.basic.Sensors;
 import brooklyn.event.feed.AbstractFeed;
 import brooklyn.event.feed.PollHandler;
 import brooklyn.event.feed.Poller;
-import brooklyn.event.feed.ssh.SshPollValue;
-import brooklyn.location.basic.SshMachineLocation;
 import brooklyn.location.basic.WinRmMachineLocation;
 import brooklyn.management.ExecutionContext;
+import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.flags.TypeCoercions;
-import brooklyn.util.guava.Maybe;
-import brooklyn.util.task.ssh.SshTasks;
-import brooklyn.util.task.system.ProcessTaskFactory;
-import brooklyn.util.task.system.ProcessTaskWrapper;
 import brooklyn.util.time.Duration;
-import io.cloudsoft.winrm4j.winrm.WinRmToolResponse;
 
 import com.google.common.base.Function;
 import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
@@ -107,8 +99,8 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed {
     private static final Joiner JOINER_ON_COMMA = Joiner.on(',');
 
     @SuppressWarnings("serial")
-    public static final ConfigKey<List<WindowsPerformanceCounterPollConfig<?>>> POLLS = ConfigKeys.newConfigKey(
-            new TypeToken<List<WindowsPerformanceCounterPollConfig<?>>>() {},
+    public static final ConfigKey<Collection<WindowsPerformanceCounterPollConfig<?>>> POLLS = ConfigKeys.newConfigKey(
+            new TypeToken<Collection<WindowsPerformanceCounterPollConfig<?>>>() {},
             "polls");
 
     public static Builder builder() {
@@ -180,13 +172,13 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed {
             if (configCopy.getPeriod() < 0) configCopy.period(builder.period);
             polls.add(configCopy);
         }
-        setConfig(POLLS, polls);
+        config().set(POLLS, polls);
         initUniqueTag(builder.uniqueTag, polls);
     }
 
     @Override
     protected void preStart() {
-        List<WindowsPerformanceCounterPollConfig<?>> polls = getConfig(POLLS);
+        Collection<WindowsPerformanceCounterPollConfig<?>> polls = getConfig(POLLS);
         
         long minPeriod = Integer.MAX_VALUE;
         List<String> performanceCounterNames = Lists.newArrayList();
@@ -200,13 +192,16 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed {
                 .add("-Counter")
                 .add(JOINER_ON_COMMA.join(Iterables.transform(performanceCounterNames, QuoteStringFunction.INSTANCE)))
                 .add("-SampleInterval")
-                .add("2") // TODO: This should be a config key
+                .add("2") // TODO: extract SampleInterval as a config key
                 .build();
         String command = String.format("(%s).CounterSamples.CookedValue", JOINER_ON_SPACE.join(allParams));
-        log.debug("Windows performance counter poll command will be: {}", command);
+        log.debug("Windows performance counter poll command for {} will be: {}", entity, command);
 
-        getPoller().scheduleAtFixedRate(new GetPerformanceCountersJob<WinRmToolResponse>(getEntity(), command),
-                new SendPerfCountersToSensors(getEntity(), getConfig(POLLS)), 100L);
+        GetPerformanceCountersJob<WinRmToolResponse> job = new GetPerformanceCountersJob<WinRmToolResponse>(getEntity(), command);
+        getPoller().scheduleAtFixedRate(
+                new CallInEntityExecutionContext<WinRmToolResponse>(entity, job),
+                new SendPerfCountersToSensors(getEntity(), getConfig(POLLS)), 
+                minPeriod);
     }
 
     private static class GetPerformanceCountersJob<T> implements Callable<T> {
@@ -240,7 +235,6 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed {
      * @param <T> The type of the {@link java.util.concurrent.Callable}.
      */
     private static class CallInEntityExecutionContext<T> implements Callable<T> {
-
         private final Callable<T> job;
         private EntityLocal entity;
 
@@ -251,25 +245,26 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed {
 
         @Override
         public T call() throws Exception {
-            final ExecutionContext executionContext
-
-                    = ((EntityInternal) entity).getManagementSupport().getExecutionContext();
+            ExecutionContext executionContext = ((EntityInternal) entity).getManagementSupport().getExecutionContext();
             return executionContext.submit(Maps.newHashMap(), job).get();
         }
     }
 
     private static class SendPerfCountersToSensors implements PollHandler<WinRmToolResponse> {
-
         private final EntityLocal entity;
         private final List<WindowsPerformanceCounterPollConfig<?>> polls;
-
-        public SendPerfCountersToSensors(EntityLocal entity, List<WindowsPerformanceCounterPollConfig<?>> polls) {
+        private final Set<AttributeSensor<?>> failedAttributes = Sets.newLinkedHashSet();
+        
+        public SendPerfCountersToSensors(EntityLocal entity, Collection<WindowsPerformanceCounterPollConfig<?>> polls) {
             this.entity = entity;
-            this.polls = polls;
+            this.polls = ImmutableList.copyOf(polls);
         }
 
         @Override
         public boolean checkSuccess(WinRmToolResponse val) {
+            // TODO not just using statucCode; also looking at absence of stderr.
+            // Status code is (empirically) unreliable: it returns 0 sometimes even when failed 
+            // (but never returns non-zero on success).
             if (val.getStatusCode() != 0) return false;
             String stderr = val.getStdErr();
             if (stderr == null || stderr.length() != 0) return false;
@@ -282,11 +277,20 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed {
         public void onSuccess(WinRmToolResponse val) {
             String[] values = val.getStdOut().split("\r\n");
             for (int i = 0; i < polls.size(); i++) {
-                Class<?> clazz = polls.get(i).getSensor().getType();
-                Maybe<? extends Object> maybeValue = TypeCoercions.tryCoerce(values[i], TypeToken.of(clazz));
-                Object value = maybeValue.isAbsent() ? null : maybeValue.get();
-                AttributeSensor<Object> attribute = (AttributeSensor<Object>) Sensors.newSensor(clazz, polls.get(i).getSensor().getName(), polls.get(i).getDescription());
-                entity.setAttribute(attribute, value);
+                WindowsPerformanceCounterPollConfig<?> config = polls.get(i);
+                Class<?> clazz = config.getSensor().getType();
+                AttributeSensor<Object> attribute = (AttributeSensor<Object>) Sensors.newSensor(clazz, config.getSensor().getName(), config.getDescription());
+                try {
+                    Object value = TypeCoercions.coerce(values[i], TypeToken.of(clazz));
+                    entity.setAttribute(attribute, value);
+                } catch (Exception e) {
+                    Exceptions.propagateIfFatal(e);
+                    if (failedAttributes.add(attribute)) {
+                        log.warn("Failed to coerce value '{}' to {} for {} -> {}", new Object[] {values[i], clazz, entity, attribute});
+                    } else {
+                        if (log.isTraceEnabled()) log.trace("Failed (repeatedly) to coerce value '{}' to {} for {} -> {}", new Object[] {values[i], clazz, entity, attribute});
+                    }
+                }
             }
         }
 
@@ -295,7 +299,9 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed {
             log.error("Windows Performance Counter query did not respond as expected. exitcode={} stdout={} stderr={}",
                     new Object[]{val.getStatusCode(), val.getStdOut(), val.getStdErr()});
             for (WindowsPerformanceCounterPollConfig<?> config : polls) {
-                entity.setAttribute(Sensors.newSensor(config.getSensor().getClass(), config.getPerformanceCounterName(), config.getDescription()), null);
+                Class<?> clazz = config.getSensor().getType();
+                AttributeSensor<?> attribute = Sensors.newSensor(clazz, config.getSensor().getName(), config.getDescription());
+                entity.setAttribute(attribute, null);
             }
         }
 
@@ -319,103 +325,6 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed {
         }
     }
 
-    /**
-     * A poll handler that takes the result of the <tt>typeperf</tt> invocation and sets the appropriate sensors.
-     */
-    private static class SendPerfCountersToSensors2 implements PollHandler<SshPollValue> {
-
-        private static final Set<? extends Class<? extends Number>> INTEGER_TYPES
-                = ImmutableSet.of(Integer.class, Long.class, Byte.class, Short.class, BigInteger.class);
-
-        private final EntityLocal entity;
-        private final SortedMap<String, AttributeSensor> sensorMap;
-
-        public SendPerfCountersToSensors2(EntityLocal entity, List<WindowsPerformanceCounterPollConfig<?>> polls) {
-            this.entity = entity;
-            
-            sensorMap = Maps.newTreeMap();
-            for (WindowsPerformanceCounterPollConfig<?> config : polls) {
-                sensorMap.put(config.getPerformanceCounterName(), config.getSensor());
-            }
-        }
-
-        @Override
-        public boolean checkSuccess(SshPollValue val) {
-            if (val.getExitStatus() != 0) return false;
-            String stderr = val.getStderr();
-            if (stderr == null || stderr.length() != 0) return false;
-            String out = val.getStdout();
-            if (out == null || out.length() == 0) return false;
-            return true;
-        }
-
-        @Override
-        public void onSuccess(SshPollValue val) {
-            String stdout = val.getStdout();
-            Matcher matcher = lineWithPerfData.matcher(stdout);
-            if (!matcher.find()) {
-                onFailure(val);
-                return;
-            }
-
-            String group = matcher.group(0);
-            Iterator<String> values = new PerfCounterValueIterator(group);
-            for (AttributeSensor sensor : sensorMap.values()) {
-                if (!values.hasNext()) {
-                    // The perf counter response has fewer elements than expected
-                    onFailure(val);
-                    return;
-                }
-                String value = values.next();
-
-                Class sensorType = sensor.getType();
-                if (INTEGER_TYPES.contains(sensorType)) {
-                    // Windows always returns decimal-formatted numbers (e.g. 1234.00000), even for integer counters.
-                    // Integer.valueOf() throws a NumberFormatException if it sees something in that format. So for
-                    // pure integer sensors, we truncate the decimal part.
-                    int decimalAt = value.indexOf('.');
-                    if (decimalAt >= 0)
-                        value = value.substring(0, decimalAt);
-                }
-                entity.setAttribute(sensor, TypeCoercions.coerce(value, sensorType));
-            }
-
-            if (values.hasNext()) {
-                // The perf counter response has more elements than expected
-                onFailure(val);
-                return;
-            }
-        }
-
-        @Override
-        public void onFailure(SshPollValue val) {
-            log.error("Windows Performance Counter query did not respond as expected. exitcode={} stdout={} stderr={}",
-                    new Object[]{val.getExitStatus(), val.getStdout(), val.getStderr()});
-            for (AttributeSensor attribute : sensorMap.values()) {
-                entity.setAttribute(attribute, null);
-            }
-        }
-
-        @Override
-        public void onException(Exception exception) {
-            log.error("Detected exception while retrieving Windows Performance Counters from entity " +
-                    entity.getDisplayName(), exception);
-            for (AttributeSensor attribute : sensorMap.values()) {
-                entity.setAttribute(attribute, null);
-            }
-        }
-        
-        @Override
-        public String toString() {
-            return super.toString()+"["+getDescription()+"]";
-        }
-        
-        @Override
-        public String getDescription() {
-            return ""+sensorMap;
-        }
-    }
-
     static class PerfCounterValueIterator implements Iterator<String> {
 
         // This pattern matches the contents of the first field, and optionally matches the rest of the line as

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/main/java/brooklyn/location/basic/WinRmMachineLocation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/basic/WinRmMachineLocation.java b/core/src/main/java/brooklyn/location/basic/WinRmMachineLocation.java
index 1f651e1..ee32568 100644
--- a/core/src/main/java/brooklyn/location/basic/WinRmMachineLocation.java
+++ b/core/src/main/java/brooklyn/location/basic/WinRmMachineLocation.java
@@ -18,6 +18,10 @@
  */
 package brooklyn.location.basic;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+import io.cloudsoft.winrm4j.winrm.WinRmTool;
+import io.cloudsoft.winrm4j.winrm.WinRmToolResponse;
+
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
@@ -31,7 +35,6 @@ import java.util.Set;
 import javax.annotation.Nullable;
 
 import org.apache.commons.codec.binary.Base64;
-import org.python.core.PyException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -41,39 +44,56 @@ import brooklyn.location.MachineDetails;
 import brooklyn.location.MachineLocation;
 import brooklyn.location.OsDetails;
 import brooklyn.util.exceptions.Exceptions;
-import brooklyn.util.flags.SetFromFlag;
+import brooklyn.util.stream.Streams;
 import brooklyn.util.time.Duration;
 import brooklyn.util.time.Time;
-import io.cloudsoft.winrm4j.winrm.WinRmTool;
-import io.cloudsoft.winrm4j.winrm.WinRmToolResponse;
 
 import com.google.common.base.Charsets;
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 
 public class WinRmMachineLocation extends AbstractLocation implements MachineLocation {
 
     private static final Logger LOG = LoggerFactory.getLogger(WinRmMachineLocation.class);
 
-    public static final ConfigKey<String> WINDOWS_USERNAME = ConfigKeys.newStringConfigKey("windows.username",
+    // FIXME Respect `port` config when using {@link WinRmTool}
+    public static final ConfigKey<Integer> WINRM_PORT = ConfigKeys.newIntegerConfigKey(
+            "port",
+            "WinRM port to use when connecting to the remote machine",
+            5985);
+    
+    // TODO merge with {link SshTool#PROP_USER} and {@link SshMachineLocation#user}
+    public static final ConfigKey<String> USER = ConfigKeys.newStringConfigKey("user",
             "Username to use when connecting to the remote machine");
 
-    public static final ConfigKey<String> WINDOWS_PASSWORD = ConfigKeys.newStringConfigKey("windows.password",
+    // TODO merge with {link SshTool#PROP_PASSWORD}
+    public static final ConfigKey<String> PASSWORD = ConfigKeys.newStringConfigKey("password",
             "Password to use when connecting to the remote machine");
 
     public static final ConfigKey<Integer> COPY_FILE_CHUNK_SIZE_BYTES = ConfigKeys.newIntegerConfigKey("windows.copy.file.size.bytes",
             "Size of file chunks (in bytes) to be used when copying a file to the remote server", 1024);
 
-    @SetFromFlag
-    protected String user;
+     public static final ConfigKey<InetAddress> ADDRESS = ConfigKeys.newConfigKey(
+            InetAddress.class,
+            "address",
+            "Address of the remote machine");
 
-    @SetFromFlag(nullable = false)
-    protected InetAddress address;
+    public static final ConfigKey<Integer> EXECUTION_ATTEMPTS = ConfigKeys.newIntegerConfigKey(
+            "windows.exec.attempts",
+            "Number of attempts to execute a remote command",
+            1);
+    
+    // TODO See SshTool#PROP_SSH_TRIES, where it was called "sshTries"; remove duplication? Merge into one well-named thing?
+    public static final ConfigKey<Integer> EXEC_TRIES = ConfigKeys.newIntegerConfigKey(
+            "execTries", 
+            "Max number of times to attempt WinRM operations", 
+            10);
 
     @Override
     public InetAddress getAddress() {
-        return address;
+        return getConfig(ADDRESS);
     }
 
     @Override
@@ -89,17 +109,19 @@ public class WinRmMachineLocation extends AbstractLocation implements MachineLoc
     @Nullable
     @Override
     public String getHostname() {
-        return address.getHostAddress();
+        InetAddress address = getAddress();
+        return (address != null) ? address.getHostAddress() : null;
     }
 
     @Override
     public Set<String> getPublicAddresses() {
-        return null;
+        InetAddress address = getAddress();
+        return (address == null) ? ImmutableSet.<String>of() : ImmutableSet.of(address.getHostAddress());
     }
-
+    
     @Override
     public Set<String> getPrivateAddresses() {
-        return null;
+        return ImmutableSet.of();
     }
 
     public WinRmToolResponse executeScript(String script) {
@@ -107,32 +129,53 @@ public class WinRmMachineLocation extends AbstractLocation implements MachineLoc
     }
 
     public WinRmToolResponse executeScript(List<String> script) {
+        int execTries = getRequiredConfig(EXEC_TRIES);
         Collection<Throwable> exceptions = Lists.newArrayList();
-        for (int i = 0; i < 10; i++) {
+        for (int i = 0; i < execTries; i++) {
             try {
-                WinRmTool winRmTool = WinRmTool.connect(getHostname(), getUsername(), getPassword());
-                WinRmToolResponse response = winRmTool.executeScript(script);
-                return response;
+                return executeScriptNoRetry(script);
             } catch (Exception e) {
-                LOG.warn("ignoring winrm exception and retrying:", e);
+                Exceptions.propagateIfFatal(e);
+                if (i == (execTries+1)) {
+                    LOG.info("Propagating WinRM exception (attempt "+(i+1)+" of "+execTries+")", e);
+                } else if (i == 0) {
+                    LOG.warn("Ignoring WinRM exception and retrying (attempt "+(i+1)+" of "+execTries+")", e);
+                } else {
+                    LOG.debug("Ignoring WinRM exception and retrying (attempt "+(i+1)+" of "+execTries+")", e);
+                }
                 exceptions.add(e);
             }
         }
         throw Exceptions.propagate("failed to execute shell script", exceptions);
     }
 
+    protected WinRmToolResponse executeScriptNoRetry(List<String> script) {
+        WinRmTool winRmTool = WinRmTool.connect(getHostname(), getUser(), getPassword());
+        WinRmToolResponse response = winRmTool.executeScript(script);
+        return response;
+    }
+
     public WinRmToolResponse executePsScript(String psScript) {
         return executePsScript(ImmutableList.of(psScript));
     }
 
     public WinRmToolResponse executePsScript(List<String> psScript) {
+        int execTries = getRequiredConfig(EXEC_TRIES);
         Collection<Throwable> exceptions = Lists.newArrayList();
-        for (int i = 0; i < 10; i++) {
+        for (int i = 0; i < execTries; i++) {
             try {
                 return executePsScriptNoRetry(psScript);
             } catch (Exception e) {
-                LOG.warn("ignoring winrm exception and retrying after 5 seconds:", e);
-                Time.sleep(Duration.FIVE_SECONDS);
+                Exceptions.propagateIfFatal(e);
+                if (i == (execTries+1)) {
+                    LOG.info("Propagating WinRM exception (attempt "+(i+1)+" of "+execTries+")", e);
+                } else if (i == 0) {
+                    LOG.warn("Ignoring WinRM exception and retrying after 5 seconds (attempt "+(i+1)+" of "+execTries+")", e);
+                    Time.sleep(Duration.FIVE_SECONDS);
+                } else {
+                    LOG.debug("Ignoring WinRM exception and retrying after 5 seconds (attempt "+(i+1)+" of "+execTries+")", e);
+                    Time.sleep(Duration.FIVE_SECONDS);
+                }
                 exceptions.add(e);
             }
         }
@@ -140,21 +183,27 @@ public class WinRmMachineLocation extends AbstractLocation implements MachineLoc
     }
 
     public WinRmToolResponse executePsScriptNoRetry(List<String> psScript) {
-        WinRmTool winRmTool = WinRmTool.connect(getHostname(), getUsername(), getPassword());
+        WinRmTool winRmTool = WinRmTool.connect(getHostname(), getUser(), getPassword());
         WinRmToolResponse response = winRmTool.executePs(psScript);
         return response;
     }
 
-    public int copyTo(File source, File destination) {
+    public int copyTo(File source, String destination) {
+        FileInputStream sourceStream = null;
         try {
-            return copyTo(new FileInputStream(source), destination);
+            sourceStream = new FileInputStream(source);
+            return copyTo(sourceStream, destination);
         } catch (FileNotFoundException e) {
             throw Exceptions.propagate(e);
+        } finally {
+            if (sourceStream != null) {
+                Streams.closeQuietly(sourceStream);
+            }
         }
     }
 
-    public int copyTo(InputStream source, File destination) {
-        executePsScript(ImmutableList.of("rm -ErrorAction SilentlyContinue " + destination.getPath()));
+    public int copyTo(InputStream source, String destination) {
+        executePsScript(ImmutableList.of("rm -ErrorAction SilentlyContinue " + destination));
         try {
             int chunkSize = getConfig(COPY_FILE_CHUNK_SIZE_BYTES);
             byte[] inputData = new byte[chunkSize];
@@ -167,8 +216,8 @@ public class WinRmMachineLocation extends AbstractLocation implements MachineLoc
                 } else {
                     chunk = Arrays.copyOf(inputData, bytesRead);
                 }
-                executePsScript(ImmutableList.of("If ((!(Test-Path " + destination.getPath() + ")) -or ((Get-Item '" + destination.getPath() + "').length -eq " +
-                        expectedFileSize + ")) {Add-Content -Encoding Byte -path " + destination.getPath() +
+                executePsScript(ImmutableList.of("If ((!(Test-Path " + destination + ")) -or ((Get-Item '" + destination + "').length -eq " +
+                        expectedFileSize + ")) {Add-Content -Encoding Byte -path " + destination +
                         " -value ([System.Convert]::FromBase64String(\"" + new String(Base64.encodeBase64(chunk)) + "\"))}"));
                 expectedFileSize += bytesRead;
             }
@@ -179,14 +228,18 @@ public class WinRmMachineLocation extends AbstractLocation implements MachineLoc
         }
     }
 
-    public String getUsername() {
-        return config().get(WINDOWS_USERNAME);
+    public String getUser() {
+        return config().get(USER);
     }
 
     private String getPassword() {
-        return config().get(WINDOWS_PASSWORD);
+        return config().get(PASSWORD);
     }
 
+    private <T> T getRequiredConfig(ConfigKey<T> key) {
+        return checkNotNull(getConfig(key), "key %s must be set", key);
+    }
+    
     public static String getDefaultUserMetadataString() {
         // Using an encoded command obviates the need to escape
         String unencodePowershell = Joiner.on("\r\n").join(ImmutableList.of(

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/main/java/brooklyn/util/flags/TypeCoercions.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/flags/TypeCoercions.java b/core/src/main/java/brooklyn/util/flags/TypeCoercions.java
index 74ada77..332c864 100644
--- a/core/src/main/java/brooklyn/util/flags/TypeCoercions.java
+++ b/core/src/main/java/brooklyn/util/flags/TypeCoercions.java
@@ -31,7 +31,6 @@ import java.math.BigInteger;
 import java.net.InetAddress;
 import java.net.URI;
 import java.net.URL;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -73,6 +72,7 @@ import com.google.common.base.Predicate;
 import com.google.common.collect.HashBasedTable;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
@@ -710,11 +710,43 @@ public class TypeCoercions {
                 return QuorumChecks.of(input);
             }
         });
-        registerAdapter(ArrayList.class, String[].class, new Function<ArrayList, String[]>() {
+        registerAdapter(Iterable.class, String[].class, new Function<Iterable, String[]>() {
             @Nullable
             @Override
-            public String[] apply(@Nullable ArrayList arrayList) {
-                return (String[]) arrayList.toArray(new String[]{});
+            public String[] apply(@Nullable Iterable list) {
+                if (list == null) return null;
+                String[] result = new String[Iterables.size(list)];
+                int count = 0;
+                for (Object element : list) {
+                    result[count++] = coerce(element, String.class);
+                }
+                return result;
+            }
+        });
+        registerAdapter(Iterable.class, Integer[].class, new Function<Iterable, Integer[]>() {
+            @Nullable
+            @Override
+            public Integer[] apply(@Nullable Iterable list) {
+                if (list == null) return null;
+                Integer[] result = new Integer[Iterables.size(list)];
+                int count = 0;
+                for (Object element : list) {
+                    result[count++] = coerce(element, Integer.class);
+                }
+                return result;
+            }
+        });
+        registerAdapter(Iterable.class, int[].class, new Function<Iterable, int[]>() {
+            @Nullable
+            @Override
+            public int[] apply(@Nullable Iterable list) {
+                if (list == null) return null;
+                int[] result = new int[Iterables.size(list)];
+                int count = 0;
+                for (Object element : list) {
+                    result[count++] = coerce(element, int.class);
+                }
+                return result;
             }
         });
         registerAdapter(String.class, Map.class, new Function<String,Map>() {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java b/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java
index 58949d7..947068d 100644
--- a/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java
+++ b/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java
@@ -936,6 +936,21 @@ public class DynamicClusterTest extends BrooklynAppUnitTestSupport {
         assertFirstAndNonFirstCounts(cluster.getMembers(), 1, 2);
     }
 
+    @Test
+    public void testPrefersMemberSpecLocation() throws Exception {
+        DynamicCluster cluster = app.createAndManageChild(EntitySpec.create(DynamicCluster.class)
+                .configure(DynamicCluster.MEMBER_SPEC, EntitySpec.create(TestEntity.class)
+                        .location(loc2))
+                .configure(DynamicCluster.INITIAL_SIZE, 1));
+        
+        cluster.start(ImmutableList.of(loc));
+        assertEquals(ImmutableList.copyOf(cluster.getLocations()), ImmutableList.of(loc));
+        
+        Entity member = Iterables.getOnlyElement(cluster.getMembers());
+        assertEquals(ImmutableList.copyOf(member.getLocations()), ImmutableList.of(loc2));
+    }
+
+
     private void assertFirstAndNonFirstCounts(Collection<Entity> members, int expectedFirstCount, int expectedNonFirstCount) {
         Set<Entity> found = MutableSet.of();
         for (Entity e: members) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/test/java/brooklyn/location/basic/ByonLocationResolverTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/location/basic/ByonLocationResolverTest.java b/core/src/test/java/brooklyn/location/basic/ByonLocationResolverTest.java
index 0d6e5ff..c83d2f0 100644
--- a/core/src/test/java/brooklyn/location/basic/ByonLocationResolverTest.java
+++ b/core/src/test/java/brooklyn/location/basic/ByonLocationResolverTest.java
@@ -60,8 +60,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 
-public class
-        ByonLocationResolverTest {
+public class ByonLocationResolverTest {
 
     private static final Logger log = LoggerFactory.getLogger(ByonLocationResolverTest.class);
     
@@ -316,19 +315,19 @@ public class
 
     @Test
     public void testWindowsMachines() throws Exception {
-        brooklynProperties.put("brooklyn.location.byon.windows.username", "myuser");
-        brooklynProperties.put("brooklyn.location.byon.windows.password", "mypassword");
+        brooklynProperties.put("brooklyn.location.byon.user", "myuser");
+        brooklynProperties.put("brooklyn.location.byon.password", "mypassword");
         String spec = "byon";
         Map<String, ?> flags = ImmutableMap.of(
                 "hosts", ImmutableList.of("1.1.1.1", "2.2.2.2"),
                 "osfamily", "windows"
         );
         MachineProvisioningLocation<MachineLocation> provisioner = resolve(spec, flags);
-        MachineLocation location = provisioner.obtain(ImmutableMap.of());
+        WinRmMachineLocation location = (WinRmMachineLocation) provisioner.obtain(ImmutableMap.of());
 
-        assertEquals(location.config().get(WinRmMachineLocation.WINDOWS_USERNAME), "myuser");
-        assertEquals(location.config().get(WinRmMachineLocation.WINDOWS_PASSWORD), "mypassword");
-        Assert.assertNotNull(provisioner);
+        assertEquals(location.config().get(WinRmMachineLocation.USER), "myuser");
+        assertEquals(location.config().get(WinRmMachineLocation.PASSWORD), "mypassword");
+        assertEquals(location.config().get(WinRmMachineLocation.ADDRESS).getHostAddress(), "1.1.1.1");
     }
 
     @Test

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/test/java/brooklyn/util/internal/TypeCoercionsTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/util/internal/TypeCoercionsTest.java b/core/src/test/java/brooklyn/util/internal/TypeCoercionsTest.java
index 3133b22..3c09e1b 100644
--- a/core/src/test/java/brooklyn/util/internal/TypeCoercionsTest.java
+++ b/core/src/test/java/brooklyn/util/internal/TypeCoercionsTest.java
@@ -22,7 +22,7 @@ import static org.testng.Assert.assertEquals;
 
 import java.math.BigDecimal;
 import java.math.BigInteger;
-import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -35,6 +35,7 @@ import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import brooklyn.entity.basic.Lifecycle;
+import brooklyn.util.collections.MutableSet;
 import brooklyn.util.flags.ClassCoercionException;
 import brooklyn.util.flags.TypeCoercions;
 import brooklyn.util.text.StringPredicates;
@@ -43,7 +44,6 @@ import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Lists;
 import com.google.common.reflect.TypeToken;
 
 public class TypeCoercionsTest {
@@ -188,6 +188,21 @@ public class TypeCoercionsTest {
     }
     
     @Test
+    public void testIterableToArrayCoercion() {
+        String[] s = TypeCoercions.coerce(ImmutableList.of("a", "b"), String[].class);
+        Assert.assertTrue(Arrays.equals(s, new String[] {"a", "b"}), "result="+Arrays.toString(s));
+        
+        Integer[] i = TypeCoercions.coerce(ImmutableList.of(1, 2), Integer[].class);
+        Assert.assertTrue(Arrays.equals(i, new Integer[] {1, 2}), "result="+Arrays.toString(i));
+        
+        int[] i2 = TypeCoercions.coerce(ImmutableList.of(1, 2), int[].class);
+        Assert.assertTrue(Arrays.equals(i2, new int[] {1, 2}), "result="+Arrays.toString(i2));
+        
+        int[] i3 = TypeCoercions.coerce(MutableSet.of("1", 2), int[].class);
+        Assert.assertTrue(Arrays.equals(i3, new int[] {1, 2}), "result="+Arrays.toString(i3));
+    }
+
+    @Test
     public void testListEntryCoercion() {
         List<?> s = TypeCoercions.coerce(ImmutableList.of("java.lang.Integer", "java.lang.Double"), new TypeToken<List<Class<?>>>() { });
         Assert.assertEquals(s, ImmutableList.of(Integer.class, Double.class));
@@ -204,7 +219,7 @@ public class TypeCoercionsTest {
         Collection<?> s = TypeCoercions.coerce(ImmutableList.of("java.lang.Integer", "java.lang.Double"), new TypeToken<Collection<Class<?>>>() { });
         Assert.assertEquals(s, ImmutableList.of(Integer.class, Double.class));
     }
-    
+
     @Test
     public void testMapValueCoercion() {
         Map<?,?> s = TypeCoercions.coerce(ImmutableMap.of("int", "java.lang.Integer", "double", "java.lang.Double"), new TypeToken<Map<String, Class<?>>>() { });
@@ -309,11 +324,6 @@ public class TypeCoercionsTest {
         assertEquals(TypeCoercions.coerce("1.0", Number.class), (Number) Double.valueOf(1.0));
     }
 
-    @Test
-    public void testArrayListToArray() {
-        assertEquals(TypeCoercions.coerce(Lists.newArrayList("Foo", "Bar", "Baz"), String[].class), new String[] {"Foo", "Bar", "Baz"});
-    }
-
     @Test(expectedExceptions = ClassCoercionException.class)
     public void testInvalidCoercionThrowsClassCoercionException() {
         TypeCoercions.coerce(new Object(), TypeToken.of(Integer.class));

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/locations/jclouds/src/main/java/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java
index 30f7700..502c40c 100644
--- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java
+++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java
@@ -52,16 +52,46 @@ public class BasicJcloudsLocationCustomizer implements JcloudsLocationCustomizer
     }
 
     @Override
+    public void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) {
+        if (machine instanceof JcloudsSshMachineLocation) {
+            customize(location, computeService, (JcloudsSshMachineLocation)machine);
+        } else {
+            // no-op
+        }
+    }
+    
+    @Override
+    public void preRelease(JcloudsMachineLocation machine) {
+        if (machine instanceof JcloudsSshMachineLocation) {
+            preRelease((JcloudsSshMachineLocation)machine);
+        } else {
+            // no-op
+        }
+    }
+
+    @Override
+    public void postRelease(JcloudsMachineLocation machine) {
+        if (machine instanceof JcloudsSshMachineLocation) {
+            postRelease((JcloudsSshMachineLocation)machine);
+        } else {
+            // no-op
+        }
+    }
+    
+    @Override
+    @Deprecated
     public void customize(JcloudsLocation location, ComputeService computeService, JcloudsSshMachineLocation machine) {
         // no-op
     }
 
     @Override
+    @Deprecated
     public void preRelease(JcloudsSshMachineLocation machine) {
         // no-op
     }
 
     @Override
+    @Deprecated
     public void postRelease(JcloudsSshMachineLocation machine) {
         // no-op
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
index df1c089..a4f6d75 100644
--- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
+++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
@@ -22,10 +22,11 @@ import static brooklyn.util.JavaGroovyEquivalents.elvis;
 import static brooklyn.util.JavaGroovyEquivalents.groovyTruth;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.jclouds.compute.options.RunScriptOptions.Builder.overrideLoginCredentials;
 import static org.jclouds.scriptbuilder.domain.Statements.exec;
+import io.cloudsoft.winrm4j.pywinrm.Session;
+import io.cloudsoft.winrm4j.pywinrm.WinRMFactory;
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
@@ -150,8 +151,6 @@ import brooklyn.util.text.Strings;
 import brooklyn.util.text.TemplateProcessor;
 import brooklyn.util.time.Duration;
 import brooklyn.util.time.Time;
-import io.cloudsoft.winrm4j.pywinrm.Session;
-import io.cloudsoft.winrm4j.pywinrm.WinRMFactory;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Charsets;
@@ -572,7 +571,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         CloudMachineNamer cloudMachineNamer = getCloudMachineNamer(setup);
         String groupId = elvis(setup.get(GROUP_ID), cloudMachineNamer.generateNewGroupId(setup));
         NodeMetadata node = null;
-        MachineLocation machineLocation = null;
+        JcloudsMachineLocation machineLocation = null;
         
         try {
             LOG.info("Creating VM "+setup.getDescription()+" in "+this);
@@ -644,11 +643,15 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             boolean windows = node.getOperatingSystem().getFamily().equals(OsFamily.WINDOWS);
 
             if (windows) {
-                // FIXME: Tidy this up and allow for user-specified credentials
+                // FIXME Allow for user-specified credentials:
+                //  - want to support setting up a given user on the VM (or resetting the Administrator password)
+                //  - and support vCloudDirector use-case where template contains username/password, so don't just use
+                //    what is returned by jclouds.
                 setup.put(USER, node.getCredentials().getUser());
                 setup.put(PASSWORD, node.getCredentials().getOptionalPassword().orNull());
             }
 
+            // TODO How do we influence the node.getLoginPort, so it is set correctly for Windows?
             // Setup port-forwarding, if required
             Optional<HostAndPort> sshHostAndPortOverride;
             if (usePortForwarding) {
@@ -662,6 +665,18 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 sshHostAndPortOverride = Optional.absent();
             }
 
+            if (waitForSshable) {
+                if (windows) {
+                    // TODO Should we generalise `waitForSshable` to a `waitForReachable`?
+                    // TODO Does jclouds support any windows user setup?
+                    waitForWinRmAvailable(node, node.getCredentials(), setup);
+                } else if (skipJcloudsSshing) {
+                    // once that host:port is definitely reachable, we can create the user
+                    waitForReachable(computeService, node, sshHostAndPortOverride, node.getCredentials(), setup);
+                    userCredentials = createUser(computeService, node, sshHostAndPortOverride, setup);
+                }
+            }
+
             if (waitForSshable && skipJcloudsSshing && !windows) {
                 // once that host:port is definitely reachable, we can create the user
                 waitForReachable(computeService, node, sshHostAndPortOverride, node.getCredentials(), setup);
@@ -701,19 +716,15 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             }
             usableTimestamp = Duration.of(provisioningStopwatch);
 
-            JcloudsSshMachineLocation jcloudsSshMachineLocation = null;
-            WinRmMachineLocation winRmMachineLocation = null;
+//            JcloudsSshMachineLocation jcloudsSshMachineLocation = null;
+//            WinRmMachineLocation winRmMachineLocation = null;
             // Create a JcloudsSshMachineLocation, and register it
             if (windows) {
-                // FIMXE: Need to write WinRM equivalent of getPublicHostname
-                String hostName = node.getPublicAddresses().iterator().next();
-                winRmMachineLocation = registerWinRmMachineLocation(hostName, setup, node.getId());
-                machineLocation = winRmMachineLocation;
+                machineLocation = registerWinRmMachineLocation(computeService, node, setup);
             } else {
-                jcloudsSshMachineLocation = registerJcloudsSshMachineLocation(computeService, node, userCredentials, sshHostAndPortOverride, setup);
-                machineLocation = jcloudsSshMachineLocation;
-                if (template!=null && jcloudsSshMachineLocation.getTemplate()==null) {
-                    jcloudsSshMachineLocation.template = template;
+                machineLocation = registerJcloudsSshMachineLocation(computeService, node, userCredentials, sshHostAndPortOverride, setup);
+                if (template!=null && machineLocation.getTemplate()==null) {
+                    ((JcloudsSshMachineLocation)machineLocation).template = template;
                 }
             }
 
@@ -737,12 +748,12 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 if (portForwardManager != null) {
                     for(Integer containerPort : portMappings.keySet()) {
                         Integer hostPort = portMappings.get(containerPort);
-                        String dockerHost = jcloudsSshMachineLocation.getSshHostAndPort().getHostText();
-                        portForwardManager.associate(node.getId(), HostAndPort.fromParts(dockerHost, hostPort), jcloudsSshMachineLocation, containerPort);
+                        String dockerHost = ((JcloudsSshMachineLocation)machineLocation).getSshHostAndPort().getHostText();
+                        portForwardManager.associate(node.getId(), HostAndPort.fromParts(dockerHost, hostPort), machineLocation, containerPort);
                     }
                 } else {
                     LOG.warn("No port-forward manager for {} so could not associate docker port-mappings for {}",
-                            this, jcloudsSshMachineLocation);
+                            this, machineLocation);
                 }
             }
 
@@ -754,8 +765,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 List<String> setupScripts = setup.get(JcloudsLocationConfig.CUSTOM_MACHINE_SETUP_SCRIPT_URL_LIST);
                 Collection<String> allScripts = new MutableList<String>().appendIfNotNull(setupScript).appendAll(setupScripts);
                 for (String setupScriptItem : allScripts) {
-                    if (Strings.isNonBlank(setupScript)) {
-                        customisationForLogging.add("custom setup script " + setupScript);
+                    if (Strings.isNonBlank(setupScriptItem)) {
+                        customisationForLogging.add("custom setup script " + setupScriptItem);
 
                         String setupVarsString = setup.get(JcloudsLocationConfig.CUSTOM_MACHINE_SETUP_SCRIPT_VARS);
                         Map<String, String> substitutions = (setupVarsString != null)
@@ -764,20 +775,20 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                         String scriptContent = ResourceUtils.create(this).getResourceAsString(setupScriptItem);
                         String script = TemplateProcessor.processTemplateContents(scriptContent, getManagementContext(), substitutions);
                         if (windows) {
-                            winRmMachineLocation.executeScript(ImmutableList.copyOf((script.replace("\r", "").split("\n"))));
+                            ((WinRmMachineLocation)machineLocation).executeScript(ImmutableList.copyOf((script.replace("\r", "").split("\n"))));
                         } else {
-                            jcloudsSshMachineLocation.execCommands("Customizing node " + this, ImmutableList.of(script));
+                            ((SshMachineLocation)machineLocation).execCommands("Customizing node " + this, ImmutableList.of(script));
                         }
                     }
                 }
 
                 if (setup.get(JcloudsLocationConfig.MAP_DEV_RANDOM_TO_DEV_URANDOM)) {
                     if (windows) {
-                        LOG.warn("Ignoring flag MAP_DEV_RANDOM_TO_DEV_URANDOM on Windows location {}", winRmMachineLocation);
+                        LOG.warn("Ignoring flag MAP_DEV_RANDOM_TO_DEV_URANDOM on Windows location {}", machineLocation);
                     } else {
                         customisationForLogging.add("point /dev/random to urandom");
 
-                        jcloudsSshMachineLocation.execCommands("using urandom instead of random",
+                        ((SshMachineLocation)machineLocation).execCommands("using urandom instead of random",
                                 Arrays.asList("sudo mv /dev/random /dev/random-real", "sudo ln -s /dev/urandom /dev/random"));
                     }
                 }
@@ -786,11 +797,11 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 if (setup.get(GENERATE_HOSTNAME)) {
                     if (windows) {
                         // TODO: Generate Windows Hostname
-                        LOG.warn("Ignoring flag GENERATE_HOSTNAME on Windows location {}", winRmMachineLocation);
+                        LOG.warn("Ignoring flag GENERATE_HOSTNAME on Windows location {}", machineLocation);
                     } else {
                         customisationForLogging.add("configure hostname");
 
-                        jcloudsSshMachineLocation.execCommands("Generate hostname " + node.getName(),
+                        ((SshMachineLocation)machineLocation).execCommands("Generate hostname " + node.getName(),
                                 Arrays.asList("sudo hostname " + node.getName(),
                                         "sudo sed -i \"s/HOSTNAME=.*/HOSTNAME=" + node.getName() + "/g\" /etc/sysconfig/network",
                                         "sudo bash -c \"echo 127.0.0.1   `hostname` >> /etc/hosts\"")
@@ -800,13 +811,13 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
                 if (setup.get(OPEN_IPTABLES)) {
                     if (windows) {
-                        LOG.warn("Ignoring flag OPEN_IPTABLES on Windows location {}", winRmMachineLocation);
+                        LOG.warn("Ignoring flag OPEN_IPTABLES on Windows location {}", machineLocation);
                     } else {
                         @SuppressWarnings("unchecked")
                         Iterable<Integer> inboundPorts = (Iterable<Integer>) setup.get(INBOUND_PORTS);
 
                         if (inboundPorts == null || Iterables.isEmpty(inboundPorts)) {
-                            LOG.info("No ports to open in iptables (no inbound ports) for {} at {}", jcloudsSshMachineLocation, this);
+                            LOG.info("No ports to open in iptables (no inbound ports) for {} at {}", machineLocation, this);
                         } else {
                             customisationForLogging.add("open iptables");
 
@@ -818,39 +829,39 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                             for (String rule : iptablesRules) {
                                 batch.add(rule);
                                 if (batch.size() == 50) {
-                                    jcloudsSshMachineLocation.execCommands("Inserting iptables rules, 50 command batch", batch);
+                                    ((SshMachineLocation)machineLocation).execCommands("Inserting iptables rules, 50 command batch", batch);
                                     batch.clear();
                                 }
                             }
                             if (batch.size() > 0) {
-                                jcloudsSshMachineLocation.execCommands("Inserting iptables rules", batch);
+                                ((SshMachineLocation)machineLocation).execCommands("Inserting iptables rules", batch);
                             }
-                            jcloudsSshMachineLocation.execCommands("List iptables rules", ImmutableList.of(IptablesCommands.listIptablesRule()));
+                            ((SshMachineLocation)machineLocation).execCommands("List iptables rules", ImmutableList.of(IptablesCommands.listIptablesRule()));
                         }
                     }
                 }
 
                 if (setup.get(STOP_IPTABLES)) {
                     if (windows) {
-                        LOG.warn("Ignoring flag OPEN_IPTABLES on Windows location {}", winRmMachineLocation);
+                        LOG.warn("Ignoring flag OPEN_IPTABLES on Windows location {}", machineLocation);
                     } else {
                         customisationForLogging.add("stop iptables");
 
                         List<String> cmds = ImmutableList.of(IptablesCommands.iptablesServiceStop(), IptablesCommands.iptablesServiceStatus());
-                        jcloudsSshMachineLocation.execCommands("Stopping iptables", cmds);
+                        ((SshMachineLocation)machineLocation).execCommands("Stopping iptables", cmds);
                     }
                 }
 
                 List<String> extraKeyUrlsToAuth = setup.get(EXTRA_PUBLIC_KEY_URLS_TO_AUTH);
                 if (extraKeyUrlsToAuth!=null && !extraKeyUrlsToAuth.isEmpty()) {
                     if (windows) {
-                        LOG.warn("Ignoring flag EXTRA_PUBLIC_KEY_URLS_TO_AUTH on Windows location", winRmMachineLocation);
+                        LOG.warn("Ignoring flag EXTRA_PUBLIC_KEY_URLS_TO_AUTH on Windows location", machineLocation);
                     } else {
                         List<String> extraKeyDataToAuth = MutableList.of();
                         for (String keyUrl : extraKeyUrlsToAuth) {
                             extraKeyDataToAuth.add(ResourceUtils.create().getResourceAsString(keyUrl));
                         }
-                        jcloudsSshMachineLocation.execCommands("Authorizing ssh keys",
+                        ((SshMachineLocation)machineLocation).execCommands("Authorizing ssh keys",
                                 ImmutableList.of(new AuthorizeRSAPublicKeys(extraKeyDataToAuth).render(org.jclouds.scriptbuilder.domain.OsFamily.UNIX)));
                     }
                 }
@@ -862,48 +873,32 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
             // Apply any optional app-specific customization.
             for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) {
-                if (windows) {
-                    LOG.warn("Ignoring customizer {} on Windows location {}", customizer, winRmMachineLocation);
-                    // TODO: WinRm based JcloudsLocationCustomizer
-                } else {
-                    customizer.customize(this, computeService, jcloudsSshMachineLocation);
-                }
+                customizer.customize(this, computeService, machineLocation);
             }
 
             customizedTimestamp = Duration.of(provisioningStopwatch);
 
-            String logMessage;
-
-            if (windows) {
-                // TODO: More complete logging for WinRmMachineLocation
-                // FIXME: Remove try-catch!
-                try {
-                    logMessage = "Finished VM " + setup.getDescription() + " creation:"
-                            + " " + winRmMachineLocation.getConfig(WinRmMachineLocation.WINDOWS_USERNAME) + "@" + machineLocation.getAddress()
-                            + " username=" + winRmMachineLocation.getUsername()
-                            + " ready after " + Duration.of(provisioningStopwatch).toStringRounded()
-                            + " (" + template + " template built in " + Duration.of(templateTimestamp).toStringRounded() + ";"
-                            + " " + node + " provisioned in " + Duration.of(provisionTimestamp).subtract(templateTimestamp).toStringRounded() + ";";
-                } catch (Exception e){
-                    logMessage = Arrays.toString(e.getStackTrace());
-                }
-            } else {
-                logMessage = "Finished VM "+setup.getDescription()+" creation:"
-                        + " "+jcloudsSshMachineLocation.getUser()+"@"+machineLocation.getAddress()+":"+jcloudsSshMachineLocation.getPort()
+            try {
+                String logMessage = "Finished VM "+setup.getDescription()+" creation:"
+                        + " "+machineLocation.getUser()+"@"+machineLocation.getAddress()+":"+machineLocation.getPort()
                         + (Boolean.TRUE.equals(setup.get(LOG_CREDENTIALS))
-                        ? "password=" + userCredentials.getOptionalPassword().or("<absent>")
-                        + " && key=" + userCredentials.getOptionalPrivateKey().or("<absent>")
-                        : "")
+                                ? "password=" + userCredentials.getOptionalPassword().or("<absent>")
+                                + " && key=" + userCredentials.getOptionalPrivateKey().or("<absent>")
+                                : "")
                         + " ready after "+Duration.of(provisioningStopwatch).toStringRounded()
                         + " ("+template+" template built in "+Duration.of(templateTimestamp).toStringRounded()+";"
                         + " "+node+" provisioned in "+Duration.of(provisionTimestamp).subtract(templateTimestamp).toStringRounded()+";"
-                        + " "+jcloudsSshMachineLocation+" ssh usable in "+Duration.of(usableTimestamp).subtract(provisionTimestamp).toStringRounded()+";"
+                        + " "+machineLocation+" connection usable in "+Duration.of(usableTimestamp).subtract(provisionTimestamp).toStringRounded()+";"
                         + " and os customized in "+Duration.of(customizedTimestamp).subtract(usableTimestamp).toStringRounded()+" - "+Joiner.on(", ").join(customisationForLogging)+")";
+                LOG.info(logMessage);
+            } catch (Exception e){
+                // TODO Remove try-catch! @Nakomis: why did you add it? What exception happened during logging?
+                Exceptions.propagateIfFatal(e);
+                LOG.warn("Problem generating log message summarising completion of jclouds machine provisioning "+machineLocation+" by "+this, e);
             }
-            
-            LOG.info(logMessage);
 
             return machineLocation;
+            
         } catch (Exception e) {
             if (e instanceof RunNodesException && ((RunNodesException)e).getNodeErrors().size() > 0) {
                 node = Iterables.get(((RunNodesException)e).getNodeErrors().keySet(), 0);
@@ -927,53 +922,6 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         }
     }
 
-    private void waitForWinRmAvailable(final NodeMetadata node, final LoginCredentials expectedCredentials, ConfigBag setup) {
-        // TODO: Reduce / remove duplication between this and waitForReachable
-        String waitForWinRmAvailable = setup.get(WAIT_FOR_WINRM_AVAILABLE);
-        checkArgument(!"false".equalsIgnoreCase(waitForWinRmAvailable), "waitForWinRmAvailable called despite waitForWinRmAvailable=%s", waitForWinRmAvailable);
-
-        long delayMs = -1;
-        try {
-            delayMs = Time.parseTimeString(""+waitForWinRmAvailable);
-        } catch (Exception e) {
-            // normal if 'true'; just fall back to default
-        }
-        if (delayMs<0)
-            delayMs = Time.parseTimeString(WAIT_FOR_WINRM_AVAILABLE.getDefaultValue());
-
-        // FIXME: remove this
-        LOG.info("Address: " + node.getPublicAddresses().iterator().next());
-        LOG.info("User: " + expectedCredentials.getUser());
-        LOG.info("Password: " + expectedCredentials.getOptionalPassword().get());
-        final Session session = WinRMFactory.INSTANCE.createSession(node.getPublicAddresses().iterator().next(),
-                expectedCredentials.getUser(), expectedCredentials.getOptionalPassword().get());
-
-        Callable<Boolean> checker = new Callable<Boolean>() {
-            public Boolean call() {
-                return session.run_cmd("hostname").getStatusCode() == 0;
-            }};
-
-        Stopwatch stopwatch = Stopwatch.createStarted();
-
-        ReferenceWithError<Boolean> reachable = new Repeater()
-                .every(1, SECONDS)
-                .until(checker)
-                .limitTimeTo(delayMs, MILLISECONDS)
-                .runKeepingError();
-
-        if (!reachable.getWithoutError()) {
-            throw new IllegalStateException("WinRm failed for "+
-                    expectedCredentials.getUser()+"@"+node.getPublicAddresses().iterator().next()+" ("+setup.getDescription()+") after waiting "+
-                    Time.makeTimeStringRounded(delayMs), reachable.getError());
-        }
-
-        LOG.debug("VM {}: is available via WinRm after {} on {}@{}",new Object[] {
-                setup.getDescription(), Time.makeTimeStringRounded(stopwatch),
-                expectedCredentials.getUser(), node.getPublicAddresses().iterator().next()});
-
-    }
-
-
     // ------------- constructing the template, etc ------------------------
 
     private static interface CustomizeTemplateBuilder {
@@ -1074,15 +1022,15 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             .put(USER_METADATA_STRING, new CustomizeTemplateOptions() {
                     public void apply(TemplateOptions t, ConfigBag props, Object v) {
                         if (t instanceof EC2TemplateOptions) {
+                            // See AWS docs: http://docs.aws.amazon.com/AWSEC2/latest/WindowsGuide/UsingConfig_WinAMI.html#user-data-execution
                             if (v==null) return;
                             String data = v.toString();
                             if (!(data.startsWith("<script>") || data.startsWith("<powershell>"))) {
                                 data = "<script> " + data + " </script>";
                             }
                             ((EC2TemplateOptions)t).userData(data.getBytes());
-                            // TODO avail in next jclouds thanks to @andreaturli
-//                          } else if (t instanceof SoftLayerTemplateOptions) {
-//                              ((SoftLayerTemplateOptions)t).userData(Strings.toString(v));
+                        } else if (t instanceof SoftLayerTemplateOptions) {
+                            ((SoftLayerTemplateOptions)t).userData(Strings.toString(v));
                         } else {
                             LOG.info("ignoring userDataString({}) in VM creation because not supported for cloud/type ({})", v, t.getClass());
                         }
@@ -1092,9 +1040,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                         if (t instanceof EC2TemplateOptions) {
                             byte[] bytes = toByteArray(v);
                             ((EC2TemplateOptions)t).userData(bytes);
-                          // TODO avail in next jclouds thanks to @andreaturli
-//                        } else if (t instanceof SoftLayerTemplateOptions) {
-//                            ((SoftLayerTemplateOptions)t).userData(Strings.toString(v));
+                        } else if (t instanceof SoftLayerTemplateOptions) {
+                            ((SoftLayerTemplateOptions)t).userData(Strings.toString(v));
                         } else {
                             LOG.info("ignoring userData({}) in VM creation because not supported for cloud/type ({})", v, t.getClass());
                         }
@@ -1932,22 +1879,41 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         }
     }
 
-    protected WinRmMachineLocation registerWinRmMachineLocation(String vmHostname, ConfigBag setup, String nodeId) {
-        WinRmMachineLocation winRmMachineLocation = createWinRmMachineLocation(vmHostname, setup);
+    protected JcloudsWinRmMachineLocation registerWinRmMachineLocation(ComputeService computeService, NodeMetadata node, ConfigBag setup) {
+        // FIMXE: Need to write WinRM equivalent of getPublicHostname
+        String vmHostname = node.getPublicAddresses().iterator().next();
+        
+        JcloudsWinRmMachineLocation winRmMachineLocation = createWinRmMachineLocation(computeService, node, vmHostname, setup);
         winRmMachineLocation.setParent(this);
-        vmInstanceIds.put(winRmMachineLocation, nodeId);
+        vmInstanceIds.put(winRmMachineLocation, node.getId());
         return winRmMachineLocation;
     }
 
-    protected WinRmMachineLocation createWinRmMachineLocation(String vmHostname, ConfigBag setup) {
+    protected JcloudsWinRmMachineLocation createWinRmMachineLocation(ComputeService computeService, NodeMetadata node, String vmHostname, ConfigBag setup) {
+        String nodeAvailabilityZone = extractAvailabilityZone(setup, node);
+        String nodeRegion = extractRegion(setup, node);
+        if (nodeRegion == null) {
+            // e.g. rackspace doesn't have "region", so rackspace-uk is best we can say (but zone="LON")
+            nodeRegion = extractProvider(setup, node);
+        }
+        
         if (isManaged()) {
-            return getManagementContext().getLocationManager().createLocation(LocationSpec.create(WinRmMachineLocation.class)
+            return getManagementContext().getLocationManager().createLocation(LocationSpec.create(JcloudsWinRmMachineLocation.class)
+                    .configure("jcloudsParent", this)
                     .configure("displayName", vmHostname)
                     .configure("address", vmHostname)
                     .configure("user", getUser(setup))
-                    .configure(WinRmMachineLocation.WINDOWS_USERNAME, setup.get(USER))
-                    .configure(WinRmMachineLocation.WINDOWS_PASSWORD, setup.get(PASSWORD))
-            );
+                    .configure(WinRmMachineLocation.USER, setup.get(USER))
+                    .configure(WinRmMachineLocation.PASSWORD, setup.get(PASSWORD))
+                    .configure("node", node)
+                    .configureIfNotNull(CLOUD_AVAILABILITY_ZONE_ID, nodeAvailabilityZone)
+                    .configureIfNotNull(CLOUD_REGION_ID, nodeRegion)
+                    .configure(CALLER_CONTEXT, setup.get(CALLER_CONTEXT))
+                    .configure(SshMachineLocation.DETECT_MACHINE_DETAILS, setup.get(SshMachineLocation.DETECT_MACHINE_DETAILS))
+                    .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)));
         } else {
             throw new UnsupportedOperationException("Cannot create WinRmMachineLocation because " + this + " is not managed");
         }
@@ -1988,31 +1954,28 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
 
     @Override
-    public void release(MachineLocation machine) {
-        String instanceId = vmInstanceIds.remove(machine);
+    public void release(MachineLocation rawMachine) {
+        String instanceId = vmInstanceIds.remove(rawMachine);
         if (instanceId == null) {
-            LOG.info("Attempted release of unknown machine "+machine+" in "+toString());
-            throw new IllegalArgumentException("Unknown machine "+machine);
+            LOG.info("Attempted release of unknown machine "+rawMachine+" in "+toString());
+            throw new IllegalArgumentException("Unknown machine "+rawMachine);
         }
-
+        JcloudsMachineLocation machine = (JcloudsMachineLocation) rawMachine;
+        
         LOG.info("Releasing machine {} in {}, instance id {}", new Object[] {machine, this, instanceId});
 
         Exception tothrow = null;
 
-        if (machine instanceof JcloudsSshMachineLocation) {
-            ConfigBag setup = config().getBag();
-            for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) {
-                try {
-                    customizer.preRelease((JcloudsSshMachineLocation) machine);
-                } catch (Exception e) {
-                    LOG.error("Problem invoking pre-release customizer "+customizer+" for machine "+machine+" in "+this+", instance id "+instanceId+
-                        "; ignoring and continuing, "
-                        + (tothrow==null ? "will throw subsequently" : "swallowing due to previous error")+": "+e, e);
-                    if (tothrow==null) tothrow = e;
-                }
+        ConfigBag setup = config().getBag();
+        for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) {
+            try {
+                customizer.preRelease(machine);
+            } catch (Exception e) {
+                LOG.error("Problem invoking pre-release customizer "+customizer+" for machine "+machine+" in "+this+", instance id "+instanceId+
+                    "; ignoring and continuing, "
+                    + (tothrow==null ? "will throw subsequently" : "swallowing due to previous error")+": "+e, e);
+                if (tothrow==null) tothrow = e;
             }
-        } else {
-            LOG.warn("Releasing non-jclouds machine "+machine+" from "+this+"; skipping customizers");
         }
 
         try {
@@ -2038,17 +2001,14 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
         removeChild(machine);
 
-        if (machine instanceof JcloudsSshMachineLocation) {
-            ConfigBag setup = config().getBag();
-            for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) {
-                try {
-                    customizer.postRelease((JcloudsSshMachineLocation) machine);
-                } catch (Exception e) {
-                    LOG.error("Problem invoking pre-release customizer "+customizer+" for machine "+machine+" in "+this+", instance id "+instanceId+
-                        "; ignoring and continuing, "
-                        + (tothrow==null ? "will throw subsequently" : "swallowing due to previous error")+": "+e, e);
-                    if (tothrow==null) tothrow = e;
-                }
+        for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) {
+            try {
+                customizer.postRelease(machine);
+            } catch (Exception e) {
+                LOG.error("Problem invoking pre-release customizer "+customizer+" for machine "+machine+" in "+this+", instance id "+instanceId+
+                    "; ignoring and continuing, "
+                    + (tothrow==null ? "will throw subsequently" : "swallowing due to previous error")+": "+e, e);
+                if (tothrow==null) tothrow = e;
             }
         }
         
@@ -2189,44 +2149,55 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         return null;
     }
 
+    private void waitForWinRmAvailable(final NodeMetadata node, final LoginCredentials expectedCredentials, ConfigBag setup) {
+        String waitForWinrmAvailable = setup.get(WAIT_FOR_WINRM_AVAILABLE);
+        checkArgument(!"false".equalsIgnoreCase(waitForWinrmAvailable), "waitForWinRmAvailable called despite waitForWinRmAvailable=%s", waitForWinrmAvailable);
+        Duration timeout = null;
+        try {
+            timeout = Duration.parse(waitForWinrmAvailable);
+        } catch (Exception e) {
+            // TODO will this just be a NumberFormatException? If so, catch that specificially
+            // normal if 'true'; just fall back to default
+        }
+        if (timeout == null) {
+            timeout = Duration.parse(WAIT_FOR_WINRM_AVAILABLE.getDefaultValue());
+        }
+        
+        // TODO Use getFirstReachableAddress, when have getLoginPort set correctly
+        String user = expectedCredentials.getUser();
+        String password = expectedCredentials.getOptionalPassword().orNull();
+        String vmIp = node.getPublicAddresses().iterator().next();
+        final Session session = WinRMFactory.INSTANCE.createSession(vmIp, user, password);
+
+        Callable<Boolean> checker = new Callable<Boolean>() {
+            public Boolean call() {
+                return session.run_cmd("hostname").getStatusCode() == 0;
+            }};
+        String connectionDetails = user+"@"+vmIp;
+
+        waitForReachable(checker, connectionDetails, expectedCredentials, setup, timeout);
+    }
+
     protected void waitForReachable(final ComputeService computeService, final NodeMetadata node, Optional<HostAndPort> hostAndPortOverride, final LoginCredentials expectedCredentials, ConfigBag setup) {
         String waitForSshable = setup.get(WAIT_FOR_SSHABLE);
         checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForReachable called despite waitForSshable=%s", waitForSshable);
 
-        String vmIp = hostAndPortOverride.isPresent() ? hostAndPortOverride.get().getHostText() : JcloudsUtil.getFirstReachableAddress(computeService.getContext(), node);
-        if (vmIp==null) LOG.warn("Unable to extract IP for "+node+" ("+setup.getDescription()+"): subsequent connection attempt will likely fail");
-
-        int vmPort = hostAndPortOverride.isPresent() ? hostAndPortOverride.get().getPortOrDefault(22) : 22;
-
-        long delayMs = -1;
+        Duration timeout = null;
         try {
-            delayMs = Time.parseTimeString(""+waitForSshable);
+            timeout = Duration.parse(waitForSshable);
         } catch (Exception e) {
             // normal if 'true'; just fall back to default
         }
-        if (delayMs<0)
-            delayMs = Time.parseTimeString(WAIT_FOR_SSHABLE.getDefaultValue());
-
-        String user = expectedCredentials.getUser();
-        if (LOG.isDebugEnabled()) {
-            Optional<String> password;
-            Optional<String> key;
-            if (Boolean.TRUE.equals(setup.get(LOG_CREDENTIALS))) {
-                password = expectedCredentials.getOptionalPassword();
-                key = expectedCredentials.getOptionalPrivateKey();
-            } else {
-                password = expectedCredentials.getOptionalPassword().isPresent() ? Optional.of("******") : Optional.<String>absent();
-                key = expectedCredentials.getOptionalPrivateKey().isPresent() ? Optional.of("******") : Optional.<String>absent();
-            }
-            LOG.debug("VM {}: reported online, now waiting {} for it to be sshable on {}@{}:{}{}; using credentials password={}; key={}",
-                    new Object[] {
-                            setup.getDescription(), Time.makeTimeStringRounded(delayMs),
-                            user, vmIp, vmPort,
-                            Objects.equal(user, getUser(setup)) ? "" : " (setup user is different: "+getUser(setup)+")",
-                            password.or("<absent>"),
-                            key.or("<absent>")
-                    });
+        if (timeout == null) {
+            timeout = Duration.parse(WAIT_FOR_SSHABLE.getDefaultValue());
         }
+        
+        String user = expectedCredentials.getUser();
+        String vmIp = hostAndPortOverride.isPresent() ? hostAndPortOverride.get().getHostText() : JcloudsUtil.getFirstReachableAddress(computeService.getContext(), node);
+        if (vmIp==null) LOG.warn("Unable to extract IP for "+node+" ("+setup.getDescription()+"): subsequent connection attempt will likely fail");
+        int vmPort = hostAndPortOverride.isPresent() ? hostAndPortOverride.get().getPortOrDefault(22) : 22;
+
+        String connectionDetails = user + "@" + vmIp + ":" + vmPort;
 
         Callable<Boolean> checker;
         if (hostAndPortOverride.isPresent()) {
@@ -2246,25 +2217,51 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 }};
         }
 
+        waitForReachable(checker, connectionDetails, expectedCredentials, setup, timeout);
+    }
+
+    protected void waitForReachable(Callable<Boolean> checker, String connectionDetails, LoginCredentials expectedCredentials, ConfigBag setup, Duration timeout) {
+        String user = expectedCredentials.getUser();
+        if (LOG.isDebugEnabled()) {
+            Optional<String> password;
+            Optional<String> key;
+            if (Boolean.TRUE.equals(setup.get(LOG_CREDENTIALS))) {
+                password = expectedCredentials.getOptionalPassword();
+                key = expectedCredentials.getOptionalPrivateKey();
+            } else {
+                password = expectedCredentials.getOptionalPassword().isPresent() ? Optional.of("******") : Optional.<String>absent();
+                key = expectedCredentials.getOptionalPrivateKey().isPresent() ? Optional.of("******") : Optional.<String>absent();
+            }
+            LOG.debug("VM {}: reported online, now waiting {} for it to be contactable on {}{}; using credentials password={}; key={}",
+                    new Object[] {
+                            setup.getDescription(), timeout,
+                            connectionDetails,
+                            Objects.equal(user, getUser(setup)) ? "" : " (setup user is different: "+getUser(setup)+")",
+                            password.or("<absent>"),
+                            key.or("<absent>")
+                    });
+        }
+
         Stopwatch stopwatch = Stopwatch.createStarted();
 
         ReferenceWithError<Boolean> reachable = new Repeater()
-            .every(1,SECONDS)
-            .until(checker)
-            .limitTimeTo(delayMs, MILLISECONDS)
-            .runKeepingError();
+                .every(1,SECONDS)
+                .until(checker)
+                .limitTimeTo(timeout)
+                .runKeepingError();
 
         if (!reachable.getWithoutError()) {
-            throw new IllegalStateException("SSH failed for "+
-                    user+"@"+vmIp+" ("+setup.getDescription()+") after waiting "+
-                    Time.makeTimeStringRounded(delayMs), reachable.getError());
+            throw new IllegalStateException("Connection failed for "
+                    +connectionDetails+" ("+setup.getDescription()+") after waiting "
+                    +Time.makeTimeStringRounded(timeout), reachable.getError());
         }
 
-        LOG.debug("VM {}: is sshable after {} on {}@{}",new Object[] {
+        LOG.debug("VM {}: connection succeeded after {} on {}",new Object[] {
                 setup.getDescription(), Time.makeTimeStringRounded(stopwatch),
-                user, vmIp});
+                connectionDetails});
     }
 
+
     // -------------------- hostnames ------------------------
     // hostnames are complicated, but irregardless, this code could be cleaned up!
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationCustomizer.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationCustomizer.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationCustomizer.java
index 94dc610..a21891c 100644
--- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationCustomizer.java
+++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationCustomizer.java
@@ -63,17 +63,43 @@ public interface JcloudsLocationCustomizer {
      * <p/>
      * If {@link brooklyn.location.jclouds.JcloudsLocationConfig#WAIT_FOR_SSHABLE} is true the
      * machine is guaranteed to be SSHable when this method is called.
+     * 
+     * @since 0.7.0; use {@link #customize(JcloudsLocation, ComputeService, JcloudsMachineLocation)}
      */
+    @Deprecated
     void customize(JcloudsLocation location, ComputeService computeService, JcloudsSshMachineLocation machine);
     
     /**
      * Override to handle machine-related cleanup before Jclouds is called to release (destroy) the machine.
+     * 
+     * @since 0.7.0; use {@link #preRelease(JcloudsMachineLocation)}
      */
+    @Deprecated
     void preRelease(JcloudsSshMachineLocation machine);
 
     /**
      * Override to handle machine-related cleanup after Jclouds is called to release (destroy) the machine.
+     * 
+     * @since 0.7.0; use {@link #postRelesae(JcloudsMachineLocation)}
      */
+    @Deprecated
     void postRelease(JcloudsSshMachineLocation machine);
 
+    /**
+     * Override to configure the given machine once it has been created and started by Jclouds.
+     * <p/>
+     * If {@link brooklyn.location.jclouds.JcloudsLocationConfig#WAIT_FOR_SSHABLE} is true the
+     * machine is guaranteed to be SSHable when this method is called.
+     */
+    void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine);
+    
+    /**
+     * Override to handle machine-related cleanup before Jclouds is called to release (destroy) the machine.
+     */
+    void preRelease(JcloudsMachineLocation machine);
+
+    /**
+     * Override to handle machine-related cleanup after Jclouds is called to release (destroy) the machine.
+     */
+    void postRelease(JcloudsMachineLocation machine);
 }


Mime
View raw message