Enforce a minimum vcpu

With too little cpu allocated, nodes become unable to perform their duties as citizens
of the cloud, such as responding timely to health probes.

3 files changed, 55 insertions, 22 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 39f784a6aac..223d88c4dc0 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
@@ -14,6 +14,7 @@ import com.yahoo.vespa.hosted.provision.NodeRepository;
  * Defines the policies for assigning cluster capacity in various environments
  *
  * @author bratseth
+ * @see NodeResourceLimits
  */
 public class CapacityPolicies {
 
@@ -43,14 +44,14 @@ public class CapacityPolicies {
 
         if (capacity.isRequired()) return target;
 
-        // Allow slow storage in zones which are not performance sensitive
-        if (zone.system().isCd() || zone.environment() == Environment.dev || zone.environment() == Environment.test)
-            target = target.with(NodeResources.DiskSpeed.any).with(NodeResources.StorageType.any);
-
         // Dev does not cap the cpu of containers since usage is spotty: Allocate just a small amount exclusively
         if (zone.environment() == Environment.dev && zone.getCloud().allowHostSharing())
             target = target.withVcpu(0.1);
 
+        // Allow slow storage in zones which are not performance sensitive
+        if (zone.system().isCd() || zone.environment() == Environment.dev || zone.environment() == Environment.test)
+            target = target.with(NodeResources.DiskSpeed.any).with(NodeResources.StorageType.any);
+
         return target;
     }
 
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 f9eaad072de..e4d7709ec74 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
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.provision.provisioning;
 
 import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
@@ -14,6 +15,7 @@ import java.util.Locale;
  * Defines the resource limits for nodes in various zones
  *
  * @author bratseth
+ * @see CapacityPolicies
  */
 public class NodeResourceLimits {
 
@@ -27,10 +29,12 @@ public class NodeResourceLimits {
     public void ensureWithinAdvertisedLimits(String type, NodeResources requested, ClusterSpec cluster) {
         if (requested.isUnspecified()) return;
 
+        if (requested.vcpu() < minAdvertisedVcpu(cluster.type()))
+            illegal(type, "vcpu", "", cluster, requested.vcpu(), minAdvertisedVcpu(cluster.type()));
         if (requested.memoryGb() < minAdvertisedMemoryGb(cluster.type()))
-            illegal(type, "memory", cluster, requested.memoryGb(), minAdvertisedMemoryGb(cluster.type()));
+            illegal(type, "memoryGb", "Gb", cluster, requested.memoryGb(), minAdvertisedMemoryGb(cluster.type()));
         if (requested.diskGb() < minAdvertisedDiskGb(requested))
-            illegal(type, "disk", cluster, requested.diskGb(), minAdvertisedDiskGb(requested));
+            illegal(type, "diskGb", "Gb", cluster, requested.diskGb(), minAdvertisedDiskGb(requested));
     }
 
     /** Returns whether the real resources we'll end up with on a given tenant node are within limits */
@@ -43,6 +47,7 @@ public class NodeResourceLimits {
     public boolean isWithinRealLimits(NodeResources realResources, ClusterSpec.Type clusterType) {
         if (realResources.isUnspecified()) return true;
 
+        if (realResources.vcpu() < minRealVcpu(clusterType)) return false;
         if (realResources.memoryGb() < minRealMemoryGb(clusterType)) return false;
         if (realResources.diskGb() < minRealDiskGb()) return false;
        return true;
@@ -51,25 +56,25 @@ public class NodeResourceLimits {
     public NodeResources enlargeToLegal(NodeResources requested, ClusterSpec.Type clusterType) {
         if (requested.isUnspecified()) return requested;
 
-        return requested.withMemoryGb(Math.max(minAdvertisedMemoryGb(clusterType), requested.memoryGb()))
-                                  .withDiskGb(Math.max(minAdvertisedDiskGb(requested), requested.diskGb()));
+        return requested.withVcpu(Math.max(minAdvertisedVcpu(clusterType), requested.vcpu()))
+                        .withMemoryGb(Math.max(minAdvertisedMemoryGb(clusterType), requested.memoryGb()))
+                        .withDiskGb(Math.max(minAdvertisedDiskGb(requested), requested.diskGb()));
+    }
+
+    private double minAdvertisedVcpu(ClusterSpec.Type clusterType) {
+        if (zone().environment() == Environment.dev && zone().getCloud().allowHostSharing()) return 0.1;
+        return 0.5;
     }
 
     private double minAdvertisedMemoryGb(ClusterSpec.Type clusterType) {
-        if (nodeRepository.zone().system() == SystemName.dev) return 1; // Allow small containers in dev system
+        if (zone().system() == SystemName.dev) return 1; // Allow small containers in dev system
         if (clusterType == ClusterSpec.Type.admin) return 2;
         return 4;
     }
 
-    private double minRealMemoryGb(ClusterSpec.Type clusterType) {
-        return minAdvertisedMemoryGb(clusterType) - 1.7;
-    }
-
     private double minAdvertisedDiskGb(NodeResources requested) {
-
-        if (requested.storageType() == NodeResources.StorageType.local
-            && nodeRepository.zone().getCloud().dynamicProvisioning()) {
-            if (nodeRepository.zone().system() == SystemName.Public)
+        if (requested.storageType() == NodeResources.StorageType.local && zone().getCloud().dynamicProvisioning()) {
+            if (zone().system() == SystemName.Public)
                 return 10 + minRealDiskGb();
             else
                 return 55 + minRealDiskGb();
@@ -77,15 +82,27 @@ public class NodeResourceLimits {
         return 4 + minRealDiskGb();
     }
 
+    private double minRealVcpu(ClusterSpec.Type clusterType) {
+        return minAdvertisedVcpu(clusterType);
+    }
+
+    private double minRealMemoryGb(ClusterSpec.Type clusterType) {
+        return minAdvertisedMemoryGb(clusterType) - 1.7;
+    }
+
     private double minRealDiskGb() {
         return 6;
     }
 
-    private void illegal(String type, String resource, ClusterSpec cluster, double requested, double minAllowed) {
+    private Zone zone() { return nodeRepository.zone(); }
+
+    private void illegal(String type, String resource, String unit, ClusterSpec cluster, double requested, double minAllowed) {
+        if ( ! unit.isEmpty())
+            unit = " " + unit;
         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);
+                                       " size is %.2f%s but must be at least %.2f%s",
+                                       cluster.type().name(), cluster.id().value(), requested, unit, minAllowed, unit);
         throw new IllegalArgumentException(message);
     }
 
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 1b8330f22fc..5c730912c49 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
@@ -521,7 +521,7 @@ public class ProvisioningTest {
     }
 
     @Test
-    public void below_resource_limit() {
+    public void below_memory_resource_limit() {
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build();
 
         ApplicationId application = tester.makeApplicationId();
@@ -531,7 +531,22 @@ public class ProvisioningTest {
                     new NodeResources(2, 2, 10, 2), tester);
         }
         catch (IllegalArgumentException e) {
-            assertEquals("container cluster 'container0': Min memory size is 2.00 Gb but must be at least 4.00 Gb", e.getMessage());
+            assertEquals("container cluster 'container0': Min memoryGb size is 2.00 Gb but must be at least 4.00 Gb", e.getMessage());
+        }
+    }
+
+    @Test
+    public void below_vcpu_resource_limit() {
+        ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build();
+
+        ApplicationId application = tester.makeApplicationId();
+        tester.makeReadyHosts(10, defaultResources).deployZoneApp();
+        try {
+            prepare(application, 2, 2, 3, 3,
+                    new NodeResources(0.4, 4, 10, 2), tester);
+        }
+        catch (IllegalArgumentException e) {
+            assertEquals("container cluster 'container0': Min vcpu size is 0.40 but must be at least 0.50", e.getMessage());
         }
     }