Allow removing host when children are in dirty

2 files changed, 9 insertions, 9 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
index dfd190b14f5..68123092dfa 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
@@ -27,12 +27,12 @@ import com.yahoo.vespa.hosted.provision.node.NodeAcl;
 import com.yahoo.vespa.hosted.provision.node.filter.NodeFilter;
 import com.yahoo.vespa.hosted.provision.node.filter.NodeListFilter;
 import com.yahoo.vespa.hosted.provision.node.filter.StateFilter;
+import com.yahoo.vespa.hosted.provision.os.OsVersions;
 import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient;
 import com.yahoo.vespa.hosted.provision.persistence.DnsNameResolver;
 import com.yahoo.vespa.hosted.provision.persistence.NameResolver;
 import com.yahoo.vespa.hosted.provision.provisioning.DockerImages;
 import com.yahoo.vespa.hosted.provision.provisioning.FirmwareChecks;
-import com.yahoo.vespa.hosted.provision.os.OsVersions;
 import com.yahoo.vespa.hosted.provision.restapi.v2.NotFoundException;
 
 import java.time.Clock;
@@ -608,7 +608,7 @@ public class NodeRepository extends AbstractComponent {
      *  Non-Docker-container node: iff in state provisioned|failed|parked
      *  Docker-container-node:
      *    If only removing the container node: node in state ready
-     *    If also removing the parent node: child is in state provisioned|failed|parked|ready
+     *    If also removing the parent node: child is in state provisioned|failed|parked|dirty|ready
      */
     private boolean canRemove(Node node, boolean deletingAsChild) {
         if (node.type() == NodeType.tenant && node.allocation().isPresent()) {
@@ -623,7 +623,7 @@ public class NodeRepository extends AbstractComponent {
 
         } else if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER) {
             Set<Node.State> legalStates = EnumSet.of(Node.State.provisioned, Node.State.failed, Node.State.parked,
-                                                     Node.State.ready);
+                                                     Node.State.dirty, Node.State.ready);
 
             if (! legalStates.contains(node.state())) {
                 throw new IllegalArgumentException(String.format("Child node %s can only be removed from following states: %s",
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
index b958f7cd09c..c9671aeafbe 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
@@ -10,7 +10,6 @@ import org.junit.Test;
 import java.time.Instant;
 import java.util.Arrays;
 import java.util.HashSet;
-import java.util.Optional;
 import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -119,11 +118,11 @@ public class NodeRepositoryTest {
         tester.addNode("node20", "node20", "host2", "docker", NodeType.tenant);
         assertEquals(6, tester.nodeRepository().getNodes().size());
 
-        tester.setNodeState("node11", Node.State.dirty);
+        tester.setNodeState("node11", Node.State.active);
 
         try {
             tester.nodeRepository().removeRecursively("host1");
-            fail("Should not be able to delete host node, one of the children is in state dirty");
+            fail("Should not be able to delete host node, one of the children is in state active");
         } catch (IllegalArgumentException ignored) {
             // Expected
         }
@@ -133,9 +132,10 @@ public class NodeRepositoryTest {
         tester.nodeRepository().removeRecursively("host2");
         assertEquals(4, tester.nodeRepository().getNodes().size());
 
-        // Now node10 and node12 are in provisioned, set node11 to ready, and it should be OK to delete host1
-        tester.nodeRepository().setReady("node11", Agent.system, getClass().getSimpleName());
-        tester.nodeRepository().removeRecursively("node11"); // Remove one of the children first instead
+        // Now node10 is in provisioned, set node11 to failed and node12 to ready, and it should be OK to delete host1
+        tester.nodeRepository().fail("node11", Agent.system, getClass().getSimpleName());
+        tester.nodeRepository().setReady("node12", Agent.system, getClass().getSimpleName());
+        tester.nodeRepository().removeRecursively("node12"); // Remove one of the children first instead
         assertEquals(3, tester.nodeRepository().getNodes().size());
 
         tester.nodeRepository().removeRecursively("host1");