Merge pull request #24959 from vespa-engine/bratseth/revert-exclusive…

…-cleanup

Bratseth/revert exclusive cleanup MERGEOK

config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java

@@ -84,10 +84,6 @@ public ClusterSpec with(Optional<Group> newGroup) {
         return new ClusterSpec(type, id, newGroup, vespaVersion, exclusive, combinedId, dockerImageRepo, stateful);
     }
 
-    public ClusterSpec withExclusivity(boolean exclusive) {
-        return new ClusterSpec(type, id, groupId, vespaVersion, exclusive, combinedId, dockerImageRepo, stateful);
-    }
-
     public ClusterSpec exclusive(boolean exclusive) {
         return new ClusterSpec(type, id, groupId, vespaVersion, exclusive, combinedId, dockerImageRepo, stateful);
     }

node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java

@@ -7,15 +7,11 @@
 import com.yahoo.config.provision.ApplicationTransaction;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.DockerImage;
-import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.config.provisioning.NodeRepositoryConfig;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.flags.FlagSource;
-import com.yahoo.vespa.flags.JacksonFlag;
-import com.yahoo.vespa.flags.PermanentFlags;
-import com.yahoo.vespa.flags.custom.SharedHost;
 import com.yahoo.vespa.hosted.provision.Node.State;
 import com.yahoo.vespa.hosted.provision.applications.Applications;
 import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb;
