Disk limit handling improvements

- Propagate unspecified node resources through system limit enlarging
- Enlarge disk size to lower limit
- Set aside headroom for disk tax on premise
- Reactivate real limit verification on allocation

7 files changed, 50 insertions, 17 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 fdbf58cda3d..5c990f0a3f3 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
@@ -52,8 +52,7 @@ public class CapacityPolicies {
             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
-        // Do not cap in AWS as hosts are allocated on demand and 1-to-1, so the node can use the entire host
-        if (zone.environment() == Environment.dev && !zone.region().value().contains("aws-"))
+        if (zone.environment() == Environment.dev && zone.getCloud().allowHostSharing())
             target = target.withVcpu(0.1);
 
         return target;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
index ca9cbc418bb..0544b675a6f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
@@ -113,7 +113,7 @@ class NodeAllocation {
 
                 if (requestedNodes.considerRetiring()) {
                     boolean wantToRetireNode = false;
-                    // if ( ! nodeResourceLimits.isWithinRealLimits(offered, cluster)) wantToRetireNode = true;
+                    if ( ! nodeResourceLimits.isWithinRealLimits(offered, cluster)) wantToRetireNode = true;
                     if (violatesParentHostPolicy(this.nodes, offered)) wantToRetireNode = true;
                     if ( ! hasCompatibleFlavor(node)) wantToRetireNode = true;
                     if (offered.status().wantToRetire()) wantToRetireNode = true;
@@ -127,12 +127,10 @@ class NodeAllocation {
                 }
             }
             else if (! saturated() && hasCompatibleFlavor(node)) {
-                /*
                 if ( ! nodeResourceLimits.isWithinRealLimits(offered, cluster)) {
                     ++rejectedDueToInsufficientRealResources;
                     continue;
                 }
-                */
                 if ( violatesParentHostPolicy(this.nodes, offered)) {
                     ++rejectedDueToClashingParentHost;
                     continue;
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 4f66794fa84..913357b16ca 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
@@ -163,11 +163,11 @@ public class NodeRepositoryProvisioner implements Provisioner {
         AllocatableClusterResources currentResources =
                 nodes.isEmpty() ? new AllocatableClusterResources(requested.minResources(), clusterSpec.type()) // new deployment: Use min
                                 : new AllocatableClusterResources(nodes, nodeRepository);
-        return ensureWithin(Limits.of(requested), currentResources);
+        return within(Limits.of(requested), currentResources);
     }
 
     /** Make the minimal adjustments needed to the current resources to stay within the limits */
-    private ClusterResources ensureWithin(Limits limits, AllocatableClusterResources current) {
+    private ClusterResources within(Limits limits, AllocatableClusterResources current) {
         if (limits.isEmpty()) return current.toAdvertisedClusterResources();
         if (limits.min().equals(limits.max())) return limits.min();
 
@@ -202,7 +202,7 @@ public class NodeRepositoryProvisioner implements Provisioner {
                                    Optional.of(nodeAllocation.membership()),
                                    node.status().vespaVersion(),
                                    nodeAllocation.networkPorts(),
-                                   requestedResources == NodeResources.unspecified ? Optional.empty() : Optional.of(requestedResources),
+                                   requestedResources.isUnspecified() ? Optional.empty() : Optional.of(requestedResources),
                                    node.status().dockerImage()));
             if (nodeAllocation.networkPorts().isPresent()) {
                 log.log(Level.FINE, () -> "Prepared node " + node.hostname() + " has port allocations");
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 94d25d7cabc..db54fd316f6 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
@@ -43,8 +43,11 @@ public class NodeResourceLimits {
         return true;
     }
 
-    public NodeResources enlargeToLegal(NodeResources advertisedResources, ClusterSpec.Type clusterType) {
-        return advertisedResources.withMemoryGb(Math.max(minAdvertisedMemoryGb(clusterType), advertisedResources.memoryGb()));
+    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()));
     }
 
     private double minAdvertisedMemoryGb(ClusterSpec.Type clusterType) {
@@ -58,6 +61,7 @@ public class NodeResourceLimits {
     }
 
     private double minAdvertisedDiskGb(NodeResources requested) {
+
         if (requested.storageType() == NodeResources.StorageType.local
             && nodeRepository.zone().getCloud().dynamicProvisioning()) {
             if (nodeRepository.zone().system() == SystemName.Public)
@@ -65,11 +69,11 @@ public class NodeResourceLimits {
             else
                 return 55 + minRealDiskGb();
         }
-        return minRealDiskGb();
+        return 4 + minRealDiskGb();
     }
 
     private double minRealDiskGb() {
-        return 10;
+        return 6;
     }
 
     private void illegal(String type, String resource, ClusterSpec cluster, double requested, double minAllowed) {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
index 8b01868e75e..5b7d67a86a5 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
@@ -161,7 +161,7 @@ public class AutoscalingTest {
         tester.addMeasurements(Resource.memory, 0.05f, 120, application1);
         tester.addMeasurements(Resource.disk,   0.05f, 120, application1);
         tester.assertResources("Scaling down to limit since resource usage is low",
-                               4, 1, 1.8,  7.4, 8.5,
+                               4, 1, 1.8,  7.4, 10.0,
                                tester.autoscale(application1, cluster1.id(), min, max));
     }
 
@@ -179,7 +179,7 @@ public class AutoscalingTest {
         tester.deploy(application1, cluster1, 5, 5, new NodeResources(3.0, 10, 10, 1));
         tester.addMeasurements(Resource.cpu,  0.3f, 1f, 240, application1);
         tester.assertResources("Scaling up since resource usage is too high",
-                               6, 6, 3.6,  8.0, 8.0,
+                               6, 6, 3.6,  8.0, 10.0,
                                tester.autoscale(application1, cluster1.id(), min, max));
     }
 
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 5eb3c394cab..f8fe0d22e84 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
@@ -408,6 +408,24 @@ public class ProvisioningTest {
     }
 
     @Test
+    public void test_node_limits_only() {
+        Flavor hostFlavor = new Flavor(new NodeResources(20, 40, 100, 4));
+        ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east")))
+                                                                    .flavors(List.of(hostFlavor))
+                                                                    .build();
+        tester.makeReadyHosts(30, hostFlavor.resources()).deployZoneApp();
+
+        ApplicationId app1 = tester.makeApplicationId("app1");
+        ClusterSpec cluster1 = ClusterSpec.request(ClusterSpec.Type.content, new ClusterSpec.Id("cluster1")).vespaVersion("7").build();
+
+        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, NodeResources.unspecified),
+                                                      new ClusterResources(4, 1, NodeResources.unspecified)));
+        tester.assertNodes("Initial allocation at min with default resources",
+                           2, 1, 1.5, 8, 50, 0.3,
+                           app1, cluster1);
+    }
+
+    @Test
     public void test_changing_limits() {
         Flavor hostFlavor = new Flavor(new NodeResources(20, 40, 100, 4));
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east")))
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
index f7fa3b29b67..227ebf8e692 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
@@ -236,10 +236,24 @@ public class ProvisioningTester {
     /** Assert on the current *non retired* nodes */
     public void assertNodes(String explanation, int nodes, int groups, double vcpu, double memory, double disk,
                             ApplicationId app, ClusterSpec cluster) {
-        assertNodes(explanation, nodes, groups, vcpu, memory, disk, DiskSpeed.getDefault(), StorageType.getDefault(), app, cluster);
+        assertNodes(explanation, nodes, groups, vcpu, memory, disk, 0.1, app, cluster);
     }
 
-    public void assertNodes(String explanation, int nodes, int groups, double vcpu, double memory, double disk,
+    /** Assert on the current *non retired* nodes */
+    public void assertNodes(String explanation, int nodes, int groups, double vcpu, double memory, double disk, double bandwidth,
+                            ApplicationId app, ClusterSpec cluster) {
+        assertNodes(explanation, nodes, groups, vcpu, memory, disk, bandwidth, DiskSpeed.getDefault(), StorageType.getDefault(), app, cluster);
+    }
+
+    public void assertNodes(String explanation, int nodes, int groups,
+                            double vcpu, double memory, double disk,
+                            DiskSpeed diskSpeed, StorageType storageType,
+                            ApplicationId app, ClusterSpec cluster) {
+        assertNodes(explanation, nodes, groups, vcpu, memory, disk, 0.1, diskSpeed, storageType, app, cluster);
+    }
+
+    public void assertNodes(String explanation, int nodes, int groups,
+                            double vcpu, double memory, double disk, double bandwidth,
                             DiskSpeed diskSpeed, StorageType storageType,
                             ApplicationId app, ClusterSpec cluster) {
         List<Node> nodeList = nodeRepository.list().owner(app).cluster(cluster.id()).not().retired().asList();
@@ -250,7 +264,7 @@ public class ProvisioningTester {
                      groups,
                      nodeList.stream().map(n -> n.allocation().get().membership().cluster().group().get()).distinct().count());
         for (Node node : nodeList) {
-            var expected = new NodeResources(vcpu, memory, disk, 0.1, diskSpeed, storageType);
+            var expected = new NodeResources(vcpu, memory, disk, bandwidth, diskSpeed, storageType);
             assertTrue(explanation + ": Resources: Expected " + expected + " but was " + node.flavor().resources(),
                        expected.compatibleWith(node.flavor().resources()));
         }