Don't allow transition to ready for allocated nodes

4 files changed, 62 insertions, 3 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 6b03fcaa3eb..099213d24e6 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
@@ -225,7 +225,12 @@ public class NodesApiHandler extends LoggingRequestHandler {
         if ( ! node.isPresent())
             node = nodeRepository.getNode(Node.State.failed, hostname);
         if ( ! node.isPresent())
-            throw new IllegalArgumentException("Could not set " + hostname + " ready: Not registered as provisioned, dirty or failed");
+            node = nodeRepository.getNode(Node.State.parked, hostname);
+        if ( ! node.isPresent())
+            throw new IllegalArgumentException("Could not set " + hostname + " ready: Not registered as provisioned, dirty, failed or parked");
+
+        if (node.get().allocation().isPresent())
+            throw new IllegalArgumentException("Could not set " + hostname + " ready: Node is allocated and must be moved to dirty instead");
 
         nodeRepository.setReady(Collections.singletonList(node.get()));
         return "Moved " + hostname + " to ready";
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 c8993557cb3..e37b6360291 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
@@ -154,6 +154,36 @@ public class RestApiTest {
 
     @Test
     public void testInvalidRequests() throws IOException {
+        // Attempt to fail and ready an allocated node without going through dirty
+        assertResponse(new Request("http://localhost:8080/nodes/v2/state/failed/host1.yahoo.com",
+                                   new byte[0], Request.Method.PUT),
+                       "{\"message\":\"Moved host1.yahoo.com to failed\"}");
+        assertResponse(new Request("http://localhost:8080/nodes/v2/state/ready/host1.yahoo.com",
+                                   new byte[0], Request.Method.PUT),
+                       400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Could not set host1.yahoo.com ready: Node is allocated and must be moved to dirty instead\"}");
+        // (... while dirty then ready works (the ready move will be initiated by node maintenance))
+        assertResponse(new Request("http://localhost:8080/nodes/v2/state/dirty/host1.yahoo.com",
+                                   new byte[0], Request.Method.PUT),
+                       "{\"message\":\"Moved host1.yahoo.com to dirty\"}");
+        assertResponse(new Request("http://localhost:8080/nodes/v2/state/ready/host1.yahoo.com",
+                                   new byte[0], Request.Method.PUT),
+                       "{\"message\":\"Moved host1.yahoo.com to ready\"}");
+
+        // Attempt to park and ready an allocated node without going through dirty
+        assertResponse(new Request("http://localhost:8080/nodes/v2/state/parked/host2.yahoo.com",
+                                   new byte[0], Request.Method.PUT),
+                       "{\"message\":\"Moved host2.yahoo.com to parked\"}");
+        assertResponse(new Request("http://localhost:8080/nodes/v2/state/ready/host2.yahoo.com",
+                                   new byte[0], Request.Method.PUT),
+                       400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Could not set host2.yahoo.com ready: Node is allocated and must be moved to dirty instead\"}");
+        // (... while dirty then ready works (the ready move will be initiated by node maintenance))
+        assertResponse(new Request("http://localhost:8080/nodes/v2/state/dirty/host2.yahoo.com",
+                                   new byte[0], Request.Method.PUT),
+                       "{\"message\":\"Moved host2.yahoo.com to dirty\"}");
+        assertResponse(new Request("http://localhost:8080/nodes/v2/state/ready/host2.yahoo.com",
+                                   new byte[0], Request.Method.PUT),
+                       "{\"message\":\"Moved host2.yahoo.com to ready\"}");
+
         // Attempt to DELETE a node which is not put in failed first
         assertResponse(new Request("http://localhost:8080/nodes/v2/node/host8.yahoo.com",
                                    new byte[0], Request.Method.DELETE),
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/parent-nodes.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/parent-nodes.json
index 28a17b03cc6..7352c4f4455 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/parent-nodes.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/parent-nodes.json
@@ -1,5 +1,5 @@
 {
   "nodes": [
     @include(node10.json)
-]
+  ]
 }
\ No newline at end of file
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/parent1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/parent1.json
index d6369fa51d3..c2d663fcb15 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/parent1.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/parent1.json
@@ -1 +1,25 @@
-{"url":"http://localhost:8080/nodes/v2/node/parent1.yahoo.com","id":"parent1.yahoo.com","state":"ready","type":"host","hostname":"parent1.yahoo.com","openStackId":"parent1","flavor":"default","minDiskAvailableGb":400.0,"minMainMemoryAvailableGb":16.0,"description":"Flavor-name-is-default","minCpuCores":2.0,"canonicalFlavor":"default","environment":"env","rebootGeneration":0,"currentRebootGeneration":0,"failCount":0,"hardwareFailure":false,"history":[{"event":"readied","at":123}]}
+{
+  "url": "http://localhost:8080/nodes/v2/node/parent1.yahoo.com",
+  "id": "parent1.yahoo.com",
+  "state": "ready",
+  "type": "host",
+  "hostname": "parent1.yahoo.com",
+  "openStackId": "parent1",
+  "flavor": "default",
+  "minDiskAvailableGb": 400.0,
+  "minMainMemoryAvailableGb": 16.0,
+  "description": "Flavor-name-is-default",
+  "minCpuCores": 2.0,
+  "canonicalFlavor": "default",
+  "environment": "env",
+  "rebootGeneration": 0,
+  "currentRebootGeneration": 0,
+  "failCount": 0,
+  "hardwareFailure": false,
+  "history": [
+    {
+      "event": "readied",
+      "at": 123
+    }
+  ]
+}