brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [2/3] incubator-brooklyn git commit: jclouds machine: don’t persist node+template
Date Thu, 17 Dec 2015 12:13:11 GMT
jclouds machine: don’t persist node+template

- Handle rebinding to persisted state has node+template;
  re-perist with just the ids/fields, rather than node.
- On init, extract the fields and just persist those.
- JcloudsWinRmMachineLocation doesn't persist node either
- JcloudsSshMachineLocation.getSubnetHostname: single code-path
  - Previously we’d do one thing if we had the “node” reference, and
    another thing if we didn’t. Instead, simplify (and make more
    consistent behaviour) by only having the one code path.
- JcloudsSshMachineLocation.getSubnetIp: always return ip
- JcloudsWinRmMachineLocation doesn't persist node either


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

Branch: refs/heads/master
Commit: 6f81aaf9709931db3c513b09ec61a69ab08571b2
Parents: b97370b
Author: Aled Sage <aled.sage@gmail.com>
Authored: Wed Oct 21 20:41:50 2015 +0100
Committer: Aled Sage <aled.sage@gmail.com>
Committed: Wed Dec 16 14:07:27 2015 +0000

----------------------------------------------------------------------
 .../location/ssh/SshMachineLocation.java        |  10 +
 .../location/jclouds/JcloudsLocation.java       |  93 +++--
 .../jclouds/JcloudsMachineLocation.java         |  17 +
 .../jclouds/JcloudsSshMachineLocation.java      | 341 +++++++++++++++----
 .../jclouds/JcloudsWinRmMachineLocation.java    | 239 ++++++++++---
 .../location/jclouds/JcloudsLocationTest.java   |  10 +-
 .../location/jclouds/JcloudsRebindLiveTest.java |  75 +++-
 .../location/jclouds/JcloudsRebindStubTest.java |  16 +-
 .../jclouds/RebindJcloudsLocationLiveTest.java  | 217 ++++++++++--
 .../jclouds/persisted-aws-machine-aKEcbxKN      | 329 ++++++++++++++++++
 .../jclouds/persisted-aws-parent-lCYB3mTb       |  78 +++++
 .../persisted-aws-winrm-machine-KYSryzW8        | 184 ++++++++++
 .../jclouds/persisted-aws-winrm-parent-fKc0Ofyn |  75 ++++
 .../jclouds/persisted-azure-machine-VNapYjwp    | 271 +++++++++++++++
 .../jclouds/persisted-azure-parent-briByOel     |  65 ++++
 15 files changed, 1837 insertions(+), 183 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f81aaf9/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 0ee5938..3e5f80e 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
@@ -102,6 +102,7 @@ import org.slf4j.LoggerFactory;
 import com.google.common.annotations.Beta;
 import com.google.common.base.Function;
 import com.google.common.base.Objects;
+import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.base.Supplier;
@@ -1003,6 +1004,15 @@ public class SshMachineLocation extends AbstractLocation implements MachineLocat
         return getMachineDetails().getOsDetails();
     }
 
+    /**
+     * Returns the machine details only if they are already loaded, or available directly as 
+     * config.
+     */
+    protected Optional<MachineDetails> getOptionalMachineDetails() {
+        MachineDetails result = machineDetails != null ? machineDetails : config().get(MACHINE_DETAILS);
+        return Optional.fromNullable(result);
+    }
+    
     @Override
     public MachineDetails getMachineDetails() {
         synchronized (machineDetailsLock) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f81aaf9/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 7578b8c..20897d9 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
@@ -20,7 +20,6 @@ package org.apache.brooklyn.location.jclouds;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
-import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.brooklyn.util.JavaGroovyEquivalents.elvis;
 import static org.apache.brooklyn.util.JavaGroovyEquivalents.groovyTruth;
 import static org.apache.brooklyn.util.ssh.BashCommands.sbinPath;
@@ -184,9 +183,6 @@ import com.google.common.collect.Sets.SetView;
 import com.google.common.io.Files;
 import com.google.common.net.HostAndPort;
 
-import io.cloudsoft.winrm4j.pywinrm.Session;
-import io.cloudsoft.winrm4j.pywinrm.WinRMFactory;
-
 /**
  * For provisioning and managing VMs in a particular provider/region, using jclouds.
  * Configuration flags are defined in {@link JcloudsLocationConfig}.
@@ -816,10 +812,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             if (windows) {
                 machineLocation = registerWinRmMachineLocation(computeService, node, userCredentials, sshHostAndPortOverride, setup);
             } else {
-                machineLocation = registerJcloudsSshMachineLocation(computeService, node, userCredentials, sshHostAndPortOverride, setup);
-                if (template!=null && machineLocation.getTemplate()==null) {
-                    ((JcloudsSshMachineLocation)machineLocation).template = template;
-                }
+                machineLocation = registerJcloudsSshMachineLocation(computeService, node, Optional.fromNullable(template), userCredentials, sshHostAndPortOverride, setup);
             }
 
             if (usePortForwarding && sshHostAndPortOverride.isPresent()) {
@@ -2026,7 +2019,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             return registerWinRmMachineLocation(computeService, node, null, Optional.<HostAndPort>absent(), setup);
         } else {
             try {
-                return registerJcloudsSshMachineLocation(computeService, node, null, Optional.<HostAndPort>absent(), setup);
+                return registerJcloudsSshMachineLocation(computeService, node, Optional.<Template>absent(), null, Optional.<HostAndPort>absent(), setup);
             } catch (IOException e) {
                 throw Exceptions.propagate(e);
             }
@@ -2157,16 +2150,25 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
     @Deprecated
     protected final JcloudsSshMachineLocation registerJcloudsSshMachineLocation(NodeMetadata node, String vmHostname, Optional<HostAndPort> sshHostAndPort, ConfigBag setup) throws IOException {
         LOG.warn("Using deprecated registerJcloudsSshMachineLocation: now wants computeService passed", new Throwable("source of deprecated registerJcloudsSshMachineLocation invocation"));
-        return registerJcloudsSshMachineLocation(null, node, null, sshHostAndPort, setup);
+        return registerJcloudsSshMachineLocation(null, node, Optional.<Template>absent(), null, sshHostAndPort, setup);
     }
 
-    protected JcloudsSshMachineLocation registerJcloudsSshMachineLocation(ComputeService computeService, NodeMetadata node, LoginCredentials userCredentials, Optional<HostAndPort> sshHostAndPort, ConfigBag setup) throws IOException {
+    /**
+     * @deprecated since 0.9.0; see {@link #registerJcloudsSshMachineLocation(ComputeService, NodeMetadata, Optional, LoginCredentials, Optional, ConfigBag)}.
+     *             Marked as final to warn those trying to sub-class. 
+     */
+    @Deprecated
+    protected final JcloudsSshMachineLocation registerJcloudsSshMachineLocation(ComputeService computeService, NodeMetadata node, LoginCredentials userCredentials, Optional<HostAndPort> sshHostAndPort, ConfigBag setup) throws IOException {
+        return registerJcloudsSshMachineLocation(computeService, node, Optional.<Template>absent(), userCredentials, sshHostAndPort, setup);
+    }
+    
+    protected JcloudsSshMachineLocation registerJcloudsSshMachineLocation(ComputeService computeService, NodeMetadata node, Optional<Template> template, LoginCredentials userCredentials, Optional<HostAndPort> sshHostAndPort, ConfigBag setup) throws IOException {
         if (userCredentials == null)
             userCredentials = node.getCredentials();
 
         String vmHostname = getPublicHostname(node, sshHostAndPort, setup);
 
-        JcloudsSshMachineLocation machine = createJcloudsSshMachineLocation(computeService, node, vmHostname, sshHostAndPort, userCredentials, setup);
+        JcloudsSshMachineLocation machine = createJcloudsSshMachineLocation(computeService, node, template, vmHostname, sshHostAndPort, userCredentials, setup);
         registerJcloudsMachineLocation(node.getId(), machine);
         return machine;
     }
@@ -2176,8 +2178,17 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         machine.setParent(this);
         vmInstanceIds.put(machine, nodeId);
     }
+
+    /**
+     * @deprecated since 0.9.0; see {@link #createJcloudsSshMachineLocation(ComputeService, NodeMetadata, Optional, String, Optional, LoginCredentials, ConfigBag)}.
+     *             Marked as final to warn those trying to sub-class. 
+     */
+    @Deprecated
+    protected final JcloudsSshMachineLocation createJcloudsSshMachineLocation(ComputeService computeService, NodeMetadata node, String vmHostname, Optional<HostAndPort> sshHostAndPort, LoginCredentials userCredentials, ConfigBag setup) throws IOException {
+        return createJcloudsSshMachineLocation(computeService, node, Optional.<Template>absent(), vmHostname, sshHostAndPort, userCredentials, setup);
+    }
     
