Merge pull request #11398 from vespa-engine/freva/patch-resources

Allow adding node with resources

3 files changed, 102 insertions, 26 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeSerializer.java
index 57695a0a22c..5ca2667ad5d 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeSerializer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeSerializer.java
@@ -70,6 +70,15 @@ public class NodeSerializer {
         }
     }
 
+    public NodeResources.DiskSpeed diskSpeedFrom(String diskSpeed) {
+        switch (diskSpeed) {
+            case "fast": return NodeResources.DiskSpeed.fast;
+            case "slow": return NodeResources.DiskSpeed.slow;
+            case "any" : return NodeResources.DiskSpeed.any;
+            default: throw new IllegalArgumentException("Unknown disk speed '" + diskSpeed + "'");
+        }
+    }
+
     public String toString(NodeResources.DiskSpeed diskSpeed) {
         switch (diskSpeed) {
             case fast : return "fast";
@@ -79,6 +88,15 @@ public class NodeSerializer {
         }
     }
 
+    public NodeResources.StorageType storageTypeFrom(String storageType) {
+        switch (storageType) {
+            case "local" : return NodeResources.StorageType.local;
+            case "remote": return NodeResources.StorageType.remote;
+            case "any"   : return NodeResources.StorageType.any;
+            default: throw new IllegalArgumentException("Unknown storage type '" + storageType + "'");
+        }
+    }
+
     public String toString(NodeResources.StorageType storageType) {
         switch (storageType) {
             case remote : return "remote";
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 ee1e5c7fa45..c291e23cefe 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
@@ -51,8 +51,8 @@ import java.util.stream.Collectors;
 
 import static com.yahoo.config.provision.NodeResources.DiskSpeed.fast;
 import static com.yahoo.config.provision.NodeResources.DiskSpeed.slow;
-import static com.yahoo.config.provision.NodeResources.StorageType.remote;
 import static com.yahoo.config.provision.NodeResources.StorageType.local;
+import static com.yahoo.config.provision.NodeResources.StorageType.remote;
 import static com.yahoo.vespa.config.SlimeUtils.optionalString;
 
 /**
@@ -241,29 +241,55 @@ public class NodesApiHandler extends LoggingRequestHandler {
 
     private Flavor flavorFromSlime(Inspector inspector) {
         Inspector flavorInspector = inspector.field("flavor");
+        Inspector resourcesInspector = inspector.field("resources");
         if ( ! flavorInspector.valid()) {
-            return new Flavor(new NodeResources(
-                    requiredField(inspector, "minCpuCores", Inspector::asDouble),
-                    requiredField(inspector, "minMainMemoryAvailableGb", Inspector::asDouble),
-                    requiredField(inspector, "minDiskAvailableGb", Inspector::asDouble),
-                    requiredField(inspector, "bandwidthGbps", Inspector::asDouble),
-                    requiredField(inspector, "fastDisk", Inspector::asBool) ? fast : slow,
-                    requiredField(inspector, "remoteStorage", Inspector::asBool) ? remote : local));
+            return new Flavor(resourcesInspector.valid() ?
+                    new NodeResources(
+                            requiredField(resourcesInspector, "vcpu", Inspector::asDouble),
+                            requiredField(resourcesInspector, "memoryGb", Inspector::asDouble),
+                            requiredField(resourcesInspector, "diskGb", Inspector::asDouble),
+                            requiredField(resourcesInspector, "bandwidthGbps", Inspector::asDouble),
+                            optionalString(resourcesInspector.field("diskSpeed")).map(serializer::diskSpeedFrom).orElse(NodeResources.DiskSpeed.getDefault()),
+                            optionalString(resourcesInspector.field("storageType")).map(serializer::storageTypeFrom).orElse(NodeResources.StorageType.getDefault())) :
+                    // TODO: Remove when all clients have migrated to the above format
+                    new NodeResources(
+                            requiredField(inspector, "minCpuCores", Inspector::asDouble),
+                            requiredField(inspector, "minMainMemoryAvailableGb", Inspector::asDouble),
+                            requiredField(inspector, "minDiskAvailableGb", Inspector::asDouble),
+                            requiredField(inspector, "bandwidthGbps", Inspector::asDouble),
+                            requiredField(inspector, "fastDisk", Inspector::asBool) ? fast : slow,
+                            requiredField(inspector, "remoteStorage", Inspector::asBool) ? remote : local));
         }
 
         Flavor flavor = nodeFlavors.getFlavorOrThrow(flavorInspector.asString());
-        if (inspector.field("minCpuCores").valid())
-            flavor = flavor.with(flavor.resources().withVcpu(inspector.field("minCpuCores").asDouble()));
-        if (inspector.field("minMainMemoryAvailableGb").valid())
-            flavor = flavor.with(flavor.resources().withMemoryGb(inspector.field("minMainMemoryAvailableGb").asDouble()));
-        if (inspector.field("minDiskAvailableGb").valid())
-            flavor = flavor.with(flavor.resources().withDiskGb(inspector.field("minDiskAvailableGb").asDouble()));
-        if (inspector.field("bandwidthGbps").valid())
-            flavor = flavor.with(flavor.resources().withBandwidthGbps(inspector.field("bandwidthGbps").asDouble()));
-        if (inspector.field("fastDisk").valid())
-            flavor = flavor.with(flavor.resources().with(inspector.field("fastDisk").asBool() ? fast : slow));
-        if (inspector.field("remoteStorage").valid())
-            flavor = flavor.with(flavor.resources().with(inspector.field("remoteStorage").asBool() ? remote : local));
+        if (resourcesInspector.valid()) {
+            if (resourcesInspector.field("vcpu").valid())
+                flavor = flavor.with(flavor.resources().withVcpu(resourcesInspector.field("vcpu").asDouble()));
+            if (resourcesInspector.field("memoryGb").valid())
+                flavor = flavor.with(flavor.resources().withMemoryGb(resourcesInspector.field("memoryGb").asDouble()));
+            if (resourcesInspector.field("diskGb").valid())
+                flavor = flavor.with(flavor.resources().withDiskGb(resourcesInspector.field("diskGb").asDouble()));
+            if (resourcesInspector.field("bandwidthGbps").valid())
+                flavor = flavor.with(flavor.resources().withBandwidthGbps(resourcesInspector.field("bandwidthGbps").asDouble()));
+            if (resourcesInspector.field("diskSpeed").valid())
+                flavor = flavor.with(flavor.resources().with(serializer.diskSpeedFrom(resourcesInspector.field("diskSpeed").asString())));
+            if (resourcesInspector.field("storageType").valid())
+                flavor = flavor.with(flavor.resources().with(serializer.storageTypeFrom(resourcesInspector.field("storageType").asString())));
+        } else {
+            // TODO: Remove when all clients have migrated to the above format
+            if (inspector.field("minCpuCores").valid())
+                flavor = flavor.with(flavor.resources().withVcpu(inspector.field("minCpuCores").asDouble()));
+            if (inspector.field("minMainMemoryAvailableGb").valid())
+                flavor = flavor.with(flavor.resources().withMemoryGb(inspector.field("minMainMemoryAvailableGb").asDouble()));
+            if (inspector.field("minDiskAvailableGb").valid())
+                flavor = flavor.with(flavor.resources().withDiskGb(inspector.field("minDiskAvailableGb").asDouble()));
+            if (inspector.field("bandwidthGbps").valid())
+                flavor = flavor.with(flavor.resources().withBandwidthGbps(inspector.field("bandwidthGbps").asDouble()));
+            if (inspector.field("fastDisk").valid())
+                flavor = flavor.with(flavor.resources().with(inspector.field("fastDisk").asBool() ? fast : slow));
+            if (inspector.field("remoteStorage").valid())
+                flavor = flavor.with(flavor.resources().with(inspector.field("remoteStorage").asBool() ? remote : local));
+        }
         return flavor;
     }
 
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 1bae70c8d60..9ec516779b4 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
@@ -8,7 +8,6 @@ import com.yahoo.application.container.handler.Response;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.io.IOUtils;
 import com.yahoo.text.Utf8;
-import com.yahoo.vespa.config.SlimeUtils;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.maintenance.OsUpgradeActivator;
 import com.yahoo.vespa.hosted.provision.testutils.ContainerConfig;
@@ -829,7 +828,7 @@ public class RestApiTest {
     }
 
     @Test
-    public void test_flavor_overrides() throws Exception {
+    public void test_flavor_overrides_old_format() throws Exception {
         String hostname = "parent2.yahoo.com";
         // Test adding with overrides
         assertResponse(new Request("http://localhost:8080/nodes/v2/node",
@@ -847,20 +846,53 @@ public class RestApiTest {
                         Request.Method.POST),
                 "{\"message\":\"Added 1 nodes to the provisioned state\"}");
         assertResponseContains(new Request("http://localhost:8080/nodes/v2/node/" + hostname),
+                "\"resources\":{\"vcpu\":64.0,\"memoryGb\":128.0,\"diskGb\":1234.0,\"bandwidthGbps\":15.0,\"storageType\":\"remote\"}");
+    }
+
+    @Test
+    public void test_flavor_overrides() throws Exception {
+        String host = "parent2.yahoo.com";
+        // Test adding with overrides
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node",
+                ("[{\"hostname\":\"" + host + "\"," + createIpAddresses("::1") + "\"openStackId\":\"osid-123\"," +
+                        "\"flavor\":\"large-variant\",\"resources\":{\"diskGb\":1234,\"memoryGb\":4321}}]").
+                        getBytes(StandardCharsets.UTF_8),
+                Request.Method.POST),
+                400,
+                "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Can only override disk GB for configured flavor\"}");
+
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node",
+                        ("[{\"hostname\":\"" + host + "\"," + createIpAddresses("::1") + "\"openStackId\":\"osid-123\"," +
+                                "\"flavor\":\"large-variant\",\"type\":\"host\",\"resources\":{\"diskGb\":1234}}]").
+                                getBytes(StandardCharsets.UTF_8),
+                        Request.Method.POST),
+                "{\"message\":\"Added 1 nodes to the provisioned state\"}");
+        assertResponseContains(new Request("http://localhost:8080/nodes/v2/node/" + host),
                 "\"minDiskAvailableGb\":1234.0,\"minMainMemoryAvailableGb\":128.0,\"minCpuCores\":64.0,\"fastDisk\":true,\"remoteStorage\":true,\"bandwidthGbps\":15.0,");
 
+        // Test adding tenant node
+        String tenant = "node-1-3.yahoo.com";
+        String resources = "\"resources\":{\"vcpu\":64.0,\"memoryGb\":128.0,\"diskGb\":1234.0,\"bandwidthGbps\":15.0,\"diskSpeed\":\"slow\",\"storageType\":\"remote\"}";
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node",
+                        ("[{\"hostname\":\"" + tenant + "\"," + createIpAddresses("::2") + "\"openStackId\":\"osid-124\"," +
+                                "\"type\":\"tenant\"," + resources + "}]").
+                                getBytes(StandardCharsets.UTF_8),
+                        Request.Method.POST),
+                "{\"message\":\"Added 1 nodes to the provisioned state\"}");
+        assertResponseContains(new Request("http://localhost:8080/nodes/v2/node/" + tenant), resources);
+
         // Test patching with overrides
-        assertResponse(new Request("http://localhost:8080/nodes/v2/node/" + hostname,
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node/" + host,
                         "{\"minDiskAvailableGb\":5432,\"minMainMemoryAvailableGb\":2345}".getBytes(StandardCharsets.UTF_8),
                         Request.Method.PATCH),
                 400,
                 "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Could not set field 'minMainMemoryAvailableGb': Can only override disk GB for configured flavor\"}");
 
-        assertResponse(new Request("http://localhost:8080/nodes/v2/node/" + hostname,
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node/" + host,
                         "{\"minDiskAvailableGb\":5432}".getBytes(StandardCharsets.UTF_8),
                         Request.Method.PATCH),
-                "{\"message\":\"Updated " + hostname + "\"}");
-        assertResponseContains(new Request("http://localhost:8080/nodes/v2/node/" + hostname),
+                "{\"message\":\"Updated " + host + "\"}");
+        assertResponseContains(new Request("http://localhost:8080/nodes/v2/node/" + host),
                 "\"minDiskAvailableGb\":5432.0,\"minMainMemoryAvailableGb\":128.0,\"minCpuCores\":64.0,\"fastDisk\":true,\"remoteStorage\":true,\"bandwidthGbps\":15.0,");
     }