Set wantToDeprovision recursively

When requesting deprovisioning of a host we always want its children to lose
their allocation (via parked), so that the host can be deprovisioned.

5 files changed, 8 insertions, 11 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
index 2044844e9ef..13cb998bb68 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
@@ -194,8 +194,6 @@ public final class Node implements Nodelike {
      * If both given wantToRetire and wantToDeprovision are equal to the current values, the method is no-op.
      */
     public Node withWantToRetire(boolean wantToRetire, boolean wantToDeprovision, Agent agent, Instant at) {
-        if (!type.isHost() && wantToDeprovision)
-            throw new IllegalArgumentException("wantToDeprovision can only be set for hosts");
         if (wantToRetire == status.wantToRetire() &&
             wantToDeprovision == status.wantToDeprovision()) return this;
         Node node = this.with(status.withWantToRetire(wantToRetire, wantToDeprovision));
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java
index 2979940ee22..d0c02d7baaf 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java
@@ -169,7 +169,7 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer {
 
         return candidatesForRemoval(nodes).stream()
                 .sorted(Comparator.comparing(node -> node.history().events().stream()
-                        .map(History.Event::at).min(Comparator.naturalOrder()).orElseGet(() -> Instant.MIN)))
+                                                         .map(History.Event::at).min(Comparator.naturalOrder()).orElse(Instant.MIN)))
                 .filter(node -> {
                     if (!sharedHosts.containsKey(node.hostname()) || sharedHosts.size() > minCount) {
                         sharedHosts.remove(node.hostname());
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 cdaaa870e5d..66fc07b51f2 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
@@ -574,7 +574,9 @@ public class Nodes {
             // This takes allocationLock to prevent any further allocation of nodes on this host
             host = lock.node();
             NodeList children = list(allocationLock).childrenOf(host);
-            result = retire(NodeListFilter.from(children.asList()), agent, instant);
+            result = performOn(NodeListFilter.from(children.asList()),
+                               (node, nodeLock) -> write(node.withWantToRetire(true, true, agent, instant),
+                                                         nodeLock));
             result.add(write(host.withWantToRetire(true, true, agent, instant), lock));
         }
         return result;
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
index 72788d5cdab..3047661e62b 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
@@ -244,7 +244,10 @@ public class OsVersionsTest {
     }
 
     private NodeList retiringChildrenOf(Node parent) {
-        return tester.nodeRepository().nodes().list().childrenOf(parent).matching(child -> child.status().wantToRetire());
+        return tester.nodeRepository().nodes().list()
+                     .childrenOf(parent)
+                     .matching(child -> child.status().wantToRetire() &&
+                                        child.status().wantToDeprovision());
     }
 
     private List<Node> provisionInfraApplication(int nodeCount) {
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 6b4a57310e7..598692ad598 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
@@ -207,12 +207,6 @@ public class NodesV2ApiTest {
                         Utf8.toBytes("{\"wantToRetire\": true}"), Request.Method.PATCH),
                 "{\"message\":\"Updated dockerhost1.yahoo.com\"}");
 
-        // wantToDeprovision on non-hosts is not allowed
-        tester.assertResponse(new Request("http://localhost:8080/nodes/v2/node/host5.yahoo.com",
-                        Utf8.toBytes("{\"wantToDeprovision\": true, \"wantToRetire\": true}"), Request.Method.PATCH),
-                400,
-                "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Could not set field 'wantToDeprovision': wantToDeprovision can only be set for hosts\"}");
-
         assertResponse(new Request("http://localhost:8080/nodes/v2/node/dockerhost1.yahoo.com",
                                    Utf8.toBytes("{\"wantToDeprovision\": true}"), Request.Method.PATCH),
                        "{\"message\":\"Updated dockerhost1.yahoo.com\"}");