From 705b17364972038bc2b74079692324fef42bb90a Mon Sep 17 00:00:00 2001 From: Svetoslav Neykov Date: Wed, 6 Jul 2016 20:49:54 +0300 Subject: [PATCH] Don't serialize NodeMetadata in location config --- .../jclouds/JcloudsSshMachineLocation.java | 26 +++++++++++-------- .../RebindJcloudsLocationLiveTest.java | 6 +++++ 2 files changed, 21 insertions(+), 11 deletions(-) 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 51a476fd92..6a3e90533a 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 @@ -114,9 +114,15 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements Jcl * 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! + * + * Note that fields in locations behave differently from those in entities. Locations will + * update the persisted state with the latest values of fields and will skip transient + * properties. Entities don't read back the field values. * * TODO We will rename these to get rid of the ugly underscore when the old node/template * fields are deleted. + * TODO Should we change callers to pass all the bits of node & template we are interested + * in instead of the objects themselves so we don't have to clear them here? */ private transient Optional _node; @@ -146,19 +152,15 @@ public void init() { 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. + // Note that the happy path for rebind will go through the above case! if (LOG.isDebugEnabled()) LOG.debug("Not doing init() of {} because parent not set; presuming rebinding", this); } + clearDeprecatedProperties(); } - + @Override public void rebind() { super.rebind(); @@ -171,15 +173,15 @@ public void rebind() { LOG.warn("Location {} does not have parent; cannot retrieve jclouds compute-service or " + "run-script factory; later operations may fail (continuing)", this); } - + clearDeprecatedProperties(); + } + + protected void clearDeprecatedProperties() { if (node != null) { setNode(node); - node = null; } - if (template != null) { setTemplate(template); - template = null; } } @@ -200,6 +202,7 @@ public String toVerboseString() { protected void setNode(NodeMetadata node) { this.node = null; + config().removeFromLocalBag("node"); nodeId = node.getId(); imageId = node.getImageId(); privateAddresses = node.getPrivateAddresses(); @@ -209,6 +212,7 @@ protected void setNode(NodeMetadata node) { protected void setTemplate(Template template) { this.template = null; + config().removeFromLocalBag("template"); _template = Optional.of(template); _image = Optional.fromNullable(template.getImage()); } 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 dd6d58f5b0..446aaa98b0 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 @@ -43,6 +43,7 @@ import com.google.common.base.Optional; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.io.Files; @@ -150,6 +151,8 @@ public void testRebindsToJcloudsSshMachineWithTemplateAndNode() throws Exception assertEquals(ImmutableSet.copyOf(loc2.getChildren()), ImmutableSet.of(machine2)); String errmsg = "loc="+loc2.toVerboseString()+"; machine="+machine2.toVerboseString(); + assertNull(machine2.config().getLocalBag().getAllConfig().get("node"), errmsg); + assertNull(machine2.config().getLocalBag().getAllConfig().get("template"), errmsg); 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); @@ -165,6 +168,9 @@ public void testRebindsToJcloudsSshMachineWithTemplateAndNode() throws Exception assertEquals(machine2.getOsDetails().getArch(), "x86_64", errmsg); assertEquals(machine2.getOsDetails().getVersion(), "6.5", errmsg); assertEquals(machine2.getOsDetails().is64bit(), true, errmsg); + + // Re-populates the @SetFromFlag fields from config + machine2.configure(ImmutableMap.of()); // Force it to be persisted again. Expect to pesist without the NodeMetadata and Template. app2.getManagementContext().getRebindManager().getChangeListener().onChanged(loc2);