From 7d578d6c49bc68cffe1672185080b51c848f4580 Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@yahooinc.com>
Date: Sun, 26 Mar 2023 20:54:08 +0200
Subject: More cleanup

---
 .../core/NodeStateChangeCheckerTest.java           | 39 ++++++++++++----------
 1 file changed, 22 insertions(+), 17 deletions(-)

(limited to 'clustercontroller-core')

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 757aced0fc0..e1e0063f01a 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
@@ -335,7 +335,7 @@ public class NodeStateChangeCheckerTest {
 
     @Test
     void testCanUpgradeStorageSafeYes() {
-        Result result = transitionToMaintenanceWithNoStorageNodesDown();
+        Result result = transitionToMaintenanceWithNoStorageNodesDown(createCluster(4), defaultAllUpClusterState());
         assertTrue(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
     }
@@ -486,8 +486,7 @@ public class NodeStateChangeCheckerTest {
         assertFalse(result.wantedStateAlreadySet());
     }
 
-    private Result transitionToMaintenanceWithOneStorageNodeDown(int storageNodeIndex) {
-        ContentCluster cluster = createCluster(4);
+    private Result transitionToMaintenanceWithOneStorageNodeDown(ContentCluster cluster, ClusterState clusterState) {
         NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster);
 
         for (int x = 0; x < cluster.clusterInfo().getConfiguredNodes().size(); x++) {
@@ -498,14 +497,6 @@ public class NodeStateChangeCheckerTest {
             cluster.clusterInfo().getStorageNodeInfo(x).setReportedState(new NodeState(STORAGE, state), 0);
         }
 
-        ClusterState clusterState = defaultAllUpClusterState();
-
-        if (storageNodeIndex >= 0) { // TODO: Move this into the calling test
-            NodeState downNodeState = new NodeState(STORAGE, DOWN);
-            cluster.clusterInfo().getStorageNodeInfo(storageNodeIndex).setReportedState(downNodeState, 4 /* time */);
-            clusterState.setNodeState(new Node(STORAGE, storageNodeIndex), downNodeState);
-        }
-
         return nodeStateChangeChecker.evaluateTransition(
                 nodeStorage, clusterState, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE);
     }
@@ -519,27 +510,35 @@ public class NodeStateChangeCheckerTest {
         }
     }
 
-    private Result transitionToMaintenanceWithNoStorageNodesDown() {
-        return transitionToMaintenanceWithOneStorageNodeDown(-1);
+    private Result transitionToMaintenanceWithNoStorageNodesDown(ContentCluster cluster, ClusterState clusterState) {
+        return transitionToMaintenanceWithOneStorageNodeDown(cluster, clusterState);
     }
 
     @Test
     void testCanUpgradeWhenAllUp() {
-        Result result = transitionToMaintenanceWithNoStorageNodesDown();
+        Result result = transitionToMaintenanceWithNoStorageNodesDown(createCluster(4), defaultAllUpClusterState());
         assertTrue(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
     }
 
     @Test
     void testCanUpgradeWhenAllUpOrRetired() {
-        Result result = transitionToMaintenanceWithNoStorageNodesDown();
+        Result result = transitionToMaintenanceWithNoStorageNodesDown(createCluster(4), defaultAllUpClusterState());
         assertTrue(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
     }
 
     @Test
     void testCanUpgradeWhenStorageIsDown() {
-        Result result = transitionToMaintenanceWithOneStorageNodeDown(nodeStorage.getIndex());
+        ClusterState clusterState = defaultAllUpClusterState();
+        var storageNodeIndex = nodeStorage.getIndex();
+
+        ContentCluster cluster = createCluster(4);
+        NodeState downNodeState = new NodeState(STORAGE, DOWN);
+        cluster.clusterInfo().getStorageNodeInfo(storageNodeIndex).setReportedState(downNodeState, 4 /* time */);
+        clusterState.setNodeState(new Node(STORAGE, storageNodeIndex), downNodeState);
+
+        Result result = transitionToMaintenanceWithOneStorageNodeDown(cluster, clusterState);
         assertTrue(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
     }
@@ -550,7 +549,13 @@ public class NodeStateChangeCheckerTest {
         // If this fails, just set otherIndex to some other valid index.
         assertNotEquals(nodeStorage.getIndex(), otherIndex);
 
-        Result result = transitionToMaintenanceWithOneStorageNodeDown(otherIndex);
+        ContentCluster cluster = createCluster(4);
+        ClusterState clusterState = defaultAllUpClusterState();
+        NodeState downNodeState = new NodeState(STORAGE, DOWN);
+        cluster.clusterInfo().getStorageNodeInfo(otherIndex).setReportedState(downNodeState, 4 /* time */);
+        clusterState.setNodeState(new Node(STORAGE, otherIndex), downNodeState);
+
+        Result result = transitionToMaintenanceWithOneStorageNodeDown(cluster, clusterState);
         assertFalse(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
         assertTrue(result.getReason().contains("Another storage node has state DOWN: 2"));
-- 
cgit v1.2.3