From 21de9943fb135a3cdde3957b4ace79be5edc55ec Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Mon, 9 Jan 2023 16:26:51 +0100 Subject: Patch wantToDeprovision recursively --- .../provision/maintenance/FailedExpirer.java | 2 +- .../provision/maintenance/ProvisionedExpirer.java | 2 +- .../yahoo/vespa/hosted/provision/node/Nodes.java | 22 +++++++++++----------- .../hosted/provision/restapi/NodePatcher.java | 22 ++++++++++++++-------- .../provision/restapi/NodesV2ApiHandler.java | 7 +++++-- .../maintenance/ProvisionedExpirerTest.java | 6 +++--- .../hosted/provision/restapi/NodesV2ApiTest.java | 3 +-- 7 files changed, 36 insertions(+), 28 deletions(-) (limited to 'node-repository/src') 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..d78b6b2e773 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 @@ -24,7 +24,7 @@ import java.util.List; public class ProvisionedExpirer extends Expirer { private final NodeRepository nodeRepository; - private static final int MAXIMUM_ALLOWED_EXPIRED_HOSTS = 20; + private static final int MAXIMUM_ALLOWED_EXPIRED_HOSTS = 5; 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 2590db9434b..4575633a379 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 @@ -331,8 +331,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)); } /** @@ -370,15 +370,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); } /** @@ -426,15 +426,15 @@ public class Nodes { } /** Move a node to given state */ - private Node move(String hostname, Node.State toState, Agent agent, boolean wantToDeprovision, Optional reason) { + private Node move(String hostname, Node.State toState, Agent agent, boolean forceDeprovision, Optional 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 reason, NestedTransaction transaction) { + private Node move(String hostname, Node.State toState, Agent agent, boolean forceDeprovision, Optional 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)) { @@ -447,8 +447,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\"}"); -- cgit v1.2.3 From 6ace72518ea9b56b8192c13ec732047029f8e717 Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Mon, 9 Jan 2023 16:44:50 +0100 Subject: Dummy commit to trigger Vespa check --- .../yahoo/vespa/hosted/provision/maintenance/ProvisionedExpirer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'node-repository/src') 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 d78b6b2e773..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,9 +23,10 @@ import java.util.List; */ public class ProvisionedExpirer extends Expirer { - private final NodeRepository nodeRepository; private static final int MAXIMUM_ALLOWED_EXPIRED_HOSTS = 5; + private final NodeRepository nodeRepository; + ProvisionedExpirer(NodeRepository nodeRepository, Duration timeout, Metric metric) { super(Node.State.provisioned, History.Event.Type.provisioned, nodeRepository, timeout, metric); this.nodeRepository = nodeRepository; -- cgit v1.2.3