Validate all specified limits

5 files changed, 28 insertions, 26 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java
index ee46d58ff9f..fdbf58cda3d 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java
@@ -10,8 +10,6 @@ import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 
-import java.util.Locale;
-
 /**
  * Defines the policies for assigning cluster capacity in various environments
  *
@@ -21,14 +19,11 @@ public class CapacityPolicies {
 
     private final Zone zone;
 
-    private final NodeResourceLimits nodeResourceLimits;
-
     /* Deployments must match 1-to-1 the advertised resources of a physical host */
     private final boolean isUsingAdvertisedResources;
 
     public CapacityPolicies(NodeRepository nodeRepository) {
         this.zone = nodeRepository.zone();
-        this.nodeResourceLimits = new NodeResourceLimits(nodeRepository);
         this.isUsingAdvertisedResources = zone.getCloud().dynamicProvisioning();
     }
 
@@ -49,7 +44,6 @@ public class CapacityPolicies {
     public NodeResources decideNodeResources(NodeResources target, Capacity capacity, ClusterSpec cluster) {
         if (target.isUnspecified())
             target = defaultNodeResources(cluster.type());
-        nodeResourceLimits.ensureWithinAdvertisedLimits(target, cluster);
 
         if (capacity.isRequired()) return target;
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
index 28002463586..4f66794fa84 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
@@ -56,6 +56,7 @@ public class NodeRepositoryProvisioner implements Provisioner {
     private final Preparer preparer;
     private final Activator activator;
     private final Optional<LoadBalancerProvisioner> loadBalancerProvisioner;
+    private final NodeResourceLimits nodeResourceLimits;
 
     int getSpareCapacityProd() {
         return SPARE_CAPACITY_PROD;
@@ -69,6 +70,7 @@ public class NodeRepositoryProvisioner implements Provisioner {
         this.capacityPolicies = new CapacityPolicies(nodeRepository);
         this.zone = zone;
         this.loadBalancerProvisioner = provisionServiceProvider.getLoadBalancerService().map(lbService -> new LoadBalancerProvisioner(nodeRepository, lbService));
+        this.nodeResourceLimits = new NodeResourceLimits(nodeRepository);
         this.preparer = new Preparer(nodeRepository,
                                      zone.environment() == Environment.prod ? SPARE_CAPACITY_PROD : SPARE_CAPACITY_NONPROD,
                                      provisionServiceProvider.getHostProvisioner(),
@@ -95,6 +97,9 @@ public class NodeRepositoryProvisioner implements Provisioner {
             throw new IllegalArgumentException(requested + " requested for " + cluster +
                                                ". Max value exceeds your quota. Resolve this at https://cloud.vespa.ai/quota");
 
+        nodeResourceLimits.ensureWithinAdvertisedLimits("Min", requested.minResources().nodeResources(), cluster);
+        nodeResourceLimits.ensureWithinAdvertisedLimits("Max", requested.maxResources().nodeResources(), cluster);
+
         int groups;
         NodeResources resources;
         NodeSpec nodeSpec;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java
index c255db4722c..94d25d7cabc 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java
@@ -24,18 +24,13 @@ public class NodeResourceLimits {
     }
 
     /** Validates the resources applications ask for (which are in "advertised" resource space) */
-    public void ensureWithinAdvertisedLimits(NodeResources requestedResources, ClusterSpec cluster) {
-        double minMemoryGb = minAdvertisedMemoryGb(cluster.type());
-        if (requestedResources.memoryGb() < minMemoryGb)
-            throw new IllegalArgumentException(String.format(Locale.ENGLISH,
-                                                             "Must specify at least %.2f Gb of memory for %s cluster '%s', was: %.2f Gb",
-                                                             minMemoryGb, cluster.type().name(), cluster.id().value(), requestedResources.memoryGb()));
-
-        double minDiskGb = minAdvertisedDiskGb(requestedResources);
-        if (requestedResources.diskGb() < minDiskGb)
-            throw new IllegalArgumentException(String.format(Locale.ENGLISH,
-                                                             "Must specify at least %.2f Gb of disk for %s cluster '%s', was: %.2f Gb",
-                                                             minDiskGb, cluster.type().name(), cluster.id().value(), requestedResources.diskGb()));
+    public void ensureWithinAdvertisedLimits(String type, NodeResources requested, ClusterSpec cluster) {
+        if (requested.isUnspecified()) return;
+
+        if (requested.memoryGb() < minAdvertisedMemoryGb(cluster.type()))
+            illegal(type, "memory", cluster, requested.memoryGb(), minAdvertisedMemoryGb(cluster.type()));
+        if (requested.diskGb() < minAdvertisedDiskGb(requested))
+            illegal(type, "disk", cluster, requested.diskGb(), minAdvertisedDiskGb(requested));
     }
 
     /** Returns whether the real resources we'll end up with on a given tenant node are within limits */
@@ -77,4 +72,12 @@ public class NodeResourceLimits {
         return 10;
     }
 
+    private void illegal(String type, String resource, ClusterSpec cluster, double requested, double minAllowed) {
+        String message = String.format(Locale.ENGLISH,
+                                       "%s cluster '%s': " + type + " " + resource +
+                                       " size is %.2f Gb but must be at least %.2f Gb",
+                                       cluster.type().name(), cluster.id().value(), requested, minAllowed);
+        throw new IllegalArgumentException(message);
+    }
+
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java
index 59507f277e4..73e337dd47c 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java
@@ -262,7 +262,7 @@ public class DynamicDockerProvisionTest {
                            app1, cluster1);
 
         // Changing group size
-        tester.activate(app1, cluster1, Capacity.from(resources(6, 3, 0.5,  5,  5),
+        tester.activate(app1, cluster1, Capacity.from(resources(6, 3, 0.5,  5,  10),
                                                       resources(9, 3,   5, 20, 15)));
         tester.assertNodes("Groups changed",
                            6, 3, 1, 10, 15,
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
index c9eb6466cd7..5eb3c394cab 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
@@ -447,24 +447,24 @@ public class ProvisioningTest {
                            app1, cluster1);
 
         // Changing limits in opposite directions cause a mixture of min and max
-        tester.activate(app1, cluster1, Capacity.from(resources(2, 1, 10, 30,  5),
-                                                      resources(4, 2, 14, 40, 10)));
+        tester.activate(app1, cluster1, Capacity.from(resources(2, 1, 10, 30,  10),
+                                                      resources(4, 2, 14, 40, 13)));
         tester.assertNodes("A mix of min and max",
-                           4, 2, 10, 30, 10,
+                           4, 2, 10, 30, 13,
                            app1, cluster1);
 
         // Changing group size
-        tester.activate(app1, cluster1, Capacity.from(resources(6, 3, 8, 25,  5),
+        tester.activate(app1, cluster1, Capacity.from(resources(6, 3, 8, 25,  10),
                                                       resources(9, 3, 12, 35, 15)));
         tester.assertNodes("Groups changed",
-                           6, 3, 8, 30, 10,
+                           6, 3, 8, 30, 13,
                            app1, cluster1);
 
         // Stop specifying node resources
         tester.activate(app1, cluster1, Capacity.from(new ClusterResources(6, 3, NodeResources.unspecified),
                                                       new ClusterResources(9, 3, NodeResources.unspecified)));
         tester.assertNodes("No change",
-                           6, 3, 8, 30, 10,
+                           6, 3, 8, 30, 13,
                            app1, cluster1);
     }
 
@@ -488,7 +488,7 @@ public class ProvisioningTest {
                     new NodeResources(2, 2, 10, 2), tester);
         }
         catch (IllegalArgumentException e) {
-            assertEquals("Must specify at least 4.00 Gb of memory for container cluster 'container0', was: 2.00 Gb", e.getMessage());
+            assertEquals("container cluster 'container0': Min memory size is 2.00 Gb but must be at least 4.00 Gb", e.getMessage());
         }
     }