Merge pull request #5497 from vespa-engine/freva/make-availabe-for-new-allocations-state-ready

Use makeAvailiableForNewAllocations to handle ready state transition

3 files changed, 19 insertions, 23 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 0c256f9bee8..2ef79ec53dd 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
@@ -477,22 +477,21 @@ public class NodeRepository extends AbstractComponent {
     }
 
     /*
-     * This method is used to enable a smooth rollout of dynamic docker flavor allocations. Once we have switch
-     * everything this can be simplified to only deleting the node.
-     *
-     * Should only be called by node-admin for docker containers
+     * 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.
      */
-    public List<Node> markNodeAvailableForNewAllocation(String hostname) {
+    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) {
-            throw new IllegalArgumentException(
-                    "Cannot make " + hostname + " available for new allocation, must be a docker container node");
-        } else 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());
+        if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER) {
+            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());
+            }
+            return removeRecursively(node, true).get(0);
         }
 
-        return removeRecursively(node, true);
+        if (node.state() == Node.State.ready) return node;
+        return setReady(Collections.singletonList(node), agent, reason).get(0);
     }
 
     /**
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 c6667fd9d47..2f7d3120211 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
@@ -6,7 +6,6 @@ import com.yahoo.config.provision.NodeType;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.container.jdisc.LoggingRequestHandler;
-import com.yahoo.container.logging.AccessLog;
 import com.yahoo.io.IOUtils;
 import com.yahoo.slime.ArrayTraverser;
 import com.yahoo.slime.Inspector;
@@ -107,8 +106,9 @@ public class NodesApiHandler extends LoggingRequestHandler {
     private HttpResponse handlePUT(HttpRequest request) {
         String path = request.getUri().getPath();
         // Check paths to disallow illegal state changes
-        if (path.startsWith("/nodes/v2/state/ready/")) {
-            nodeRepository.setReady(lastElement(path), Agent.operator, "Readied through the nodes/v2 API");
+        if (path.startsWith("/nodes/v2/state/ready/") ||
+                path.startsWith("/nodes/v2/state/availablefornewallocations/")) {
+            nodeRepository.markNodeAvailableForNewAllocation(lastElement(path), Agent.operator, "Readied through the nodes/v2 API");
             return new MessageResponse("Moved " + lastElement(path) + " to ready");
         }
         else if (path.startsWith("/nodes/v2/state/failed/")) {
@@ -129,12 +129,6 @@ public class NodesApiHandler extends LoggingRequestHandler {
             nodeRepository.reactivate(lastElement(path), Agent.operator, "Reactivated through nodes/v2 API");
             return new MessageResponse("Moved " + lastElement(path) + " to active");
         }
-        else if (path.startsWith("/nodes/v2/state/availablefornewallocations/")) {
-            String hostname = lastElement(path);
-            List<Node> available = nodeRepository.markNodeAvailableForNewAllocation(hostname);
-            return new MessageResponse("Marked following nodes as available for new allocation: " +
-                    available.stream().map(Node::hostname).collect(Collectors.joining(", ")));
-        }
 
         throw new NotFoundException("Cannot put to path '" + path + "'");
     }
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 dbf08597d6b..2e69867e9b1 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
@@ -169,9 +169,12 @@ public class RestApiTest {
                        "{\"message\":\"Moved test-container-1 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/test-container-1",
+        assertResponse(new Request("http://localhost:8080/nodes/v2/state/ready/test-container-1",
                         new byte[0], Request.Method.PUT),
-                "{\"message\":\"Marked following nodes as available for new allocation: test-container-1\"}");
+                "{\"message\":\"Moved test-container-1 to ready\"}");
+
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node/test-container-1",  new byte[0], Request.Method.GET),
+                404, "{\"error-code\":\"NOT_FOUND\",\"message\":\"No node with hostname 'test-container-1'\"}");
 
         // Put a host in failed and make sure it's children are also failed
         assertResponse(new Request("http://localhost:8080/nodes/v2/state/failed/dockerhost1.yahoo.com", new byte[0], Request.Method.PUT),