Patch wantToDeprovision recursively

7 files changed, 36 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..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<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)) {
@@ -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\"}");