Revert "Bratseth/apply policies to limits"

5 files changed, 49 insertions, 53 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java
index ad20f68ca33..e0ccbe10b10 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java
@@ -26,7 +26,7 @@ public class Cluster {
     private final ClusterSpec.Id id;
     private final boolean exclusive;
     private final ClusterResources min, max;
-    private final boolean required;
+    private boolean required;
     private final Optional<Suggestion> suggested;
     private final Optional<ClusterResources> target;
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java
index 2755030e2b3..078b0621a99 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java
@@ -139,20 +139,23 @@ public class AllocatableClusterResources {
     public static Optional<AllocatableClusterResources> from(ClusterResources wantedResources,
                                                              ClusterSpec clusterSpec,
                                                              Limits applicationLimits,
+                                                             boolean required,
                                                              NodeList hosts,
                                                              NodeRepository nodeRepository) {
         var capacityPolicies = new CapacityPolicies(nodeRepository);
         var systemLimits = new NodeResourceLimits(nodeRepository);
         boolean exclusive = clusterSpec.isExclusive();
+        int actualNodes = capacityPolicies.decideSize(wantedResources.nodes(), required, true, false, clusterSpec);
         if ( !clusterSpec.isExclusive() && !nodeRepository.zone().getCloud().dynamicProvisioning()) {
             // We decide resources: Add overhead to what we'll request (advertised) to make sure real becomes (at least) cappedNodeResources
             var advertisedResources = nodeRepository.resourcesCalculator().realToRequest(wantedResources.nodeResources(), exclusive);
             advertisedResources = systemLimits.enlargeToLegal(advertisedResources, clusterSpec.type(), exclusive); // Ask for something legal
             advertisedResources = applicationLimits.cap(advertisedResources); // Overrides other conditions, even if it will then fail
+            advertisedResources = capacityPolicies.decideNodeResources(advertisedResources, required, clusterSpec); // Adjust to what we can request
             var realResources = nodeRepository.resourcesCalculator().requestToReal(advertisedResources, exclusive); // What we'll really get
             if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec.type())) return Optional.empty();
             if (matchesAny(hosts, advertisedResources))
-                    return Optional.of(new AllocatableClusterResources(wantedResources.with(realResources),
+                    return Optional.of(new AllocatableClusterResources(wantedResources.withNodes(actualNodes).with(realResources),
                                                                        advertisedResources,
                                                                        wantedResources,
                                                                        clusterSpec));
@@ -165,6 +168,7 @@ public class AllocatableClusterResources {
             for (Flavor flavor : nodeRepository.flavors().getFlavors()) {
                 // Flavor decide resources: Real resources are the worst case real resources we'll get if we ask for these advertised resources
                 NodeResources advertisedResources = nodeRepository.resourcesCalculator().advertisedResourcesOf(flavor);
+                advertisedResources = capacityPolicies.decideNodeResources(advertisedResources, required, clusterSpec); // Adjust to what we can get
                 NodeResources realResources = nodeRepository.resourcesCalculator().requestToReal(advertisedResources, exclusive);
 
                 // Adjust where we don't need exact match to the flavor
@@ -180,7 +184,7 @@ public class AllocatableClusterResources {
 
                 if ( ! between(applicationLimits.min().nodeResources(), applicationLimits.max().nodeResources(), advertisedResources)) continue;
                 if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec.type())) continue;
-                var candidate = new AllocatableClusterResources(wantedResources.with(realResources),
+                var candidate = new AllocatableClusterResources(wantedResources.withNodes(actualNodes).with(realResources),
                                                                 advertisedResources,
                                                                 wantedResources,
                                                                 clusterSpec);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
index d727757b07e..b8a80a9bd2b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
@@ -66,6 +66,7 @@ public class AllocationOptimizer {
                                                                                groupsAdjustedForRedundancy,
                                                                                limits, target, current, clusterModel));
                 var allocatableResources = AllocatableClusterResources.from(next, current.clusterSpec(), limits,
+                                                                            clusterModel.cluster().required(),
                                                                             hosts, nodeRepository);
 
                 if (allocatableResources.isEmpty()) continue;
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 2b9c5396724..0c2c3c48df1 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
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.provision.provisioning;
 
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Capacity;
-import com.yahoo.config.provision.ClusterResources;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.NodeResources;
@@ -30,21 +29,10 @@ public class CapacityPolicies {
         this.sharedHosts = type -> PermanentFlags.SHARED_HOST.bindTo(nodeRepository.flagSource()).value().isEnabled(type.name());
     }
 
-    public Capacity applyOn(Capacity capacity, ApplicationId application) {
-        return capacity.withLimits(applyOn(capacity.minResources(), capacity, application),
-                                   applyOn(capacity.maxResources(), capacity, application));
-    }
-
-    private ClusterResources applyOn(ClusterResources resources, Capacity capacity, ApplicationId application) {
-        int nodes = decideSize(resources.nodes(), capacity.isRequired(), application.instance().isTester());
-        int groups = Math.min(resources.groups(), nodes); // cannot have more groups than nodes
-        var nodeResources = decideNodeResources(resources.nodeResources(), capacity.isRequired());
-        return new ClusterResources(nodes, groups, nodeResources);
-    }
-
-    private int decideSize(int requested, boolean required, boolean isTester) {
+    public int decideSize(int requested, boolean required, boolean canFail, boolean isTester, ClusterSpec cluster) {
         if (isTester) return 1;
 
+        ensureRedundancy(requested, cluster, canFail);
         if (required) return requested;
         switch(zone.environment()) {
             case dev : case test : return 1;
@@ -55,7 +43,10 @@ public class CapacityPolicies {
         }
     }
 
-    private NodeResources decideNodeResources(NodeResources target, boolean required) {
+    public NodeResources decideNodeResources(NodeResources target, boolean required, ClusterSpec cluster) {
+        if (target.isUnspecified())
+            target = defaultNodeResources(cluster.type());
+
         if (required) return target;
 
         // Dev does not cap the cpu or network of containers since usage is spotty: Allocate just a small amount exclusively
@@ -86,11 +77,28 @@ public class CapacityPolicies {
     }
 
     /**
-     * Returns whether the nodes requested can share physical host with other applications.
+     * Whether or not the nodes requested can share physical host with other applications.
      * A security feature which only makes sense for prod.
      */
     public boolean decideExclusivity(Capacity capacity, boolean requestedExclusivity) {
         return requestedExclusivity && (capacity.isRequired() || zone.environment() == Environment.prod);
     }
 
+    /**
+     * Throw if the node count is 1 for container and content clusters and we're in a production zone
+     *
+     * @throws IllegalArgumentException if only one node is requested and we can fail
+     */
+    private void ensureRedundancy(int nodeCount, ClusterSpec cluster, boolean canFail) {
+        if (canFail &&
+            nodeCount == 1 &&
+            requiresRedundancy(cluster.type()) &&
+            zone.environment().isProduction())
+            throw new IllegalArgumentException("Deployments to prod require at least 2 nodes per cluster for redundancy. Not fulfilled for " + cluster);
+    }
+
+    private static boolean requiresRedundancy(ClusterSpec.Type clusterType) {
+        return clusterType.isContent() || clusterType.isContainer();
+    }
+
 }
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 5dac2004931..b35b0a5e301 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
@@ -84,8 +84,8 @@ public class NodeRepositoryProvisioner implements Provisioner {
     @Override
     public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, Capacity requested,
                                   ProvisionLogger logger) {
-        log.log(Level.FINE, "Received deploy prepare request for " + requested +
-                            " for application " + application + ", cluster " + cluster);
+        log.log(Level.FINE, () -> "Received deploy prepare request for " + requested +
+                                  " for application " + application + ", cluster " + cluster);
 
         if (cluster.group().isPresent()) throw new IllegalArgumentException("Node requests cannot specify a group");
 
@@ -96,16 +96,17 @@ public class NodeRepositoryProvisioner implements Provisioner {
         NodeResources resources;
         NodeSpec nodeSpec;
         if (requested.type() == NodeType.tenant) {
-            var actual = capacityPolicies.applyOn(requested, application);
-            ClusterResources target = decideTargetResources(application, cluster, actual);
-            boolean exclusive = capacityPolicies.decideExclusivity(actual, cluster.isExclusive());
-            ensureRedundancy(target.nodes(), cluster, actual.canFail(), application);
-            logIfDownscaled(requested.minResources().nodes(), actual.minResources().nodes(), cluster, logger);
-
-            groups = target.groups();
-            resources = target.nodeResources().isUnspecified() ? capacityPolicies.defaultNodeResources(cluster.type())
-                                                               : target.nodeResources();
-            nodeSpec = NodeSpec.from(target.nodes(), resources, exclusive, actual.canFail());
+            ClusterResources target = decideTargetResources(application, cluster, requested);
+            int nodeCount = capacityPolicies.decideSize(target.nodes(),
+                                                        requested.isRequired(),
+                                                        requested.canFail(),
+                                                        application.instance().isTester(),
+                                                        cluster);
+            groups = Math.min(target.groups(), nodeCount); // cannot have more groups than nodes
+            resources = capacityPolicies.decideNodeResources(target.nodeResources(), requested.isRequired(), cluster);
+            boolean exclusive = capacityPolicies.decideExclusivity(requested, cluster.isExclusive());
+            nodeSpec = NodeSpec.from(nodeCount, resources, exclusive, requested.canFail());
+            logIfDownscaled(target.nodes(), nodeCount, cluster, logger);
         }
         else {
             groups = 1; // type request with multiple groups is not supported
@@ -189,28 +190,10 @@ public class NodeRepositoryProvisioner implements Provisioner {
                                   .advertisedResources();
     }
 
-    /**
-     * Throw if the node count is 1 for container and content clusters and we're in a production zone
-     *
-     * @throws IllegalArgumentException if only one node is requested and we can fail
-     */
-    private void ensureRedundancy(int nodeCount, ClusterSpec cluster, boolean canFail, ApplicationId application) {
-        if (! application.instance().isTester() &&
-            canFail &&
-            nodeCount == 1 &&
-            requiresRedundancy(cluster.type()) &&
-            zone.environment().isProduction())
-            throw new IllegalArgumentException("Deployments to prod require at least 2 nodes per cluster for redundancy. Not fulfilled for " + cluster);
-    }
-
-    private static boolean requiresRedundancy(ClusterSpec.Type clusterType) {
-        return clusterType.isContent() || clusterType.isContainer();
-    }
-
-    private void logIfDownscaled(int requestedMinNodes, int actualMinNodes, ClusterSpec cluster, ProvisionLogger logger) {
-        if (zone.environment().isManuallyDeployed() && actualMinNodes < requestedMinNodes)
-            logger.log(Level.INFO, "Requested " + requestedMinNodes + " nodes for " + cluster +
-                                   ", downscaling to " + actualMinNodes + " nodes in " + zone.environment());
+    private void logIfDownscaled(int targetNodes, int actualNodes, ClusterSpec cluster, ProvisionLogger logger) {
+        if (zone.environment().isManuallyDeployed() && actualNodes < targetNodes)
+            logger.log(Level.INFO, "Requested " + targetNodes + " nodes for " + cluster +
+                                   ", downscaling to " + actualNodes + " nodes in " + zone.environment());
     }
 
     private List<HostSpec> asSortedHosts(List<Node> nodes, NodeResources requestedResources) {