Also deny maintenance when another node is in maintenance

The cluster controller today already denies setting a node X safely to
maintenance M, if there is another node Y in another group that has wanted
state M.  Which means that if Y is in M but wanted state is not M, X is allowed
to be set in M.  This is an edge case which is rare.

2 files changed, 7 insertions, 4 deletions

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java
index 8901dc8ae6b..acddfa478b1 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java
@@ -226,7 +226,7 @@ public class NodeStateChangeChecker {
             return Result.allowSettingOfWantedState();
         }
 
-        Result ongoingChanges = anyNodeSetToMaintenance();
+        Result ongoingChanges = anyNodeSetToMaintenance(clusterState);
         if (!ongoingChanges.settingWantedStateIsAllowed()) {
             return ongoingChanges;
         }
@@ -281,10 +281,13 @@ public class NodeStateChangeChecker {
         return false;
     }
 
-    private Result anyNodeSetToMaintenance() {
+    private Result anyNodeSetToMaintenance(ClusterState clusterState) {
         for (NodeInfo nodeInfo : clusterInfo.getAllNodeInfo()) {
+            if (clusterState.getNodeState(nodeInfo.getNode()).getState() == State.MAINTENANCE) {
+                return Result.createDisallowed("Another node is already in maintenance:" + nodeInfo.getNodeIndex());
+            }
             if (nodeInfo.getWantedState().getState() == State.MAINTENANCE) {
-                return Result.createDisallowed("There is a node already in maintenance:" + nodeInfo.getNodeIndex());
+                return Result.createDisallowed("Another node wants maintenance:" + nodeInfo.getNodeIndex());
             }
         }
         return Result.allowSettingOfWantedState();
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 e8ded1971ac..eb3261bacaf 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
@@ -281,7 +281,7 @@ public class SetNodeStateTest extends StateRestApiTest {
                 .setNewState("user", state.toString().toLowerCase(), "whatever reason.")
                 .setCondition(SetUnitStateRequest.Condition.SAFE));
 
-        String regex = "^There is a node already in maintenance:([0-9]+)$";
+        String regex = "^Another node wants maintenance:([0-9]+)$";
         Matcher matcher = Pattern.compile(regex).matcher(setResponse.getReason());
 
         String errorMessage = "Expected reason to match '" + regex + "', but got: " + setResponse.getReason() + "'";