Stay within lower node resource limits

4 files changed, 66 insertions, 15 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
index 8f3bed75fa6..9ede545833f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
@@ -9,6 +9,7 @@ import com.yahoo.config.provision.NodeResources;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator;
+import com.yahoo.vespa.hosted.provision.provisioning.NodeResourceLimits;
 
 import java.time.Duration;
 import java.util.List;
@@ -47,6 +48,7 @@ public class Autoscaler {
     private final HostResourcesCalculator hostResourcesCalculator;
     private final NodeMetricsDb metricsDb;
     private final NodeRepository nodeRepository;
+    private final NodeResourceLimits nodeResourceLimits;
 
     public Autoscaler(HostResourcesCalculator hostResourcesCalculator,
                       NodeMetricsDb metricsDb,
@@ -54,6 +56,7 @@ public class Autoscaler {
         this.hostResourcesCalculator = hostResourcesCalculator;
         this.metricsDb = metricsDb;
         this.nodeRepository = nodeRepository;
+        this.nodeResourceLimits = new NodeResourceLimits(nodeRepository.zone());
     }
 
     public Optional<ClusterResources> autoscale(ApplicationId applicationId, ClusterSpec cluster, List<Node> clusterNodes) {
@@ -71,7 +74,8 @@ public class Autoscaler {
         Optional<ClusterResources> bestAllocation = findBestAllocation(totalCpuSpent.get(),
                                                                        totalMemorySpent.get(),
                                                                        totalDiskSpent.get(),
-                                                                       currentAllocation);
+                                                                       currentAllocation,
+                                                                       cluster);
         System.out.println("  Best allocation: " + bestAllocation);
         if (bestAllocation.isPresent() && isSimilar(bestAllocation.get(), currentAllocation))
             return Optional.empty(); // Avoid small changes
@@ -79,12 +83,12 @@ public class Autoscaler {
     }
 
     private Optional<ClusterResources> findBestAllocation(double totalCpu, double totalMemory, double totalDisk,
-                                                          ClusterResources currentAllocation) {
+                                                          ClusterResources currentAllocation, ClusterSpec cluster) {
         Optional<ClusterResourcesWithCost> bestAllocation = Optional.empty();
         for (ResourceIterator i = new ResourceIterator(totalCpu, totalMemory, totalDisk, currentAllocation); i.hasNext(); ) {
             ClusterResources allocation = i.next();
             System.out.println("    Considering " + allocation.nodes() + " nodes:");
-            Optional<ClusterResourcesWithCost> allocatableResources = toAllocatableResources(allocation);
+            Optional<ClusterResourcesWithCost> allocatableResources = toAllocatableResources(allocation, cluster);
             if (allocatableResources.isEmpty()) continue;
             System.out.println("    -- Candidate: " + allocatableResources);
             if (bestAllocation.isEmpty() || allocatableResources.get().cost() < bestAllocation.get().cost())
@@ -108,13 +112,14 @@ public class Autoscaler {
      * Returns the smallest allocatable node resources larger than the given node resources,
      * or empty if none available.
      */
-    private Optional<ClusterResourcesWithCost> toAllocatableResources(ClusterResources resources) {
+    private Optional<ClusterResourcesWithCost> toAllocatableResources(ClusterResources resources, ClusterSpec cluster) {
         if (allowsHostSharing(nodeRepository.zone().cloud())) {
-            // Return the requested resources, or empty if they cannot fit on existing hosts
+            // Return the requested resources, adjusted to be legal or empty if they cannot fit on existing hosts
+            NodeResources nodeResources = nodeResourceLimits.enlargeToLegal(resources.nodeResources(), cluster.type());
             for (Flavor flavor : nodeRepository.getAvailableFlavors().getFlavors())
-                if (flavor.resources().satisfies(resources.nodeResources()))
-                    return Optional.of(new ClusterResourcesWithCost(resources,
-                                                                    costOf(resources.nodeResources()) * resources.nodes()));
+                if (flavor.resources().satisfies(nodeResources))
+                    return Optional.of(new ClusterResourcesWithCost(resources.with(nodeResources),
+                                                                    costOf(nodeResources) * resources.nodes()));
             return Optional.empty();
         }
         else {
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 7c5ff35878b..179d7f2703c 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
@@ -19,11 +19,15 @@ import java.util.Locale;
 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(Zone zone) {
         this.zone = zone;
+        this.nodeResourceLimits = new NodeResourceLimits(zone);
         this.isUsingAdvertisedResources = zone.cloud().value().equals("aws");
     }
 
@@ -64,7 +68,7 @@ public class CapacityPolicies {
     }
 
     private void ensureSufficientResources(NodeResources resources, ClusterSpec cluster) {
-        double minMemoryGb = minMemoryGb(cluster.type());
+        double minMemoryGb = nodeResourceLimits.minMemoryGb(cluster.type());
         if (resources.memoryGb() >= minMemoryGb) return;
 
         throw new IllegalArgumentException(String.format(Locale.ENGLISH,
@@ -72,12 +76,6 @@ public class CapacityPolicies {
                 minMemoryGb, cluster.type().name(), cluster.id().value(), resources.memoryGb()));
     }
 
-    private int minMemoryGb(ClusterSpec.Type clusterType) {
-        if (zone.system() == SystemName.dev) return 1; // Allow small containers in dev system
-        if (clusterType == ClusterSpec.Type.admin) return 2;
-        return 4;
-    }
-
     private NodeResources defaultNodeResources(ClusterSpec.Type clusterType) {
         if (clusterType == ClusterSpec.Type.admin) {
             if (zone.system() == SystemName.dev) {
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
new file mode 100644
index 00000000000..ca04bf66ce3
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java
@@ -0,0 +1,32 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.provisioning;
+
+import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.NodeResources;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.Zone;
+
+/**
+ * Defines the resource limits for nodes in various zones
+ *
+ * @author bratseth
+ */
+public class NodeResourceLimits {
+
+    private final Zone zone;
+
+    public NodeResourceLimits(Zone zone) {
+        this.zone = zone;
+    }
+
+    public int minMemoryGb(ClusterSpec.Type clusterType) {
+        if (zone.system() == SystemName.dev) return 1; // Allow small containers in dev system
+        if (clusterType == ClusterSpec.Type.admin) return 2;
+        return 4;
+    }
+
+    public NodeResources enlargeToLegal(NodeResources resources, ClusterSpec.Type clusterType) {
+        return resources.withMemoryGb(Math.max(minMemoryGb(clusterType), resources.memoryGb()));
+    }
+
+}
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 ddc04b80043..459f4438f34 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
@@ -95,6 +95,22 @@ public class AutoscalingTest {
     }
 
     @Test
+    public void testAutoscalingAvoidsIllegalConfigurations() {
+        NodeResources resources = new NodeResources(3, 100, 100, 1);
+        AutoscalingTester tester = new AutoscalingTester(resources);
+
+        ApplicationId application1 = tester.applicationId("application1");
+        ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1");
+
+        // deploy
+        tester.deploy(application1, cluster1, 6, 1, resources);
+        tester.addMeasurements(Resource.memory,  0.02f, 1f, 120, application1);
+        tester.assertResources("Scaling down",
+                               8, 1, 2.1, 4.0, 71.4,
+                               tester.autoscale(application1, cluster1));
+    }
+
+    @Test
     public void testAutoscalingAws() {
         List<Flavor> flavors = new ArrayList<>();
         flavors.add(new Flavor("aws-xlarge", new NodeResources(3, 200, 100, 1, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote)));