Revert "Revert "Revert "Enforce CC timeouts in Orchestrator 2"""

5 files changed, 21 insertions, 64 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 eecdcc75228..4d6738940a8 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,7 +37,6 @@ public class SetNodeStateRequest extends Request<SetResponse> {
     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);
@@ -52,7 +51,6 @@ public class SetNodeStateRequest extends Request<SetResponse> {
         this.responseWait = setUnitStateRequest.getResponseWait();
         this.wantedState = wantedState;
         this.timeBudget = setUnitStateRequest.timeBudget();
-        this.probe = setUnitStateRequest.isProbe();
     }
 
     @Override
@@ -63,8 +61,7 @@ public class SetNodeStateRequest extends Request<SetResponse> {
                 newStates,
                 id.getNode(),
                 context.nodeStateOrHostInfoChangeHandler,
-                context.currentConsolidatedState,
-                probe);
+                context.currentConsolidatedState);
     }
 
     static NodeState getRequestedNodeState(Map<String, UnitState> newStates, Node n) throws StateRestApiException {
@@ -103,8 +100,7 @@ public class SetNodeStateRequest extends Request<SetResponse> {
             Map<String, UnitState> newStates,
             Node node,
             NodeStateOrHostInfoChangeHandler stateListener,
-            ClusterState currentClusterState,
-            boolean probe) throws StateRestApiException {
+            ClusterState currentClusterState) throws StateRestApiException {
         if ( ! cluster.hasConfiguredNode(node.getIndex())) {
             throw new MissingIdException(cluster.getName(), node);
         }
@@ -130,8 +126,7 @@ public class SetNodeStateRequest extends Request<SetResponse> {
                 condition,
                 nodeInfo,
                 cluster,
-                stateListener,
-                probe);
+                stateListener);
 
         // If the state was successfully set, just return an "ok" message back.
         String reason = success ? "ok" : result.getReason();
@@ -148,10 +143,9 @@ public class SetNodeStateRequest extends Request<SetResponse> {
             SetUnitStateRequest.Condition condition,
             NodeInfo nodeInfo,
             ContentCluster cluster,
-            NodeStateOrHostInfoChangeHandler stateListener,
-            boolean probe) {
+            NodeStateOrHostInfoChangeHandler stateListener) {
         if (result.settingWantedStateIsAllowed()) {
-            setNewWantedState(nodeInfo, newWantedState, stateListener, probe);
+            setNewWantedState(nodeInfo, newWantedState, stateListener);
         }
 
         // True if the wanted state was or has just been set to newWantedState
@@ -162,7 +156,7 @@ public class SetNodeStateRequest extends Request<SetResponse> {
             // 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, probe);
+            setDistributorWantedState(cluster, nodeInfo.getNodeIndex(), newWantedState, stateListener);
         }
 
         return success;
@@ -175,8 +169,7 @@ public class SetNodeStateRequest extends Request<SetResponse> {
     private static void setDistributorWantedState(ContentCluster cluster,
                                                   int index,
                                                   NodeState newStorageWantedState,
-                                                  NodeStateOrHostInfoChangeHandler stateListener,
-                                                  boolean probe) {
+                                                  NodeStateOrHostInfoChangeHandler stateListener) {
         Node distributorNode = new Node(NodeType.DISTRIBUTOR, index);
         NodeInfo nodeInfo = cluster.getNodeInfo(distributorNode);
         if (nodeInfo == null) {
@@ -207,15 +200,13 @@ public class SetNodeStateRequest extends Request<SetResponse> {
         if (newWantedState.getState() != currentWantedState.getState() ||
                 !Objects.equals(newWantedState.getDescription(),
                         currentWantedState.getDescription())) {
-            setNewWantedState(nodeInfo, newWantedState, stateListener, probe);
+            setNewWantedState(nodeInfo, newWantedState, stateListener);
         }
     }
 
     private static void setNewWantedState(NodeInfo nodeInfo,
                                           NodeState newWantedState,
-                                          NodeStateOrHostInfoChangeHandler stateListener,
-                                          boolean probe) {
-        if (probe) return;
+                                          NodeStateOrHostInfoChangeHandler stateListener) {
         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 d7820722887..b4d189bcd55 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,7 +27,6 @@ public class SetNodeStatesForClusterRequest extends Request<SetResponse> {
     private final Map<String, UnitState> newStates;
     private final SetUnitStateRequest.Condition condition;
     private final TimeBudget timeBudget;
-    private final boolean probe;
 
 
     public SetNodeStatesForClusterRequest(Id.Cluster cluster, SetUnitStateRequest request) {
@@ -36,7 +35,6 @@ public class SetNodeStatesForClusterRequest extends Request<SetResponse> {
         this.newStates = request.getNewState();
         this.condition = request.getCondition();
         this.timeBudget = request.timeBudget();
-        this.probe = request.isProbe();
     }
 
     @Override
@@ -71,8 +69,7 @@ public class SetNodeStatesForClusterRequest extends Request<SetResponse> {
                     newStates,
                     node,
                     context.nodeStateOrHostInfoChangeHandler,
-                    context.currentConsolidatedState,
-                    probe);
+                    context.currentConsolidatedState);
 
             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 c3090a5e832..6fa7d536c67 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,6 +22,5 @@ public interface WantedStateSetter {
                     Map<String, UnitState> newStates,
                     Node node,
                     NodeStateOrHostInfoChangeHandler stateListener,
-                    ClusterState currentClusterState,
-                    boolean probe) throws StateRestApiException;
+                    ClusterState currentClusterState) 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 6cf4b7989e7..f3a4be5ac2f 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,7 +33,6 @@ 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;
 
@@ -47,7 +46,6 @@ 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);
@@ -100,11 +98,6 @@ 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 {
@@ -465,7 +458,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(), anyBoolean())).thenReturn(response);
+        when(wantedStateSetter.set(any(), any(), any(), any(), any(), any())).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 7161fb1be79..9239b8774b0 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,23 +23,20 @@ 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 {
-    private static final String REASON = "operator";
-    private ContentCluster cluster = mock(ContentCluster.class);
-    private SetUnitStateRequest.Condition condition = SetUnitStateRequest.Condition.SAFE;
-    private Map<String, UnitState> newStates = new HashMap<>();
-    private UnitState unitState = mock(UnitState.class);
+    public static final String REASON = "operator";
+    ContentCluster cluster = mock(ContentCluster.class);
+    SetUnitStateRequest.Condition condition = SetUnitStateRequest.Condition.SAFE;
+    Map<String, UnitState> newStates = new HashMap<>();
+    UnitState unitState = mock(UnitState.class);
     private final int NODE_INDEX = 2;
-    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;
+    Node storageNode = new Node(NodeType.STORAGE, NODE_INDEX);
+    NodeStateOrHostInfoChangeHandler stateListener = mock(NodeStateOrHostInfoChangeHandler.class);
+    ClusterState currentClusterState = mock(ClusterState.class);
 
     @Before
     public void setUp() {
@@ -56,16 +53,6 @@ public class SetNodeStateRequestTest {
     }
 
     @Test
-    public void testProbingDoesntChangeState() throws StateRestApiException {
-        probe = true;
-        testSetStateRequest(
-                "maintenance",
-                State.UP, State.UP,
-                NodeStateChangeChecker.Result.allowSettingOfWantedState(),
-                Optional.empty(), Optional.empty());
-    }
-
-    @Test
     public void testUpToDown() throws StateRestApiException {
         testSetStateRequest(
                 "down",
@@ -137,9 +124,6 @@ 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();
@@ -149,9 +133,6 @@ 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()) {
@@ -159,9 +140,6 @@ 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());
         }
     }
 
@@ -172,7 +150,6 @@ public class SetNodeStateRequestTest {
                 newStates,
                 storageNode,
                 stateListener,
-                currentClusterState,
-                probe);
+                currentClusterState);
     }
 }
\ No newline at end of file