From 0ccd403b2eed0476a17548a55107bb036567c3f0 Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Wed, 24 Oct 2018 17:21:49 +0200 Subject: set-node-state probing in CC --- .../restapiv2/requests/SetNodeStateRequest.java | 27 +++++++++----- .../requests/SetNodeStatesForClusterRequest.java | 5 ++- .../core/restapiv2/requests/WantedStateSetter.java | 3 +- .../core/restapiv2/SetNodeStateTest.java | 9 ++++- .../requests/SetNodeStateRequestTest.java | 41 +++++++++++++++++----- .../staterestapi/requests/SetUnitStateRequest.java | 3 ++ .../utils/staterestapi/server/JsonReader.java | 10 ++++-- .../utils/staterestapi/server/RestApiHandler.java | 2 ++ 8 files changed, 76 insertions(+), 24 deletions(-) diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java index 4d6738940a8..eecdcc75228 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java @@ -37,6 +37,7 @@ public class SetNodeStateRequest extends Request { private final SetUnitStateRequest.ResponseWait responseWait; private final WantedStateSetter wantedState; private final TimeBudget timeBudget; + private final boolean probe; public SetNodeStateRequest(Id.Node id, SetUnitStateRequest setUnitStateRequest) { this(id, setUnitStateRequest, SetNodeStateRequest::setWantedState); @@ -51,6 +52,7 @@ public class SetNodeStateRequest extends Request { this.responseWait = setUnitStateRequest.getResponseWait(); this.wantedState = wantedState; this.timeBudget = setUnitStateRequest.timeBudget(); + this.probe = setUnitStateRequest.isProbe(); } @Override @@ -61,7 +63,8 @@ public class SetNodeStateRequest extends Request { newStates, id.getNode(), context.nodeStateOrHostInfoChangeHandler, - context.currentConsolidatedState); + context.currentConsolidatedState, + probe); } static NodeState getRequestedNodeState(Map newStates, Node n) throws StateRestApiException { @@ -100,7 +103,8 @@ public class SetNodeStateRequest extends Request { Map newStates, Node node, NodeStateOrHostInfoChangeHandler stateListener, - ClusterState currentClusterState) throws StateRestApiException { + ClusterState currentClusterState, + boolean probe) throws StateRestApiException { if ( ! cluster.hasConfiguredNode(node.getIndex())) { throw new MissingIdException(cluster.getName(), node); } @@ -126,7 +130,8 @@ public class SetNodeStateRequest extends Request { condition, nodeInfo, cluster, - stateListener); + stateListener, + probe); // If the state was successfully set, just return an "ok" message back. String reason = success ? "ok" : result.getReason(); @@ -143,9 +148,10 @@ public class SetNodeStateRequest extends Request { SetUnitStateRequest.Condition condition, NodeInfo nodeInfo, ContentCluster cluster, - NodeStateOrHostInfoChangeHandler stateListener) { + NodeStateOrHostInfoChangeHandler stateListener, + boolean probe) { if (result.settingWantedStateIsAllowed()) { - setNewWantedState(nodeInfo, newWantedState, stateListener); + setNewWantedState(nodeInfo, newWantedState, stateListener, probe); } // True if the wanted state was or has just been set to newWantedState @@ -156,7 +162,7 @@ public class SetNodeStateRequest extends Request { // of the distributor. E.g. setting the storage node to maintenance may cause // feeding issues unless distributor is also set down. - setDistributorWantedState(cluster, nodeInfo.getNodeIndex(), newWantedState, stateListener); + setDistributorWantedState(cluster, nodeInfo.getNodeIndex(), newWantedState, stateListener, probe); } return success; @@ -169,7 +175,8 @@ public class SetNodeStateRequest extends Request { private static void setDistributorWantedState(ContentCluster cluster, int index, NodeState newStorageWantedState, - NodeStateOrHostInfoChangeHandler stateListener) { + NodeStateOrHostInfoChangeHandler stateListener, + boolean probe) { Node distributorNode = new Node(NodeType.DISTRIBUTOR, index); NodeInfo nodeInfo = cluster.getNodeInfo(distributorNode); if (nodeInfo == null) { @@ -200,13 +207,15 @@ public class SetNodeStateRequest extends Request { if (newWantedState.getState() != currentWantedState.getState() || !Objects.equals(newWantedState.getDescription(), currentWantedState.getDescription())) { - setNewWantedState(nodeInfo, newWantedState, stateListener); + setNewWantedState(nodeInfo, newWantedState, stateListener, probe); } } private static void setNewWantedState(NodeInfo nodeInfo, NodeState newWantedState, - NodeStateOrHostInfoChangeHandler stateListener) { + NodeStateOrHostInfoChangeHandler stateListener, + boolean probe) { + if (probe) return; nodeInfo.setWantedState(newWantedState); stateListener.handleNewWantedNodeState(nodeInfo, newWantedState); } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java index b4d189bcd55..d7820722887 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java @@ -27,6 +27,7 @@ public class SetNodeStatesForClusterRequest extends Request { private final Map newStates; private final SetUnitStateRequest.Condition condition; private final TimeBudget timeBudget; + private final boolean probe; public SetNodeStatesForClusterRequest(Id.Cluster cluster, SetUnitStateRequest request) { @@ -35,6 +36,7 @@ public class SetNodeStatesForClusterRequest extends Request { this.newStates = request.getNewState(); this.condition = request.getCondition(); this.timeBudget = request.timeBudget(); + this.probe = request.isProbe(); } @Override @@ -69,7 +71,8 @@ public class SetNodeStatesForClusterRequest extends Request { newStates, node, context.nodeStateOrHostInfoChangeHandler, - context.currentConsolidatedState); + context.currentConsolidatedState, + probe); if (!setResponse.getWasModified()) { throw new InternalFailure("We have not yet implemented the meaning of " + diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java index 6fa7d536c67..c3090a5e832 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java @@ -22,5 +22,6 @@ public interface WantedStateSetter { Map newStates, Node node, NodeStateOrHostInfoChangeHandler stateListener, - ClusterState currentClusterState) throws StateRestApiException; + ClusterState currentClusterState, + boolean probe) throws StateRestApiException; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java index f3a4be5ac2f..6cf4b7989e7 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java @@ -33,6 +33,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -46,6 +47,7 @@ public class SetNodeStateTest extends StateRestApiTest { private Condition condition = Condition.FORCE; private ResponseWait responseWait = ResponseWait.WAIT_UNTIL_CLUSTER_ACKED; private TimeBudget timeBudget = TimeBudget.fromNow(Clock.systemUTC(), Duration.ofSeconds(10)); + private boolean probe = false; public SetUnitStateRequestImpl(String req) { super(req, 0); @@ -98,6 +100,11 @@ public class SetNodeStateTest extends StateRestApiTest { public TimeBudget timeBudget() { return timeBudget; } + + @Override + public boolean isProbe() { + return probe; + } } private void verifyStateSet(String state, String reason) throws Exception { @@ -458,7 +465,7 @@ public class SetNodeStateTest extends StateRestApiTest { new SetUnitStateRequestImpl("music/storage/1").setNewState("user", "maintenance", "whatever reason."), wantedStateSetter); SetResponse response = new SetResponse("some reason", wasModified); - when(wantedStateSetter.set(any(), any(), any(), any(), any(), any())).thenReturn(response); + when(wantedStateSetter.set(any(), any(), any(), any(), any(), any(), anyBoolean())).thenReturn(response); RemoteClusterControllerTask.Context context = mock(RemoteClusterControllerTask.Context.class); MasterInterface masterInterface = mock(MasterInterface.class); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java index 9239b8774b0..b8fe0303cfc 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java @@ -23,20 +23,23 @@ import java.util.Map; import java.util.Optional; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class SetNodeStateRequestTest { - public static final String REASON = "operator"; - ContentCluster cluster = mock(ContentCluster.class); - SetUnitStateRequest.Condition condition = SetUnitStateRequest.Condition.SAFE; - Map newStates = new HashMap<>(); - UnitState unitState = mock(UnitState.class); + private static final String REASON = "operator"; + private ContentCluster cluster = mock(ContentCluster.class); + private SetUnitStateRequest.Condition condition = SetUnitStateRequest.Condition.SAFE; + private Map newStates = new HashMap<>(); + private UnitState unitState = mock(UnitState.class); private final int NODE_INDEX = 2; - Node storageNode = new Node(NodeType.STORAGE, NODE_INDEX); - NodeStateOrHostInfoChangeHandler stateListener = mock(NodeStateOrHostInfoChangeHandler.class); - ClusterState currentClusterState = mock(ClusterState.class); + private Node storageNode = new Node(NodeType.STORAGE, NODE_INDEX); + private NodeStateOrHostInfoChangeHandler stateListener = mock(NodeStateOrHostInfoChangeHandler.class); + private ClusterState currentClusterState = mock(ClusterState.class); + private boolean probe = false; @Before public void setUp() { @@ -52,6 +55,16 @@ public class SetNodeStateRequestTest { Optional.of(State.MAINTENANCE), Optional.of(State.DOWN)); } + @Test + public void testProbing() throws StateRestApiException { + probe = true; + testSetStateRequest( + "maintenance", + State.UP, State.UP, + NodeStateChangeChecker.Result.allowSettingOfWantedState(), + Optional.empty(), Optional.empty()); + } + @Test public void testUpToDown() throws StateRestApiException { testSetStateRequest( @@ -124,6 +137,9 @@ public class SetNodeStateRequestTest { when(cluster.getNodeInfo(distributorNode)).thenReturn(distributorNodeInfo); NodeState distributorNodeState = new NodeState(distributorNode.getType(), distributorWantedState); + if (distributorWantedState != State.UP) { + distributorNodeState.setDescription(REASON); + } when(distributorNodeInfo.getUserWantedState()).thenReturn(distributorNodeState); setWantedState(); @@ -133,6 +149,9 @@ public class SetNodeStateRequestTest { new NodeState(NodeType.STORAGE, expectedNewStorageWantedState.get()); verify(storageNodeInfo).setWantedState(expectedNewStorageNodeState); verify(stateListener).handleNewWantedNodeState(storageNodeInfo, expectedNewStorageNodeState); + } else { + verify(storageNodeInfo, times(0)).setWantedState(any()); + verify(stateListener, times(0)).handleNewWantedNodeState(eq(storageNodeInfo), any()); } if (expectedNewDistributorWantedState.isPresent()) { @@ -140,6 +159,9 @@ public class SetNodeStateRequestTest { new NodeState(NodeType.DISTRIBUTOR, expectedNewDistributorWantedState.get()); verify(distributorNodeInfo).setWantedState(expectedNewDistributorNodeState); verify(stateListener).handleNewWantedNodeState(distributorNodeInfo, expectedNewDistributorNodeState); + } else { + verify(distributorNodeInfo, times(0)).setWantedState(any()); + verify(stateListener, times(0)).handleNewWantedNodeState(eq(distributorNodeInfo), any()); } } @@ -150,6 +172,7 @@ public class SetNodeStateRequestTest { newStates, storageNode, stateListener, - currentClusterState); + currentClusterState, + probe); } } \ No newline at end of file diff --git a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/requests/SetUnitStateRequest.java b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/requests/SetUnitStateRequest.java index a28ddb3539b..27f18c3664b 100644 --- a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/requests/SetUnitStateRequest.java +++ b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/requests/SetUnitStateRequest.java @@ -64,4 +64,7 @@ public interface SetUnitStateRequest extends UnitRequest { ResponseWait getResponseWait(); TimeBudget timeBudget(); + + /** A probe request is a non-committal request to see if an identical (but non-probe) request would have succeeded. */ + boolean isProbe(); } diff --git a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java index d871a8ed6bc..dab6895cc9d 100644 --- a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java +++ b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java @@ -33,13 +33,16 @@ public class JsonReader { } static class SetRequestData { + final boolean probe; final Map stateMap; final SetUnitStateRequest.Condition condition; final SetUnitStateRequest.ResponseWait responseWait; - public SetRequestData(Map stateMap, + public SetRequestData(boolean probe, + Map stateMap, SetUnitStateRequest.Condition condition, SetUnitStateRequest.ResponseWait responseWait) { + this.probe = probe; this.stateMap = stateMap; this.condition = condition; this.responseWait = responseWait; @@ -49,8 +52,9 @@ public class JsonReader { public SetRequestData getStateRequestData(HttpRequest request) throws Exception { JSONObject json = new JSONObject(request.getPostContent().toString()); - final SetUnitStateRequest.Condition condition; + final boolean probe = json.has("probe") && json.getBoolean("probe"); + final SetUnitStateRequest.Condition condition; if (json.has("condition")) { condition = SetUnitStateRequest.Condition.fromString(json.getString("condition")); } else { @@ -100,6 +104,6 @@ public class JsonReader { stateMap.put(type, new UnitStateImpl(code, reason)); } - return new SetRequestData(stateMap, condition, responseWait); + return new SetRequestData(probe, stateMap, condition, responseWait); } } diff --git a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java index c38f7aec8c6..46f5d964245 100644 --- a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java +++ b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java @@ -97,6 +97,8 @@ public class RestApiHandler implements HttpRequestHandler { public ResponseWait getResponseWait() { return setRequestData.responseWait; } @Override public TimeBudget timeBudget() { return TimeBudget.from(clock, start, timeout); } + @Override + public boolean isProbe() { return setRequestData.probe; } }); return new JsonHttpResult().setJson(jsonWriter.createJson(setResponse)); } -- cgit v1.2.3