Disallow >1 group to suspend

If there is more than one group, disallow suspending a node if there is a node
in another group that has a user wanted state != UP.

If there is 1 group, disallow suspending more than 1 node.

5 files changed, 277 insertions, 27 deletions

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/HierarchicalGroupVisiting.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/HierarchicalGroupVisiting.java
index 1e8fb9e2ffb..0ff370fc57d 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/HierarchicalGroupVisiting.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/HierarchicalGroupVisiting.java
@@ -4,8 +4,10 @@ package com.yahoo.vespa.clustercontroller.core;
 
 import com.yahoo.vdslib.distribution.GroupVisitor;
 
-@FunctionalInterface
 public interface HierarchicalGroupVisiting {
+    /** Returns true if the group contains more than one (leaf) group. */
+    boolean isHierarchical();
+
     /**
      * Invoke the visitor for each leaf group of an implied group.  If the group is non-hierarchical
      * (flat), the visitor will not be invoked.
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/HierarchicalGroupVisitingAdapter.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/HierarchicalGroupVisitingAdapter.java
index b0d69750c77..b638604c311 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/HierarchicalGroupVisitingAdapter.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/HierarchicalGroupVisitingAdapter.java
@@ -18,18 +18,20 @@ public class HierarchicalGroupVisitingAdapter implements HierarchicalGroupVisiti
     }
 
     @Override
-    public void visit(GroupVisitor visitor) {
-        if (distribution.getRootGroup().isLeafGroup()) {
-            // A flat non-hierarchical cluster
-            return;
-        }
+    public boolean isHierarchical() {
+        return !distribution.getRootGroup().isLeafGroup();
+    }
 
-        distribution.visitGroups(group -> {
-            if (group.isLeafGroup()) {
-                return visitor.visitGroup(group);
-            }
+    @Override
+    public void visit(GroupVisitor visitor) {
+        if (isHierarchical()) {
+            distribution.visitGroups(group -> {
+                if (group.isLeafGroup()) {
+                    return visitor.visitGroup(group);
+                }
 
-            return true;
-        });
+                return true;
+            });
+        }
     }
 }
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 f28fa37c7b7..413e0bbf03f 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
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.clustercontroller.core;
 
 import com.yahoo.lang.MutableBoolean;
+import com.yahoo.lang.SettableOptional;
 import com.yahoo.vdslib.distribution.ConfiguredNode;
 import com.yahoo.vdslib.distribution.Group;
 import com.yahoo.vdslib.state.ClusterState;
@@ -212,6 +213,11 @@ public class NodeStateChangeChecker {
                     oldWantedState.getState() + ": " + oldWantedState.getDescription());
         }
 
+        Result otherGroupCheck = anotherNodeInAnotherGroupHasWantedState(nodeInfo);
+        if (!otherGroupCheck.settingWantedStateIsAllowed()) {
+            return otherGroupCheck;
+        }
+
         if (clusterState.getNodeState(nodeInfo.getNode()).getState() == State.DOWN) {
             return Result.allowSettingOfWantedState();
         }
@@ -233,6 +239,85 @@ public class NodeStateChangeChecker {
         return Result.allowSettingOfWantedState();
     }
 
+    /**
+     * Returns a disallow-result if there is another node (in another group, if hierarchical)
+     * that has a wanted state != UP.  We disallow more than 1 suspended node/group at a time.
+     */
+    private Result anotherNodeInAnotherGroupHasWantedState(StorageNodeInfo nodeInfo) {
+        if (groupVisiting.isHierarchical()) {
+            SettableOptional<Result> anotherNodeHasWantedState = new SettableOptional<>();
+
+            groupVisiting.visit(group -> {
+                if (!groupContainsNode(group, nodeInfo.getNode())) {
+                    Result result = otherNodeInGroupHasWantedState(group);
+                    if (!result.settingWantedStateIsAllowed()) {
+                        anotherNodeHasWantedState.set(result);
+                        // Have found a node that is suspended, halt the visiting
+                        return false;
+                    }
+                }
+
+                return true;
+            });
+
+            return anotherNodeHasWantedState.asOptional().orElseGet(Result::allowSettingOfWantedState);
+        } else {
+            // Return a disallow-result if there is another node with a wanted state
+            return otherNodeHasWantedState(nodeInfo);
+        }
+    }
+
+    /** Returns a disallow-result, if there is a node in the group with wanted state != UP. */
+    private Result otherNodeInGroupHasWantedState(Group group) {
+        for (var configuredNode : group.getNodes()) {
+            StorageNodeInfo storageNodeInfo = clusterInfo.getStorageNodeInfo(configuredNode.index());
+            if (storageNodeInfo == null) continue;  // needed for tests only
+            State storageNodeWantedState = storageNodeInfo
+                    .getUserWantedState().getState();
+            if (storageNodeWantedState != State.UP) {
+                return Result.createDisallowed(
+                        "At most one group can have wanted state: Other storage node " + configuredNode.index() +
+                        " in group " + group.getIndex() + " has wanted state " + storageNodeWantedState);
+            }
+
+            State distributorWantedState = clusterInfo.getDistributorNodeInfo(configuredNode.index())
+                    .getUserWantedState().getState();
+            if (distributorWantedState != State.UP) {
+                return Result.createDisallowed(
+                        "At most one group can have wanted state: Other distributor " + configuredNode.index() +
+                        " in group " + group.getIndex() + " has wanted state " + distributorWantedState);
+            }
+        }
+
+        return Result.allowSettingOfWantedState();
+    }
+
+    private Result otherNodeHasWantedState(StorageNodeInfo nodeInfo) {
+        for (var configuredNode : clusterInfo.getConfiguredNodes().values()) {
+            if (configuredNode.index() == nodeInfo.getNodeIndex()) {
+                continue;
+            }
+
+            State storageNodeWantedState = clusterInfo.getStorageNodeInfo(configuredNode.index())
+                    .getUserWantedState().getState();
+            if (storageNodeWantedState != State.UP) {
+                return Result.createDisallowed(
+                        "At most one node can have a wanted state when #groups = 1: Other storage node " +
+                                configuredNode.index() + " has wanted state " + storageNodeWantedState);
+            }
+
+            State distributorWantedState = clusterInfo.getDistributorNodeInfo(configuredNode.index())
+                    .getUserWantedState().getState();
+            if (distributorWantedState != State.UP) {
+                return Result.createDisallowed(
+                        "At most one node can have a wanted state when #groups = 1: Other distributor " +
+                                configuredNode.index() + " has wanted state " + distributorWantedState);
+            }
+        }
+
+        return Result.allowSettingOfWantedState();
+    }
+
     private boolean anotherNodeInGroupAlreadyAllowed(StorageNodeInfo nodeInfo, String newDescription) {
         MutableBoolean alreadyAllowed = new MutableBoolean(false);
 
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java
index 5e3dbbe713b..0c67a96cba2 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.clustercontroller.core;
 import com.yahoo.vdslib.distribution.ConfiguredNode;
 import com.yahoo.vdslib.distribution.Distribution;
 import com.yahoo.vdslib.distribution.Group;
+import com.yahoo.vdslib.distribution.GroupVisitor;
 import com.yahoo.vdslib.state.ClusterState;
 import com.yahoo.vdslib.state.Node;
 import com.yahoo.vdslib.state.NodeState;
@@ -11,6 +12,7 @@ import com.yahoo.vdslib.state.NodeType;
 import com.yahoo.vdslib.state.State;
 import com.yahoo.vespa.clustercontroller.core.hostinfo.HostInfo;
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest;
+import com.yahoo.vespa.config.content.StorDistributionConfig;
 import org.junit.Test;
 
 import java.text.ParseException;
@@ -40,6 +42,11 @@ public class NodeStateChangeCheckerTest {
     private static final NodeState MAINTENANCE_NODE_STATE = createNodeState(State.MAINTENANCE, "Orchestrator");
     private static final NodeState DOWN_NODE_STATE = createNodeState(State.DOWN, "RetireEarlyExpirer");
 
+    private static final HierarchicalGroupVisiting noopVisiting = new HierarchicalGroupVisiting() {
+        @Override public boolean isHierarchical() { return false; }
+        @Override public void visit(GroupVisitor visitor) { }
+    };
+
     private static NodeState createNodeState(State state, String description) {
         return new NodeState(NodeType.STORAGE, state).setDescription(description);
     }
@@ -57,12 +64,12 @@ public class NodeStateChangeCheckerTest {
     }
 
     private NodeStateChangeChecker createChangeChecker(ContentCluster cluster) {
-        return new NodeStateChangeChecker(requiredRedundancy, visitor -> {}, cluster.clusterInfo(), false);
+        return new NodeStateChangeChecker(requiredRedundancy, noopVisiting, cluster.clusterInfo(), false);
     }
 
     private ContentCluster createCluster(Collection<ConfiguredNode> nodes) {
         Distribution distribution = mock(Distribution.class);
-        Group group = new Group(2, "to");
+        Group group = new Group(2, "two");
         when(distribution.getRootGroup()).thenReturn(group);
         return new ContentCluster("Clustername", nodes, distribution);
     }
@@ -117,7 +124,7 @@ public class NodeStateChangeCheckerTest {
     public void testDeniedInMoratorium() {
         ContentCluster cluster = createCluster(createNodes(4));
         NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker(
-                requiredRedundancy, visitor -> {}, cluster.clusterInfo(), true);
+                requiredRedundancy, noopVisiting, cluster.clusterInfo(), true);
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 new Node(NodeType.STORAGE, 10), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE,
                 UP_NODE_STATE, MAINTENANCE_NODE_STATE);
@@ -130,7 +137,7 @@ public class NodeStateChangeCheckerTest {
     public void testUnknownStorageNode() {
         ContentCluster cluster = createCluster(createNodes(4));
         NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker(
-                requiredRedundancy, visitor -> {}, cluster.clusterInfo(), false);
+                requiredRedundancy, noopVisiting, cluster.clusterInfo(), false);
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 new Node(NodeType.STORAGE, 10), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE,
                 UP_NODE_STATE, MAINTENANCE_NODE_STATE);
@@ -140,6 +147,158 @@ public class NodeStateChangeCheckerTest {
     }
 
     @Test
+    public void testWhenOtherStorageNodeIsSuspended() {
+        // Nodes 0-3, storage node 0 being in maintenance with "Orchestrator" description.
+        ContentCluster cluster = createCluster(createNodes(4));
+        cluster.clusterInfo().getStorageNodeInfo(0).setWantedState(new NodeState(NodeType.STORAGE, State.MAINTENANCE).setDescription("Orchestrator"));
+        NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker(
+                requiredRedundancy, noopVisiting, cluster.clusterInfo(), false);
+        ClusterState clusterStateWith0InMaintenance = clusterState(String.format(
+                "version:%d distributor:4 storage:4 .0.s:m",
+                currentClusterStateVersion));
+
+        NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
+                new Node(NodeType.STORAGE, 1), clusterStateWith0InMaintenance,
+                SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE);
+        assertFalse(result.settingWantedStateIsAllowed());
+        assertFalse(result.wantedStateAlreadySet());
+        assertThat(result.getReason(), is("At most one node can have a wanted state when #groups = 1: " +
+                "Other storage node 0 has wanted state Maintenance"));
+    }
+
+    @Test
+    public void testWhenOtherDistributorIsDown() {
+        // Nodes 0-3, storage node 0 being in maintenance with "Orchestrator" description.
+        ContentCluster cluster = createCluster(createNodes(4));
+        cluster.clusterInfo().getDistributorNodeInfo(0)
+                .setWantedState(new NodeState(NodeType.DISTRIBUTOR, State.DOWN).setDescription("Orchestrator"));
+        NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker(
+                requiredRedundancy, noopVisiting, cluster.clusterInfo(), false);
+        ClusterState clusterStateWith0InMaintenance = clusterState(String.format(
+                "version:%d distributor:4 .0.s:d storage:4",
+                currentClusterStateVersion));
+
+        NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
+                new Node(NodeType.STORAGE, 1), clusterStateWith0InMaintenance,
+                SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE);
+        assertFalse(result.settingWantedStateIsAllowed());
+        assertFalse(result.wantedStateAlreadySet());
+        assertThat(result.getReason(), is("At most one node can have a wanted state when #groups = 1: " +
+                "Other distributor 0 has wanted state Down"));
+    }
+
+    @Test
+    public void testWhenOtherDistributorInOtherGroupIsDown() {
+        // Nodes 0-3, distributor 0 being in maintenance with "Orchestrator" description.
+        // 2 groups: nodes 0-1 is group 0, 2-3 is group 1.
+        ContentCluster cluster = createCluster(createNodes(4));
+        cluster.clusterInfo().getDistributorNodeInfo(0)
+                .setWantedState(new NodeState(NodeType.STORAGE, State.DOWN).setDescription("Orchestrator"));
+        HierarchicalGroupVisiting visiting = makeHierarchicalGroupVisitingWith2Groups(4);
+        NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker(
+                requiredRedundancy, visiting, cluster.clusterInfo(), false);
+        ClusterState clusterStateWith0InMaintenance = clusterState(String.format(
+                "version:%d distributor:4 .0.s:d storage:4",
+                currentClusterStateVersion));
+
+        {
+            // Denied for node 2 in group 1, since distributor 0 in group 0 is down
+            NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
+                    new Node(NodeType.STORAGE, 2), clusterStateWith0InMaintenance,
+                    SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE);
+            assertFalse(result.settingWantedStateIsAllowed());
+            assertFalse(result.wantedStateAlreadySet());
+            assertThat(result.getReason(), is("At most one group can have wanted state: " +
+                    "Other distributor 0 in group 0 has wanted state Down"));
+        }
+
+        {
+            // Even node 1 of group 0 is not permitted, as node 0 is not considered
+            // suspended since only the distributor has been set down.
+            NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
+                    new Node(NodeType.STORAGE, 1), clusterStateWith0InMaintenance,
+                    SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE);
+            assertFalse(result.getReason(), result.settingWantedStateIsAllowed());
+            assertEquals("Another distributor wants state DOWN: 0", result.getReason());
+        }
+    }
+
+    @Test
+    public void testWhenOtherStorageNodeInOtherGroupIsSuspended() {
+        // Nodes 0-3, storage node 0 being in maintenance with "Orchestrator" description.
+        // 2 groups: nodes 0-1 is group 0, 2-3 is group 1.
+        ContentCluster cluster = createCluster(createNodes(4));
+        cluster.clusterInfo().getStorageNodeInfo(0).setWantedState(new NodeState(NodeType.STORAGE, State.MAINTENANCE).setDescription("Orchestrator"));
+        HierarchicalGroupVisiting visiting = makeHierarchicalGroupVisitingWith2Groups(4);
+        NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker(
+                requiredRedundancy, visiting, cluster.clusterInfo(), false);
+        ClusterState clusterStateWith0InMaintenance = clusterState(String.format(
+                "version:%d distributor:4 storage:4 .0.s:m",
+                currentClusterStateVersion));
+
+        {
+            // Denied for node 2 in group 1, since node 0 in group 0 is in maintenance
+            NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
+                    new Node(NodeType.STORAGE, 2), clusterStateWith0InMaintenance,
+                    SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE);
+            assertFalse(result.settingWantedStateIsAllowed());
+            assertFalse(result.wantedStateAlreadySet());
+            assertThat(result.getReason(), is("At most one group can have wanted state: " +
+                    "Other storage node 0 in group 0 has wanted state Maintenance"));
+        }
+
+        {
+            // Permitted for node 1 in group 0, since node 0 is already in maintenance with
+            // description Orchestrator, and it is in the same group
+            NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
+                    new Node(NodeType.STORAGE, 1), clusterStateWith0InMaintenance,
+                    SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE);
+            assertTrue(result.getReason(), result.settingWantedStateIsAllowed());
+            assertFalse(result.wantedStateAlreadySet());
+        }
+    }
+
+    /**
+     * Make a HierarchicalGroupVisiting with the given number of nodes, with 2 groups:
+     * Group "0" is nodes 0-1, group "1" is 2-3.
+     */
+    private HierarchicalGroupVisiting makeHierarchicalGroupVisitingWith2Groups(int nodes) {
+        int groups = 2;
+        if (nodes % groups != 0) {
+            throw new IllegalArgumentException("Cannot have 2 groups with an odd number of nodes: " + nodes);
+        }
+        int nodesPerGroup = nodes / groups;
+
+        var configBuilder = new StorDistributionConfig.Builder()
+                .active_per_leaf_group(true)
+                .ready_copies(2)
+                .redundancy(2)
+                .initial_redundancy(2);
+
+        configBuilder.group(new StorDistributionConfig.Group.Builder()
+                .index("invalid")
+                .name("invalid")
+                .capacity(nodes)
+                .partitions("1|*"));
+
+        int nodeIndex = 0;
+        for (int i = 0; i < groups; ++i) {
+            var groupBuilder = new StorDistributionConfig.Group.Builder()
+                    .index(String.valueOf(i))
+                    .name(String.valueOf(i))
+                    .capacity(nodesPerGroup)
+                    .partitions("");
+            for (int j = 0; j < nodesPerGroup; ++j, ++nodeIndex) {
+                groupBuilder.nodes(new StorDistributionConfig.Group.Nodes.Builder()
+                        .index(nodeIndex));
+            }
+            configBuilder.group(groupBuilder);
+        }
+
+        return new HierarchicalGroupVisitingAdapter(new Distribution(configBuilder.build()));
+    }
+
+    @Test
     public void testSafeSetStateDistributors() {
         NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(createCluster(createNodes(1)));
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
@@ -162,7 +321,7 @@ public class NodeStateChangeCheckerTest {
 
         // We should then be denied setting storage node 1 safely to maintenance.
         NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker(
-                requiredRedundancy, visitor -> {}, cluster.clusterInfo(), false);
+                requiredRedundancy, noopVisiting, cluster.clusterInfo(), false);
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 nodeStorage, clusterStateWith3Down, SetUnitStateRequest.Condition.SAFE,
                 UP_NODE_STATE, MAINTENANCE_NODE_STATE);
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 2ba19d4f5be..83dbdbe31c8 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
@@ -223,28 +223,28 @@ public class SetNodeStateTest extends StateRestApiTest {
         assertSetUnitState(1, State.MAINTENANCE, null);  // sanity-check
 
         // Because 2 is in a different group maintenance should be denied
-        assertSetUnitStateCausesAlreadyInWantedMaintenance(2, State.MAINTENANCE);
+        assertSetUnitStateCausesAtMostOneGroupError(2, State.MAINTENANCE);
 
         // Because 3 and 5 are in the same group as 1, these should be OK
         assertSetUnitState(3, State.MAINTENANCE, null);
         assertUnitState(1, "user", State.MAINTENANCE, "whatever reason.");  // sanity-check
         assertUnitState(3, "user", State.MAINTENANCE, "whatever reason.");  // sanity-check
         assertSetUnitState(5, State.MAINTENANCE, null);
-        assertSetUnitStateCausesAlreadyInWantedMaintenance(2, State.MAINTENANCE);  // sanity-check
+        assertSetUnitStateCausesAtMostOneGroupError(2, State.MAINTENANCE);  // sanity-check
 
         // Set all to up
         assertSetUnitState(1, State.UP, null);
         assertSetUnitState(1, State.UP, null); // sanity-check
         assertSetUnitState(3, State.UP, null);
-        assertSetUnitStateCausesAlreadyInWantedMaintenance(2, State.MAINTENANCE);  // sanity-check
+        assertSetUnitStateCausesAtMostOneGroupError(2, State.MAINTENANCE);  // sanity-check
         assertSetUnitState(5, State.UP, null);
 
         // Now we should be allowed to upgrade second group, while the first group will be denied
         assertSetUnitState(2, State.MAINTENANCE, null);
-        assertSetUnitStateCausesAlreadyInWantedMaintenance(1, State.MAINTENANCE);  // sanity-check
+        assertSetUnitStateCausesAtMostOneGroupError(1, State.MAINTENANCE);  // sanity-check
         assertSetUnitState(0, State.MAINTENANCE, null);
         assertSetUnitState(4, State.MAINTENANCE, null);
-        assertSetUnitStateCausesAlreadyInWantedMaintenance(1, State.MAINTENANCE);  // sanity-check
+        assertSetUnitStateCausesAtMostOneGroupError(1, State.MAINTENANCE);  // sanity-check
 
         // And set second group up again
         assertSetUnitState(0, State.MAINTENANCE, null);
@@ -264,14 +264,14 @@ public class SetNodeStateTest extends StateRestApiTest {
         assertSetUnitState(1, State.MAINTENANCE, null);  // sanity-check
 
         // Because 2 is in a different group maintenance should be denied
-        assertSetUnitStateCausesAlreadyInWantedMaintenance(2, State.MAINTENANCE);
+        assertSetUnitStateCausesAtMostOneGroupError(2, State.MAINTENANCE);
 
         // Because 3 and 5 are in the same group as 1, these should be OK
         assertSetUnitState(3, State.MAINTENANCE, null);
         assertUnitState(1, "user", State.MAINTENANCE, "whatever reason.");  // sanity-check
         assertUnitState(3, "user", State.MAINTENANCE, "whatever reason.");  // sanity-check
         assertSetUnitState(5, State.MAINTENANCE, null);
-        assertSetUnitStateCausesAlreadyInWantedMaintenance(2, State.MAINTENANCE);  // sanity-check
+        assertSetUnitStateCausesAtMostOneGroupError(2, State.MAINTENANCE);  // sanity-check
 
         // Set all to up
         assertSetUnitState(1, State.UP, null);
@@ -306,12 +306,14 @@ public class SetNodeStateTest extends StateRestApiTest {
         }
     }
 
-    private void assertSetUnitStateCausesAlreadyInWantedMaintenance(int index, State state) throws StateRestApiException {
-        assertSetUnitStateFails(index, state, "^Another storage node wants state MAINTENANCE: ([0-9]+)$");
+    private void assertSetUnitStateCausesAtMostOneGroupError(int index, State state) throws StateRestApiException {
+        assertSetUnitStateFails(index, state, "^At most one group can have wanted state: " +
+                "Other storage node ([0-9]+) in group ([0-9]+) has wanted state Maintenance$");
     }
 
     private void assertSetUnitStateCausesAnotherNodeHasStateError(int index, State state) throws StateRestApiException {
-        assertSetUnitStateFails(index, state, "^Another storage node has state DOWN: ([0-9]+)$");
+        assertSetUnitStateFails(index, state, "^At most one group can have wanted state: " +
+                "Other storage node ([0-9]+) in group ([0-9]+) has wanted state Maintenance$");
     }
 
     private void assertSetUnitStateFails(int index, State state, String reasonRegex)