From 25c4bd41d2c724e3779054b2a94c18ec0502b063 Mon Sep 17 00:00:00 2001 From: Aled Sage Date: Wed, 4 May 2016 21:07:01 +0100 Subject: [PATCH 1/2] Fix clocker-pattern for reloadProperties Previously if mgmt.reloadBrooklynProperties() was called, the clocker registered locations would be lost. Now we re-register them when reloadProperties is called. --- .../ClockerDynamicLocationPatternTest.java | 82 +++++++++++++++++-- .../dynamic/clocker/StubHostLocation.java | 11 +++ .../clocker/StubInfrastructureLocation.java | 11 +++ 3 files changed, 98 insertions(+), 6 deletions(-) diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/ClockerDynamicLocationPatternTest.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/ClockerDynamicLocationPatternTest.java index b67b11c790..122cc56c08 100644 --- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/ClockerDynamicLocationPatternTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/ClockerDynamicLocationPatternTest.java @@ -20,17 +20,23 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertSame; +import static org.testng.Assert.fail; + +import java.util.Map; import org.apache.brooklyn.api.entity.EntitySpec; +import org.apache.brooklyn.api.location.LocationDefinition; import org.apache.brooklyn.api.location.MachineLocation; import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.location.BasicLocationRegistry; import org.apache.brooklyn.core.location.Locations; import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport; -import org.apache.brooklyn.core.test.entity.TestApplication; import org.apache.brooklyn.location.localhost.LocalhostMachineProvisioningLocation; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -43,6 +49,8 @@ */ public class ClockerDynamicLocationPatternTest extends BrooklynAppUnitTestSupport { + private static final Logger LOG = LoggerFactory.getLogger(ClockerDynamicLocationPatternTest.class); + private LocalhostMachineProvisioningLocation loc; @BeforeMethod(alwaysRun=true) @@ -85,19 +93,72 @@ public void testThroughLocationRegistry() throws Exception { String infraLocSpec = infra.sensors().get(StubInfrastructure.LOCATION_SPEC); String infraLocName = infra.sensors().get(StubInfrastructure.LOCATION_NAME); - StubInfrastructureLocation infraLoc = (StubInfrastructureLocation) mgmt.getLocationRegistry().resolve(infraLocSpec); - StubInfrastructureLocation infraLoc2 = (StubInfrastructureLocation) mgmt.getLocationRegistry().resolve(infraLocName); + StubInfrastructureLocation infraLoc = (StubInfrastructureLocation) mgmt.getLocationRegistry().getLocationManaged(infraLocSpec); + StubInfrastructureLocation infraLoc2 = (StubInfrastructureLocation) mgmt.getLocationRegistry().getLocationManaged(infraLocName); assertSame(infraLoc, infraLoc2); - + assertHasLocation(mgmt.getLocationRegistry().getDefinedLocations().values(), infraLocName); + + MachineLocation machine = infraLoc.obtain(ImmutableMap.of()); StubHost host = (StubHost) Iterables.getOnlyElement(infra.getStubHostCluster().getMembers()); String hostLocSpec = host.sensors().get(StubHost.LOCATION_SPEC); String hostLocName = host.sensors().get(StubHost.LOCATION_NAME); - StubHostLocation hostLoc = (StubHostLocation) mgmt.getLocationRegistry().resolve(hostLocSpec); - StubHostLocation hostLoc2 = (StubHostLocation) mgmt.getLocationRegistry().resolve(hostLocName); + StubHostLocation hostLoc = (StubHostLocation) mgmt.getLocationRegistry().getLocationManaged(hostLocSpec); + StubHostLocation hostLoc2 = (StubHostLocation) mgmt.getLocationRegistry().getLocationManaged(hostLocName); + assertSame(hostLoc, hostLoc2); + assertHasLocation(mgmt.getLocationRegistry().getDefinedLocations().values(), hostLocName); + + StubContainer container = (StubContainer) Iterables.getOnlyElement(host.getDockerContainerCluster().getMembers()); + StubContainerLocation containerLoc = container.getDynamicLocation(); + assertEquals(containerLoc, machine); + assertEquals(Iterables.getOnlyElement(hostLoc.getChildren()), machine); + + infraLoc.release(machine); + assertFalse(Entities.isManaged(container)); + assertFalse(Locations.isManaged(containerLoc)); + } + + @Test + public void testThroughLocationRegistryAfterReloadProperties() throws Exception { + StubInfrastructure infra = mgmt.getEntityManager().createEntity(EntitySpec.create(StubInfrastructure.class) + .configure(BrooklynConfigKeys.SKIP_ON_BOX_BASE_DIR_RESOLUTION, shouldSkipOnBoxBaseDirResolution())); + infra.start(ImmutableList.of(loc)); + + String infraLocSpec = infra.sensors().get(StubInfrastructure.LOCATION_SPEC); + String infraLocName = infra.sensors().get(StubInfrastructure.LOCATION_NAME); + + StubHost host = (StubHost) Iterables.getOnlyElement(infra.getStubHostCluster().getMembers()); + String hostLocSpec = host.sensors().get(StubHost.LOCATION_SPEC); + String hostLocName = host.sensors().get(StubHost.LOCATION_NAME); + + // Force a reload of brooklyn.properties (causes location-registry to be repopulated from scratch) + mgmt.reloadBrooklynProperties(); + + // Should still be listed + assertHasLocation(mgmt.getLocationRegistry().getDefinedLocations().values(), infraLocName); + assertHasLocation(mgmt.getLocationRegistry().getDefinedLocations().values(), hostLocName); + + // We are willing to re-register our resolver, because in real-life we'd register it through + // services in META-INF. + ((BasicLocationRegistry)mgmt.getLocationRegistry()).registerResolver(new StubResolver()); + LOG.info("Locations to look up: infraLocSpec="+infraLocSpec+"; infraLocName="+infraLocName+"; hostLocSpec="+hostLocSpec+"; hostLocName="+hostLocName); + + // Confirm can still load infra-location + StubInfrastructureLocation infraLoc = (StubInfrastructureLocation) mgmt.getLocationRegistry().getLocationManaged(infraLocSpec); + StubInfrastructureLocation infraLoc2 = (StubInfrastructureLocation) mgmt.getLocationRegistry().getLocationManaged(infraLocName); + assertNotNull(infraLoc); + assertSame(infraLoc, infraLoc2); + + // Confirm can still load host-location + StubHostLocation hostLoc = (StubHostLocation) mgmt.getLocationRegistry().getLocationManaged(hostLocSpec); + StubHostLocation hostLoc2 = (StubHostLocation) mgmt.getLocationRegistry().getLocationManaged(hostLocName); + assertNotNull(hostLoc); assertSame(hostLoc, hostLoc2); + // Confirm can still use infra-loc (to obtain a machine) + MachineLocation machine = infraLoc.obtain(ImmutableMap.of()); + StubContainer container = (StubContainer) Iterables.getOnlyElement(host.getDockerContainerCluster().getMembers()); StubContainerLocation containerLoc = container.getDynamicLocation(); assertEquals(containerLoc, machine); @@ -107,4 +168,13 @@ public void testThroughLocationRegistry() throws Exception { assertFalse(Entities.isManaged(container)); assertFalse(Locations.isManaged(containerLoc)); } + + private void assertHasLocation(Iterable locs, String expectedName) { + for (LocationDefinition loc : locs) { + if (expectedName.equals(loc.getName())) { + return; + } + } + fail("No location named '"+expectedName+"' in "+locs); + } } diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostLocation.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostLocation.java index 9c76848fea..bdea2f0a9a 100644 --- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostLocation.java +++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostLocation.java @@ -28,6 +28,7 @@ import org.apache.brooklyn.api.location.LocationDefinition; import org.apache.brooklyn.api.location.MachineProvisioningLocation; import org.apache.brooklyn.api.location.NoMachinesAvailableException; +import org.apache.brooklyn.api.mgmt.ManagementContext.PropertiesReloadListener; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.entity.Entities; @@ -69,6 +70,7 @@ public void init() { super.init(); dockerHost = (StubHost) checkNotNull(getConfig(OWNER), "owner"); machine = (SshMachineLocation) checkNotNull(getConfig(MACHINE), "machine"); + addReloadListener(); } @Override @@ -81,6 +83,15 @@ public void rebind() { if (getConfig(LOCATION_NAME) != null) { register(); } + addReloadListener(); + } + + @SuppressWarnings("serial") + private void addReloadListener() { + getManagementContext().addPropertiesReloadListener(new PropertiesReloadListener() { + @Override public void reloaded() { + register(); + }}); } @Override diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureLocation.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureLocation.java index 5d5695c232..57c52a49fd 100644 --- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureLocation.java +++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureLocation.java @@ -28,6 +28,7 @@ import org.apache.brooklyn.api.location.MachineLocation; import org.apache.brooklyn.api.location.MachineProvisioningLocation; import org.apache.brooklyn.api.location.NoMachinesAvailableException; +import org.apache.brooklyn.api.mgmt.ManagementContext.PropertiesReloadListener; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.location.AbstractLocation; @@ -63,6 +64,7 @@ public class StubInfrastructureLocation extends AbstractLocation implements Mach public void init() { super.init(); infrastructure = (StubInfrastructure) checkNotNull(getConfig(OWNER), "owner"); + addReloadListener(); } @Override @@ -74,6 +76,15 @@ public void rebind() { if (getConfig(LOCATION_NAME) != null) { register(); } + addReloadListener(); + } + + @SuppressWarnings("serial") + private void addReloadListener() { + getManagementContext().addPropertiesReloadListener(new PropertiesReloadListener() { + @Override public void reloaded() { + register(); + }}); } @Override From 2a818bd4be58aef7cba3bb25426cd50e89c8092e Mon Sep 17 00:00:00 2001 From: Aled Sage Date: Wed, 4 May 2016 21:10:21 +0100 Subject: [PATCH 2/2] Clocker-pattern: update for checkNotNull on rebind --- .../location/dynamic/clocker/StubHostLocation.java | 14 +++++++++++--- .../clocker/StubInfrastructureLocation.java | 11 +++++++++-- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostLocation.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostLocation.java index bdea2f0a9a..8ab2df9b06 100644 --- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostLocation.java +++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostLocation.java @@ -68,9 +68,17 @@ public class StubHostLocation extends AbstractLocation implements MachineProvisi @Override public void init() { super.init(); - dockerHost = (StubHost) checkNotNull(getConfig(OWNER), "owner"); - machine = (SshMachineLocation) checkNotNull(getConfig(MACHINE), "machine"); - addReloadListener(); + + // TODO BasicLocationRebindsupport.addCustoms currently calls init() unfortunately! + // Don't checkNotNull in that situation - it could be this location is orphaned! + if (isRebinding()) { + dockerHost = (StubHost) config().get(OWNER); + machine = config().get(MACHINE); + } else { + dockerHost = (StubHost) checkNotNull(getConfig(OWNER), "owner"); + machine = (SshMachineLocation) checkNotNull(getConfig(MACHINE), "machine"); + addReloadListener(); + } } @Override diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureLocation.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureLocation.java index 57c52a49fd..aaa7d3043d 100644 --- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureLocation.java +++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureLocation.java @@ -63,8 +63,15 @@ public class StubInfrastructureLocation extends AbstractLocation implements Mach @Override public void init() { super.init(); - infrastructure = (StubInfrastructure) checkNotNull(getConfig(OWNER), "owner"); - addReloadListener(); + + // TODO BasicLocationRebindsupport.addCustoms currently calls init() unfortunately! + // Don't checkNotNull in that situation - it could be this location is orphaned! + if (isRebinding()) { + infrastructure = (StubInfrastructure) getConfig(OWNER); + } else { + infrastructure = (StubInfrastructure) checkNotNull(getConfig(OWNER), "owner"); + addReloadListener(); + } } @Override