Addressed PR#542 comments
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/363989b9 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/363989b9 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/363989b9 Branch: refs/heads/master Commit: 363989b95ad20a51148f8140a5dae9cdef9a5434 Parents: a8a27f8 Author: Svetoslav Neykov <[email protected]> Authored: Tue Mar 17 23:36:05 2015 +0200 Committer: Svetoslav Neykov <[email protected]> Committed: Thu Mar 19 16:03:08 2015 +0200 ---------------------------------------------------------------------- .../management/ha/HighAvailabilityManager.java | 2 +- .../entity/brooklynnode/BrooklynNodeImpl.java | 7 +- .../software/MachineLifecycleEffectorTasks.java | 42 ++++--- .../BrooklynJavascriptGuiLauncherTest.java | 7 +- .../rest/filter/HaHotCheckResourceFilter.java | 1 + .../rest/filter/HaHotStateRequired.java | 8 ++ .../brooklyn/rest/BrooklynRestApiLauncher.java | 2 +- .../test/java/brooklyn/rest/HaHotCheckTest.java | 19 ++- .../mocks/HighAvailabilityManagerMock.java | 121 ------------------- .../mocks/HighAvailabilityManagerStub.java | 121 +++++++++++++++++++ .../testing/mocks/ManagementContextMock.java | 2 +- 11 files changed, 170 insertions(+), 162 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/363989b9/api/src/main/java/brooklyn/management/ha/HighAvailabilityManager.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/brooklyn/management/ha/HighAvailabilityManager.java b/api/src/main/java/brooklyn/management/ha/HighAvailabilityManager.java index 2a1af9a..c530116 100644 --- a/api/src/main/java/brooklyn/management/ha/HighAvailabilityManager.java +++ b/api/src/main/java/brooklyn/management/ha/HighAvailabilityManager.java @@ -44,7 +44,7 @@ public interface HighAvailabilityManager { ManagementNodeState getNodeState(); - /** The time in milliseconds when the state was last changed */ + /** The time in milliseconds when the state was last changed. -1 if no state transition has occurred yet.*/ long getLastStateChange(); /** http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/363989b9/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeImpl.java ---------------------------------------------------------------------- diff --git a/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeImpl.java b/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeImpl.java index 7856b8f..d4b4992 100644 --- a/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeImpl.java +++ b/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeImpl.java @@ -176,8 +176,9 @@ public class BrooklynNodeImpl extends SoftwareProcessImpl implements BrooklynNod @Override protected void preRestart() { super.preRestart(); + //restart will kill the process, try to shut down before that shutdownGracefully(); - DynamicTasks.queue("post-shutdown", new Runnable() { public void run() { + DynamicTasks.queue("pre-restart", new Runnable() { public void run() { //set by shutdown - clear it so the entity starts cleanly. Does the indicator bring any value at all? ServiceNotUpLogic.clearNotUpIndicator(BrooklynNodeImpl.this, SHUTDOWN.getName()); }}); @@ -220,7 +221,7 @@ public class BrooklynNodeImpl extends SoftwareProcessImpl implements BrooklynNod @Override protected void postStop() { super.postStop(); - if (isStopMachine()) { + if (isMachineStopped()) { // Don't unmanage in entity's task context as it will self-cancel the task. Wait for the stop effector to complete. // If this is not enough (still getting Caused by: java.util.concurrent.CancellationException: null) then // we could search for the top most task with entity context == this and wait on it. Even stronger would be @@ -230,7 +231,7 @@ public class BrooklynNodeImpl extends SoftwareProcessImpl implements BrooklynNod } } - private boolean isStopMachine() { + private boolean isMachineStopped() { // Don't rely on effector parameters, check if there is still a machine running. // If the entity was previously stopped with STOP_MACHINE_MODE=StopMode.NEVER // and a second time with STOP_MACHINE_MODE=StopMode.IF_NOT_STOPPED, then the http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/363989b9/software/base/src/main/java/brooklyn/entity/software/MachineLifecycleEffectorTasks.java ---------------------------------------------------------------------- diff --git a/software/base/src/main/java/brooklyn/entity/software/MachineLifecycleEffectorTasks.java b/software/base/src/main/java/brooklyn/entity/software/MachineLifecycleEffectorTasks.java index e64e078..4fd6b09 100644 --- a/software/base/src/main/java/brooklyn/entity/software/MachineLifecycleEffectorTasks.java +++ b/software/base/src/main/java/brooklyn/entity/software/MachineLifecycleEffectorTasks.java @@ -552,24 +552,8 @@ public abstract class MachineLifecycleEffectorTasks { log.info("Stopping {} in {}", entity(), entity().getLocations()); - @SuppressWarnings("deprecation") - final boolean hasStopMachine = parameters.containsKey(StopSoftwareParameters.STOP_MACHINE); - @SuppressWarnings("deprecation") - final Boolean isStopMachine = parameters.get(StopSoftwareParameters.STOP_MACHINE); - - final StopMode stopProcessMode = parameters.get(StopSoftwareParameters.STOP_PROCESS_MODE); - - final boolean hasStopMachineMode = parameters.containsKey(StopSoftwareParameters.STOP_MACHINE_MODE); - StopMode stopMachineMode = parameters.get(StopSoftwareParameters.STOP_MACHINE_MODE); - - if (hasStopMachine && isStopMachine != null) { - checkCompatibleMachineModes(isStopMachine, hasStopMachineMode, stopMachineMode); - if (isStopMachine) { - stopMachineMode = StopMode.IF_NOT_STOPPED; - } else { - stopMachineMode = StopMode.NEVER; - } - } + StopMode stopMachineMode = getStopMachineMode(parameters); + StopMode stopProcessMode = parameters.get(StopSoftwareParameters.STOP_PROCESS_MODE); DynamicTasks.queue("pre-stop", new Callable<String>() { public String call() { if (entity().getAttribute(SoftwareProcess.SERVICE_STATE_ACTUAL)==Lifecycle.STOPPED) { @@ -650,6 +634,26 @@ public abstract class MachineLifecycleEffectorTasks { if (log.isDebugEnabled()) log.debug("Stopped software process entity "+entity()); } + public static StopMode getStopMachineMode(ConfigBag parameters) { + @SuppressWarnings("deprecation") + final boolean hasStopMachine = parameters.containsKey(StopSoftwareParameters.STOP_MACHINE); + @SuppressWarnings("deprecation") + final Boolean isStopMachine = parameters.get(StopSoftwareParameters.STOP_MACHINE); + + final boolean hasStopMachineMode = parameters.containsKey(StopSoftwareParameters.STOP_MACHINE_MODE); + final StopMode stopMachineMode = parameters.get(StopSoftwareParameters.STOP_MACHINE_MODE); + + if (hasStopMachine && isStopMachine != null) { + checkCompatibleMachineModes(isStopMachine, hasStopMachineMode, stopMachineMode); + if (isStopMachine) { + return StopMode.IF_NOT_STOPPED; + } else { + return StopMode.NEVER; + } + } + return stopMachineMode; + } + public static boolean canStop(StopMode stopMode, Entity entity) { boolean isEntityStopped = entity.getAttribute(SoftwareProcess.SERVICE_STATE_ACTUAL)==Lifecycle.STOPPED; return canStop(stopMode, isEntityStopped); @@ -660,7 +664,7 @@ public abstract class MachineLifecycleEffectorTasks { stopMode == StopMode.IF_NOT_STOPPED && !isStopped; } - private void checkCompatibleMachineModes(Boolean isStopMachine, boolean hasStopMachineMode, StopMode stopMachineMode) { + private static void checkCompatibleMachineModes(Boolean isStopMachine, boolean hasStopMachineMode, StopMode stopMachineMode) { if (hasStopMachineMode && (isStopMachine && stopMachineMode != StopMode.IF_NOT_STOPPED || !isStopMachine && stopMachineMode != StopMode.NEVER)) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/363989b9/usage/jsgui/src/test/java/brooklyn/rest/jsgui/BrooklynJavascriptGuiLauncherTest.java ---------------------------------------------------------------------- diff --git a/usage/jsgui/src/test/java/brooklyn/rest/jsgui/BrooklynJavascriptGuiLauncherTest.java b/usage/jsgui/src/test/java/brooklyn/rest/jsgui/BrooklynJavascriptGuiLauncherTest.java index ab55d66..9ebb40b 100644 --- a/usage/jsgui/src/test/java/brooklyn/rest/jsgui/BrooklynJavascriptGuiLauncherTest.java +++ b/usage/jsgui/src/test/java/brooklyn/rest/jsgui/BrooklynJavascriptGuiLauncherTest.java @@ -64,12 +64,7 @@ public class BrooklynJavascriptGuiLauncherTest { protected void checkUrlContains(final String path, final String text) { //Server may return 403 until it loads completely, wait a bit //until it stabilizes. - Asserts.succeedsEventually(new Runnable() { - @Override - public void run() { - HttpTestUtils.assertContentContainsText(rootUrl()+path, text); - } - }); + HttpTestUtils.assertContentEventuallyContainsText(rootUrl()+path, text); } protected void checkEventuallyHealthy() { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/363989b9/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java index 8b1d67f..5367439 100644 --- a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java +++ b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java @@ -89,6 +89,7 @@ public class HaHotCheckResourceFilter implements ResourceFilterFactory { // and starting rebind so add a time offset just to be sure. private boolean recentlySwitchedState() { long lastStateChange = mgmt.getHighAvailabilityManager().getLastStateChange(); + if (lastStateChange == -1) return false; return System.currentTimeMillis() - lastStateChange < STATE_CHANGE_SETTLE_OFFSET; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/363989b9/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotStateRequired.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotStateRequired.java b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotStateRequired.java index 271da8f..09eea5f 100644 --- a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotStateRequired.java +++ b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotStateRequired.java @@ -23,6 +23,14 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +/** + * When a REST method (or its containing class) is marked with this annotation + * requests to it will fail with a 403 response if the instance is not in MASTER + * mode (or has recently switched or is still rebinding). Guards the method so + * that when it returns the caller can be certain of the response. For example + * if the response is 404, then the resource doesn't exist as opposed to + * not being loaded from persistence store yet. + */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.METHOD, ElementType.TYPE}) public @interface HaHotStateRequired {} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/363989b9/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncher.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncher.java b/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncher.java index cb7b29d..5643614 100644 --- a/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncher.java +++ b/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncher.java @@ -152,7 +152,7 @@ public class BrooklynRestApiLauncher { this.deployJsgui = false; return this; } - + public BrooklynRestApiLauncher disableHighAvailability(boolean value) { this.disableHighAvailability = value; return this; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/363989b9/usage/rest-server/src/test/java/brooklyn/rest/HaHotCheckTest.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/test/java/brooklyn/rest/HaHotCheckTest.java b/usage/rest-server/src/test/java/brooklyn/rest/HaHotCheckTest.java index 8f28728..58a9e6b 100644 --- a/usage/rest-server/src/test/java/brooklyn/rest/HaHotCheckTest.java +++ b/usage/rest-server/src/test/java/brooklyn/rest/HaHotCheckTest.java @@ -23,7 +23,6 @@ import static org.testng.Assert.assertEquals; import javax.ws.rs.core.MediaType; import org.testng.annotations.AfterClass; -import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -46,24 +45,24 @@ public class HaHotCheckTest extends BrooklynRestResourceTest { private ManagementContextMock mgmtMock; - //Treat before/after class methods as before/after method - //otherwise they should be static. - @BeforeClass(alwaysRun=true) - public void setUpClass() {} - @AfterClass(alwaysRun=true) - public void tearDownClass() {} - - @BeforeMethod(alwaysRun = true) + @Override + @BeforeClass(alwaysRun = true) public void setUp() throws Exception { mgmtMock = new ManagementContextMock(); super.setUp(); } - @AfterMethod(alwaysRun = true) + @Override + @AfterClass(alwaysRun = true) public void tearDown() throws Exception { super.tearDown(); } + @BeforeMethod(alwaysRun = true) + public void setUpMethod() { + mgmtMock.setState(ManagementNodeState.MASTER); + } + @Override protected void addBrooklynResources() { config.getSingletons().add(new ManagementContextProvider(mgmtMock)); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/363989b9/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/HighAvailabilityManagerMock.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/HighAvailabilityManagerMock.java b/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/HighAvailabilityManagerMock.java deleted file mode 100644 index 5c99183..0000000 --- a/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/HighAvailabilityManagerMock.java +++ /dev/null @@ -1,121 +0,0 @@ -/* - * 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 brooklyn.rest.testing.mocks; - -import java.util.Map; - -import brooklyn.management.ha.HighAvailabilityManager; -import brooklyn.management.ha.HighAvailabilityMode; -import brooklyn.management.ha.ManagementNodeState; -import brooklyn.management.ha.ManagementPlaneSyncRecord; -import brooklyn.management.ha.ManagementPlaneSyncRecordPersister; - -public class HighAvailabilityManagerMock implements HighAvailabilityManager { - - private ManagementNodeState state = ManagementNodeState.MASTER; - - public void setState(ManagementNodeState state) { - this.state = state; - } - - private static RuntimeException fail() { - throw new UnsupportedOperationException("Mocked method not implemented"); - } - - @Override - public boolean isRunning() { - return state != ManagementNodeState.TERMINATED; - } - - @Override - public ManagementNodeState getNodeState() { - return state; - } - - @Override - public long getLastStateChange() { - return 0; - } - - @Override - public HighAvailabilityManager setPersister(ManagementPlaneSyncRecordPersister persister) { - throw fail(); - } - - @Override - public void disabled() { - throw fail(); - } - - @Override - public void start(HighAvailabilityMode startMode) { - throw fail(); - } - - @Override - public void stop() { - throw fail(); - } - - @Override - public void changeMode(HighAvailabilityMode mode) { - throw fail(); - } - - @Override - public void setPriority(long priority) { - throw fail(); - } - - @Override - public long getPriority() { - throw fail(); - } - - @Override - public void publishClearNonMaster() { - throw fail(); - } - - @Override - public ManagementPlaneSyncRecord getLastManagementPlaneSyncRecord() { - throw fail(); - } - - @Override - public ManagementPlaneSyncRecord getManagementPlaneSyncState() { - throw fail(); - } - - @Override - public ManagementPlaneSyncRecord loadManagementPlaneSyncRecord(boolean useLocalKnowledgeForThisNode) { - throw fail(); - } - - @Override - public ManagementPlaneSyncRecordPersister getPersister() { - throw fail(); - } - - @Override - public Map<String, Object> getMetrics() { - throw fail(); - } - -} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/363989b9/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/HighAvailabilityManagerStub.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/HighAvailabilityManagerStub.java b/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/HighAvailabilityManagerStub.java new file mode 100644 index 0000000..98fb8d5 --- /dev/null +++ b/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/HighAvailabilityManagerStub.java @@ -0,0 +1,121 @@ +/* + * 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 brooklyn.rest.testing.mocks; + +import java.util.Map; + +import brooklyn.management.ha.HighAvailabilityManager; +import brooklyn.management.ha.HighAvailabilityMode; +import brooklyn.management.ha.ManagementNodeState; +import brooklyn.management.ha.ManagementPlaneSyncRecord; +import brooklyn.management.ha.ManagementPlaneSyncRecordPersister; + +public class HighAvailabilityManagerStub implements HighAvailabilityManager { + + private ManagementNodeState state = ManagementNodeState.MASTER; + + public void setState(ManagementNodeState state) { + this.state = state; + } + + private static RuntimeException fail() { + throw new UnsupportedOperationException("Mocked method not implemented"); + } + + @Override + public boolean isRunning() { + return state != ManagementNodeState.TERMINATED; + } + + @Override + public ManagementNodeState getNodeState() { + return state; + } + + @Override + public long getLastStateChange() { + return 0; + } + + @Override + public HighAvailabilityManager setPersister(ManagementPlaneSyncRecordPersister persister) { + throw fail(); + } + + @Override + public void disabled() { + throw fail(); + } + + @Override + public void start(HighAvailabilityMode startMode) { + throw fail(); + } + + @Override + public void stop() { + throw fail(); + } + + @Override + public void changeMode(HighAvailabilityMode mode) { + throw fail(); + } + + @Override + public void setPriority(long priority) { + throw fail(); + } + + @Override + public long getPriority() { + throw fail(); + } + + @Override + public void publishClearNonMaster() { + throw fail(); + } + + @Override + public ManagementPlaneSyncRecord getLastManagementPlaneSyncRecord() { + throw fail(); + } + + @Override + public ManagementPlaneSyncRecord getManagementPlaneSyncState() { + throw fail(); + } + + @Override + public ManagementPlaneSyncRecord loadManagementPlaneSyncRecord(boolean useLocalKnowledgeForThisNode) { + throw fail(); + } + + @Override + public ManagementPlaneSyncRecordPersister getPersister() { + throw fail(); + } + + @Override + public Map<String, Object> getMetrics() { + throw fail(); + } + +} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/363989b9/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/ManagementContextMock.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/ManagementContextMock.java b/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/ManagementContextMock.java index 6a29fed..90edf1e 100644 --- a/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/ManagementContextMock.java +++ b/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/ManagementContextMock.java @@ -44,7 +44,7 @@ import brooklyn.management.ha.ManagementNodeState; import brooklyn.util.guava.Maybe; public class ManagementContextMock implements ManagementContext { - private HighAvailabilityManagerMock haMock = new HighAvailabilityManagerMock(); + private HighAvailabilityManagerStub haMock = new HighAvailabilityManagerStub(); public void setState(ManagementNodeState state) { haMock.setState(state);