-    protected JcloudsSshMachineLocation createJcloudsSshMachineLocation(ComputeService computeService, NodeMetadata node, String vmHostname, Optional<HostAndPort> sshHostAndPort, LoginCredentials userCredentials, ConfigBag setup) throws IOException {
+    protected JcloudsSshMachineLocation createJcloudsSshMachineLocation(ComputeService computeService, NodeMetadata node, Optional<Template> template, String vmHostname, Optional<HostAndPort> sshHostAndPort, LoginCredentials userCredentials, ConfigBag setup) throws IOException {
         Map<?,?> sshConfig = extractSshConfig(setup, node);
         String nodeAvailabilityZone = extractAvailabilityZone(setup, node);
         String nodeRegion = extractRegion(setup, node);
@@ -2226,6 +2237,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                     .configure(SshMachineLocation.PRIVATE_KEY_DATA, userCredentials.getOptionalPrivateKey().orNull())
                     .configure("jcloudsParent", this)
                     .configure("node", node)
+                    .configure("template", template.orNull())
                     .configureIfNotNull(CLOUD_AVAILABILITY_ZONE_ID, nodeAvailabilityZone)
                     .configureIfNotNull(CLOUD_REGION_ID, nodeRegion)
                     .configure(CALLER_CONTEXT, setup.get(CALLER_CONTEXT))
@@ -2367,8 +2379,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
         try {
             // FIXME: Needs to release port forwarding for WinRmMachineLocations
-            if (machine instanceof SshMachineLocation) {
-                releasePortForwarding((SshMachineLocation)machine);
+            if (machine instanceof JcloudsMachineLocation) {
+                releasePortForwarding((JcloudsMachineLocation)machine);
             }
         } catch (Exception e) {
             LOG.error("Problem releasing port-forwarding for machine "+machine+" in "+this+", instance id "+instanceId+
@@ -2444,30 +2456,43 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         }
     }
 
-    protected void releasePortForwarding(final SshMachineLocation machine) {
+    protected void releasePortForwarding(final JcloudsMachineLocation machine) {
         // TODO Implementation needs revisisted. It relies on deprecated PortForwardManager methods.
 
         boolean usePortForwarding = Boolean.TRUE.equals(machine.getConfig(USE_PORT_FORWARDING));
         final JcloudsPortForwarderExtension portForwarder = machine.getConfig(PORT_FORWARDER);
         PortForwardManager portForwardManager = machine.getConfig(PORT_FORWARDING_MANAGER);
-        final NodeMetadata node = (machine instanceof JcloudsSshMachineLocation) ? ((JcloudsSshMachineLocation) machine).getNode() : null;
+        final String nodeId = machine.getJcloudsId();
         final Map<String, Runnable> subtasks = Maps.newLinkedHashMap();
 
         if (portForwarder == null) {
             LOG.debug("No port-forwarding to close (because portForwarder null) on release of " + machine);
         } else {
+            final Optional<NodeMetadata> node = machine.getOptionalNode();
             // Release the port-forwarding for the login-port, which was explicitly created by JcloudsLocation
-            if (usePortForwarding && node != null) {
-                final HostAndPort sshHostAndPortOverride = machine.getSshHostAndPort();
-                final int loginPort = node.getLoginPort();
-                subtasks.put(
-                        "Close port-forward "+sshHostAndPortOverride+"->"+node.getLoginPort(),
-                        new Runnable() {
-                            public void run() {
-                                LOG.debug("Closing port-forwarding at {} for machine {}: {}->{}", new Object[] {this, machine, sshHostAndPortOverride, node.getLoginPort()});
-                                portForwarder.closePortForwarding(node, loginPort, sshHostAndPortOverride, Protocol.TCP);
-                            }
-                        });
+            if (usePortForwarding && node.isPresent()) {
+                final HostAndPort hostAndPortOverride;
+                if (machine instanceof SshMachineLocation) {
+                    hostAndPortOverride = ((SshMachineLocation)machine).getSshHostAndPort();
+                } else if (machine instanceof WinRmMachineLocation) {
+                    String host = ((WinRmMachineLocation)machine).getAddress().getHostAddress();
+                    int port = ((WinRmMachineLocation)machine).config().get(WinRmMachineLocation.WINRM_PORT);
+                    hostAndPortOverride = HostAndPort.fromParts(host, port);
+                } else {
+                    LOG.warn("Unexpected machine {} of type {}; expected SSH or WinRM", machine, (machine != null ? machine.getClass() : null));
+                    hostAndPortOverride = null;
+                }
+                if (hostAndPortOverride != null) {
+                    final int loginPort = node.get().getLoginPort();
+                    subtasks.put(
+                            "Close port-forward "+hostAndPortOverride+"->"+loginPort,
+                            new Runnable() {
+                                public void run() {
+                                    LOG.debug("Closing port-forwarding at {} for machine {}: {}->{}", new Object[] {this, machine, hostAndPortOverride, loginPort});
+                                    portForwarder.closePortForwarding(node.get(), loginPort, hostAndPortOverride, Protocol.TCP);
+                                }
+                            });
+                }
             }
 
             // Get all the other port-forwarding mappings for this VM, and release all of those
@@ -2475,8 +2500,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             if (portForwardManager != null) {
                 mappings = Sets.newLinkedHashSet();
                 mappings.addAll(portForwardManager.getLocationPublicIpIds(machine));
-                if (node != null) {
-                    mappings.addAll(portForwardManager.getPortMappingWithPublicIpId(node.getId()));
+                if (nodeId != null) {
+                    mappings.addAll(portForwardManager.getPortMappingWithPublicIpId(nodeId));
                 }
             } else {
                 mappings = ImmutableSet.of();
@@ -2486,13 +2511,13 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 final HostAndPort publicEndpoint = mapping.getPublicEndpoint();
                 final int targetPort = mapping.getPrivatePort();
                 final Protocol protocol = Protocol.TCP;
-                if (publicEndpoint != null) {
+                if (publicEndpoint != null && node.isPresent()) {
                     subtasks.put(
                             "Close port-forward "+publicEndpoint+"->"+targetPort,
                             new Runnable() {
                                 public void run() {
                                     LOG.debug("Closing port-forwarding at {} for machine {}: {}->{}", new Object[] {this, machine, publicEndpoint, targetPort});
-                                    portForwarder.closePortForwarding(node, targetPort, publicEndpoint, protocol);
+                                    portForwarder.closePortForwarding(node.get(), targetPort, publicEndpoint, protocol);
                                 }
                             });
                 }
@@ -2527,8 +2552,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         // Forget all port mappings associated with this VM
         if (portForwardManager != null) {
             portForwardManager.forgetPortMappings(machine);
-            if (node != null) {
-                portForwardManager.forgetPortMappings(node.getId());
+            if (nodeId != null) {
+                portForwardManager.forgetPortMappings(nodeId);
             }
         }
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f81aaf9/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsMachineLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsMachineLocation.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsMachineLocation.java
index 173b695..a0fa874 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsMachineLocation.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsMachineLocation.java
@@ -23,13 +23,30 @@ import org.apache.brooklyn.core.location.HasSubnetHostname;
 import org.jclouds.compute.domain.NodeMetadata;
 import org.jclouds.compute.domain.Template;
 
+import com.google.common.base.Optional;
+
 public interface JcloudsMachineLocation extends MachineLocation, HasSubnetHostname {
     
     @Override
     public JcloudsLocation getParent();
     
+    public Optional<NodeMetadata> getOptionalNode();
+
+    /**
+     * @deprecated since 0.9.0; instead use {@link #getOptionalNode()}. After rebind, the node will 
+     *             not be available if the VM is no longer running.
+     * 
+     * @throws IllegalStateException If the node is not available (i.e. not cached, and cannot be  
+     *         found from cloud provider).
+     */
+    @Deprecated
     public NodeMetadata getNode();
     
+    /**
+     * @deprecated since 0.9.0; instead use {@link #getOptionalNode()}. After rebind, the node will not
+     * be available if the VM is no longer running.
+     */
+    @Deprecated
     public Template getTemplate();
 
     public String getJcloudsId();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f81aaf9/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 2db43d1..e59f752 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
@@ -21,7 +21,6 @@ package org.apache.brooklyn.location.jclouds;
 import static org.apache.brooklyn.util.JavaGroovyEquivalents.groovyTruth;
 
 import java.net.InetAddress;
-import java.net.UnknownHostException;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -46,6 +45,7 @@ import org.jclouds.compute.ComputeServiceContext;
 import org.jclouds.compute.callables.RunScriptOnNode;
 import org.jclouds.compute.domain.ExecResponse;
 import org.jclouds.compute.domain.Hardware;
+import org.jclouds.compute.domain.Image;
 import org.jclouds.compute.domain.NodeMetadata;
 import org.jclouds.compute.domain.OperatingSystem;
 import org.jclouds.compute.domain.OsFamily;
@@ -58,11 +58,12 @@ import org.jclouds.scriptbuilder.domain.Statement;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Objects;
 import com.google.common.base.Optional;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.net.HostAndPort;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.util.concurrent.ListenableFuture;
 
 public class JcloudsSshMachineLocation extends SshMachineLocation implements JcloudsMachineLocation {
@@ -72,12 +73,53 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements Jcl
     @SetFromFlag
     JcloudsLocation jcloudsParent;
     
+    /**
+     * @deprecated since 0.9.0; the node should not be persisted.
+     */
     @SetFromFlag
+    @Deprecated
     NodeMetadata node;
     
+    /**
+     * @deprecated since 0.9.0; the template should not be persisted.
+     */
     @SetFromFlag
+    @Deprecated
     Template template;
     
+    @SetFromFlag
+    String nodeId;
+
+    @SetFromFlag
+    String imageId;
+
+    @SetFromFlag
+    Set<String> privateAddresses;
+    
+    @SetFromFlag
+    Set<String> publicAddresses;
+
+    @SetFromFlag
+    String hostname;
+
+    /**
+     * Historically, "node" and "template" were persisted. However that is a very bad idea!
+     * It means we pull in lots of jclouds classes into the persisted state. We are at an  
+     * intermediate stage, where we want to handle rebinding to old state that has "node"
+     * and new state that should not. We therefore leave in the {@code @SetFromFlag} on node
+     * so that we read it back, but we ensure the value is null when we write it out!
+     * 
+     * TODO We will rename these to get rid of the ugly underscore when the old node/template 
+     * fields are deleted.
+     */
+    private transient Optional<NodeMetadata> _node;
+
+    private transient Optional<Template> _template;
+
+    private transient Optional<Image> _image;
+
+    private transient String _privateHostname;
+    
     private RunScriptOnNode.Factory runScriptFactory;
     
     public JcloudsSshMachineLocation() {
@@ -90,7 +132,6 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements Jcl
     public JcloudsSshMachineLocation(Map<?,?> flags, JcloudsLocation jcloudsParent, NodeMetadata node) {
         super(flags);
         this.jcloudsParent = jcloudsParent;
-        this.node = node;
         
         init();
     }
@@ -101,6 +142,12 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements Jcl
             super.init();
             ComputeServiceContext context = jcloudsParent.getComputeService().getContext();
             runScriptFactory = context.utils().injector().getInstance(RunScriptOnNode.Factory.class);
+            if (node != null) {
+                setNode(node);
+            }
+            if (template != null) {
+                setTemplate(template);
+            }
         } else {
             // TODO Need to fix the rebind-detection, and not call init() on rebind.
             // This will all change when locations become entities.
@@ -113,6 +160,16 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements Jcl
         super.rebind();
         ComputeServiceContext context = jcloudsParent.getComputeService().getContext();
         runScriptFactory = context.utils().injector().getInstance(RunScriptOnNode.Factory.class);
+        
+        if (node != null) {
+            setNode(node);
+            node = null;
+        }
+
+        if (template != null) {
+            setTemplate(template);
+            template = null;
+        }
     }
     
     @Override
@@ -120,23 +177,93 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements Jcl
         return Objects.toStringHelper(this).omitNullValues()
                 .add("id", getId()).add("name", getDisplayName())
                 .add("user", getUser()).add("address", getAddress()).add("port", getConfig(SSH_PORT))
-                .add("node", getNode())
-                .add("jcloudsId", getJcloudsId())
-                .add("privateAddresses", node.getPrivateAddresses())
-                .add("publicAddresses", node.getPublicAddresses())
+                .add("node", _node)
+                .add("nodeId", getJcloudsId())
+                .add("imageId", getImageId())
+                .add("privateAddresses", getPrivateAddresses())
+                .add("publicAddresses", getPublicAddresses())
                 .add("parentLocation", getParent())
-                .add("osDetails", getOsDetails())
+                .add("osDetails", getOptionalOsDetails())
                 .toString();
     }
 
+    protected void setNode(NodeMetadata node) {
+        this.node = null;
+        nodeId = node.getId();
+        imageId = node.getImageId();
+        privateAddresses = node.getPrivateAddresses();
+        publicAddresses = node.getPublicAddresses();
+        hostname = node.getHostname();
+        _node = Optional.of(node);
+    }
+
+    protected void setTemplate(Template template) {
+        this.template = null;
+        _template = Optional.of(template);
+        _image = Optional.fromNullable(template.getImage());
+    }
+
     @Override
+    public Optional<NodeMetadata> getOptionalNode() {
+      if (_node == null) {
+          _node = Optional.fromNullable(getParent().getComputeService().getNodeMetadata(nodeId));
+      }
+      return _node;
+    }
+
+    protected Optional<Image> getOptionalImage() {
+      if (_image == null) {
+          _image = Optional.fromNullable(getParent().getComputeService().getImage(imageId));
+      }
+      return _image;
+    }
+
+    /**
+     * @since 0.9.0
+     * @deprecated since 0.9.0 (only added as aid until the deprecated {@link #getTemplate()} is deleted)
+     */
+    @Deprecated
+    protected Optional<Template> getOptionalTemplate() {
+        if (_template == null) {
+            _template = Optional.absent();
+        }
+        return _template;
+    }
+
+    /**
+     * @deprecated since 0.9.0
+     */
+    @Override
+    @Deprecated
     public NodeMetadata getNode() {
-        return node;
+        Optional<NodeMetadata> result = getOptionalNode();
+        if (result.isPresent()) {
+            return result.get();
+        } else {
+            throw new IllegalStateException("Node "+nodeId+" not present in "+getParent());
+        }
+    }
+    
+    @VisibleForTesting
+    Optional<NodeMetadata> peekNode() {
+        return _node;
     }
     
+    /**
+     * @deprecated since 0.9.0
+     */
     @Override
+    @Deprecated
     public Template getTemplate() {
-        return template;
+        Optional<Template> result = getOptionalTemplate();
+        if (result.isPresent()) {
+            String msg = "Deprecated use of getTemplate() for "+this;
+            LOG.warn(msg + " - see debug log for stacktrace");
+            LOG.debug(msg, new Exception("for stacktrace"));
+            return result.get();
+        } else {
+            throw new IllegalStateException("Template for "+nodeId+" (in "+getParent()+") not cached (deprecated use of getTemplate())");
+        }
     }
     
     @Override
@@ -146,77 +273,125 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements Jcl
     
     @Override
     public String getHostname() {
-        return node.getHostname();
+        if (hostname != null) {
+            return hostname;
+        }
+        InetAddress address = getAddress();
+        return (address != null) ? address.getHostAddress() : null;
     }
     
-    /** In most clouds, the public hostname is the only way to ensure VMs in different zones can access each other. */
+    /** In clouds like AWS, the public hostname is the only way to ensure VMs in different zones can access each other. */
     @Override
     public Set<String> getPublicAddresses() {
-        return node.getPublicAddresses();
+        return (publicAddresses == null) ? ImmutableSet.<String>of() : publicAddresses;
     }
     
     @Override
     public Set<String> getPrivateAddresses() {
-        return node.getPrivateAddresses();
+        return (privateAddresses == null) ? ImmutableSet.<String>of() : privateAddresses;
     }
 
     @Override
     public String getSubnetHostname() {
-        String privateHostname = jcloudsParent.getPrivateHostname(node, Optional.<HostAndPort>absent(), config().getBag());
-        return privateHostname;
+        // Changed behaviour in Brooklyn 0.9.0. Previously it delegated to jcloudsParent.getPrivateHostname(node),
+        // which would treat AWS as a special case to try to get the AWS hostname. Now it just does the generic
+        // lookup, based on the private/public addresses retrieved from node during at construction time.
+        if (_privateHostname == null) {
+            // Impl taken from jcloudsParent.getPrivateHostnameGeneric(NodeMetadata, ConfigBag).
+            // But we won't always have a node object (e.g. after rebind).
+            //
+            //prefer the private address to the hostname because hostname is sometimes wrong/abbreviated
+            //(see that javadoc; also e.g. on rackspace/cloudstack, the hostname is not registered with any DNS).
+            //Don't return local-only address (e.g. never 127.0.0.1)
+            for (String p : getPrivateAddresses()) {
+                if (Networking.isLocalOnly(p)) continue;
+                _privateHostname = p;
+            }
+            if (groovyTruth(getPublicAddresses())) {
+                _privateHostname = getPublicAddresses().iterator().next();
+            } else if (groovyTruth(getHostname())) {
+                _privateHostname = getHostname();
+            } else {
+                return null;
+            }
+        }
+        return _privateHostname;
     }
 
     @Override
     public String getSubnetIp() {
+        // Previous to Brooklyn 0.9.0, this could return the hostname or would try to do
+        // jcloudsParent.getPublicHostname, and return the resolved IP. That was clearly 
+        // not a "subnet ip"!
         Optional<String> privateAddress = getPrivateAddress();
         if (privateAddress.isPresent()) {
             return privateAddress.get();
         }
-
-        String hostname = jcloudsParent.getPublicHostname(node, Optional.<HostAndPort>absent(), config().getBag());
-        if (hostname != null && !Networking.isValidIp4(hostname)) {
-            try {
-                return InetAddress.getByName(hostname).getHostAddress();
-            } catch (UnknownHostException e) {
-                LOG.debug("Cannot resolve IP for hostname {} of machine {} (so returning hostname): {}", new Object[] {hostname, this, e});
-            }
+        if (groovyTruth(node.getPublicAddresses())) {
+            return node.getPublicAddresses().iterator().next();
         }
-        return hostname;
+        return null;
     }
 
     protected Optional<String> getPrivateAddress() {
-        if (groovyTruth(node.getPrivateAddresses())) {
-            for (String p : node.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);
-            }
+        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);
         }
         return Optional.absent();
     }
     
     @Override
     public String getJcloudsId() {
-        return node.getId();
+        return nodeId;
     }
     
+    protected String getImageId() {
+        return imageId;
+    }
+
     /** executes the given statements on the server using jclouds ScriptBuilder,
      * wrapping in a script which is polled periodically.
      * the output is returned once the script completes (disadvantage compared to other methods)
      * but the process is nohupped and the SSH session is not kept, 
      * so very useful for long-running processes
+     * 
+     * @deprecated since 0.9.0; use standard {@link #execScript(String, List)} and the other variants.
      */
+    @Deprecated
     public ListenableFuture<ExecResponse> submitRunScript(String ...statements) {
         return submitRunScript(new InterpretableStatement(statements));
     }
+
+    /**
+     * @deprecated since 0.9.0; use standard {@link #execScript(String, List)} and the other variants.
+     */
+    @Deprecated
     public ListenableFuture<ExecResponse> submitRunScript(Statement script) {
         return submitRunScript(script, new RunScriptOptions());            
     }
+    
+    /**
+     * @deprecated since 0.9.0; use standard {@link #execScript(String, List)} and the other variants.
+     */
+    @Deprecated
     public ListenableFuture<ExecResponse> submitRunScript(Statement script, RunScriptOptions options) {
-        return runScriptFactory.submit(node, script, options);
+        Optional<NodeMetadata> node = getOptionalNode();
+        if (node.isPresent()) {
+            return runScriptFactory.submit(node.get(), script, options);
+        } else {
+            throw new IllegalStateException("Node "+nodeId+" not present in "+getParent());
+        }
     }
-    /** uses submitRunScript to execute the commands, and throws error if it fails or returns non-zero */
+    
+    /**
+     * Uses submitRunScript to execute the commands, and throws error if it fails or returns non-zero
+     * 
+     * @deprecated since 0.9.0; use standard {@link #execScript(String, List)} and the other variants.
+     */
+    @Deprecated
     public void execRemoteScript(String ...commands) {
         try {
             ExecResponse result = submitRunScript(commands).get();
@@ -234,44 +409,79 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements Jcl
      * Retrieves the password for this VM, if one exists. The behaviour/implementation is different for different clouds.
      * e.g. on Rackspace, the password for a windows VM is available immediately; on AWS-EC2, for a Windows VM you need 
      * to poll repeatedly until the password is available which can take up to 15 minutes.
+     * 
+     * @deprecated since 0.9.0; use the machine to execute commands, so no need to extract the password
      */
+    @Deprecated
     public String waitForPassword() {
-        // TODO Hacky; don't want aws specific stuff here but what to do?!
-        if (jcloudsParent.getProvider().equals("aws-ec2")) {
-            try {
-                return JcloudsUtil.waitForPasswordOnAws(jcloudsParent.getComputeService(), node, 15, TimeUnit.MINUTES);
-            } catch (TimeoutException e) {
-                throw Throwables.propagate(e);
+        Optional<NodeMetadata> node = getOptionalNode();
+        if (node.isPresent()) {
+            // TODO Hacky; don't want aws specific stuff here but what to do?!
+            if (jcloudsParent.getProvider().equals("aws-ec2")) {
+                try {
+                    return JcloudsUtil.waitForPasswordOnAws(jcloudsParent.getComputeService(), node.get(), 15, TimeUnit.MINUTES);
+                } catch (TimeoutException e) {
+                    throw Throwables.propagate(e);
+                }
+            } else {
+                LoginCredentials credentials = node.get().getCredentials();
+                return (credentials != null) ? credentials.getOptionalPassword().orNull() : null;
             }
         } else {
-            LoginCredentials credentials = node.getCredentials();
-            return (credentials != null) ? credentials.getPassword() : null;
+            throw new IllegalStateException("Node "+nodeId+" not present in "+getParent());
+        }
+    }
+
+    /**
+     * Returns the known OsDetails, without any attempt to retrieve them if not known.
+     */
+    protected Optional<OsDetails> getOptionalOsDetails() {
+        Optional<MachineDetails> machineDetails = getOptionalMachineDetails();
+        OsDetails result = machineDetails.isPresent() ? machineDetails.get().getOsDetails() : null;
+        return Optional.fromNullable(result);
+    }
+
+    protected Optional<OperatingSystem> getOptionalOperatingSystem() {
+        Optional<NodeMetadata> node = getOptionalNode();
+        
+        OperatingSystem os = node.isPresent() ? node.get().getOperatingSystem() : null;
+        if (os == null) {
+            // some nodes (eg cloudstack, gce) might not get OS available on the node,
+            // so also try taking it from the image if available
+            Optional<Image> image = getOptionalImage();
+            if (image.isPresent()) {
+                os = image.get().getOperatingSystem();
+            }
         }
+        return Optional.fromNullable(os);
     }
 
+    protected Optional<Hardware> getOptionalHardware() {
+        Optional<NodeMetadata> node = getOptionalNode();
+        return Optional.fromNullable(node.isPresent() ? node.get().getHardware() : null);
+    }
+    
     @Override
     protected MachineDetails inferMachineDetails() {
         Optional<String> name = Optional.absent();
         Optional<String> version = Optional.absent();
         Optional<String> architecture = Optional.absent();
-
-        OperatingSystem os = node.getOperatingSystem();
-        if (os == null && getTemplate() != null && getTemplate().getImage() != null)
-            // some nodes (eg cloudstack, gce) might not get OS available on the node,
-            // so also try taking it from the template if available
-            os = getTemplate().getImage().getOperatingSystem();
-
-        if (os != null) {
+        Optional<OperatingSystem> os = getOptionalOperatingSystem();
+        Optional<Hardware> hardware = getOptionalHardware();
+        
+        if (os.isPresent()) {
             // Note using family rather than name. Name is often unset.
-            name = Optional.fromNullable(os.getFamily() != null && !OsFamily.UNRECOGNIZED.equals(os.getFamily()) ? os.getFamily().toString() : null);
-            version = Optional.fromNullable(!Strings.isBlank(os.getVersion()) ? os.getVersion() : null);
             // Using is64Bit rather then getArch because getArch often returns "paravirtual"
-            architecture = Optional.fromNullable(os.is64Bit() ? BasicOsDetails.OsArchs.X_86_64 : BasicOsDetails.OsArchs.I386);
+            OsFamily family = os.get().getFamily();
+            String versionRaw = os.get().getVersion();
+            boolean is64Bit = os.get().is64Bit();
+            name = Optional.fromNullable(family != null && !OsFamily.UNRECOGNIZED.equals(family) ? family.toString() : null);
+            version = Optional.fromNullable(Strings.isNonBlank(versionRaw) ? versionRaw : null);
+            architecture = Optional.fromNullable(is64Bit ? BasicOsDetails.OsArchs.X_86_64 : BasicOsDetails.OsArchs.I386);
         }
 
-        Hardware hardware = node.getHardware();
-        Optional<Integer> ram = hardware==null ? Optional.<Integer>absent() : Optional.fromNullable(hardware.getRam());
-        Optional<Integer> cpus = hardware==null ? Optional.<Integer>absent() : Optional.fromNullable(hardware.getProcessors() != null ? hardware.getProcessors().size() : null);
+        Optional<Integer> ram = hardware.isPresent() ? Optional.fromNullable(hardware.get().getRam()) : Optional.<Integer>absent();
+        Optional<Integer> cpus = hardware.isPresent() ? Optional.fromNullable(hardware.get().getProcessors() != null ? hardware.get().getProcessors().size() : null) : Optional.<Integer>absent();
 
         // Skip superclass' SSH to machine if all data is present, otherwise defer to super
         if (name.isPresent() && version.isPresent() && architecture.isPresent() && ram.isPresent() && cpus.isPresent()) {
@@ -302,18 +512,21 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements Jcl
 
     @Override
     public Map<String, String> toMetadataRecord() {
-        Hardware hardware = node.getHardware();
-        List<? extends Processor> processors = (hardware != null) ? hardware.getProcessors() : null;
+        Optional<NodeMetadata> node = getOptionalNode();
+        
+        Optional<Hardware> hardware = getOptionalHardware();
+        List<? extends Processor> processors = hardware.isPresent() ? hardware.get().getProcessors() : null;
         
         ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
         builder.putAll(super.toMetadataRecord());
         putIfNotNull(builder, "provider", getParent().getProvider());
         putIfNotNull(builder, "account", getParent().getIdentity());
-        putIfNotNull(builder, "serverId", node.getProviderId());
-        putIfNotNull(builder, "imageId", node.getImageId());
-        putIfNotNull(builder, "instanceTypeName", (hardware != null ? hardware.getName() : null));
-        putIfNotNull(builder, "instanceTypeId", (hardware != null ? hardware.getProviderId() : null));
-        putIfNotNull(builder, "ram", "" + (hardware != null ? hardware.getRam() : null));
+        putIfNotNull(builder, "region", getParent().getRegion());
+        putIfNotNull(builder, "serverId", getJcloudsId());
+        putIfNotNull(builder, "imageId", getImageId());
+        putIfNotNull(builder, "instanceTypeName", (hardware.isPresent() ? hardware.get().getName() : null));
+        putIfNotNull(builder, "instanceTypeId", (hardware.isPresent() ? hardware.get().getProviderId() : null));
+        putIfNotNull(builder, "ram", "" + (hardware.isPresent() ? hardware.get().getRam() : null));
         putIfNotNull(builder, "cpus", "" + (processors != null ? processors.size() : null));
         
         try {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f81aaf9/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsWinRmMachineLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsWinRmMachineLocation.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsWinRmMachineLocation.java
index 7b84432..b58e783 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsWinRmMachineLocation.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsWinRmMachineLocation.java
@@ -21,21 +21,23 @@ package org.apache.brooklyn.location.jclouds;
 import static org.apache.brooklyn.util.JavaGroovyEquivalents.groovyTruth;
 
 import java.net.InetAddress;
-import java.net.UnknownHostException;
-import java.util.Iterator;
 import java.util.Set;
 
 import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
 import org.apache.brooklyn.util.core.flags.SetFromFlag;
 import org.apache.brooklyn.util.net.Networking;
+import org.jclouds.compute.ComputeServiceContext;
+import org.jclouds.compute.callables.RunScriptOnNode;
+import org.jclouds.compute.domain.Image;
 import org.jclouds.compute.domain.NodeMetadata;
 import org.jclouds.compute.domain.Template;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Objects;
 import com.google.common.base.Optional;
-import com.google.common.net.HostAndPort;
+import com.google.common.collect.ImmutableSet;
 
 public class JcloudsWinRmMachineLocation extends WinRmMachineLocation implements JcloudsMachineLocation {
 
@@ -44,110 +46,263 @@ public class JcloudsWinRmMachineLocation extends WinRmMachineLocation implements
     @SetFromFlag
     JcloudsLocation jcloudsParent;
     
+    /**
+     * @deprecated since 0.9.0; the node should not be persisted.
+     */
     @SetFromFlag
+    @Deprecated
     NodeMetadata node;
     
+    /**
+     * @deprecated since 0.9.0; the template should not be persisted.
+     */
     @SetFromFlag
+    @Deprecated
     Template template;
+    
+    @SetFromFlag
+    String nodeId;
+
+    @SetFromFlag
+    String imageId;
+
+    @SetFromFlag
+    Set<String> privateAddresses;
+    
+    @SetFromFlag
+    Set<String> publicAddresses;
+
+    @SetFromFlag
+    String hostname;
+
+    /**
+     * Historically, "node" and "template" were persisted. However that is a very bad idea!
+     * It means we pull in lots of jclouds classes into the persisted state. We are at an  
+     * intermediate stage, where we want to handle rebinding to old state that has "node"
+     * and new state that should not. We therefore leave in the {@code @SetFromFlag} on node
+     * so that we read it back, but we ensure the value is null when we write it out!
+     * 
+     * TODO We will rename these to get rid of the ugly underscore when the old node/template 
+     * fields are deleted.
+     */
+    private transient Optional<NodeMetadata> _node;
+
+    private transient Optional<Template> _template;
 
+    private transient Optional<Image> _image;
+
+    private transient String _privateHostname;
+    
     public JcloudsWinRmMachineLocation() {
     }
 
     @Override
+    public void init() {
+        if (jcloudsParent != null) {
+            super.init();
+            if (node != null) {
+                setNode(node);
+            }
+            if (template != null) {
+                setTemplate(template);
+            }
+        } else {
+            // TODO Need to fix the rebind-detection, and not call init() on rebind.
+            // This will all change when locations become entities.
+            if (LOG.isDebugEnabled()) LOG.debug("Not doing init() of {} because parent not set; presuming rebinding", this);
+        }
+    }
+    
+    @Override
+    public void rebind() {
+        super.rebind();
+        
+        if (node != null) {
+            setNode(node);
+            node = null;
+        }
+
+        if (template != null) {
+            setTemplate(template);
+            template = null;
+        }
+    }
+    
+    @Override
     public String toVerboseString() {
         return Objects.toStringHelper(this).omitNullValues()
                 .add("id", getId()).add("name", getDisplayName())
                 .add("user", getUser())
                 .add("address", getAddress())
                 .add("port", getPort())
-                .add("node", getNode())
-                .add("jcloudsId", getJcloudsId())
-                .add("privateAddresses", node.getPrivateAddresses())
-                .add("publicAddresses", node.getPublicAddresses())
+                .add("node", _node)
+                .add("nodeId", getJcloudsId())
+                .add("imageId", getImageId())
+                .add("privateAddresses", getPrivateAddresses())
+                .add("publicAddresses", getPublicAddresses())
                 .add("parentLocation", getParent())
                 .add("osDetails", getOsDetails())
                 .toString();
     }
 
+    protected void setNode(NodeMetadata node) {
+        this.node = null;
+        nodeId = node.getId();
+        imageId = node.getImageId();
+        privateAddresses = node.getPrivateAddresses();
+        publicAddresses = node.getPublicAddresses();
+        hostname = node.getHostname();
+        _node = Optional.of(node);
+    }
+
+    protected void setTemplate(Template template) {
+        this.template = null;
+        _template = Optional.of(template);
+        _image = Optional.fromNullable(template.getImage());
+    }
+
     @Override
     public int getPort() {
         return getConfig(WINRM_PORT);
     }
     
     @Override
-    public NodeMetadata getNode() {
-        return node;
+    public JcloudsLocation getParent() {
+        return jcloudsParent;
     }
     
     @Override
-    public Template getTemplate() {
-        return template;
+    public Optional<NodeMetadata> getOptionalNode() {
+      if (_node == null) {
+          _node = Optional.fromNullable(getParent().getComputeService().getNodeMetadata(nodeId));
+      }
+      return _node;
     }
-    
+
+    @VisibleForTesting
+    Optional<NodeMetadata> peekNode() {
+        return _node;
+    }
+
+    protected Optional<Image> getOptionalImage() {
+        if (_image == null) {
+            _image = Optional.fromNullable(getParent().getComputeService().getImage(imageId));
+        }
+        return _image;
+    }
+
+    /**
+     * @since 0.9.0
+     * @deprecated since 0.9.0 (only added as aid until the deprecated {@link #getTemplate()} is deleted)
+     */
+    @Deprecated
+    protected Optional<Template> getOptionalTemplate() {
+        if (_template == null) {
+            _template = Optional.absent();
+        }
+        return _template;
+    }
+
+    /**
+     * @deprecated since 0.9.0
+     */
     @Override
-    public JcloudsLocation getParent() {
-        return jcloudsParent;
+    @Deprecated
+    public NodeMetadata getNode() {
+        Optional<NodeMetadata> result = getOptionalNode();
+        if (result.isPresent()) {
+            return result.get();
+        } else {
+            throw new IllegalStateException("Node "+nodeId+" not present in "+getParent());
+        }
     }
-    
+
+    /**
+     * @deprecated since 0.9.0
+     */
+    @Override
+    @Deprecated
+    public Template getTemplate() {
+        Optional<Template> result = getOptionalTemplate();
+        if (result.isPresent()) {
+            String msg = "Deprecated use of getTemplate() for "+this;
+            LOG.warn(msg + " - see debug log for stacktrace");
+            LOG.debug(msg, new Exception("for stacktrace"));
+            return result.get();
+        } else {
+            throw new IllegalStateException("Template for "+nodeId+" (in "+getParent()+") not cached (deprecated use of getTemplate())");
+        }
+    }
+
     @Override
     public String getHostname() {
+        if (hostname != null) {
+            return hostname;
+        }
         InetAddress address = getAddress();
         return (address != null) ? address.getHostAddress() : null;
     }
-    
+
+
+    /** In clouds like AWS, the public hostname is the only way to ensure VMs in different zones can access each other. */
     @Override
     public Set<String> getPublicAddresses() {
-        return node.getPublicAddresses();
+        return (publicAddresses == null) ? ImmutableSet.<String>of() : publicAddresses;
     }
     
     @Override
     public Set<String> getPrivateAddresses() {
-        return node.getPrivateAddresses();
+        return (privateAddresses == null) ? ImmutableSet.<String>of() : privateAddresses;
     }
 
+
     @Override
     public String getSubnetHostname() {
-        // TODO: TEMP FIX: WAS:
-        // String publicHostname = jcloudsParent.getPublicHostname(node, Optional.<HostAndPort>absent(), config().getBag());
-        // but this causes a call to JcloudsUtil.getFirstReachableAddress, which searches for accessible SSH service.
-        // This workaround is good for public nodes but not private-subnet ones.
-        return getHostname();
+        // Same impl as JcloudsSshMachineLocation
+        if (_privateHostname == null) {
+            for (String p : getPrivateAddresses()) {
+                if (Networking.isLocalOnly(p)) continue;
+                _privateHostname = p;
+            }
+            if (groovyTruth(getPublicAddresses())) {
+                _privateHostname = getPublicAddresses().iterator().next();
+            } else if (groovyTruth(getHostname())) {
+                _privateHostname = getHostname();
+            } else {
+                return null;
+            }
+        }
+        return _privateHostname;
     }
 
     @Override
     public String getSubnetIp() {
+        // Same impl as JcloudsSshMachineLocation
         Optional<String> privateAddress = getPrivateAddress();
         if (privateAddress.isPresent()) {
             return privateAddress.get();
         }
-
-        String hostname = jcloudsParent.getPublicHostname(node, Optional.<HostAndPort>absent(), config().getBag());
-        if (hostname != null && !Networking.isValidIp4(hostname)) {
-            try {
-                return InetAddress.getByName(hostname).getHostAddress();
-            } catch (UnknownHostException e) {
-                LOG.debug("Cannot resolve IP for hostname {} of machine {} (so returning hostname): {}", new Object[] {hostname, this, e});
-            }
+        if (groovyTruth(node.getPublicAddresses())) {
+            return node.getPublicAddresses().iterator().next();
         }
-        return hostname;
+        return null;
     }
 
     protected Optional<String> getPrivateAddress() {
-        if (groovyTruth(node.getPrivateAddresses())) {
-            Iterator<String> pi = node.getPrivateAddresses().iterator();
-            while (pi.hasNext()) {
-                String p = pi.next();
-                // 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);
-            }
+        // Same impl as JcloudsSshMachineLocation
+        for (String p : getPrivateAddresses()) {
+            if (Networking.isLocalOnly(p)) continue;
+            return Optional.of(p);
         }
         return Optional.absent();
     }
     
     @Override
     public String getJcloudsId() {
-        return node.getId();
+        return nodeId;
+    }
+    
+    protected String getImageId() {
+        return imageId;
     }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f81aaf9/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsLocationTest.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsLocationTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsLocationTest.java
index 851505d..2a8900a 100644
--- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsLocationTest.java
+++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsLocationTest.java
@@ -62,6 +62,7 @@ import com.google.common.collect.ContiguousSet;
 import com.google.common.collect.DiscreteDomain;
 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.collect.Range;
 import com.google.common.primitives.Ints;
@@ -100,7 +101,7 @@ public class JcloudsLocationTest implements JcloudsLocationConfig {
     
     @BeforeMethod(alwaysRun=true)
     public void setUp() throws Exception {
-        managementContext = LocalManagementContextForTests.newInstance(BrooklynProperties.Factory.builderEmpty().build());
+        managementContext = LocalManagementContextForTests.newInstance(BrooklynProperties.Factory.newDefault());
     }
     
     @AfterMethod(alwaysRun=true)
@@ -472,7 +473,12 @@ public class JcloudsLocationTest implements JcloudsLocationConfig {
                 .configure("address", "127.0.0.1") 
                 .configure("port", 22) 
                 .configure("user", "bob")
-                .configure("jcloudsParent", this));
+                .configure("jcloudsParent", this)
+                .configure("nodeId", "myNodeId")
+                .configure("imageId", "myImageId")
+                .configure("privateAddresses", ImmutableSet.of("10.0.0.1"))
+                .configure("publicAddresses", ImmutableSet.of("56.0.0.1"))
+                );
             registerJcloudsMachineLocation("bogus", result);
             
             // explicitly invoke this customizer, to comply with tests

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f81aaf9/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsRebindLiveTest.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsRebindLiveTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsRebindLiveTest.java
index b81d930..de3fb1c 100644
--- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsRebindLiveTest.java
+++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsRebindLiveTest.java
@@ -20,15 +20,21 @@ package org.apache.brooklyn.location.jclouds;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.fail;
 
 import java.util.List;
 import java.util.Map;
 
 import org.apache.brooklyn.api.location.LocationSpec;
+import org.apache.brooklyn.api.location.MachineLocation;
 import org.apache.brooklyn.api.location.MachineProvisioningLocation;
 import org.apache.brooklyn.core.location.Locations;
 import org.apache.brooklyn.core.mgmt.rebind.RebindTestFixtureWithApp;
 import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
+import org.apache.brooklyn.util.core.internal.winrm.WinRmToolResponse;
 import org.apache.brooklyn.util.exceptions.CompoundRuntimeException;
 import org.apache.brooklyn.util.stream.Streams;
 import org.jclouds.compute.domain.NodeMetadata;
@@ -64,7 +70,7 @@ public class JcloudsRebindLiveTest extends RebindTestFixtureWithApp {
 
     public static final String SOFTLAYER_LOCATION_SPEC = "jclouds:" + AbstractJcloudsLiveTest.SOFTLAYER_PROVIDER;
     
-    protected List<JcloudsSshMachineLocation> machines;
+    protected List<JcloudsMachineLocation> machines;
     
     @BeforeMethod(alwaysRun=true)
     public void setUp() throws Exception {
@@ -99,13 +105,25 @@ public class JcloudsRebindLiveTest extends RebindTestFixtureWithApp {
     public void testEc2Rebind() throws Exception {
         ImmutableMap<String, Object> obtainFlags = ImmutableMap.<String,Object>builder()
                 .put("imageId", AWS_EC2_CENTOS_IMAGE_ID)
-                .put("hardwareId", AbstractJcloudsLiveTest.AWS_EC2_SMALL_HARDWARE_ID)
+                .put("hardwareId", AbstractJcloudsLiveTest.AWS_EC2_MEDIUM_HARDWARE_ID)
                 .put("inboundPorts", ImmutableList.of(22))
                 .build();
         runTest(AWS_EC2_LOCATION_SPEC, obtainFlags);
     }
     
     @Test(groups = {"Live"})
+    public void testEc2WinrmRebind() throws Exception {
+        ImmutableMap<String, Object> obtainFlags = ImmutableMap.<String,Object>builder()
+                .put("imageNameRegex", "Windows_Server-2012-R2_RTM-English-64Bit-Base-.*")
+                .put("imageOwner", "801119661308")
+                .put("hardwareId", AbstractJcloudsLiveTest.AWS_EC2_MEDIUM_HARDWARE_ID)
+                .put("useJcloudsSshInit", false)
+                .put("inboundPorts", ImmutableList.of(5985, 3389))
+                .build();
+        runTest(AWS_EC2_LOCATION_SPEC, obtainFlags);
+    }
+
+    @Test(groups = {"Live"})
     public void testSoftlayerRebind() throws Exception {
         runTest(SOFTLAYER_LOCATION_SPEC, ImmutableMap.of("inboundPorts", ImmutableList.of(22)));
     }
@@ -113,21 +131,31 @@ public class JcloudsRebindLiveTest extends RebindTestFixtureWithApp {
     protected void runTest(String locSpec, Map<String, ?> obtainFlags) throws Exception {
         JcloudsLocation location = (JcloudsLocation) mgmt().getLocationRegistry().resolve(locSpec);
         
-        JcloudsSshMachineLocation origMachine = obtainMachine(location, obtainFlags);
+        JcloudsMachineLocation origMachine = obtainMachine(location, obtainFlags);
         String origHostname = origMachine.getHostname();
         NodeMetadata origNode = origMachine.getNode();
-        Template origTemplate = origMachine.getTemplate();
-        assertSshable(origMachine);
+        if (origMachine instanceof JcloudsSshMachineLocation) {
+            Template origTemplate = origMachine.getTemplate(); // WinRM machines don't bother with template!
+        }
+        assertConnectable(origMachine);
 
         rebind();
         
-        // Check the machine is as before
-        JcloudsSshMachineLocation newMachine = (JcloudsSshMachineLocation) newManagementContext.getLocationManager().getLocation(origMachine.getId());
+        // Check the machine is as before; but won't have persisted node+template.
+        // We'll be able to re-create the node object by querying the cloud-provider again though.
+        JcloudsMachineLocation newMachine = (JcloudsMachineLocation) newManagementContext.getLocationManager().getLocation(origMachine.getId());
         JcloudsLocation newLocation = newMachine.getParent();
         String newHostname = newMachine.getHostname();
-        NodeMetadata newNode = newMachine.getNode();
-        Template newTemplate = newMachine.getTemplate();
-        assertSshable(newMachine);
+        if (newMachine instanceof JcloudsSshMachineLocation) {
+            assertFalse(((JcloudsSshMachineLocation)newMachine).getOptionalTemplate().isPresent());
+            assertNull(((JcloudsSshMachineLocation)newMachine).peekNode());
+        } else if (newMachine instanceof JcloudsWinRmMachineLocation) {
+            assertNull(((JcloudsWinRmMachineLocation)newMachine).peekNode());
+        } else {
+            fail("Unexpected new machine type: machine="+newMachine+"; type="+(newMachine == null ? null : newMachine.getClass()));
+        }
+        NodeMetadata newNode = newMachine.getOptionalNode().get();
+        assertConnectable(newMachine);
         
         assertEquals(newHostname, origHostname);
         assertEquals(origNode.getId(), newNode.getId());
@@ -153,29 +181,44 @@ public class JcloudsRebindLiveTest extends RebindTestFixtureWithApp {
         }
     }
 
+    protected void assertConnectable(MachineLocation machine) {
+        if (machine instanceof SshMachineLocation) {
+            assertSshable((SshMachineLocation)machine);
+        } else if (machine instanceof WinRmMachineLocation) {
+            assertWinrmable((WinRmMachineLocation)machine);
+        } else {
+            throw new UnsupportedOperationException("Unsupported machine type: machine="+machine+"; type="+(machine == null ? null : machine.getClass()));
+        }
+    }
+    
     protected void assertSshable(SshMachineLocation machine) {
         int result = machine.execScript("simplecommand", ImmutableList.of("true"));
         assertEquals(result, 0);
     }
 
+    protected void assertWinrmable(WinRmMachineLocation machine) {
+        WinRmToolResponse result = machine.executePsScript("echo mycmd");
+        assertEquals(result.getStatusCode(), 0, "status="+result.getStatusCode()+"; stdout="+result.getStdOut()+"; stderr="+result.getStdErr());
+    }
+
     // Use this utility method to ensure machines are released on tearDown
-    protected JcloudsSshMachineLocation obtainMachine(MachineProvisioningLocation<?> location, Map<?, ?> conf) throws Exception {
-        JcloudsSshMachineLocation result = (JcloudsSshMachineLocation)location.obtain(conf);
+    protected JcloudsMachineLocation obtainMachine(MachineProvisioningLocation<?> location, Map<?, ?> conf) throws Exception {
+        JcloudsMachineLocation result = (JcloudsMachineLocation)location.obtain(conf);
         machines.add(checkNotNull(result, "result"));
         return result;
     }
 
-    protected void releaseMachine(JcloudsSshMachineLocation machine) {
+    protected void releaseMachine(JcloudsMachineLocation machine) {
         if (!Locations.isManaged(machine)) return;
         machines.remove(machine);
         machine.getParent().release(machine);
     }
     
-    protected List<Exception> releaseMachineSafely(Iterable<? extends JcloudsSshMachineLocation> machines) {
+    protected List<Exception> releaseMachineSafely(Iterable<? extends JcloudsMachineLocation> machines) {
         List<Exception> exceptions = Lists.newArrayList();
-        List<JcloudsSshMachineLocation> machinesCopy = ImmutableList.copyOf(machines);
+        List<JcloudsMachineLocation> machinesCopy = ImmutableList.copyOf(machines);
         
-        for (JcloudsSshMachineLocation machine : machinesCopy) {
+        for (JcloudsMachineLocation machine : machinesCopy) {
             try {
                 releaseMachine(machine);
             } catch (Exception e) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f81aaf9/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsRebindStubTest.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsRebindStubTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsRebindStubTest.java
index ac7fd56..ea9e1c6 100644
--- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsRebindStubTest.java
+++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsRebindStubTest.java
@@ -20,6 +20,7 @@ package org.apache.brooklyn.location.jclouds;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
 
 import java.net.URI;
 import java.util.List;
@@ -54,6 +55,7 @@ import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import com.google.common.base.Optional;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
@@ -142,7 +144,7 @@ public class JcloudsRebindStubTest extends RebindTestFixtureWithApp {
         NodeMetadata node = new NodeMetadataImpl(
                 "softlayer", 
                 "myname", 
-                "myid", 
+                "123", // ids in SoftLayer are numeric
                 locImpl,
                 URI.create("http://myuri.com"), 
                 ImmutableMap.<String, String>of(), // userMetadata 
@@ -192,15 +194,19 @@ public class JcloudsRebindStubTest extends RebindTestFixtureWithApp {
 
         rebind();
         
-        // Check the machine is as before
+        // Check the machine is as before.
+        // Call to getOptionalNode() will cause it to try to resolve this node in Softlayer; but it won't find it.
         JcloudsSshMachineLocation newMachine = (JcloudsSshMachineLocation) newManagementContext.getLocationManager().getLocation(origMachine.getId());
         JcloudsLocation newJcloudsLoc = newMachine.getParent();
         String newHostname = newMachine.getHostname();
-        NodeMetadata newNode = newMachine.getNode();
-        Template newTemplate = newMachine.getTemplate();
+        String newNodeId = newMachine.getJcloudsId();
+        Optional<NodeMetadata> newNode = newMachine.getOptionalNode();
+        Optional<Template> newTemplate = newMachine.getOptionalTemplate();
         
         assertEquals(newHostname, origHostname);
-        assertEquals(origNode.getId(), newNode.getId());
+        assertEquals(origNode.getId(), newNodeId);
+        assertFalse(newNode.isPresent(), "newNode="+newNode);
+        assertFalse(newTemplate.isPresent(), "newTemplate="+newTemplate);
         
         assertEquals(newJcloudsLoc.getProvider(), origJcloudsLoc.getProvider());
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f81aaf9/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/RebindJcloudsLocationLiveTest.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/RebindJcloudsLocationLiveTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/RebindJcloudsLocationLiveTest.java
index c9de22b..57c803c 100644
--- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/RebindJcloudsLocationLiveTest.java
+++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/RebindJcloudsLocationLiveTest.java
@@ -19,6 +19,7 @@
 package org.apache.brooklyn.location.jclouds;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertTrue;
 
@@ -29,17 +30,23 @@ import org.apache.brooklyn.api.location.OsDetails;
 import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.factory.ApplicationBuilder;
 import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
+import org.apache.brooklyn.core.mgmt.rebind.RebindOptions;
 import org.apache.brooklyn.core.mgmt.rebind.RebindTestUtils;
 import org.apache.brooklyn.core.test.entity.TestApplication;
+import org.apache.brooklyn.util.core.ResourceUtils;
 import org.apache.brooklyn.util.core.config.ConfigBag;
+import org.apache.commons.io.FileUtils;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import com.google.common.base.Optional;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.io.Files;
+import com.google.common.net.HostAndPort;
 
 public class RebindJcloudsLocationLiveTest extends AbstractJcloudsLiveTest {
 
@@ -47,27 +54,28 @@ public class RebindJcloudsLocationLiveTest extends AbstractJcloudsLiveTest {
     public static final String AWS_EC2_LOCATION_SPEC = "jclouds:" + AWS_EC2_PROVIDER + ":" + AWS_EC2_REGION_NAME;
 
     private ClassLoader classLoader = getClass().getClassLoader();
-    private TestApplication origApp;
-    private LiveTestEntity origEntity;
     private File mementoDir;
+    private TestApplication origApp;
     
     @BeforeMethod(alwaysRun=true)
     @Override
     public void setUp() throws Exception {
         super.setUp();
-        origApp = ApplicationBuilder.newManagedApp(EntitySpec.create(TestApplication.class), managementContext);
-        origEntity = origApp.createAndManageChild(EntitySpec.create(LiveTestEntity.class));
 
         jcloudsLocation = (JcloudsLocation) managementContext.getLocationRegistry().resolve(AWS_EC2_LOCATION_SPEC);
         jcloudsLocation.config().set(JcloudsLocation.HARDWARE_ID, AWS_EC2_SMALL_HARDWARE_ID);
+        
+        origApp = TestApplication.Factory.newManagedInstanceForTests(managementContext);
     }
 
     @AfterMethod(alwaysRun = true)
     @Override
     public void tearDown() throws Exception {
-        super.tearDown();
-        if (origApp != null) Entities.destroyAll(origApp.getManagementContext());
-        if (mementoDir != null) RebindTestUtils.deleteMementoDir(mementoDir);
+        try {
+            super.tearDown();
+        } finally {
+            if (mementoDir != null) RebindTestUtils.deleteMementoDir(mementoDir);
+        }
     }
     
     @Override
@@ -78,6 +86,8 @@ public class RebindJcloudsLocationLiveTest extends AbstractJcloudsLiveTest {
     
     @Test(groups="Live")
     public void testRebindsToJcloudsMachine() throws Exception {
+        LiveTestEntity origEntity = origApp.addChild(EntitySpec.create(LiveTestEntity.class));
+
         origApp.start(ImmutableList.of(jcloudsLocation));
         JcloudsLocation origJcloudsLocation = jcloudsLocation;
         System.out.println("orig locations: " + origEntity.getLocations());
@@ -87,33 +97,194 @@ public class RebindJcloudsLocationLiveTest extends AbstractJcloudsLiveTest {
         LiveTestEntity newEntity = (LiveTestEntity) Iterables.find(newApp.getChildren(), Predicates.instanceOf(LiveTestEntity.class));
         JcloudsSshMachineLocation newMachine = (JcloudsSshMachineLocation) Iterables.find(newEntity.getLocations(), Predicates.instanceOf(JcloudsSshMachineLocation.class));
 
-        assertMachineEquals(newMachine, origMachine);
+        assertMachineEquals(newMachine, origMachine, true); // Don't expect OsDetails, because that is not persisted.
         assertTrue(newMachine.isSshable());
 
         JcloudsLocation newJcloudsLoction = newMachine.getParent();
         assertJcloudsLocationEquals(newJcloudsLoction, origJcloudsLocation);
     }
 
-    private void assertMachineEquals(JcloudsSshMachineLocation actual, JcloudsSshMachineLocation expected) {
+    // TODO In jclouds-azure, the AzureComputeTemplateOptions fields changed, which meant old 
+    // persisted state could not be deserialized. These files are examples of the old format.
+    @Test(groups={"Live", "WIP"}, enabled=false)
+    public void testRebindsToJcloudsMachineWithInvalidTemplate() throws Exception {
+        ResourceUtils resourceUtils = ResourceUtils.create(this);
+        FileUtils.write(
+                new File(mementoDir, "locations/briByOel"),
+                resourceUtils.getResourceAsString("classpath://org/apache/brooklyn/location/jclouds/persisted-azure-parent-briByOel"));
+        FileUtils.write(
+                new File(mementoDir, "locations/VNapYjwp"),
+                resourceUtils.getResourceAsString("classpath://org/apache/brooklyn/location/jclouds/persisted-azure-machine-VNapYjwp"));
+        
+        TestApplication newApp = rebind();
+        
+        JcloudsLocation loc = (JcloudsLocation) newApp.getManagementContext().getLocationManager().getLocation("briByOel");
+        JcloudsSshMachineLocation machine = (JcloudsSshMachineLocation) newApp.getManagementContext().getLocationManager().getLocation("VNapYjwp");
+        assertEquals(ImmutableSet.of(loc.getChildren()), ImmutableSet.of(machine));
+    }
+
+    @Test(groups={"Live", "Live-sanity"})
+    public void testRebindsToJcloudsSshMachineWithTemplateAndNode() throws Exception {
+        // Populate the mementoDir with some old-style peristed state
+        ResourceUtils resourceUtils = ResourceUtils.create(this);
+        String origParentXml = resourceUtils.getResourceAsString("classpath://org/apache/brooklyn/location/jclouds/persisted-aws-parent-lCYB3mTb");
+        String origMachineXml = resourceUtils.getResourceAsString("classpath://org/apache/brooklyn/location/jclouds/persisted-aws-machine-aKEcbxKN");
+        File persistedParentFile = new File(mementoDir, "locations/lCYB3mTb");
+        File persistedMachineFile = new File(mementoDir, "locations/aKEcbxKN");
+        FileUtils.write(
+                persistedParentFile,
+                origParentXml);
+        FileUtils.write(
+                persistedMachineFile,
+                origMachineXml);
+
+        assertTrue(origMachineXml.contains("AWSEC2TemplateOptions"), origMachineXml);
+        assertTrue(origMachineXml.contains("NodeMetadataImpl"), origMachineXml);
+
+        // Rebind to the old-style persisted state, which includes the NodeMetadata and the Template objects.
+        // Expect to parse that ok.
+        TestApplication app2 = rebind();
+        
+        JcloudsLocation loc2 = (JcloudsLocation) app2.getManagementContext().getLocationManager().getLocation("lCYB3mTb");
+        JcloudsSshMachineLocation machine2 = (JcloudsSshMachineLocation) app2.getManagementContext().getLocationManager().getLocation("aKEcbxKN");
+        assertEquals(ImmutableSet.copyOf(loc2.getChildren()), ImmutableSet.of(machine2));
+        
+        String errmsg = "loc="+loc2.toVerboseString()+"; machine="+machine2.toVerboseString();
+        assertEquals(machine2.getId(), "aKEcbxKN", errmsg);
+        assertEquals(machine2.getJcloudsId(), "ap-southeast-1/i-56fd53f2", errmsg);
+        assertEquals(machine2.getSshHostAndPort(), HostAndPort.fromParts("ec2-54-254-23-53.ap-southeast-1.compute.amazonaws.com", 22), errmsg);
+        assertEquals(machine2.getPrivateAddresses(), ImmutableSet.of("10.144.66.5"), errmsg);
+        assertEquals(machine2.getPublicAddresses(), ImmutableSet.of("54.254.23.53"), errmsg);
+        assertEquals(machine2.getPrivateAddress(), Optional.of("10.144.66.5"), errmsg);
+        assertEquals(machine2.getHostname(), "ip-10-144-66-5", errmsg); // TODO would prefer the hostname that works inside and out
+        assertEquals(machine2.getOsDetails().isWindows(), false, errmsg);
+        assertEquals(machine2.getOsDetails().isLinux(), true, errmsg);
+        assertEquals(machine2.getOsDetails().isMac(), false, errmsg);
+        assertEquals(machine2.getOsDetails().getName(), "centos", errmsg);
+        assertEquals(machine2.getOsDetails().getArch(), "x86_64", errmsg);
+        assertEquals(machine2.getOsDetails().getVersion(), "6.5", errmsg);
+        assertEquals(machine2.getOsDetails().is64bit(), true, errmsg);
+
+        // Force it to be persisted again. Expect to pesist without the NodeMetadata and Template.
+        app2.getManagementContext().getRebindManager().getChangeListener().onChanged(loc2);
+        app2.getManagementContext().getRebindManager().getChangeListener().onChanged(machine2);
+        RebindTestUtils.waitForPersisted(app2);
+        
+        String newMachineXml = new String(java.nio.file.Files.readAllBytes(persistedMachineFile.toPath()));
+        assertFalse(newMachineXml.contains("AWSEC2TemplateOptions"), newMachineXml);
+        assertFalse(newMachineXml.contains("NodeMetadataImpl"), newMachineXml);
+        
+        // Rebind again, with the re-written persisted state.
+        TestApplication app3 = rebind();
+        
+        JcloudsLocation loc3 = (JcloudsLocation) app3.getManagementContext().getLocationManager().getLocation("lCYB3mTb");
+        JcloudsSshMachineLocation machine3 = (JcloudsSshMachineLocation) app3.getManagementContext().getLocationManager().getLocation("aKEcbxKN");
+        assertEquals(ImmutableSet.copyOf(loc3.getChildren()), ImmutableSet.of(machine3));
+        
+        errmsg = "loc="+loc3.toVerboseString()+"; machine="+machine3.toVerboseString();
+        assertEquals(machine3.getId(), "aKEcbxKN", errmsg);
+        assertEquals(machine3.getJcloudsId(), "ap-southeast-1/i-56fd53f2", errmsg);
+        assertEquals(machine3.getSshHostAndPort(), HostAndPort.fromParts("ec2-54-254-23-53.ap-southeast-1.compute.amazonaws.com", 22), errmsg);
+        assertEquals(machine3.getPrivateAddresses(), ImmutableSet.of("10.144.66.5"), errmsg);
+        assertEquals(machine3.getPublicAddresses(), ImmutableSet.of("54.254.23.53"), errmsg);
+        assertEquals(machine3.getPrivateAddress(), Optional.of("10.144.66.5"), errmsg);
+        assertEquals(machine3.getHostname(), "ip-10-144-66-5", errmsg); // TODO would prefer the hostname that works inside and out
+        
+        // The VM is no longer running, so won't be able to infer OS Details.
+        assertFalse(machine3.getOptionalOsDetails().isPresent(), errmsg);
+    }
+
+    @Test(groups={"Live", "Live-sanity"})
+    public void testRebindsToJcloudsWinrmMachineWithTemplateAndNode() throws Exception {
+        // Populate the mementoDir with some old-style peristed state
+        ResourceUtils resourceUtils = ResourceUtils.create(this);
+        String origParentXml = resourceUtils.getResourceAsString("classpath://org/apache/brooklyn/location/jclouds/persisted-aws-winrm-parent-fKc0Ofyn");
+        String origMachineXml = resourceUtils.getResourceAsString("classpath://org/apache/brooklyn/location/jclouds/persisted-aws-winrm-machine-KYSryzW8");
+        File persistedParentFile = new File(mementoDir, "locations/fKc0Ofyn");
+        File persistedMachineFile = new File(mementoDir, "locations/KYSryzW8");
+        FileUtils.write(
+                persistedParentFile,
+                origParentXml);
+        FileUtils.write(
+                persistedMachineFile,
+                origMachineXml);
+
+        assertTrue(origMachineXml.contains("NodeMetadataImpl"), origMachineXml);
+
+        // Rebind to the old-style persisted state, which includes the NodeMetadata and the Template objects.
+        // Expect to parse that ok.
+        TestApplication app2 = rebind();
+        
+        JcloudsLocation loc2 = (JcloudsLocation) app2.getManagementContext().getLocationManager().getLocation("fKc0Ofyn");
+        JcloudsWinRmMachineLocation machine2 = (JcloudsWinRmMachineLocation) app2.getManagementContext().getLocationManager().getLocation("KYSryzW8");
+        assertEquals(ImmutableSet.copyOf(loc2.getChildren()), ImmutableSet.of(machine2));
+        
+        String errmsg = "loc="+loc2.toVerboseString()+"; machine="+machine2.toVerboseString();
+        assertEquals(machine2.getId(), "KYSryzW8", errmsg);
+        assertEquals(machine2.getJcloudsId(), "eu-central-1/i-372eda8a", errmsg);
+        assertEquals(machine2.getAddress().getHostAddress(), "52.28.153.46", errmsg);
+        assertEquals(machine2.getPort(), 5985, errmsg);
+        // FIXME assertEquals(machine2.getAddress().getHostAddress(), HostAndPort.fromParts("ec2-52-28-153-46.eu-central-1.compute.amazonaws.com", 22), errmsg);
+        assertEquals(machine2.getPrivateAddresses(), ImmutableSet.of("172.31.18.175"), errmsg);
+        assertEquals(machine2.getPublicAddresses(), ImmutableSet.of("52.28.153.46"), errmsg);
+        assertEquals(machine2.getPrivateAddress(), Optional.of("172.31.18.175"), errmsg);
+        assertEquals(machine2.getHostname(), "ip-172-31-18-175", errmsg); // TODO would prefer the hostname that works inside and out
+        assertNull(machine2.getOsDetails(), errmsg); // JcloudsWinRmMachineLocation never had OsDetails
+
+        // Force it to be persisted again. Expect to pesist without the NodeMetadata and Template.
+        app2.getManagementContext().getRebindManager().getChangeListener().onChanged(loc2);
+        app2.getManagementContext().getRebindManager().getChangeListener().onChanged(machine2);
+        RebindTestUtils.waitForPersisted(app2);
+        
+        String newMachineXml = new String(java.nio.file.Files.readAllBytes(persistedMachineFile.toPath()));
+        assertFalse(newMachineXml.contains("NodeMetadataImpl"), newMachineXml);
+        
+        // Rebind again, with the re-written persisted state.
+        TestApplication app3 = rebind();
+        
+        JcloudsLocation loc3 = (JcloudsLocation) app3.getManagementContext().getLocationManager().getLocation("fKc0Ofyn");
+        JcloudsWinRmMachineLocation machine3 = (JcloudsWinRmMachineLocation) app3.getManagementContext().getLocationManager().getLocation("KYSryzW8");
+        assertEquals(ImmutableSet.copyOf(loc3.getChildren()), ImmutableSet.of(machine3));
+        
+        errmsg = "loc="+loc3.toVerboseString()+"; machine="+machine3.toVerboseString();
+        assertEquals(machine3.getId(), "KYSryzW8", errmsg);
+        assertEquals(machine3.getJcloudsId(), "eu-central-1/i-372eda8a", errmsg);
+        assertEquals(machine3.getAddress().getHostAddress(), "52.28.153.46", errmsg);
+        assertEquals(machine3.getPort(), 5985, errmsg);
+        assertEquals(machine3.getPrivateAddresses(), ImmutableSet.of("172.31.18.175"), errmsg);
+        assertEquals(machine3.getPublicAddresses(), ImmutableSet.of("52.28.153.46"), errmsg);
+        assertEquals(machine3.getPrivateAddress(), Optional.of("172.31.18.175"), errmsg);
+        assertEquals(machine3.getHostname(), "ip-172-31-18-175", errmsg); // TODO would prefer the hostname that works inside and out
+        assertNull(machine2.getOsDetails(), errmsg); // JcloudsWinRmMachineLocation never had OsDetails
+    }
+
+    private void assertMachineEquals(JcloudsSshMachineLocation actual, JcloudsSshMachineLocation expected, boolean expectNoOsDetails) {
         String errmsg = "actual="+actual.toVerboseString()+"; expected="+expected.toVerboseString();
         assertEquals(actual.getId(), expected.getId(), errmsg);
         assertEquals(actual.getJcloudsId(), expected.getJcloudsId(), errmsg);
-        assertOsDetailEquals(actual.getOsDetails(), expected.getOsDetails());
+        if (expectNoOsDetails) {
+            assertOsDetailEquals(actual.getOptionalOsDetails(), Optional.<OsDetails>absent());
+        } else {
+            assertOsDetailEquals(actual.getOptionalOsDetails(), expected.getOptionalOsDetails());
+        }
         assertEquals(actual.getSshHostAndPort(), expected.getSshHostAndPort());
         assertEquals(actual.getPrivateAddress(), expected.getPrivateAddress());
         assertConfigBagEquals(actual.config().getBag(), expected.config().getBag(), errmsg);
     }
 
-    private void assertOsDetailEquals(OsDetails actual, OsDetails expected) {
+    private void assertOsDetailEquals(Optional<OsDetails> actual, Optional<OsDetails> expected) {
         String errmsg = "actual="+actual+"; expected="+expected;
-        if (actual == null) assertNull(expected, errmsg);
-        assertEquals(actual.isWindows(), expected.isWindows());
-        assertEquals(actual.isLinux(), expected.isLinux());
-        assertEquals(actual.isMac(), expected.isMac());
-        assertEquals(actual.getName(), expected.getName());
-        assertEquals(actual.getArch(), expected.getArch());
-        assertEquals(actual.getVersion(), expected.getVersion());
-        assertEquals(actual.is64bit(), expected.is64bit());
+        if (actual.isPresent()) {
+            assertEquals(actual.get().isWindows(), expected.get().isWindows());
+            assertEquals(actual.get().isLinux(), expected.get().isLinux());
+            assertEquals(actual.get().isMac(), expected.get().isMac());
+            assertEquals(actual.get().getName(), expected.get().getName());
+            assertEquals(actual.get().getArch(), expected.get().getArch());
+            assertEquals(actual.get().getVersion(), expected.get().getVersion());
+            assertEquals(actual.get().is64bit(), expected.get().is64bit());
+        } else {
+            assertFalse(expected.isPresent(), errmsg);
+        }
     }
 
     private void assertJcloudsLocationEquals(JcloudsLocation actual, JcloudsLocation expected) {
@@ -143,7 +314,13 @@ public class RebindJcloudsLocationLiveTest extends AbstractJcloudsLiveTest {
     }
     
     private TestApplication rebind() throws Exception {
+        return rebind(RebindOptions.create()
+                .mementoDir(mementoDir)
+                .classLoader(classLoader));
+    }
+    
+    private TestApplication rebind(RebindOptions options) throws Exception {
         RebindTestUtils.waitForPersisted(origApp);
-        return (TestApplication) RebindTestUtils.rebind(mementoDir, getClass().getClassLoader());
+        return (TestApplication) RebindTestUtils.rebind(options);
     }
 }


Mime
View raw message