Merge pull request #5652 from vespa-engine/freva/remove-if-docker-and-tenant

Only remove node if docker container and tenant

2 files changed, 24 insertions, 4 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 e6334fcfac7..5ce88522fed 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
@@ -505,12 +505,12 @@ public class NodeRepository extends AbstractComponent {
     }
 
     /*
-     * This method is used by the REST API to handle readying nodes for new allocations. For docker containers this will
-     * remove the node from node repository, otherwise the node will be moved to state ready.
+     * This method is used by the REST API to handle readying nodes for new allocations. For tenant docker
+     * containers this will remove the node from node repository, otherwise the node will be moved to state ready.
      */
     public Node markNodeAvailableForNewAllocation(String hostname, Agent agent, String reason) {
         Node node = getNode(hostname).orElseThrow(() -> new NotFoundException("No node with hostname '" + hostname + "'"));
-        if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER) {
+        if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER && node.type() == NodeType.tenant) {
             if (node.state() != Node.State.dirty) {
                 throw new IllegalArgumentException(
                         "Cannot make " + hostname + " available for new allocation, must be in state dirty, but was in " + node.state());
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 5d217ce86e5..a936beac797 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
@@ -21,7 +21,6 @@ import java.util.stream.Collectors;
 import static junit.framework.TestCase.fail;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 
 /**
  * tests basic operation of the node repository
@@ -83,6 +82,27 @@ public class NodeRepositoryTest {
     }
 
     @Test
+    public void only_remove_tenant_docker_containers_for_new_allocations() {
+        NodeRepositoryTester tester = new NodeRepositoryTester();
+        tester.addNode("host1", "host1", "default", NodeType.tenant);
+        tester.addNode("host2", "host2", "docker", NodeType.tenant);
+        tester.addNode("cfg1", "cfg1", "docker", NodeType.config);
+
+        tester.setNodeState("host1", Node.State.dirty);
+        tester.setNodeState("host2", Node.State.dirty);
+        tester.setNodeState("cfg1", Node.State.dirty);
+
+        tester.nodeRepository().markNodeAvailableForNewAllocation("host1", Agent.system, getClass().getSimpleName());
+        assertEquals(Node.State.ready, tester.nodeRepository().getNode("host1").get().state());
+
+        tester.nodeRepository().markNodeAvailableForNewAllocation("host2", Agent.system, getClass().getSimpleName());
+        assertFalse(tester.nodeRepository().getNode("host2").isPresent());
+
+        tester.nodeRepository().markNodeAvailableForNewAllocation("cfg1", Agent.system, getClass().getSimpleName());
+        assertEquals(Node.State.ready, tester.nodeRepository().getNode("cfg1").get().state());
+    }
+
+    @Test
     public void delete_host_only_after_all_the_children_have_been_deleted() {
         NodeRepositoryTester tester = new NodeRepositoryTester();