Add more tests to nodeadmin -> marknodeasavailable usage

4 files changed, 31 insertions, 4 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java
index 5194614c9b3..45d0890d8cb 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java
@@ -129,13 +129,17 @@ public class NodesApiHandler extends LoggingRequestHandler {
              * This is a temporary "state" or rest call that we use to enable a smooth rollout of
              * dynamic docker flavor allocations. Once we have switch everything we remove this
              * and change the code in the nodeadmin to delete directly.
+             *
+             * Should only be called by node-admin for docker containers (the docker constraint is
+             * enforced in the remove method)
              */
+            String hostname = lastElement(path);
             if (nodeRepository.dynamicAllocationEnabled()) {
-                nodeRepository.remove(lastElement(path));
-                return new MessageResponse("Removed " + lastElement(path));
+                nodeRepository.remove(hostname);
+                return new MessageResponse("Removed " + hostname);
             } else {
-                nodeRepository.setReady(lastElement(path));
-                return new MessageResponse("Moved " + lastElement(path) + " to ready");
+                nodeRepository.setReady(hostname);
+                return new MessageResponse("Moved " + hostname + " to ready");
             }
         }
 
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 40ba82be67e..46fa5c3ce98 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
@@ -66,4 +66,21 @@ public class NodeRepositoryTest {
         tester.curator().set(Path.fromString("/provision/v1/dynamicDockerAllocation"), new byte[0]);
         assertTrue(tester.nodeRepository().dynamicAllocationEnabled());
     }
+
+    @Test
+    public void only_allow_to_delete_dirty_nodes_when_dynamic_allocation_feature_enabled() {
+        NodeRepositoryTester tester = new NodeRepositoryTester();
+        try {
+            tester.addNode("id1", "host1", "default", NodeType.host);
+            tester.addNode("id2", "host2", "docker", NodeType.tenant);
+            tester.nodeRepository().setDirty("host2");
+
+            assertFalse(tester.nodeRepository().remove("host2"));
+
+            tester.curator().set(Path.fromString("/provision/v1/dynamicDockerAllocation"), new byte[0]);
+            assertTrue(tester.nodeRepository().remove("host2"));
+        } finally {
+            tester.curator().delete(Path.fromString("/provision/v1/dynamicDockerAllocation"));
+        }
+    }
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java
index ed6da7f82af..3c25729f1b8 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java
@@ -52,6 +52,7 @@ public class NodeRepositoryTester {
         FlavorConfigBuilder b = new FlavorConfigBuilder();
         b.addFlavor("default", 2., 4., 100, Flavor.Type.BARE_METAL).cost(3);
         b.addFlavor("small", 1., 2., 50, Flavor.Type.BARE_METAL).cost(2);
+        b.addFlavor("docker", 1., 2., 50, Flavor.Type.DOCKER_CONTAINER).cost(1);
         return b.build();
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
index bad123aff82..0285138c9c2 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
@@ -149,6 +149,11 @@ public class RestApiTest {
                                    new byte[0], Request.Method.PUT),
                        "{\"message\":\"Moved host6.yahoo.com to dirty\"}");
 
+        // ... and set it back to ready as if this was from the node-admin with the temporary state rest api
+        assertResponse(new Request("http://localhost:8080/nodes/v2/state/availablefornewallocations/host6.yahoo.com",
+                        new byte[0], Request.Method.PUT),
+                "{\"message\":\"Moved host6.yahoo.com to ready\"}");
+
         // Put a host in failed and make sure it's children are also failed
         assertResponse(new Request("http://localhost:8080/nodes/v2/state/failed/parent1.yahoo.com", new byte[0], Request.Method.PUT),
                 "{\"message\":\"Moved host10.yahoo.com, host5.yahoo.com, parent1.yahoo.com to failed\"}");