diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2023-01-09 16:56:50 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-01-09 16:56:50 +0100 |
commit | 5900c30d04a4b56b7e47e0c149b2542a5101b433 (patch) | |
tree | e499b3a936fdb1279c858c3350fd62943120968c /node-repository | |
parent | 14ef98ae75260ce6e42e282b89c1dc4a97d659aa (diff) | |
parent | 6ace72518ea9b56b8192c13ec732047029f8e717 (diff) |
Merge pull request #25464 from vespa-engine/hakonhall/patch-wanttodeprovision-recursively
Patch wantToDeprovision recursively
Diffstat (limited to 'node-repository')
7 files changed, 37 insertions, 28 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java index 9e674c573da..d506eb6e3d3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java @@ -74,7 +74,7 @@ public class FailedExpirer extends NodeRepositoryMaintainer { recycleIf(node -> node.allocation().isEmpty(), remainingNodes, allNodes); recycleIf(node -> !node.allocation().get().membership().cluster().isStateful() && - node.history().hasEventBefore(History.Event.Type.failed, clock().instant().minus(statelessExpiry)), + node.history().hasEventBefore(History.Event.Type.failed, clock().instant().minus(statelessExpiry)), remainingNodes, allNodes); recycleIf(node -> node.allocation().get().membership().cluster().isStateful() && diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirer.java index 2db02992f40..d2f8ae1e5a7 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirer.java @@ -23,8 +23,9 @@ import java.util.List; */ public class ProvisionedExpirer extends Expirer { + private static final int MAXIMUM_ALLOWED_EXPIRED_HOSTS = 5; + private final NodeRepository nodeRepository; - private static final int MAXIMUM_ALLOWED_EXPIRED_HOSTS = 20; ProvisionedExpirer(NodeRepository nodeRepository, Duration timeout, Metric metric) { super(Node.State.provisioned, History.Event.Type.provisioned, nodeRepository, timeout, metric); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java index 4bfcea4acd2..b507d66e14f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java @@ -332,8 +332,8 @@ public class Nodes { return fail(hostname, false, agent, reason); } - public Node fail(String hostname, boolean wantToDeprovision, Agent agent, String reason) { - return move(hostname, Node.State.failed, agent, wantToDeprovision, Optional.of(reason)); + public Node fail(String hostname, boolean forceDeprovision, Agent agent, String reason) { + return move(hostname, Node.State.failed, agent, forceDeprovision, Optional.of(reason)); } /** @@ -371,15 +371,15 @@ public class Nodes { * @return the node in its new state * @throws NoSuchNodeException if the node is not found */ - public Node park(String hostname, boolean wantToDeprovision, Agent agent, String reason) { + public Node park(String hostname, boolean forceDeprovision, Agent agent, String reason) { NestedTransaction transaction = new NestedTransaction(); - Node parked = park(hostname, wantToDeprovision, agent, reason, transaction); + Node parked = park(hostname, forceDeprovision, agent, reason, transaction); transaction.commit(); return parked; } - private Node park(String hostname, boolean wantToDeprovision, Agent agent, String reason, NestedTransaction transaction) { - return move(hostname, Node.State.parked, agent, wantToDeprovision, Optional.of(reason), transaction); + private Node park(String hostname, boolean forceDeprovision, Agent agent, String reason, NestedTransaction transaction) { + return move(hostname, Node.State.parked, agent, forceDeprovision, Optional.of(reason), transaction); } /** @@ -427,15 +427,15 @@ public class Nodes { } /** Move a node to given state */ - private Node move(String hostname, Node.State toState, Agent agent, boolean wantToDeprovision, Optional<String> reason) { + private Node move(String hostname, Node.State toState, Agent agent, boolean forceDeprovision, Optional<String> reason) { NestedTransaction transaction = new NestedTransaction(); - Node moved = move(hostname, toState, agent, wantToDeprovision, reason, transaction); + Node moved = move(hostname, toState, agent, forceDeprovision, reason, transaction); transaction.commit(); return moved; } /** Move a node to given state as part of a transaction */ - private Node move(String hostname, Node.State toState, Agent agent, boolean wantToDeprovision, Optional<String> reason, NestedTransaction transaction) { + private Node move(String hostname, Node.State toState, Agent agent, boolean forceDeprovision, Optional<String> reason, NestedTransaction transaction) { // TODO: Work out a safe lock acquisition strategy for moves. Lock is only held while adding operations to // transaction, but lock must also be held while committing try (NodeMutex lock = lockAndGetRequired(hostname)) { @@ -448,8 +448,8 @@ public class Nodes { illegal("Could not set " + node + " active: Same cluster and index as " + currentActive); } } - if (wantToDeprovision) - node = node.withWantToRetire(wantToDeprovision, wantToDeprovision, agent, clock.instant()); + if (forceDeprovision) + node = node.withWantToRetire(true, true, agent, clock.instant()); if (toState == Node.State.deprovisioned) { node = node.with(IP.Config.EMPTY); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java index f77b98cc02c..3581cd05f08 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java @@ -13,6 +13,9 @@ import com.yahoo.slime.Inspector; import com.yahoo.slime.ObjectTraverser; import com.yahoo.slime.SlimeUtils; import com.yahoo.slime.Type; +import com.yahoo.vespa.flags.BooleanFlag; +import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.provision.LockedNodeList; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; @@ -39,7 +42,6 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.TreeSet; -import java.util.stream.Collectors; import static com.yahoo.config.provision.NodeResources.DiskSpeed.fast; import static com.yahoo.config.provision.NodeResources.DiskSpeed.slow; @@ -65,11 +67,13 @@ public class NodePatcher { private final NodeRepository nodeRepository; private final NodeFlavors nodeFlavors; private final Clock clock; + private final BooleanFlag recursiveWantToDeprovision; - public NodePatcher(NodeFlavors nodeFlavors, NodeRepository nodeRepository) { + public NodePatcher(NodeFlavors nodeFlavors, NodeRepository nodeRepository, FlagSource flagSource) { this.nodeRepository = nodeRepository; this.nodeFlavors = nodeFlavors; this.clock = nodeRepository.clock(); + this.recursiveWantToDeprovision = Flags.RECURSIVE_WANT_TO_DEPROVISION.bindTo(flagSource); } /** @@ -192,12 +196,14 @@ public class NodePatcher { case WANT_TO_RETIRE: case WANT_TO_DEPROVISION: case WANT_TO_REBUILD: - boolean wantToRetire = asOptionalBoolean(root.field(WANT_TO_RETIRE)).orElse(node.status().wantToRetire()); - boolean wantToDeprovision = asOptionalBoolean(root.field(WANT_TO_DEPROVISION)).orElse(node.status().wantToDeprovision()); - boolean wantToRebuild = asOptionalBoolean(root.field(WANT_TO_REBUILD)).orElse(node.status().wantToRebuild()); - return node.withWantToRetire(wantToRetire, - wantToDeprovision && !applyingAsChild, - wantToRebuild && !applyingAsChild, + // These needs to be handled as one, because certain combinations are not allowed. + return node.withWantToRetire(asOptionalBoolean(root.field(WANT_TO_RETIRE)).orElseGet(node.status()::wantToRetire), + asOptionalBoolean(root.field(WANT_TO_DEPROVISION)) + .filter(want -> recursiveWantToDeprovision.value() || !applyingAsChild) + .orElseGet(node.status()::wantToDeprovision), + asOptionalBoolean(root.field(WANT_TO_REBUILD)) + .filter(want -> !applyingAsChild) + .orElseGet(node.status()::wantToRebuild), Agent.operator, clock.instant()); case "reports" : diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java index fce5ccbdddc..cde951d1bf1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java @@ -26,6 +26,7 @@ import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; +import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.hosted.provision.NoSuchNodeException; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; @@ -75,14 +76,16 @@ public class NodesV2ApiHandler extends ThreadedHttpRequestHandler { private final Orchestrator orchestrator; private final NodeRepository nodeRepository; private final NodeFlavors nodeFlavors; + private final FlagSource flagSource; @Inject public NodesV2ApiHandler(ThreadedHttpRequestHandler.Context parentCtx, Orchestrator orchestrator, - NodeRepository nodeRepository, NodeFlavors flavors) { + NodeRepository nodeRepository, NodeFlavors flavors, FlagSource flagSource) { super(parentCtx); this.orchestrator = orchestrator; this.nodeRepository = nodeRepository; this.nodeFlavors = flavors; + this.flagSource = flagSource; } @Override @@ -169,7 +172,7 @@ public class NodesV2ApiHandler extends ThreadedHttpRequestHandler { private HttpResponse handlePATCH(HttpRequest request) { Path path = new Path(request.getUri()); if (path.matches("/nodes/v2/node/{hostname}")) { - NodePatcher patcher = new NodePatcher(nodeFlavors, nodeRepository); + NodePatcher patcher = new NodePatcher(nodeFlavors, nodeRepository, flagSource); String hostname = path.get("hostname"); if (isTenantPeer(request)) { patcher.patchFromUntrustedTenantHost(hostname, request.getData()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirerTest.java index 32886733856..359f75c27ab 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirerTest.java @@ -37,12 +37,12 @@ public class ProvisionedExpirerTest { tester.clock().advance(Duration.ofMinutes(5)); new ProvisionedExpirer(tester.nodeRepository(), Duration.ofMinutes(4), new TestMetric()).maintain(); - assertEquals(5, tester.nodeRepository().nodes().list().deprovisioning().size()); - assertEquals(20, tester.nodeRepository().nodes().list().not().deprovisioning().size()); + assertEquals(10, tester.nodeRepository().nodes().list().deprovisioning().size()); + assertEquals(5, tester.nodeRepository().nodes().list().not().deprovisioning().size()); } private void populateNodeRepo() { - var nodes = IntStream.range(0, 25) + var nodes = IntStream.range(0, 15) .mapToObj(i -> Node.create("id-" + i, "host-" + i, new Flavor(NodeResources.unspecified()), Node.State.provisioned, NodeType.host).build()) .toList(); tester.nodeRepository().database().addNodesInState(nodes, Node.State.provisioned, Agent.system); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java index 55b401ef4e8..f5fe0e6aafd 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java @@ -218,9 +218,8 @@ public class NodesV2ApiTest { assertResponse(new Request("http://localhost:8080/nodes/v2/node/dockerhost2.yahoo.com", Utf8.toBytes("{\"wantToDeprovision\": true, \"wantToRetire\": true}"), Request.Method.PATCH), "{\"message\":\"Updated dockerhost2.yahoo.com\"}"); - // Make sure that wantToRetire is applied recursively, but wantToDeprovision isn't tester.assertResponseContains(new Request("http://localhost:8080/nodes/v2/node/host5.yahoo.com"), - "\"wantToRetire\":true,\"preferToRetire\":false,\"wantToDeprovision\":false,"); + "\"wantToRetire\":true,\"preferToRetire\":false,\"wantToDeprovision\":true,"); assertResponse(new Request("http://localhost:8080/nodes/v2/node/dockerhost1.yahoo.com", Utf8.toBytes("{\"wantToRebuild\": true, \"wantToRetire\": true}"), Request.Method.PATCH), "{\"message\":\"Updated dockerhost1.yahoo.com\"}"); |