BROOKLYN-325: address review comments Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/8bef4c7a Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/8bef4c7a Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/8bef4c7a
Branch: refs/heads/master Commit: 8bef4c7a3db3584c8c383a32eb1c4a62f90f7272 Parents: 829ae6a Author: Aled Sage <aled.s...@gmail.com> Authored: Wed Nov 16 15:46:05 2016 +0000 Committer: Aled Sage <aled.s...@gmail.com> Committed: Wed Nov 16 15:46:05 2016 +0000 ---------------------------------------------------------------------- .../entity/internal/AttributesInternal.java | 17 ++++---- .../mgmt/rebind/BasicEntityRebindSupport.java | 6 +-- .../MachineLifecycleEffectorTasks.java | 4 +- ...ftwareProcessRebindNotRunningEntityTest.java | 44 +++++++++++++------- ...eProcessStopsDuringStartJcloudsLiveTest.java | 2 +- .../SoftwareProcessStopsDuringStartTest.java | 6 +-- 6 files changed, 47 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8bef4c7a/core/src/main/java/org/apache/brooklyn/core/entity/internal/AttributesInternal.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/internal/AttributesInternal.java b/core/src/main/java/org/apache/brooklyn/core/entity/internal/AttributesInternal.java index 7bccb4d..66004fb 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/internal/AttributesInternal.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/internal/AttributesInternal.java @@ -1,4 +1,5 @@ /* + * 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 @@ -19,22 +20,21 @@ package org.apache.brooklyn.core.entity.internal; import org.apache.brooklyn.api.sensor.AttributeSensor; import org.apache.brooklyn.core.entity.Attributes; -import org.apache.brooklyn.core.sensor.BasicAttributeSensor; +import org.apache.brooklyn.core.sensor.Sensors; import com.google.common.annotations.Beta; import com.google.common.annotations.VisibleForTesting; -import com.google.common.reflect.TypeToken; public interface AttributesInternal extends Attributes { @Beta - public static final AttributeSensor<ProvisioningTaskState> INTERNAL_PROVISIONING_TASK_STATE = new BasicAttributeSensor<ProvisioningTaskState>( - TypeToken.of(ProvisioningTaskState.class), + public static final AttributeSensor<ProvisioningTaskState> INTERNAL_PROVISIONING_TASK_STATE = Sensors.newSensor( + ProvisioningTaskState.class, "internal.provisioning.task.state", "Internal transient sensor (do not use) for tracking the provisioning of a machine (to better handle aborting/rebind)"); @Beta - public static final AttributeSensor<ProvisioningTaskState> INTERNAL_TERMINATION_TASK_STATE = new BasicAttributeSensor<ProvisioningTaskState>( - TypeToken.of(ProvisioningTaskState.class), + public static final AttributeSensor<ProvisioningTaskState> INTERNAL_TERMINATION_TASK_STATE = Sensors.newSensor( + ProvisioningTaskState.class, "internal.termination.task.state", "Internal transient sensor (do not use) for tracking the termination of a machine (to better handle aborting/rebind)"); @@ -42,11 +42,12 @@ public interface AttributesInternal extends Attributes { * Used only internally by {@link org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks} * to track provisioning/termination. This is used so the machine can be terminated if stopped while opaque provision * call is being made; and is used to report if termination was prematurely aborted (e.g. during Brooklyn restart). + * + * @since 0.10.0; before that moved from org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks$ProvisioningTaskState */ @Beta @VisibleForTesting public enum ProvisioningTaskState { - RUNNING, - DONE; + RUNNING } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8bef4c7a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java index f7b5cd8..4165191 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java @@ -269,7 +269,7 @@ public class BasicEntityRebindSupport extends AbstractBrooklynObjectRebindSuppor ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator( entity, "Task aborted on rebind", - "Set to on-fire (from previous expected state "+expectedState+") because tasks aborted on rebind"); + "Set to on-fire (from previous expected state "+expectedState+") because tasks aborted on shutdown"); // Check if we were in the process of provisioning a machine. If so, a VM might have // been left behind. E.g. we might have submitted to jclouds the request to provision @@ -282,7 +282,7 @@ public class BasicEntityRebindSupport extends AbstractBrooklynObjectRebindSuppor ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator( entity, "VM may be lost on rebind", - "VM provisioning may have been in-progress and now lost, because tasks aborted on rebind"); + "VM provisioning may have been in-progress and now lost, because tasks aborted on shutdown"); } // Similar to the provisioning case, if we were terminating the VM then we may or may @@ -294,7 +294,7 @@ public class BasicEntityRebindSupport extends AbstractBrooklynObjectRebindSuppor ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator( entity, "VM may be lost on rebind", - "VM termination may have been in-progress and now lost, because tasks aborted on rebind"); + "VM termination may have been in-progress and now lost, because tasks aborted on shutdown"); } } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8bef4c7a/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 8748861..bb7da92 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 @@ -414,7 +414,7 @@ public abstract class MachineLifecycleEffectorTasks { machine = Tasks.withBlockingDetails("Provisioning machine in " + location, new ObtainLocationTask(location, flags)); entity().sensors().set(INTERNAL_PROVISIONED_MACHINE, machine); } finally { - entity().sensors().set(AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE, ProvisioningTaskState.DONE); + entity().sensors().remove(AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE); } if (machine == null) { @@ -774,7 +774,7 @@ public abstract class MachineLifecycleEffectorTasks { @Override public Boolean call() throws Exception { ProvisioningTaskState state = entity().sensors().get(AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE); - return (state == ProvisioningTaskState.DONE); + return (state != ProvisioningTaskState.RUNNING); }}) .backoffTo(Duration.FIVE_SECONDS) .limitTimeTo(maxWait) http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8bef4c7a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessRebindNotRunningEntityTest.java ---------------------------------------------------------------------- diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessRebindNotRunningEntityTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessRebindNotRunningEntityTest.java index 89a177a..9c9987c 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessRebindNotRunningEntityTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessRebindNotRunningEntityTest.java @@ -47,6 +47,7 @@ import org.apache.brooklyn.core.entity.internal.AttributesInternal; import org.apache.brooklyn.core.entity.lifecycle.Lifecycle; import org.apache.brooklyn.core.entity.trait.Startable; import org.apache.brooklyn.core.location.AbstractLocation; +import org.apache.brooklyn.core.mgmt.rebind.RebindOptions; import org.apache.brooklyn.core.mgmt.rebind.RebindTestFixtureWithApp; import org.apache.brooklyn.core.test.entity.TestApplication; import org.apache.brooklyn.location.byon.FixedListMachineProvisioningLocation; @@ -62,6 +63,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import com.google.common.base.Predicate; @@ -87,6 +89,11 @@ public class SoftwareProcessRebindNotRunningEntityTest extends RebindTestFixture // TODO Longer term, we should investigate/fix that so tearDown finishes promptly no matter what! private List<CountDownLatch> latches; + @DataProvider + public Object[][] terminateOrigManagementContextProvider() { + return new Object[][]{{false}, {true}}; + } + @BeforeMethod(alwaysRun=true) @Override public void setUp() throws Exception { @@ -133,8 +140,8 @@ public class SoftwareProcessRebindNotRunningEntityTest extends RebindTestFixture return HighAvailabilityMode.MASTER; } - @Test - public void testRebindWhileWaitingForCheckRunning() throws Exception { + @Test(dataProvider="terminateOrigManagementContextProvider") + public void testRebindWhileWaitingForCheckRunning(boolean terminateOrigManagementContext) throws Exception { final CountDownLatch checkRunningCalledLatch = newLatch(1); RecordingSshTool.setCustomResponse(".*myCheckRunning.*", new CustomResponseGenerator() { @Override @@ -152,15 +159,15 @@ public class SoftwareProcessRebindNotRunningEntityTest extends RebindTestFixture EntityAsserts.assertAttributeEquals(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.STARTING); - TestApplication newApp = rebind(); + TestApplication newApp = rebind(RebindOptions.create().terminateOrigManagementContext(terminateOrigManagementContext)); final VanillaSoftwareProcess newEntity = (VanillaSoftwareProcess) Iterables.find(newApp.getChildren(), Predicates.instanceOf(VanillaSoftwareProcess.class)); assertMarkedAsOnfire(newEntity, Lifecycle.STARTING); assertMarkedAsOnfire(newApp, Lifecycle.STARTING); } - @Test - public void testRebindWhileLaunching() throws Exception { + @Test(dataProvider="terminateOrigManagementContextProvider") + public void testRebindWhileLaunching(boolean terminateOrigManagementContext) throws Exception { final CountDownLatch launchCalledLatch = newLatch(1); final CountDownLatch launchBlockedLatch = newLatch(1); RecordingSshTool.setCustomResponse(".*myLaunch.*", new CustomResponseGenerator() { @@ -180,15 +187,15 @@ public class SoftwareProcessRebindNotRunningEntityTest extends RebindTestFixture EntityAsserts.assertAttributeEquals(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.STARTING); - TestApplication newApp = rebind(); + TestApplication newApp = rebind(RebindOptions.create().terminateOrigManagementContext(terminateOrigManagementContext)); final VanillaSoftwareProcess newEntity = (VanillaSoftwareProcess) Iterables.find(newApp.getChildren(), Predicates.instanceOf(VanillaSoftwareProcess.class)); assertMarkedAsOnfire(newEntity, Lifecycle.STARTING); assertMarkedAsOnfire(newApp, Lifecycle.STARTING); } - @Test - public void testRebindWhileStoppingProcess() throws Exception { + @Test(dataProvider="terminateOrigManagementContextProvider") + public void testRebindWhileStoppingProcess(boolean terminateOrigManagementContext) throws Exception { final CountDownLatch stopCalledLatch = newLatch(1); final CountDownLatch stopBlockedLatch = newLatch(1); RecordingSshTool.setCustomResponse(".*myStop.*", new CustomResponseGenerator() { @@ -210,7 +217,7 @@ public class SoftwareProcessRebindNotRunningEntityTest extends RebindTestFixture EntityAsserts.assertAttributeEquals(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.STOPPING); - TestApplication newApp = rebind(); + TestApplication newApp = rebind(RebindOptions.create().terminateOrigManagementContext(terminateOrigManagementContext)); final VanillaSoftwareProcess newEntity = (VanillaSoftwareProcess) Iterables.find(newApp.getChildren(), Predicates.instanceOf(VanillaSoftwareProcess.class)); assertMarkedAsOnfire(newEntity, Lifecycle.STOPPING); @@ -218,6 +225,11 @@ public class SoftwareProcessRebindNotRunningEntityTest extends RebindTestFixture @Test public void testRebindWhileProvisioning() throws Exception { + testRebindWhileProvisioning(true); + } + + @Test(dataProvider="terminateOrigManagementContextProvider") + public void testRebindWhileProvisioning(boolean terminateOrigManagementContext) throws Exception { final CountDownLatch obtainCalledLatch = newLatch(1); final CountDownLatch obtainBlockedLatch = newLatch(1); MyProvisioningLocation blockingProvisioner = mgmt().getLocationManager().createLocation(LocationSpec.create(MyProvisioningLocation.class) @@ -233,8 +245,9 @@ public class SoftwareProcessRebindNotRunningEntityTest extends RebindTestFixture awaitOrFail(obtainCalledLatch, Asserts.DEFAULT_LONG_TIMEOUT); EntityAsserts.assertAttributeEquals(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.STARTING); + EntityAsserts.assertAttributeEquals(entity, AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE, AttributesInternal.ProvisioningTaskState.RUNNING); - TestApplication newApp = rebind(); + TestApplication newApp = rebind(RebindOptions.create().terminateOrigManagementContext(terminateOrigManagementContext)); final VanillaSoftwareProcess newEntity = (VanillaSoftwareProcess) Iterables.find(newApp.getChildren(), Predicates.instanceOf(VanillaSoftwareProcess.class)); assertMarkedAsOnfire(newEntity, Lifecycle.STARTING); @@ -246,8 +259,8 @@ public class SoftwareProcessRebindNotRunningEntityTest extends RebindTestFixture EntityAsserts.assertAttributeEquals(newEntity, AttributesInternal.INTERNAL_TERMINATION_TASK_STATE, null); } - @Test - public void testRebindWhileTerminatingVm() throws Exception { + @Test(dataProvider="terminateOrigManagementContextProvider") + public void testRebindWhileTerminatingVm(boolean terminateOrigManagementContext) throws Exception { final CountDownLatch releaseCalledLatch = newLatch(1); final CountDownLatch obtainBlockedLatch = newLatch(1); MyProvisioningLocation blockingProvisioner = mgmt().getLocationManager().createLocation(LocationSpec.create(MyProvisioningLocation.class) @@ -265,8 +278,9 @@ public class SoftwareProcessRebindNotRunningEntityTest extends RebindTestFixture awaitOrFail(releaseCalledLatch, Asserts.DEFAULT_LONG_TIMEOUT); EntityAsserts.assertAttributeEquals(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.STOPPING); + EntityAsserts.assertAttributeEquals(entity, AttributesInternal.INTERNAL_TERMINATION_TASK_STATE, AttributesInternal.ProvisioningTaskState.RUNNING); - TestApplication newApp = rebind(); + TestApplication newApp = rebind(RebindOptions.create().terminateOrigManagementContext(terminateOrigManagementContext)); final VanillaSoftwareProcess newEntity = (VanillaSoftwareProcess) Iterables.find(newApp.getChildren(), Predicates.instanceOf(VanillaSoftwareProcess.class)); @@ -338,12 +352,12 @@ public class SoftwareProcessRebindNotRunningEntityTest extends RebindTestFixture EntityAsserts.assertAttributeEqualsEventually(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.ON_FIRE); EntityAsserts.assertAttributeEqualsEventually(entity, Attributes.SERVICE_UP, false); assertNotUpIndicatorIncludesEventually(entity, "Task aborted on rebind", - "Set to on-fire (from previous expected state "+previousState+") because tasks aborted on rebind"); + "Set to on-fire (from previous expected state "+previousState+") because tasks aborted on shutdown"); } protected void assertMarkedAsVmLost(Entity entity, Lifecycle previousState) throws Exception { String expectedReason = "VM " + (previousState == Lifecycle.STARTING ? "provisioning" : "termination") - + " may have been in-progress and now lost, because tasks aborted on rebind"; + + " may have been in-progress and now lost, because tasks aborted on shutdown"; assertNotUpIndicatorIncludesEventually(entity, "VM may be lost on rebind", expectedReason); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8bef4c7a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartJcloudsLiveTest.java ---------------------------------------------------------------------- diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartJcloudsLiveTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartJcloudsLiveTest.java index f5dcf5d..9515feb 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartJcloudsLiveTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartJcloudsLiveTest.java @@ -142,7 +142,7 @@ public class SoftwareProcessStopsDuringStartJcloudsLiveTest extends BrooklynAppL } }, Asserts.DEFAULT_LONG_TIMEOUT.toMilliseconds(), TimeUnit.MILLISECONDS); EntityAsserts.assertEntityHealthy(entity); - assertEquals(entity.getAttribute(AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE), AttributesInternal.ProvisioningTaskState.DONE); + assertEquals(entity.getAttribute(AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE), null); assertEquals(entity.getAttribute(MachineLifecycleEffectorTasks.INTERNAL_PROVISIONED_MACHINE), Machines.findUniqueMachineLocation(entity.getLocations(), SshMachineLocation.class).get()); executeInLimitedTime(new Callable<Void>() { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8bef4c7a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartTest.java ---------------------------------------------------------------------- diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartTest.java index 99d9678..095da91 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartTest.java @@ -99,7 +99,7 @@ public class SoftwareProcessStopsDuringStartTest extends BrooklynAppUnitTestSupp entity.start(ImmutableList.<Location>of(loc)); SshMachineLocation machine = Machines.findUniqueMachineLocation(entity.getLocations(), SshMachineLocation.class).get(); - EntityAsserts.assertAttributeEquals(entity, AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE, AttributesInternal.ProvisioningTaskState.DONE); + EntityAsserts.assertAttributeEquals(entity, AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE, null); EntityAsserts.assertAttributeEquals(entity, MachineLifecycleEffectorTasks.INTERNAL_PROVISIONED_MACHINE, machine); Stopwatch stopwatch = Stopwatch.createStarted(); @@ -120,7 +120,7 @@ public class SoftwareProcessStopsDuringStartTest extends BrooklynAppUnitTestSupp entity.start(ImmutableList.<Location>of(loc)); SshMachineLocation machine = Machines.findUniqueMachineLocation(entity.getLocations(), SshMachineLocation.class).get(); - EntityAsserts.assertAttributeEquals(entity, AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE, AttributesInternal.ProvisioningTaskState.DONE); + EntityAsserts.assertAttributeEquals(entity, AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE, null); EntityAsserts.assertAttributeEquals(entity, MachineLifecycleEffectorTasks.INTERNAL_PROVISIONED_MACHINE, machine); entity.stop(); @@ -129,7 +129,7 @@ public class SoftwareProcessStopsDuringStartTest extends BrooklynAppUnitTestSupp entity.start(ImmutableList.<Location>of(loc)); SshMachineLocation machine2 = Machines.findUniqueMachineLocation(entity.getLocations(), SshMachineLocation.class).get(); - EntityAsserts.assertAttributeEquals(entity, AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE, AttributesInternal.ProvisioningTaskState.DONE); + EntityAsserts.assertAttributeEquals(entity, AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE, null); EntityAsserts.assertAttributeEquals(entity, MachineLifecycleEffectorTasks.INTERNAL_PROVISIONED_MACHINE, machine2); entity.stop();
