diff options
author | Harald Musum <musum@yahooinc.com> | 2023-07-09 19:51:49 +0200 |
---|---|---|
committer | Harald Musum <musum@yahooinc.com> | 2023-07-09 19:51:49 +0200 |
commit | 181513a91bdee2661450248a7a508f8b3230d8db (patch) | |
tree | 0e9dbdd00e2408856e1eedee6a8855c5bb3229a3 /clustercontroller-core | |
parent | c6ef6d07bb72038d85cb43a89187545e9757c5de (diff) |
Renames and minor refactorings, no funcational changes
Diffstat (limited to 'clustercontroller-core')
5 files changed, 226 insertions, 258 deletions
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java index 2e8e2707166..19c4e4b1e89 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java @@ -17,6 +17,7 @@ import java.util.Objects; import java.util.TreeMap; import static com.yahoo.vdslib.state.NodeState.ORCHESTRATOR_RESERVED_DESCRIPTION; +import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result; public class ContentCluster { @@ -129,7 +130,7 @@ public class ContentCluster { * @param newState state wanted to be set * @param inMoratorium whether the CC is in moratorium */ - public NodeStateChangeChecker.Result calculateEffectOfNewState( + public Result calculateEffectOfNewState( Node node, ClusterState clusterState, SetUnitStateRequest.Condition condition, NodeState oldState, NodeState newState, boolean inMoratorium) { 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 50b3f6fb7f9..f1ddf8d6366 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 @@ -11,7 +11,6 @@ import com.yahoo.vdslib.state.Node; import com.yahoo.vdslib.state.NodeState; import com.yahoo.vdslib.state.State; import com.yahoo.vespa.clustercontroller.core.hostinfo.HostInfo; -import com.yahoo.vespa.clustercontroller.core.hostinfo.Metrics; import com.yahoo.vespa.clustercontroller.core.hostinfo.StorageNode; import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest; import java.util.ArrayList; @@ -30,9 +29,9 @@ import static com.yahoo.vdslib.state.State.DOWN; import static com.yahoo.vdslib.state.State.MAINTENANCE; import static com.yahoo.vdslib.state.State.RETIRED; import static com.yahoo.vdslib.state.State.UP; -import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result.allowSettingOfWantedState; -import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result.createAlreadySet; -import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result.createDisallowed; +import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result.allow; +import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result.alreadySet; +import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result.disallow; import static com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest.Condition.FORCE; import static com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest.Condition.SAFE; import static java.util.logging.Level.FINE; @@ -67,7 +66,7 @@ public class NodeStateChangeChecker { public static class Result { public enum Action { - MUST_SET_WANTED_STATE, + ALLOWED, ALREADY_SET, DISALLOWED } @@ -80,27 +79,27 @@ public class NodeStateChangeChecker { this.reason = reason; } - public static Result createDisallowed(String reason) { + public static Result disallow(String reason) { return new Result(Action.DISALLOWED, reason); } - public static Result allowSettingOfWantedState() { - return new Result(Action.MUST_SET_WANTED_STATE, "Preconditions fulfilled and new state different"); + public static Result allow() { + return new Result(Action.ALLOWED, "Preconditions fulfilled and new state different"); } - public static Result createAlreadySet() { + public static Result alreadySet() { return new Result(Action.ALREADY_SET, "Basic preconditions fulfilled and new state is already effective"); } - public boolean settingWantedStateIsAllowed() { return action == Action.MUST_SET_WANTED_STATE; } + public boolean allowed() { return action == Action.ALLOWED; } - public boolean settingWantedStateIsNotAllowed() { return ! settingWantedStateIsAllowed(); } + public boolean notAllowed() { return ! allowed(); } - public boolean wantedStateAlreadySet() { + public boolean isAlreadySet() { return action == Action.ALREADY_SET; } - public String getReason() { + public String reason() { return reason; } @@ -111,106 +110,88 @@ public class NodeStateChangeChecker { public Result evaluateTransition(Node node, ClusterState clusterState, SetUnitStateRequest.Condition condition, NodeState oldWantedState, NodeState newWantedState) { - if (condition == FORCE) { - return allowSettingOfWantedState(); - } + if (condition == FORCE) + return allow(); - if (inMoratorium) { - return createDisallowed("Master cluster controller is bootstrapping and in moratorium"); - } + if (inMoratorium) + return disallow("Master cluster controller is bootstrapping and in moratorium"); - if (condition != SAFE) { - return createDisallowed("Condition not implemented: " + condition.name()); - } + if (condition != SAFE) + return disallow("Condition not implemented: " + condition.name()); - if (node.getType() != STORAGE) { - return createDisallowed("Safe-set of node state is only supported for storage nodes! " + + if (node.getType() != STORAGE) + return disallow("Safe-set of node state is only supported for storage nodes! " + "Requested node type: " + node.getType().toString()); - } StorageNodeInfo nodeInfo = clusterInfo.getStorageNodeInfo(node.getIndex()); - if (nodeInfo == null) { - return createDisallowed("Unknown node " + node); - } + if (nodeInfo == null) + return disallow("Unknown node " + node); // If the new state and description equals the existing, we're done. This is done for 2 cases: // - We can short-circuit setting of a new wanted state, which e.g. hits ZooKeeper. // - We ensure that clients that have previously set the wanted state, continue - // to see the same conclusion, even though they possibly would have been denied - // MUST_SET_WANTED_STATE if re-evaluated. This is important for implementing idempotent clients. + // to see the same conclusion, even though they possibly would have been + // DISALLOWED if re-evaluated. This is important for implementing idempotent clients. if (newWantedState.getState().equals(oldWantedState.getState()) && Objects.equals(newWantedState.getDescription(), oldWantedState.getDescription())) { - return createAlreadySet(); + return alreadySet(); } return switch (newWantedState.getState()) { case UP -> canSetStateUp(nodeInfo, oldWantedState); case MAINTENANCE -> canSetStateMaintenanceTemporarily(nodeInfo, clusterState, newWantedState.getDescription()); case DOWN -> canSetStateDownPermanently(nodeInfo, clusterState, newWantedState.getDescription()); - default -> createDisallowed("Destination node state unsupported in safe mode: " + newWantedState); + default -> disallow("Destination node state unsupported in safe mode: " + newWantedState); }; } private Result canSetStateDownPermanently(NodeInfo nodeInfo, ClusterState clusterState, String newDescription) { NodeState oldWantedState = nodeInfo.getUserWantedState(); - if (oldWantedState.getState() != UP && !oldWantedState.getDescription().equals(newDescription)) { + if (oldWantedState.getState() != UP && !oldWantedState.getDescription().equals(newDescription)) // Refuse to override whatever an operator or unknown entity is doing. // // Note: The new state&description is NOT equal to the old state&description: // that would have been short-circuited prior to this. - return createDisallowed("A conflicting wanted state is already set: " + - oldWantedState.getState() + ": " + oldWantedState.getDescription()); - } + return disallow("A conflicting wanted state is already set: " + + oldWantedState + ": " + oldWantedState.getDescription()); State reportedState = nodeInfo.getReportedState().getState(); - if (reportedState != UP) { - return createDisallowed("Reported state (" + reportedState - + ") is not UP, so no bucket data is available"); - } + if (reportedState != UP) + return disallow("Reported state (" + reportedState + ") is not UP, so no bucket data is available"); State currentState = clusterState.getNodeState(nodeInfo.getNode()).getState(); - if (currentState != RETIRED) { - return createDisallowed("Only retired nodes are allowed to be set to DOWN in safe mode - is " - + currentState); - } + if (currentState != RETIRED) + return disallow("Only retired nodes are allowed to be set to DOWN in safe mode - is " + currentState); HostInfo hostInfo = nodeInfo.getHostInfo(); Integer hostInfoNodeVersion = hostInfo.getClusterStateVersionOrNull(); int clusterControllerVersion = clusterState.getVersion(); - if (hostInfoNodeVersion == null || hostInfoNodeVersion != clusterControllerVersion) { - return createDisallowed("Cluster controller at version " + clusterControllerVersion - + " got info for storage node " + nodeInfo.getNodeIndex() + " at a different version " - + hostInfoNodeVersion); - } + int nodeIndex = nodeInfo.getNodeIndex(); + if (hostInfoNodeVersion == null || hostInfoNodeVersion != clusterControllerVersion) + return disallow("Cluster controller at version " + clusterControllerVersion + + " got info for storage node " + nodeIndex + " at a different version " + + hostInfoNodeVersion); - Optional<Metrics.Value> bucketsMetric; - bucketsMetric = hostInfo.getMetrics().getValueAt(BUCKETS_METRIC_NAME, BUCKETS_METRIC_DIMENSIONS); - if (bucketsMetric.isEmpty() || bucketsMetric.get().getLast() == null) { - return createDisallowed("Missing last value of the " + BUCKETS_METRIC_NAME + - " metric for storage node " + nodeInfo.getNodeIndex()); - } + var bucketsMetric = hostInfo.getMetrics().getValueAt(BUCKETS_METRIC_NAME, BUCKETS_METRIC_DIMENSIONS); + if (bucketsMetric.isEmpty() || bucketsMetric.get().getLast() == null) + return disallow("Missing last value of the " + BUCKETS_METRIC_NAME + " metric for storage node " + nodeIndex); long lastBuckets = bucketsMetric.get().getLast(); - if (lastBuckets > 0) { - return createDisallowed("The storage node manages " + lastBuckets + " buckets"); - } + if (lastBuckets > 0) + return disallow("The storage node manages " + lastBuckets + " buckets"); - return allowSettingOfWantedState(); + return allow(); } private Result canSetStateUp(NodeInfo nodeInfo, NodeState oldWantedState) { - if (oldWantedState.getState() == UP) { - // The description is not significant when wanting to set the state to UP - return createAlreadySet(); - } + if (oldWantedState.getState() == UP) + return alreadySet(); // The description is not significant when wanting to set the state to UP - if (nodeInfo.getReportedState().getState() != UP) { - return createDisallowed("Refuse to set wanted state to UP, " + - "since the reported state is not UP (" + - nodeInfo.getReportedState().getState() + ")"); - } + State reportedState = nodeInfo.getReportedState().getState(); + if (reportedState != UP) + return disallow("Refuse to set wanted state to UP, since the reported state is not UP (" + reportedState + ")"); - return allowSettingOfWantedState(); + return allow(); } private Result canSetStateMaintenanceTemporarily(StorageNodeInfo nodeInfo, ClusterState clusterState, @@ -222,18 +203,16 @@ public class NodeStateChangeChecker { // // Note: The new state&description is NOT equal to the old state&description: // that would have been short-circuited prior to this. - return createDisallowed("A conflicting wanted state is already set: " + - oldWantedState.getState() + ": " + oldWantedState.getDescription()); + return disallow("A conflicting wanted state is already set: " + + oldWantedState.getState() + ": " + oldWantedState.getDescription()); } if (maxNumberOfGroupsAllowedToBeDown == -1) { - var otherGroupCheck = anotherNodeInAnotherGroupHasWantedState(nodeInfo); - if (otherGroupCheck.settingWantedStateIsNotAllowed()) { - return otherGroupCheck; - } - if (anotherNodeInGroupAlreadyAllowed(nodeInfo, newDescription)) { - return allowSettingOfWantedState(); - } + var result = anotherNodeInAnotherGroupHasWantedState(nodeInfo); + if (result.notAllowed()) + return result; + if (anotherNodeInGroupAlreadyAllowed(nodeInfo, newDescription)) + return allow(); } else { var result = otherNodesHaveWantedState(nodeInfo, newDescription, clusterState); if (result.isPresent()) @@ -242,22 +221,22 @@ public class NodeStateChangeChecker { if (clusterState.getNodeState(nodeInfo.getNode()).getState() == DOWN) { log.log(FINE, "node is DOWN, allow"); - return allowSettingOfWantedState(); + return allow(); } - Result allNodesAreUpCheck = nodesAreUpOrRetired(clusterState); - if (allNodesAreUpCheck.settingWantedStateIsNotAllowed()) { - log.log(FINE, "allNodesAreUpCheck: " + allNodesAreUpCheck); - return allNodesAreUpCheck; + var result = nodesAreUpOrRetired(clusterState); + if (result.notAllowed()) { + log.log(FINE, "nodesAreUpOrRetired: " + result); + return result; } - Result checkDistributorsResult = checkDistributors(nodeInfo.getNode(), clusterState.getVersion()); - if (checkDistributorsResult.settingWantedStateIsNotAllowed()) { - log.log(FINE, "checkDistributors: "+ checkDistributorsResult); - return checkDistributorsResult; + result = checkDistributors(nodeInfo.getNode(), clusterState.getVersion()); + if (result.notAllowed()) { + log.log(FINE, "checkDistributors: "+ result); + return result; } - return allowSettingOfWantedState(); + return allow(); } /** @@ -271,7 +250,7 @@ public class NodeStateChangeChecker { groupVisiting.visit(group -> { if (!groupContainsNode(group, nodeInfo.getNode())) { Result result = otherNodeInGroupHasWantedState(group); - if (result.settingWantedStateIsNotAllowed()) { + if (result.notAllowed()) { anotherNodeHasWantedState.set(result); // Have found a node that is suspended, halt the visiting return false; @@ -281,7 +260,7 @@ public class NodeStateChangeChecker { return true; }); - return anotherNodeHasWantedState.asOptional().orElseGet(Result::allowSettingOfWantedState); + return anotherNodeHasWantedState.asOptional().orElseGet(Result::allow); } else { // Returns a disallow-result if there is another node with a wanted state return otherNodeHasWantedState(nodeInfo); @@ -309,12 +288,12 @@ public class NodeStateChangeChecker { Set<Integer> groupsWithSameStateAndDescription = groupsWithSameStateAndDescription(MAINTENANCE, newDescription); if (aGroupContainsNode(groupsWithSameStateAndDescription, node)) { log.log(FINE, "Node is in group with same state and description, allow"); - return Optional.of(allowSettingOfWantedState()); + return Optional.of(allow()); } // There are groups with nodes not up, but with another description, probably operator set if (groupsWithSameStateAndDescription.size() == 0) { - return Optional.of(createDisallowed("Wanted state already set for another node in groups: " + - sortSetIntoList(groupsWithNodesWantedStateNotUp))); + return Optional.of(disallow("Wanted state already set for another node in groups: " + + sortSetIntoList(groupsWithNodesWantedStateNotUp))); } Set<Integer> retiredAndNotUpGroups = groupsWithNotRetiredAndNotUp(clusterState); @@ -326,16 +305,16 @@ public class NodeStateChangeChecker { } if (numberOfGroupsToConsider < maxNumberOfGroupsAllowedToBeDown) { log.log(FINE, "Allow, retiredAndNotUpGroups=" + retiredAndNotUpGroups); - return Optional.of(allowSettingOfWantedState()); + return Optional.of(allow()); } - return Optional.of(createDisallowed(String.format("At most %d groups can have wanted state: %s", - maxNumberOfGroupsAllowedToBeDown, - sortSetIntoList(retiredAndNotUpGroups)))); + return Optional.of(disallow(String.format("At most %d groups can have wanted state: %s", + maxNumberOfGroupsAllowedToBeDown, + sortSetIntoList(retiredAndNotUpGroups)))); } else { // Return a disallow-result if there is another node with a wanted state var otherNodeHasWantedState = otherNodeHasWantedState(nodeInfo); - if (otherNodeHasWantedState.settingWantedStateIsNotAllowed()) + if (otherNodeHasWantedState.notAllowed()) return Optional.of(otherNodeHasWantedState); } return Optional.empty(); @@ -354,55 +333,46 @@ public class NodeStateChangeChecker { StorageNodeInfo storageNodeInfo = clusterInfo.getStorageNodeInfo(index); if (storageNodeInfo == null) continue; // needed for tests only State storageNodeWantedState = storageNodeInfo.getUserWantedState().getState(); - if (storageNodeWantedState != UP) { - return createDisallowed( - "At most one group can have wanted state: Other storage node " + index + - " in group " + group.getIndex() + " has wanted state " + storageNodeWantedState); - } + if (storageNodeWantedState != UP) + return disallow("At most one group can have wanted state: Other storage node " + index + + " in group " + group.getIndex() + " has wanted state " + storageNodeWantedState); State distributorWantedState = clusterInfo.getDistributorNodeInfo(index).getUserWantedState().getState(); - if (distributorWantedState != UP) { - return createDisallowed( - "At most one group can have wanted state: Other distributor " + index + - " in group " + group.getIndex() + " has wanted state " + distributorWantedState); - } + if (distributorWantedState != UP) + return disallow("At most one group can have wanted state: Other distributor " + index + + " in group " + group.getIndex() + " has wanted state " + distributorWantedState); } - return allowSettingOfWantedState(); + return allow(); } private Result otherNodeHasWantedState(StorageNodeInfo nodeInfo) { for (var configuredNode : clusterInfo.getConfiguredNodes().values()) { int index = configuredNode.index(); - if (index == nodeInfo.getNodeIndex()) { - continue; - } + if (index == nodeInfo.getNodeIndex()) continue; State storageNodeWantedState = clusterInfo.getStorageNodeInfo(index).getUserWantedState().getState(); if (storageNodeWantedState != UP) { - return createDisallowed( - "At most one node can have a wanted state when #groups = 1: Other storage node " + + return disallow("At most one node can have a wanted state when #groups = 1: Other storage node " + index + " has wanted state " + storageNodeWantedState); } State distributorWantedState = clusterInfo.getDistributorNodeInfo(index).getUserWantedState().getState(); if (distributorWantedState != UP) { - return createDisallowed( - "At most one node can have a wanted state when #groups = 1: Other distributor " + + return disallow("At most one node can have a wanted state when #groups = 1: Other distributor " + index + " has wanted state " + distributorWantedState); } } - return allowSettingOfWantedState(); + return allow(); } private boolean anotherNodeInGroupAlreadyAllowed(StorageNodeInfo nodeInfo, String newDescription) { MutableBoolean alreadyAllowed = new MutableBoolean(false); groupVisiting.visit(group -> { - if (!groupContainsNode(group, nodeInfo.getNode())) { + if (!groupContainsNode(group, nodeInfo.getNode())) return true; - } alreadyAllowed.set(anotherNodeInGroupAlreadyAllowed(group, nodeInfo.getNode(), newDescription)); @@ -425,9 +395,8 @@ public class NodeStateChangeChecker { private static boolean groupContainsNode(Group group, Node node) { for (ConfiguredNode configuredNode : group.getNodes()) { - if (configuredNode.index() == node.getIndex()) { + if (configuredNode.index() == node.getIndex()) return true; - } } return false; @@ -454,16 +423,16 @@ public class NodeStateChangeChecker { for (NodeInfo nodeInfo : clusterInfo.getAllNodeInfos()) { State wantedState = nodeInfo.getUserWantedState().getState(); if (wantedState != UP && wantedState != RETIRED) - return createDisallowed("Another " + nodeInfo.type() + " wants state " + - wantedState.toString().toUpperCase() + ": " + nodeInfo.getNodeIndex()); + return disallow("Another " + nodeInfo.type() + " wants state " + + wantedState.toString().toUpperCase() + ": " + nodeInfo.getNodeIndex()); State state = clusterState.getNodeState(nodeInfo.getNode()).getState(); if (state != UP && state != RETIRED) - return createDisallowed("Another " + nodeInfo.type() + " has state " + - state.toString().toUpperCase() + ": " + nodeInfo.getNodeIndex()); + return disallow("Another " + nodeInfo.type() + " has state " + + state.toString().toUpperCase() + ": " + nodeInfo.getNodeIndex()); } - return allowSettingOfWantedState(); + return allow(); } private Result checkStorageNodesForDistributor(DistributorNodeInfo distributorNodeInfo, Node node) { @@ -473,19 +442,18 @@ public class NodeStateChangeChecker { Integer minReplication = storageNode.getMinCurrentReplicationFactorOrNull(); // Why test on != null? Missing min-replication is OK (indicate empty/few buckets on system). if (minReplication != null && minReplication < requiredRedundancy) { - return createDisallowed("Distributor " - + distributorNodeInfo.getNodeIndex() + return disallow("Distributor " + distributorNodeInfo.getNodeIndex() + " says storage node " + node.getIndex() + " has buckets with redundancy as low as " + storageNode.getMinCurrentReplicationFactorOrNull() + ", but we require at least " + requiredRedundancy); } else { - return allowSettingOfWantedState(); + return allow(); } } } - return allowSettingOfWantedState(); + return allow(); } /** @@ -494,28 +462,28 @@ public class NodeStateChangeChecker { * @param clusterStateVersion the cluster state we expect distributors to have */ private Result checkDistributors(Node node, int clusterStateVersion) { - if (clusterInfo.getDistributorNodeInfos().isEmpty()) { - return createDisallowed("Not aware of any distributors, probably not safe to upgrade?"); - } + if (clusterInfo.getDistributorNodeInfos().isEmpty()) + return disallow("Not aware of any distributors, probably not safe to upgrade?"); + for (DistributorNodeInfo distributorNodeInfo : clusterInfo.getDistributorNodeInfos()) { Integer distributorClusterStateVersion = distributorNodeInfo.getHostInfo().getClusterStateVersionOrNull(); - if (distributorClusterStateVersion == null) { - return createDisallowed("Distributor node " + distributorNodeInfo.getNodeIndex() - + " has not reported any cluster state version yet."); - } else if (distributorClusterStateVersion != clusterStateVersion) { - return createDisallowed("Distributor node " + distributorNodeInfo.getNodeIndex() - + " does not report same version (" - + distributorNodeInfo.getHostInfo().getClusterStateVersionOrNull() - + ") as fleetcontroller (" + clusterStateVersion + ")"); + if (distributorClusterStateVersion == null) + return disallow("Distributor node " + distributorNodeInfo.getNodeIndex() + + " has not reported any cluster state version yet."); + if (distributorClusterStateVersion != clusterStateVersion) { + return disallow("Distributor node " + distributorNodeInfo.getNodeIndex() + + " does not report same version (" + + distributorNodeInfo.getHostInfo().getClusterStateVersionOrNull() + + ") as fleetcontroller (" + clusterStateVersion + ")"); } Result storageNodesResult = checkStorageNodesForDistributor(distributorNodeInfo, node); - if (storageNodesResult.settingWantedStateIsNotAllowed()) { + if (storageNodesResult.notAllowed()) { return storageNodesResult; } } - return allowSettingOfWantedState(); + return allow(); } private Set<Integer> groupsWithUserWantedStateNotUp() { 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 01a75034ddf..bfbe0f795fc 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 @@ -9,7 +9,6 @@ import com.yahoo.vdslib.state.NodeType; import com.yahoo.vdslib.state.State; import com.yahoo.vespa.clustercontroller.core.ContentCluster; import com.yahoo.vespa.clustercontroller.core.NodeInfo; -import com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker; import com.yahoo.vespa.clustercontroller.core.RemoteClusterControllerTask; import com.yahoo.vespa.clustercontroller.core.listeners.NodeListener; import com.yahoo.vespa.clustercontroller.core.restapiv2.Id; @@ -145,7 +144,7 @@ public class SetNodeStateRequest extends Request<SetResponse> { probe); // If the state was successfully set, just return an "ok" message back. - String reason = success ? "ok" : result.getReason(); + String reason = success ? "ok" : result.reason(); return new SetResponse(reason, success); } @@ -154,19 +153,19 @@ public class SetNodeStateRequest extends Request<SetResponse> { * wanted state, or the requested state has been accepted as the new wanted state. */ private static boolean setWantedStateAccordingToResult( - NodeStateChangeChecker.Result result, + Result result, NodeState newWantedState, SetUnitStateRequest.Condition condition, NodeInfo nodeInfo, ContentCluster cluster, NodeListener stateListener, boolean probe) { - if (result.settingWantedStateIsAllowed()) { + if (result.allowed()) { setNewWantedState(nodeInfo, newWantedState, stateListener, probe); } // True if the wanted state was or has just been set to newWantedState - boolean success = result.settingWantedStateIsAllowed() || result.wantedStateAlreadySet(); + boolean success = result.allowed() || result.isAlreadySet(); if (success && condition == SetUnitStateRequest.Condition.SAFE && nodeInfo.isStorage()) { // In safe-mode, setting the storage node must be accompanied by changing the state 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 43687d51937..7b20fcf694a 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 @@ -115,8 +115,8 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( nodeDistributor, defaultAllUpClusterState(), FORCE, UP_NODE_STATE, newState); - assertTrue(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed()); + assertFalse(result.isAlreadySet()); } @ParameterizedTest @@ -127,9 +127,9 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( new Node(STORAGE, 10), defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("Master cluster controller is bootstrapping and in moratorium", result.getReason()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); + assertEquals("Master cluster controller is bootstrapping and in moratorium", result.reason()); } @ParameterizedTest @@ -140,9 +140,9 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( new Node(STORAGE, 10), defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("Unknown node storage.10", result.getReason()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); + assertEquals("Unknown node storage.10", result.reason()); } @ParameterizedTest @@ -159,10 +159,10 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( new Node(STORAGE, 1), clusterStateWith0InMaintenance, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); assertEquals("At most one node can have a wanted state when #groups = 1: Other storage node 0 has wanted state Maintenance", - result.getReason()); + result.reason()); } @Test @@ -195,9 +195,9 @@ public class NodeStateChangeCheckerTest { cluster.clusterInfo().getStorageNodeInfo(nodeIndex).setReportedState(new NodeState(STORAGE, DOWN), 0); Node node = new Node(STORAGE, nodeIndex); Result result = nodeStateChangeChecker.evaluateTransition(node, clusterState, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed(), result.toString()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("At most 2 groups can have wanted state: [0, 1, 2]", result.getReason()); + assertFalse(result.allowed(), result.toString()); + assertFalse(result.isAlreadySet()); + assertEquals("At most 2 groups can have wanted state: [0, 1, 2]", result.reason()); } // Nodes in group 0 and 1 in maintenance, try to set storage node in group 2 to maintenance, should fail @@ -206,9 +206,9 @@ public class NodeStateChangeCheckerTest { int nodeIndex = 2; Node node = new Node(STORAGE, nodeIndex); Result result = nodeStateChangeChecker.evaluateTransition(node, clusterState, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed(), result.toString()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("At most 2 groups can have wanted state: [0, 1]", result.getReason()); + assertFalse(result.allowed(), result.toString()); + assertFalse(result.isAlreadySet()); + assertEquals("At most 2 groups can have wanted state: [0, 1]", result.reason()); } } @@ -251,9 +251,9 @@ public class NodeStateChangeCheckerTest { int nodeIndex = 4; Node node = new Node(STORAGE, nodeIndex); Result result = nodeStateChangeChecker.evaluateTransition(node, clusterState, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed(), result.toString()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("At most 2 groups can have wanted state: [0, 1]", result.getReason()); + assertFalse(result.allowed(), result.toString()); + assertFalse(result.isAlreadySet()); + assertEquals("At most 2 groups can have wanted state: [0, 1]", result.reason()); } // 2 nodes in group 0 and 1 in group 1 in maintenance, try to set storage node 3 in group 1 to maintenance @@ -270,9 +270,9 @@ public class NodeStateChangeCheckerTest { int nodeIndex = 4; Node node = new Node(STORAGE, nodeIndex); Result result = nodeStateChangeChecker.evaluateTransition(node, clusterState, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed(), result.toString()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("At most 2 groups can have wanted state: [0, 1]", result.getReason()); + assertFalse(result.allowed(), result.toString()); + assertFalse(result.isAlreadySet()); + assertEquals("At most 2 groups can have wanted state: [0, 1]", result.reason()); } // 2 nodes in group 0 up again but buckets not in sync and 2 nodes in group 1 in maintenance, @@ -301,8 +301,8 @@ public class NodeStateChangeCheckerTest { int nodeIndex = 2; Node node = new Node(STORAGE, nodeIndex); Result result = nodeStateChangeChecker.evaluateTransition(node, clusterState, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertTrue(result.settingWantedStateIsAllowed(), result.toString()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed(), result.toString()); + assertFalse(result.isAlreadySet()); } } @@ -321,10 +321,10 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( new Node(STORAGE, 1), clusterStateWith0InMaintenance, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); assertEquals("At most one node can have a wanted state when #groups = 1: Other distributor 0 has wanted state Down", - result.getReason()); + result.reason()); } @ParameterizedTest @@ -344,12 +344,12 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( new Node(STORAGE, 2), clusterStateWith0InMaintenance, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); if (maxNumberOfGroupsAllowedToBeDown >= 1) - assertEquals("Wanted state already set for another node in groups: [0]", result.getReason()); + assertEquals("Wanted state already set for another node in groups: [0]", result.reason()); else - assertEquals("At most one group can have wanted state: Other distributor 0 in group 0 has wanted state Down", result.getReason()); + assertEquals("At most one group can have wanted state: Other distributor 0 in group 0 has wanted state Down", result.reason()); } { @@ -359,11 +359,11 @@ public class NodeStateChangeCheckerTest { new Node(STORAGE, 1), clusterStateWith0InMaintenance, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); if (maxNumberOfGroupsAllowedToBeDown >= 1) { - assertFalse(result.settingWantedStateIsAllowed(), result.getReason()); - assertEquals("Wanted state already set for another node in groups: [0]", result.getReason()); + assertFalse(result.allowed(), result.reason()); + assertEquals("Wanted state already set for another node in groups: [0]", result.reason()); } else { - assertFalse(result.settingWantedStateIsAllowed(), result.getReason()); - assertEquals("Another distributor wants state DOWN: 0", result.getReason()); + assertFalse(result.allowed(), result.reason()); + assertEquals("Another distributor wants state DOWN: 0", result.reason()); } } } @@ -385,13 +385,13 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( new Node(STORAGE, 2), clusterStateWith0InMaintenance, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); if (maxNumberOfGroupsAllowedToBeDown >= 1) - assertEquals("At most 1 groups can have wanted state: [0]", result.getReason()); + assertEquals("At most 1 groups can have wanted state: [0]", result.reason()); else assertEquals("At most one group can have wanted state: Other storage node 0 in group 0 has wanted state Maintenance", - result.getReason()); + result.reason()); } { @@ -400,8 +400,8 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( new Node(STORAGE, 1), clusterStateWith0InMaintenance, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertTrue(result.settingWantedStateIsAllowed(), result.getReason()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed(), result.reason()); + assertFalse(result.isAlreadySet()); } } @@ -412,9 +412,9 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( nodeDistributor, defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); - assertTrue(result.getReason().contains("Safe-set of node state is only supported for storage nodes")); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); + assertTrue(result.reason().contains("Safe-set of node state is only supported for storage nodes")); } @ParameterizedTest @@ -433,17 +433,17 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, clusterStateWith3Down, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("Another storage node has state DOWN: 3", result.getReason()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); + assertEquals("Another storage node has state DOWN: 3", result.reason()); } @ParameterizedTest @ValueSource(ints = {-1, 1}) void testCanUpgradeStorageSafeYes(int maxNumberOfGroupsAllowedToBeDown) { Result result = transitionToMaintenanceWithNoStorageNodesDown(createCluster(4, 1, maxNumberOfGroupsAllowedToBeDown), defaultAllUpClusterState()); - assertTrue(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed()); + assertFalse(result.isAlreadySet()); } @ParameterizedTest @@ -456,8 +456,8 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, defaultAllUpClusterState(), SAFE, MAINTENANCE_NODE_STATE, UP_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); } // A node may be reported as Up but have a generated state of Down if it's part of @@ -477,8 +477,8 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, stateWithNodeDown, SAFE, MAINTENANCE_NODE_STATE, UP_NODE_STATE); - assertTrue(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed()); + assertFalse(result.isAlreadySet()); } @ParameterizedTest @@ -491,8 +491,8 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, defaultAllUpClusterState(), SAFE, new NodeState(STORAGE, DOWN), UP_NODE_STATE); - assertTrue(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed()); + assertFalse(result.isAlreadySet()); } @ParameterizedTest @@ -505,10 +505,10 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); assertEquals("Distributor 0 says storage node 1 has buckets with redundancy as low as 3, but we require at least 4", - result.getReason()); + result.reason()); } @ParameterizedTest @@ -521,8 +521,8 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( new Node(STORAGE, 3), defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertTrue(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed()); + assertFalse(result.isAlreadySet()); } @ParameterizedTest @@ -546,8 +546,8 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( new Node(STORAGE, 1), defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertTrue(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed()); + assertFalse(result.isAlreadySet()); } @ParameterizedTest @@ -559,9 +559,9 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("Distributor node 0 has not reported any cluster state version yet.", result.getReason()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); + assertEquals("Distributor node 0 has not reported any cluster state version yet.", result.reason()); } private Result transitionToSameState(State state, String oldDescription, String newDescription, int maxNumberOfGroupsAllowedToBeDown) { @@ -583,23 +583,23 @@ public class NodeStateChangeCheckerTest { @ValueSource(ints = {-1, 1}) void testSettingUpWhenUpCausesAlreadySet(int maxNumberOfGroupsAllowedToBeDown) { Result result = transitionToSameState(UP, "foo", "bar", maxNumberOfGroupsAllowedToBeDown); - assertTrue(result.wantedStateAlreadySet()); + assertTrue(result.isAlreadySet()); } @ParameterizedTest @ValueSource(ints = {-1, 1}) void testSettingAlreadySetState(int maxNumberOfGroupsAllowedToBeDown) { Result result = transitionToSameState("foo", "foo", maxNumberOfGroupsAllowedToBeDown); - assertFalse(result.settingWantedStateIsAllowed()); - assertTrue(result.wantedStateAlreadySet()); + assertFalse(result.allowed()); + assertTrue(result.isAlreadySet()); } @ParameterizedTest @ValueSource(ints = {-1, 1}) void testDifferentDescriptionImpliesDenied(int maxNumberOfGroupsAllowedToBeDown) { Result result = transitionToSameState("foo", "bar", maxNumberOfGroupsAllowedToBeDown); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); } private Result transitionToMaintenanceWithOneStorageNodeDown(ContentCluster cluster, ClusterState clusterState) { @@ -632,16 +632,16 @@ public class NodeStateChangeCheckerTest { @ValueSource(ints = {-1, 1}) void testCanUpgradeWhenAllUp(int maxNumberOfGroupsAllowedToBeDown) { Result result = transitionToMaintenanceWithNoStorageNodesDown(createCluster(4, maxNumberOfGroupsAllowedToBeDown), defaultAllUpClusterState()); - assertTrue(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed()); + assertFalse(result.isAlreadySet()); } @ParameterizedTest @ValueSource(ints = {-1, 1}) void testCanUpgradeWhenAllUpOrRetired(int maxNumberOfGroupsAllowedToBeDown) { Result result = transitionToMaintenanceWithNoStorageNodesDown(createCluster(4, maxNumberOfGroupsAllowedToBeDown), defaultAllUpClusterState()); - assertTrue(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed()); + assertFalse(result.isAlreadySet()); } @ParameterizedTest @@ -656,8 +656,8 @@ public class NodeStateChangeCheckerTest { clusterState.setNodeState(new Node(STORAGE, storageNodeIndex), downNodeState); Result result = transitionToMaintenanceWithOneStorageNodeDown(cluster, clusterState); - assertTrue(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed()); + assertFalse(result.isAlreadySet()); } @ParameterizedTest @@ -674,9 +674,9 @@ public class NodeStateChangeCheckerTest { 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")); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); + assertTrue(result.reason().contains("Another storage node has state DOWN: 2")); } @ParameterizedTest @@ -698,8 +698,8 @@ public class NodeStateChangeCheckerTest { Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, stateWithNodeDown, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); } @ParameterizedTest @@ -711,9 +711,9 @@ public class NodeStateChangeCheckerTest { currentClusterStateVersion, 0, maxNumberOfGroupsAllowedToBeDown); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("Only retired nodes are allowed to be set to DOWN in safe mode - is Up", result.getReason()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); + assertEquals("Only retired nodes are allowed to be set to DOWN in safe mode - is Up", result.reason()); } @ParameterizedTest @@ -725,9 +725,9 @@ public class NodeStateChangeCheckerTest { currentClusterStateVersion, 1, maxNumberOfGroupsAllowedToBeDown); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("The storage node manages 1 buckets", result.getReason()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); + assertEquals("The storage node manages 1 buckets", result.reason()); } @ParameterizedTest @@ -739,9 +739,9 @@ public class NodeStateChangeCheckerTest { currentClusterStateVersion, 0, maxNumberOfGroupsAllowedToBeDown); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("Reported state (Initializing) is not UP, so no bucket data is available", result.getReason()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); + assertEquals("Reported state (Initializing) is not UP, so no bucket data is available", result.reason()); } @ParameterizedTest @@ -753,10 +753,10 @@ public class NodeStateChangeCheckerTest { currentClusterStateVersion - 1, 0, maxNumberOfGroupsAllowedToBeDown); - assertFalse(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertFalse(result.allowed()); + assertFalse(result.isAlreadySet()); assertEquals("Cluster controller at version 2 got info for storage node 1 at a different version 1", - result.getReason()); + result.reason()); } @ParameterizedTest @@ -768,8 +768,8 @@ public class NodeStateChangeCheckerTest { currentClusterStateVersion, 0, maxNumberOfGroupsAllowedToBeDown); - assertTrue(result.settingWantedStateIsAllowed()); - assertFalse(result.wantedStateAlreadySet()); + assertTrue(result.allowed()); + assertFalse(result.isAlreadySet()); } private Result evaluateDownTransition(ClusterState clusterState, @@ -948,9 +948,9 @@ public class NodeStateChangeCheckerTest { private void checkSettingToMaintenanceIsAllowed(int nodeIndex, NodeStateChangeChecker nodeStateChangeChecker, ClusterState clusterState) { Node node = new Node(STORAGE, nodeIndex); Result result = nodeStateChangeChecker.evaluateTransition(node, clusterState, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); - assertTrue(result.settingWantedStateIsAllowed(), result.toString()); - assertFalse(result.wantedStateAlreadySet()); - assertEquals("Preconditions fulfilled and new state different", result.getReason()); + assertTrue(result.allowed(), result.toString()); + assertFalse(result.isAlreadySet()); + assertEquals("Preconditions fulfilled and new state different", result.reason()); } private void setStorageNodeWantedStateToMaintenance(ContentCluster cluster, int nodeIndex) { 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 6d93eadfe2a..f2f38954f55 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 @@ -12,7 +12,6 @@ import com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker; import com.yahoo.vespa.clustercontroller.core.listeners.NodeListener; import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.StateRestApiException; import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest; -import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.SetResponse; import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitState; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -20,6 +19,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; +import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; @@ -54,7 +54,7 @@ public class SetNodeStateRequestTest { testSetStateRequest( "maintenance", State.UP, State.UP, - NodeStateChangeChecker.Result.allowSettingOfWantedState(), + Result.allow(), Optional.of(State.MAINTENANCE), Optional.of(State.DOWN)); } @@ -64,7 +64,7 @@ public class SetNodeStateRequestTest { testSetStateRequest( "maintenance", State.UP, State.UP, - NodeStateChangeChecker.Result.allowSettingOfWantedState(), + Result.allow(), Optional.empty(), Optional.empty()); } @@ -73,7 +73,7 @@ public class SetNodeStateRequestTest { testSetStateRequest( "down", State.UP, State.UP, - NodeStateChangeChecker.Result.allowSettingOfWantedState(), + Result.allow(), Optional.of(State.DOWN), Optional.of(State.DOWN)); } @@ -82,7 +82,7 @@ public class SetNodeStateRequestTest { testSetStateRequest( "up", State.MAINTENANCE, State.DOWN, - NodeStateChangeChecker.Result.allowSettingOfWantedState(), + Result.allow(), Optional.of(State.UP), Optional.of(State.UP)); } @@ -91,7 +91,7 @@ public class SetNodeStateRequestTest { testSetStateRequest( "up", State.DOWN, State.DOWN, - NodeStateChangeChecker.Result.allowSettingOfWantedState(), + Result.allow(), Optional.of(State.UP), Optional.of(State.UP)); } @@ -100,7 +100,7 @@ public class SetNodeStateRequestTest { testSetStateRequest( "maintenance", State.MAINTENANCE, State.UP, - NodeStateChangeChecker.Result.createAlreadySet(), + Result.alreadySet(), Optional.empty(), Optional.of(State.DOWN)); } @@ -109,7 +109,7 @@ public class SetNodeStateRequestTest { testSetStateRequest( "maintenance", State.MAINTENANCE, State.DOWN, - NodeStateChangeChecker.Result.createAlreadySet(), + Result.alreadySet(), Optional.empty(), Optional.empty()); } @@ -168,8 +168,8 @@ public class SetNodeStateRequestTest { } } - private SetResponse setWantedState() throws StateRestApiException { - return SetNodeStateRequest.setWantedState( + private void setWantedState() throws StateRestApiException { + SetNodeStateRequest.setWantedState( cluster, condition, newStates, |