This closes #444

core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java

@@ -70,33 +70,41 @@ protected void addConfig(RebindContext rebindContext, LocationMemento memento) {
         // 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);
+                }
             }
         }
     }

core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindLocationTest.java

@@ -270,6 +270,25 @@ public void testLocationTags() throws Exception {
         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 MyLocation() {
         public MyLocation(Map<?,?> flags) {
             super(flags);
         }
+
+        @Override
+        public void requestPersist() {
+            super.requestPersist();
+        }
     }
 
     public static class MyLocationReffingOthers extends AbstractLocation {

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

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);
+    }
+}

software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java

@@ -420,11 +420,12 @@ public MachineLocation call() throws Exception {
             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;
         }
     }