Disallow incremental non-exclusive container allocation

Disallow shared allocation of containers also when there is
an existing non-exclusive host which can fit the requested node.

7 files changed, 45 insertions, 24 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
index 510c4041efb..5738c8f3945 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
@@ -205,7 +205,7 @@ public class NodeRepository extends AbstractComponent {
      */
     public boolean exclusiveAllocation(ClusterSpec clusterSpec) {
         return clusterSpec.isExclusive() ||
-               ( clusterSpec.type().isContainer() && zone.system().isPublic() &&  !zone.environment().isTest() ) ||
+               ( clusterSpec.type().isContainer() && zone.system().isPublic() && !zone.environment().isTest() ) ||
                ( !zone().cloud().allowHostSharing() && !sharedHosts.value().isEnabled(clusterSpec.type().name()));
     }
 
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 a2ef76e84d0..f0c20137894 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
@@ -188,6 +188,7 @@ public class AllocatableClusterResources {
 
             if ( ! systemLimits.isWithinRealLimits(allocatableResources.realResources, applicationId, clusterSpec))
                 return Optional.empty();
+            System.out.println("  Any satisfies " + allocatableResources.realResources + "?");
             if ( ! anySatisfies(allocatableResources.realResources, availableRealHostResources))
                 return Optional.empty();
             return Optional.of(allocatableResources);
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 b56e8d1b247..f344bcff746 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
@@ -48,6 +48,7 @@ public class AllocationOptimizer {
                                          : nodeRepository.nodes().list().hosts().stream().map(host -> host.flavor().resources())
                                                          .map(hostResources -> maxResourcesOf(hostResources, clusterModel))
                                                          .toList();
+        System.out.println("Dynamic provisioning: " + (nodeRepository.zone().cloud().dynamicProvisioning()) + ". Finding best allocation given host resources " + availableRealHostResources);
         for (int groups = limits.min().groups(); groups <= limits.max().groups(); groups++) {
             for (int nodes = limits.min().nodes(); nodes <= limits.max().nodes(); nodes++) {
                 if (nodes % groups != 0) continue;
@@ -68,6 +69,7 @@ public class AllocationOptimizer {
                 }
             }
         }
+        System.out.println("... best allocation: " + bestAllocation);
         return bestAllocation;
     }
 
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 c6971f0fe02..175af185e72 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
@@ -194,27 +194,34 @@ class NodeAllocation {
         return false;
     }
 
+    /**
+     * Returns whether allocating the candidate on this host would violate exclusivity constraints.
+     * Note that while we currently require that exclusive allocations uses the entire host,
+     * this method also handles the case where smaller exclusive nodes are allocated on it.
+     */
     private boolean violatesExclusivity(NodeCandidate candidate) {
-        if (candidate.parentHostname().isEmpty()) return false;
-
-        // In nodes which does not allow host sharing, exclusivity is violated if...
-        if ( ! nodeRepository.zone().cloud().allowHostSharing()) {
-            // TODO: Write this in a way that is simple to read
-            // If either the parent is dedicated to a cluster type different from this cluster
-            return  ! candidate.parent.flatMap(Node::exclusiveToClusterType).map(cluster.type()::equals).orElse(true) ||
-                    // or this cluster is requiring exclusivity, but the host is exclusive to a different owner
-                    (requestedNodes.isExclusive() && !candidate.parent.flatMap(Node::exclusiveToApplicationId).map(application::equals).orElse(false));
+        if (candidate.parent.isEmpty()) return false;
+
+        if (nodeRepository.exclusiveAllocation(cluster)) {
+            // Node must allocate the host entirely and not violate application or cluster type constraints
+            var parent = candidate.parent.get();
+            if (!candidate.resources().isUnspecified() &&
+                ! nodeRepository.resourcesCalculator().advertisedResourcesOf(parent.flavor()).compatibleWith(candidate.resources())) return true;
+            if (parent.exclusiveToApplicationId().isPresent() && !parent.exclusiveToApplicationId().get().equals(application)) return true;
+            if (parent.exclusiveToClusterType().isPresent() && !parent.exclusiveToClusterType().get().equals(cluster.type())) return true;
+            return false;
         }
-
-        // In zones with shared hosts we require that if either of the nodes on the host requires exclusivity,
-        // then all the nodes on the host must have the same owner
-        for (Node nodeOnHost : allNodes.childrenOf(candidate.parentHostname().get())) {
-            if (nodeOnHost.allocation().isEmpty()) continue;
-            if (requestedNodes.isExclusive() || nodeOnHost.allocation().get().membership().cluster().isExclusive()) {
-                if ( ! nodeOnHost.allocation().get().owner().equals(application)) return true;
+        else {
+            // If any of the nodes on the host requires exclusivity to another application, allocating on it is a violation
+            for (Node nodeOnHost : allNodes.childrenOf(candidate.parentHostname().get())) {
+                if (nodeOnHost.allocation().isEmpty()) continue;
+                if (requestedNodes.isExclusive() || nodeOnHost.allocation().get().membership().cluster().isExclusive()) {
+                    if (!nodeOnHost.allocation().get().owner().equals(application))
+                        return true;
+                }
             }
+            return false;
         }
-        return false;
     }
 
     /**
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java
index 24ea9361823..6efb9b6f4aa 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java
@@ -47,7 +47,7 @@ public class MockHostProvisioner implements HostProvisioner {
 
     private int deprovisionedHosts = 0;
     private EnumSet<Behaviour> behaviours = EnumSet.noneOf(Behaviour.class);
-    private Map<ClusterSpec.Type, Flavor> hostFlavors = new HashMap<>();
+    private final Map<ClusterSpec.Type, Flavor> hostFlavors = new HashMap<>();
 
     public MockHostProvisioner(List<Flavor> flavors, MockNameResolver nameResolver, int memoryTaxGb) {
         this.flavors = List.copyOf(flavors);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
index 0a614cc9b2b..51eeb8717f5 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
@@ -97,10 +97,10 @@ public class MockNodeRepository extends NodeRepository {
         defaultCloudAccount = zone.cloud().account();
 
         curator.setZooKeeperEnsembleConnectionSpec("cfg1:1234,cfg2:1234,cfg3:1234");
-        populate();
+        populate(zone);
     }
 
-    private void populate() {
+    private void populate(Zone zone) {
         NodeRepositoryProvisioner provisioner = new NodeRepositoryProvisioner(this, Zone.defaultZone(), new MockProvisionServiceProvider());
         List<Node> nodes = new ArrayList<>();
 
@@ -202,16 +202,26 @@ public class MockNodeRepository extends NodeRepository {
                                             .vespaVersion("6.42")
                                             .loadBalancerSettings(new ZoneEndpoint(false, true, List.of(new AllowedUrn(AccessType.awsPrivateLink, "arne"))))
                                             .build();
+        ClusterResources min, max;
+        if (zone.system().isPublic()) { // resources must match one of the flavors used in this mock ("large"), since this is a container cluster
+            min = new ClusterResources(2, 1, new NodeResources(4, 32, 1600, 1));
+            max = new ClusterResources(8, 2, new NodeResources(8, 64, 3200, 1));
+        }
+        else { // resources must fit on actually provisioned hosts
+            min = new ClusterResources(2, 1, new NodeResources(2, 8, 50, 1));
+            max = new ClusterResources(8, 2, new NodeResources(4, 16, 1000, 1));
+        }
+
         activate(provisioner.prepare(app1Id,
                                      cluster1Id,
-                                     Capacity.from(new ClusterResources(2, 1, new NodeResources(2, 8, 50, 1)),
-                                                   new ClusterResources(8, 2, new NodeResources(4, 16, 1000, 1)),
+                                     Capacity.from(min,
+                                                   max,
                                                    IntRange.empty(),
                                                    false,
                                                    true,
                                                    Optional.empty(),
                                                    ClusterInfo.empty()),
-                                null), app1Id, provisioner);
+                                     null), app1Id, provisioner);
         Application app1 = applications().get(app1Id).get();
         Cluster cluster1 = app1.cluster(cluster1Id.id()).get();
         cluster1 = cluster1.withSuggested(new Autoscaling(Autoscaling.Status.unavailable,
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisionServiceProvider.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisionServiceProvider.java
index 81947251e64..a8be7ac3af4 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisionServiceProvider.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisionServiceProvider.java
@@ -50,4 +50,5 @@ public class MockProvisionServiceProvider implements ProvisionServiceProvider {
     public HostResourcesCalculator getHostResourcesCalculator() {
         return hostResourcesCalculator;
     }
+
 }