@@ -66,7 +62,6 @@ public class NodeRepository extends AbstractComponent {
     private final MetricsDb metricsDb;
     private final Orchestrator orchestrator;
     private final int spareCount;
-    private final JacksonFlag<SharedHost> sharedHosts;
 
     /**
      * Creates a node repository from a zookeeper provider.
@@ -139,7 +134,6 @@ public NodeRepository(NodeFlavors flavors,
         this.metricsDb = metricsDb;
         this.orchestrator = orchestrator;
         this.spareCount = spareCount;
-        this.sharedHosts = PermanentFlags.SHARED_HOST.bindTo(flagSource());
         nodes.rewrite();
     }
 
@@ -203,8 +197,7 @@ public NodeRepository(NodeFlavors flavors,
      * perfectly.
      */
     public boolean exclusiveAllocation(ClusterSpec clusterSpec) {
-        return clusterSpec.isExclusive() ||
-               ( !zone().cloud().allowHostSharing() && !sharedHosts.value().isEnabled(clusterSpec.type().name()));
+        return clusterSpec.isExclusive() || ! zone().cloud().allowHostSharing();
     }
 
     /**

node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java

@@ -164,10 +164,10 @@ public static Optional<AllocatableClusterResources> from(ClusterResources wanted
         if (! exclusive) {
             // 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, exclusive); // Ask for something legal
+            advertisedResources = systemLimits.enlargeToLegal(advertisedResources, clusterSpec.type(), exclusive); // Ask for something legal
             advertisedResources = applicationLimits.cap(advertisedResources); // Overrides other conditions, even if it will then fail
             var realResources = nodeRepository.resourcesCalculator().requestToReal(advertisedResources, exclusive); // What we'll really get
-            if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec))
+            if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec.type()))
                 return Optional.empty();
             if (anySatisfies(realResources, availableRealHostResources))
                     return Optional.of(new AllocatableClusterResources(wantedResources.with(realResources),
@@ -187,7 +187,7 @@ public static Optional<AllocatableClusterResources> from(ClusterResources wanted
 
                 // Adjust where we don't need exact match to the flavor
                 if (flavor.resources().storageType() == NodeResources.StorageType.remote) {
-                    double diskGb = systemLimits.enlargeToLegal(cappedWantedResources, clusterSpec, exclusive).diskGb();
+                    double diskGb = systemLimits.enlargeToLegal(cappedWantedResources, clusterSpec.type(), exclusive).diskGb();
                     advertisedResources = advertisedResources.withDiskGb(diskGb);
                     realResources = realResources.withDiskGb(diskGb);
                 }
@@ -197,7 +197,7 @@ public static Optional<AllocatableClusterResources> from(ClusterResources wanted
                 }
 
                 if ( ! between(applicationLimits.min().nodeResources(), applicationLimits.max().nodeResources(), advertisedResources)) continue;
-                if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec)) continue;
+                if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec.type())) continue;
                 var candidate = new AllocatableClusterResources(wantedResources.with(realResources),
                                                                 advertisedResources,
                                                                 wantedResources,

node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Limits.java

@@ -63,7 +63,7 @@ public NodeResources cap(NodeResources resources) {
     public Limits fullySpecified(ClusterSpec clusterSpec, NodeRepository nodeRepository, ApplicationId applicationId) {
         if (this.isEmpty()) throw new IllegalStateException("Unspecified limits can not be made fully specified");
 
-        var defaultResources = new CapacityPolicies(nodeRepository).defaultNodeResources(clusterSpec, applicationId);
+        var defaultResources = new CapacityPolicies(nodeRepository).defaultNodeResources(clusterSpec, applicationId, clusterSpec.isExclusive());
         var specifiedMin = min.nodeResources().isUnspecified() ? min.with(defaultResources) : min;
         var specifiedMax = max.nodeResources().isUnspecified() ? max.with(defaultResources) : max;
         return new Limits(specifiedMin, specifiedMax);

node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java

@@ -10,8 +10,10 @@
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.Zone;
+import com.yahoo.vespa.flags.JacksonFlag;
 import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.flags.StringFlag;
+import com.yahoo.vespa.flags.custom.SharedHost;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import java.util.Map;
 import java.util.TreeMap;
@@ -28,13 +30,13 @@
  */
 public class CapacityPolicies {
 
-    private final NodeRepository nodeRepository;
     private final Zone zone;
+    private final JacksonFlag<SharedHost> sharedHosts;
     private final StringFlag adminClusterNodeArchitecture;
 
     public CapacityPolicies(NodeRepository nodeRepository) {
-        this.nodeRepository = nodeRepository;
         this.zone = nodeRepository.zone();
+        this.sharedHosts = PermanentFlags.SHARED_HOST.bindTo(nodeRepository.flagSource());
         this.adminClusterNodeArchitecture = PermanentFlags.ADMIN_CLUSTER_NODE_ARCHITECTURE.bindTo(nodeRepository.flagSource());
     }
 
@@ -77,15 +79,16 @@ private NodeResources decideNodeResources(NodeResources target, boolean required
         return target;
     }
 
-    public NodeResources defaultNodeResources(ClusterSpec clusterSpec, ApplicationId applicationId) {
+    public NodeResources defaultNodeResources(ClusterSpec clusterSpec, ApplicationId applicationId, boolean exclusive) {
         if (clusterSpec.type() == ClusterSpec.Type.admin) {
             Architecture architecture = adminClusterArchitecture(applicationId);
 
             if (clusterSpec.id().value().equals("cluster-controllers")) {
-                return clusterControllerResources(clusterSpec).with(architecture);
+                return clusterControllerResources(clusterSpec, exclusive)
+                        .with(architecture);
             }
 
-            return (nodeRepository.exclusiveAllocation(clusterSpec)
+            return (requiresExclusiveHost(clusterSpec.type(), exclusive)
                     ? versioned(clusterSpec, Map.of(new Version(0), smallestExclusiveResources()))
                     : versioned(clusterSpec, Map.of(new Version(0), smallestSharedResources())))
                     .with(architecture);
@@ -104,8 +107,8 @@ public NodeResources defaultNodeResources(ClusterSpec clusterSpec, ApplicationId
         }
     }
 
-    private NodeResources clusterControllerResources(ClusterSpec clusterSpec) {
-        if (nodeRepository.exclusiveAllocation(clusterSpec)) {
+    private NodeResources clusterControllerResources(ClusterSpec clusterSpec, boolean exclusive) {
+        if (requiresExclusiveHost(clusterSpec.type(), exclusive)) {
             return versioned(clusterSpec, Map.of(new Version(0), smallestExclusiveResources()));
         }
         return versioned(clusterSpec, Map.of(new Version(0), new NodeResources(0.25, 1.14, 10, 0.3)));
@@ -115,6 +118,11 @@ private Architecture adminClusterArchitecture(ApplicationId instance) {
         return Architecture.valueOf(adminClusterNodeArchitecture.with(APPLICATION_ID, instance.serializedForm()).value());
     }
 
+    /** Returns whether an exclusive host is required for given cluster type and exclusivity requirement */
+    private boolean requiresExclusiveHost(ClusterSpec.Type type, boolean exclusive) {
+        return ! zone.cloud().allowHostSharing() && (exclusive || !sharedHosts.value().isEnabled(type.name()));
+    }
+
     /** Returns the resources for the newest version not newer than that requested in the cluster spec. */
     static NodeResources versioned(ClusterSpec spec, Map<Version, NodeResources> resources) {
         return requireNonNull(new TreeMap<>(resources).floorEntry(spec.vespaVersion()),
@@ -137,10 +145,9 @@ private NodeResources smallestSharedResources() {
     }
 
     /** Returns whether the nodes requested can share physical host with other applications */
-    public ClusterSpec decideExclusivity(Capacity capacity, ClusterSpec requestedCluster) {
-        if (capacity.cloudAccount().isPresent()) return requestedCluster.withExclusivity(true); // Implicit exclusive
-        boolean exclusive = requestedCluster.isExclusive() && (capacity.isRequired() || zone.environment() == Environment.prod);
-        return requestedCluster.withExclusivity(exclusive);
+    public boolean decideExclusivity(Capacity capacity, boolean requestedExclusivity) {
+        if (capacity.cloudAccount().isPresent()) return true; // Implicit exclusive when using custom cloud account
+        return requestedExclusivity && (capacity.isRequired() || zone.environment() == Environment.prod);
     }
 
 }

node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java

@@ -95,29 +95,28 @@ public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, Ca
         NodeResources resources;
         NodeSpec nodeSpec;
         if (requested.type() == NodeType.tenant) {
-            cluster = capacityPolicies.decideExclusivity(requested, cluster);
-            Capacity actual = capacityPolicies.applyOn(requested, application, cluster.isExclusive());
+            boolean exclusive = capacityPolicies.decideExclusivity(requested, cluster.isExclusive());
+            Capacity actual = capacityPolicies.applyOn(requested, application, exclusive);
             ClusterResources target = decideTargetResources(application, cluster, actual);
             ensureRedundancy(target.nodes(), cluster, actual.canFail(), application);
             logIfDownscaled(requested.minResources().nodes(), actual.minResources().nodes(), cluster, logger);
 
             groups = target.groups();
-            resources = getNodeResources(cluster, target.nodeResources(), application);
-            nodeSpec = NodeSpec.from(target.nodes(), resources, cluster.isExclusive(), actual.canFail(),
+            resources = getNodeResources(cluster, target.nodeResources(), application, exclusive);
+            nodeSpec = NodeSpec.from(target.nodes(), resources, exclusive, actual.canFail(),
                                      requested.cloudAccount().orElse(nodeRepository.zone().cloud().account()));
         }
         else {
             groups = 1; // type request with multiple groups is not supported
-            cluster = cluster.withExclusivity(true);
-            resources = getNodeResources(cluster, requested.minResources().nodeResources(), application);
+            resources = getNodeResources(cluster, requested.minResources().nodeResources(), application, true);
             nodeSpec = NodeSpec.from(requested.type(), nodeRepository.zone().cloud().account());
         }
         return asSortedHosts(preparer.prepare(application, cluster, nodeSpec, groups), resources);
     }
 
-    private NodeResources getNodeResources(ClusterSpec cluster, NodeResources nodeResources, ApplicationId applicationId) {
+    private NodeResources getNodeResources(ClusterSpec cluster, NodeResources nodeResources, ApplicationId applicationId, boolean exclusive) {
         return nodeResources.isUnspecified()
-                ? capacityPolicies.defaultNodeResources(cluster, applicationId)
+                ? capacityPolicies.defaultNodeResources(cluster, applicationId, exclusive)
                 : nodeResources;
     }
 
@@ -178,7 +177,8 @@ private ClusterResources currentResources(Application application,
     private ClusterResources initialResourcesFrom(Capacity requested, ClusterSpec clusterSpec, ApplicationId applicationId) {
         var initial = requested.minResources();
         if (initial.nodeResources().isUnspecified())
-            initial = initial.with(capacityPolicies.defaultNodeResources(clusterSpec, applicationId));
+            initial = initial.with(capacityPolicies.defaultNodeResources(clusterSpec, applicationId,
+                                                                         capacityPolicies.decideExclusivity(requested, clusterSpec.isExclusive())));
         return initial;
     }
 
@@ -260,7 +260,8 @@ private void validate(Collection<HostSpec> hosts) {
     private IllegalArgumentException newNoAllocationPossible(ClusterSpec spec, Limits limits) {
         StringBuilder message = new StringBuilder("No allocation possible within ").append(limits);
 
-        if (nodeRepository.exclusiveAllocation(spec))
+        boolean exclusiveHosts = spec.isExclusive() || ! nodeRepository.zone().cloud().allowHostSharing();
+        if (exclusiveHosts)
             message.append(". Nearest allowed node resources: ").append(findNearestNodeResources(limits));
 
         return new IllegalArgumentException(message.toString());

node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java

@@ -28,10 +28,10 @@ public NodeResourceLimits(NodeRepository nodeRepository) {
     public void ensureWithinAdvertisedLimits(String type, NodeResources requested, ClusterSpec cluster) {
         if (requested.isUnspecified()) return;
 
-        if (requested.vcpu() < minAdvertisedVcpu(cluster))
-            illegal(type, "vcpu", "", cluster, requested.vcpu(), minAdvertisedVcpu(cluster));
-        if (requested.memoryGb() < minAdvertisedMemoryGb(cluster))
-            illegal(type, "memoryGb", "Gb", cluster, requested.memoryGb(), minAdvertisedMemoryGb(cluster));
+        if (requested.vcpu() < minAdvertisedVcpu(cluster.type()))
+            illegal(type, "vcpu", "", cluster, requested.vcpu(), minAdvertisedVcpu(cluster.type()));
+        if (requested.memoryGb() < minAdvertisedMemoryGb(cluster.type()))
+            illegal(type, "memoryGb", "Gb", cluster, requested.memoryGb(), minAdvertisedMemoryGb(cluster.type()));
         if (requested.diskGb() < minAdvertisedDiskGb(requested, cluster.isExclusive()))
             illegal(type, "diskGb", "Gb", cluster, requested.diskGb(), minAdvertisedDiskGb(requested, cluster.isExclusive()));
     }
@@ -40,36 +40,36 @@ public void ensureWithinAdvertisedLimits(String type, NodeResources requested, C
     public boolean isWithinRealLimits(NodeCandidate candidateNode, ClusterSpec cluster) {
         if (candidateNode.type() != NodeType.tenant) return true; // Resource limits only apply to tenant nodes
         return isWithinRealLimits(nodeRepository.resourcesCalculator().realResourcesOf(candidateNode, nodeRepository),
-                                  cluster);
+                                  cluster.type());
     }
 
     /** Returns whether the real resources we'll end up with on a given tenant node are within limits */
-    public boolean isWithinRealLimits(NodeResources realResources, ClusterSpec cluster) {
+    public boolean isWithinRealLimits(NodeResources realResources, ClusterSpec.Type clusterType) {
         if (realResources.isUnspecified()) return true;
 
-        if (realResources.vcpu() < minRealVcpu(cluster)) return false;
-        if (realResources.memoryGb() < minRealMemoryGb(cluster)) return false;
+        if (realResources.vcpu() < minRealVcpu(clusterType)) return false;
+        if (realResources.memoryGb() < minRealMemoryGb(clusterType)) return false;
         if (realResources.diskGb() < minRealDiskGb()) return false;
        return true;
     }
 
-    public NodeResources enlargeToLegal(NodeResources requested, ClusterSpec cluster, boolean exclusive) {
+    public NodeResources enlargeToLegal(NodeResources requested, ClusterSpec.Type clusterType, boolean exclusive) {
         if (requested.isUnspecified()) return requested;
 
-        return requested.withVcpu(Math.max(minAdvertisedVcpu(cluster), requested.vcpu()))
-                        .withMemoryGb(Math.max(minAdvertisedMemoryGb(cluster), requested.memoryGb()))
+        return requested.withVcpu(Math.max(minAdvertisedVcpu(clusterType), requested.vcpu()))
+                        .withMemoryGb(Math.max(minAdvertisedMemoryGb(clusterType), requested.memoryGb()))
                         .withDiskGb(Math.max(minAdvertisedDiskGb(requested, exclusive), requested.diskGb()));
     }
 
-    private double minAdvertisedVcpu(ClusterSpec cluster) {
-        if (zone().environment() == Environment.dev && ! nodeRepository.exclusiveAllocation(cluster)) return 0.1;
-        if (cluster.type().isContent() && zone().environment().isProduction()) return 1.0;
-        if (cluster.type() == ClusterSpec.Type.admin) return 0.1;
+    private double minAdvertisedVcpu(ClusterSpec.Type clusterType) {
+        if (zone().environment() == Environment.dev && zone().cloud().allowHostSharing()) return 0.1;
+        if (clusterType.isContent() && zone().environment().isProduction()) return 1.0;
+        if (clusterType == ClusterSpec.Type.admin) return 0.1;
         return 0.5;
     }
 
-    private double minAdvertisedMemoryGb(ClusterSpec cluster) {
-        if (cluster.type() == ClusterSpec.Type.admin) return 1;
+    private double minAdvertisedMemoryGb(ClusterSpec.Type clusterType) {
+        if (clusterType == ClusterSpec.Type.admin) return 1;
         return 4;
     }
 
@@ -85,10 +85,10 @@ private long reservedDiskSpaceGb(NodeResources.StorageType storageType, boolean
             return 4;
     }
 
-    private double minRealVcpu(ClusterSpec cluster) { return minAdvertisedVcpu(cluster); }
+    private double minRealVcpu(ClusterSpec.Type clusterType) { return minAdvertisedVcpu(clusterType); }
 
-    private double minRealMemoryGb(ClusterSpec cluster) {
-        return minAdvertisedMemoryGb(cluster) - 1.7;
+    private double minRealMemoryGb(ClusterSpec.Type clusterType) {
+        return minAdvertisedMemoryGb(clusterType) - 1.7;
     }
 
     private double minRealDiskGb() { return 6; }