You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2016/11/18 21:59:16 UTC
[1/2] brooklyn-server git commit: BROOKLYN-396: test and fix
Repository: brooklyn-server
Updated Branches:
refs/heads/master a569463dd -> d7eb99e10
BROOKLYN-396: test and fix
When rebinding a location, if a config flag corresponds to a field
with \u201c@SetFromFlag\u201d then just set the field, and don\u2019t leave the
key-value inside \u201cconfig().getBag()\u201d.
Some classes use fields to store internal state, which should not be
passed to child locations
(e.g. `LocalhostMachineProvisioningLocation.origConfigs`).
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/3563d479
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/3563d479
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/3563d479
Branch: refs/heads/master
Commit: 3563d4794064b2c2eecf516d7fb96c7e1556e478
Parents: a569463
Author: Aled Sage <al...@gmail.com>
Authored: Fri Nov 18 16:07:53 2016 +0000
Committer: Aled Sage <al...@gmail.com>
Committed: Fri Nov 18 16:07:53 2016 +0000
----------------------------------------------------------------------
.../mgmt/rebind/BasicLocationRebindSupport.java | 52 ++++++----
.../core/mgmt/rebind/RebindLocationTest.java | 24 +++++
.../byon/ByonLocationResolverRebindTest.java | 31 +++++-
.../LocalhostLocationResolverRebindTest.java | 101 +++++++++++++++++++
.../MachineLifecycleEffectorTasks.java | 3 +-
5 files changed, 183 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3563d479/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java
index 99f5a24..e603395 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java
@@ -70,33 +70,41 @@ public class BasicLocationRebindSupport extends AbstractBrooklynObjectRebindSupp
// Note that the flags have been set in the constructor
// Sept 2016 - now ignores unused and config description
- location.config().putAll(memento.getLocationConfig());
-
for (Map.Entry<String, Object> entry : memento.getLocationConfig().entrySet()) {
String flagName = entry.getKey();
+ Object value = entry.getValue();
+
+ Field field;
try {
- Field field = FlagUtils.findFieldForFlag(flagName, location);
- Class<?> fieldType = field.getType();
- Object value = entry.getValue();
-
- // Field is either of type ConfigKey, or it's a vanilla java field annotated with @SetFromFlag.
- // If the former, need to look up the field value (i.e. the ConfigKey) to find out the type.
- // If the latter, just want to look at the field itself to get the type.
- // Then coerce the value we have to that type.
- // And use magic of setFieldFromFlag's magic to either set config or field as appropriate.
- if (ConfigKey.class.isAssignableFrom(fieldType)) {
- ConfigKey<?> configKey = (ConfigKey<?>) FlagUtils.getField(location, field);
- location.config().set((ConfigKey<Object>)configKey, entry.getValue());
- } else {
- value = TypeCoercions.coerce(entry.getValue(), fieldType);
- if (value != null) {
- location.config().putAll(MutableMap.of(flagName, value));
- FlagUtils.setFieldFromFlag(location, flagName, value);
- }
- }
-
+ field = FlagUtils.findFieldForFlag(flagName, location);
} catch (NoSuchElementException e) {
// FIXME How to do findFieldForFlag without throwing exception if it's not there?
+ field = null;
+ }
+ if (field == null) {
+ // This is anonymous config: just add it using the string key
+ location.config().putAll(MutableMap.of(flagName, value));
+ continue;
+ }
+
+ Class<?> fieldType = field.getType();
+
+ // Field is either of type ConfigKey, or it's a vanilla java field annotated with @SetFromFlag.
+ // If the former, need to look up the field value (i.e. the ConfigKey) to find out the type.
+ // If the latter, just want to look at the field itself to get the type.
+ // Then coerce the value we have to that type.
+ // And use magic of setFieldFromFlag's magic to either set config or field as appropriate.
+ if (ConfigKey.class.isAssignableFrom(fieldType)) {
+ ConfigKey<?> configKey = (ConfigKey<?>) FlagUtils.getField(location, field);
+ location.config().set((ConfigKey<Object>)configKey, entry.getValue());
+ } else {
+ // Fields annotated with `@SetFromFlag` are very "special" - don't treat them
+ // like normal config (because that's not how they are normally treated before
+ // rebind - see https://issues.apache.org/jira/browse/BROOKLYN-396
+ value = TypeCoercions.coerce(entry.getValue(), fieldType);
+ if (value != null) {
+ FlagUtils.setFieldFromFlag(location, flagName, value);
+ }
}
}
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3563d479/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindLocationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindLocationTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindLocationTest.java
index 327dd96..53e9a5c 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindLocationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindLocationTest.java
@@ -270,6 +270,25 @@ public class RebindLocationTest extends RebindTestFixtureWithApp {
Asserts.assertEqualsIgnoringOrder(newLoc.tags().getTags(), ImmutableSet.of("foo", newApp));
}
+ // See https://issues.apache.org/jira/browse/BROOKLYN-396
+ @Test
+ public void testFlagFieldsNotReturnedInConfig() throws Exception {
+ MyLocation origLoc = mgmt().getLocationManager().createLocation(LocationSpec.create(MyLocation.class));
+ origLoc.myfield = "myval";
+ origLoc.requestPersist();
+
+ // Check (before rebind) that the 'myfield' isn't also in the config
+ assertNull(origLoc.config().getBag().getStringKey("myfield"));
+
+ // Check after rebind that we are the same: 'myfield' isn't also in the config
+ rebind();
+ MyLocation newLoc = (MyLocation) mgmt().getLocationManager().getLocation(origLoc.getId());
+
+ assertEquals(newLoc.myfield, "myval");
+ assertNull(newLoc.config().getBag().getStringKey("myfield"));
+ }
+
+
public static class LocationChecksIsRebinding extends AbstractLocation {
boolean isRebindingValWhenRebinding;
@@ -328,6 +347,11 @@ public class RebindLocationTest extends RebindTestFixtureWithApp {
public MyLocation(Map<?,?> flags) {
super(flags);
}
+
+ @Override
+ public void requestPersist() {
+ super.requestPersist();
+ }
}
public static class MyLocationReffingOthers extends AbstractLocation {
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3563d479/core/src/test/java/org/apache/brooklyn/location/byon/ByonLocationResolverRebindTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/location/byon/ByonLocationResolverRebindTest.java b/core/src/test/java/org/apache/brooklyn/location/byon/ByonLocationResolverRebindTest.java
index 9f3b3a3..ccf72d3 100644
--- a/core/src/test/java/org/apache/brooklyn/location/byon/ByonLocationResolverRebindTest.java
+++ b/core/src/test/java/org/apache/brooklyn/location/byon/ByonLocationResolverRebindTest.java
@@ -19,11 +19,13 @@
package org.apache.brooklyn.location.byon;
import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.assertNull;
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.config.ConfigKeys;
+import org.apache.brooklyn.core.location.internal.LocationInternal;
import org.apache.brooklyn.core.mgmt.rebind.RebindTestFixtureWithApp;
import org.apache.brooklyn.location.ssh.SshMachineLocation;
import org.testng.annotations.Test;
@@ -34,15 +36,34 @@ public class ByonLocationResolverRebindTest extends RebindTestFixtureWithApp {
@Test
public void testRebindByon() throws Exception {
- String spec = "byon(hosts=\"1.1.1.1\")";
+ String spec = "byon(hosts=\"1.1.1.1,1.1.1.2\")";
MachineProvisioningLocation<MachineLocation> provisioner = resolve(spec);
-
+ MachineLocation machine1 = provisioner.obtain(ImmutableMap.of());
+ machine1.config().set(ConfigKeys.newStringConfigKey("mykey1"), "myval1");
+
rebind();
@SuppressWarnings("unchecked")
MachineProvisioningLocation<MachineLocation> newProvisioner = (MachineProvisioningLocation<MachineLocation>) mgmt().getLocationManager().getLocation(provisioner.getId());
- MachineLocation newLocation = newProvisioner.obtain(ImmutableMap.of());
- assertTrue(newLocation instanceof SshMachineLocation, "Expected location to be SshMachineLocation, found " + newLocation);
+
+ SshMachineLocation newMachine1 = (SshMachineLocation) mgmt().getLocationManager().getLocation(machine1.getId());
+ assertEquals(newMachine1.config().get(ConfigKeys.newStringConfigKey("mykey1")), "myval1");
+
+ SshMachineLocation newMachine2 = (SshMachineLocation) newProvisioner.obtain(ImmutableMap.of());
+
+ // See https://issues.apache.org/jira/browse/BROOKLYN-396 (though didn't fail for byon, only localhost)
+ ((LocationInternal)newProvisioner).config().getLocalBag().toString();
+ ((LocationInternal)newMachine1).config().getLocalBag().toString();
+ ((LocationInternal)newMachine2).config().getLocalBag().toString();
+
+ // Confirm when machine is released that we get it back (without the modifications to config)
+ newMachine1.config().set(ConfigKeys.newStringConfigKey("mykey2"), "myval2");
+ newProvisioner.release(newMachine1);
+
+ MachineLocation newMachine1b = newProvisioner.obtain(ImmutableMap.of());
+ assertEquals(newMachine1b.getAddress(), machine1.getAddress());
+ assertNull(newMachine1b.config().get(ConfigKeys.newStringConfigKey("mykey1")));
+ assertNull(newMachine1b.config().get(ConfigKeys.newStringConfigKey("mykey2")));
}
@Test
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3563d479/core/src/test/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolverRebindTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolverRebindTest.java b/core/src/test/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolverRebindTest.java
new file mode 100644
index 0000000..43c6c6e
--- /dev/null
+++ b/core/src/test/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolverRebindTest.java
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.localhost;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
+
+import org.apache.brooklyn.api.location.LocationSpec;
+import org.apache.brooklyn.api.location.MachineLocation;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.location.internal.LocationInternal;
+import org.apache.brooklyn.core.mgmt.rebind.RebindTestFixtureWithApp;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.ImmutableMap;
+
+public class LocalhostLocationResolverRebindTest extends RebindTestFixtureWithApp {
+
+ // Test is motivated by https://issues.apache.org/jira/browse/BROOKLYN-396, to
+ // compare behaviour with and without rebind.
+ @Test
+ public void testWithoutRebind() throws Exception {
+ runTest(false);
+ }
+
+ @Test
+ public void testRebind() throws Exception {
+ runTest(true);
+ }
+
+ protected void runTest(boolean doRebind) throws Exception {
+ LocalhostMachineProvisioningLocation provisioner = resolve("localhost");
+ MachineLocation machine1 = provisioner.obtain(ImmutableMap.of());
+ machine1.config().set(ConfigKeys.newStringConfigKey("mykey1"), "myval1");
+
+ if (doRebind) {
+ rebind();
+ }
+
+ LocalhostMachineProvisioningLocation newProvisioner = (LocalhostMachineProvisioningLocation) mgmt().getLocationManager().getLocation(provisioner.getId());
+
+ SshMachineLocation newMachine1 = (SshMachineLocation) mgmt().getLocationManager().getLocation(machine1.getId());
+ assertEquals(newMachine1.config().get(ConfigKeys.newStringConfigKey("mykey1")), "myval1");
+
+ SshMachineLocation newMachine2 = newProvisioner.obtain(ImmutableMap.of());
+
+ // See https://issues.apache.org/jira/browse/BROOKLYN-396
+ ((LocationInternal)newProvisioner).config().getLocalBag().toString();
+ ((LocationInternal)newMachine1).config().getLocalBag().toString();
+ ((LocationInternal)newMachine2).config().getLocalBag().toString();
+
+ // Release a machine, and get a new one.
+ // With a normal BYON, it would give us the same machine.
+ // For localhost, we don't care if it's the same or different - as long as it doesn't
+ // have the specific config set on the original MachineLocation
+ newMachine1.config().set(ConfigKeys.newStringConfigKey("mykey2"), "myval2");
+ newProvisioner.release(newMachine1);
+
+ MachineLocation newMachine1b = newProvisioner.obtain(ImmutableMap.of());
+ assertNull(newMachine1b.config().get(ConfigKeys.newStringConfigKey("mykey1")));
+ assertNull(newMachine1b.config().get(ConfigKeys.newStringConfigKey("mykey2")));
+ }
+
+ @Test
+ public void testRebindWhenOnlyByonLocationSpec() throws Exception {
+ int before = mgmt().getLocationManager().getLocations().size();
+ getLocationSpec("localhost");
+
+ rebind();
+
+ int after = mgmt().getLocationManager().getLocations().size();
+ assertEquals(after, before);
+ }
+
+ @SuppressWarnings("unchecked")
+ private LocationSpec<LocalhostMachineProvisioningLocation> getLocationSpec(String val) {
+ return (LocationSpec<LocalhostMachineProvisioningLocation>) mgmt().getLocationRegistry().getLocationSpec(val).get();
+ }
+
+ private LocalhostMachineProvisioningLocation resolve(String val) {
+ return (LocalhostMachineProvisioningLocation) mgmt().getLocationRegistry().getLocationManaged(val);
+ }
+}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3563d479/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
index bb7da92..83a34de 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
@@ -420,11 +420,12 @@ public abstract class MachineLifecycleEffectorTasks {
if (machine == null) {
throw new NoMachinesAvailableException("Failed to obtain machine in " + location.toString());
}
- if (log.isDebugEnabled())
+ if (log.isDebugEnabled()) {
log.debug("While starting {}, obtained new location instance {}", entity(),
(machine instanceof SshMachineLocation
? machine + ", details " + ((SshMachineLocation) machine).getUser() + ":" + Sanitizer.sanitize(((SshMachineLocation) machine).config().getLocalBag())
: machine));
+ }
return machine;
}
}
[2/2] brooklyn-server git commit: This closes #444
Posted by al...@apache.org.
This closes #444
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/d7eb99e1
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/d7eb99e1
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/d7eb99e1
Branch: refs/heads/master
Commit: d7eb99e1081fe843d4e2135f3029ca17d2a0a867
Parents: a569463 3563d47
Author: Aled Sage <al...@gmail.com>
Authored: Fri Nov 18 21:56:17 2016 +0000
Committer: Aled Sage <al...@gmail.com>
Committed: Fri Nov 18 21:56:17 2016 +0000
----------------------------------------------------------------------
.../mgmt/rebind/BasicLocationRebindSupport.java | 52 ++++++----
.../core/mgmt/rebind/RebindLocationTest.java | 24 +++++
.../byon/ByonLocationResolverRebindTest.java | 31 +++++-
.../LocalhostLocationResolverRebindTest.java | 101 +++++++++++++++++++
.../MachineLifecycleEffectorTasks.java | 3 +-
5 files changed, 183 insertions(+), 28 deletions(-)
----------------------------------------------------------------------