Correctly check for exclusivity violation in dynamically provisioned zones

2 files changed, 33 insertions, 5 deletions

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 b202a10c4d3..5e48da9f291 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
@@ -204,20 +204,20 @@ class NodeAllocation {
     private boolean violatesExclusivity(NodeCandidate candidate) {
         if (candidate.parentHostname().isEmpty()) return false;
 
-        // In nodes which does not allow host sharing, exclusivity is violated if...
+        // In zones 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));
+                    (nodeRepository.exclusiveAllocation(cluster) && !candidate.parent.flatMap(Node::exclusiveToApplicationId).map(application::equals).orElse(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 (nodeRepository.exclusiveAllocation(cluster) || nodeOnHost.allocation().get().membership().cluster().isExclusive()) {
                 if ( ! nodeOnHost.allocation().get().owner().equals(application)) return true;
             }
         }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
index ced29b28d41..7dbed807480 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
@@ -3,8 +3,10 @@ package com.yahoo.vespa.hosted.provision.provisioning;
 
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Capacity;
+import com.yahoo.config.provision.Cloud;
 import com.yahoo.config.provision.ClusterResources;
 import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.HostSpec;
 import com.yahoo.config.provision.NodeFlavors;
@@ -13,8 +15,13 @@ import com.yahoo.config.provision.NodeResources.Architecture;
 import com.yahoo.config.provision.NodeResources.DiskSpeed;
 import com.yahoo.config.provision.NodeResources.StorageType;
 import com.yahoo.config.provision.NodeType;
+import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.flags.PermanentFlags;
+import com.yahoo.vespa.flags.custom.HostResources;
+import com.yahoo.vespa.flags.custom.SharedHost;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.Node.State;
 import com.yahoo.vespa.hosted.provision.NodeList;
@@ -163,7 +170,7 @@ public class DynamicProvisioningTest {
 
     @Test
     public void avoids_allocating_to_empty_hosts() {
-        var tester = tester(false);
+        var tester = tester(true);
         tester.makeReadyHosts(6, new NodeResources(12, 12, 200, 12));
         tester.activateTenantHosts();
 
@@ -185,6 +192,27 @@ public class DynamicProvisioningTest {
     }
 
     @Test
+    public void does_not_allocate_container_nodes_to_shared_hosts() {
+        assertHostSharing(Environment.prod, ClusterSpec.Type.container, false);
+        assertHostSharing(Environment.prod, ClusterSpec.Type.content, true);
+        assertHostSharing(Environment.staging, ClusterSpec.Type.container, true);
+        assertHostSharing(Environment.staging, ClusterSpec.Type.content, true);
+    }
+
+    private void assertHostSharing(Environment environment, ClusterSpec.Type clusterType, boolean expectShared) {
+        Zone zone = new Zone(Cloud.builder().dynamicProvisioning(true).allowHostSharing(false).build(), SystemName.Public, environment, RegionName.defaultName());
+        MockHostProvisioner hostProvisioner = new MockHostProvisioner(new NodeFlavors(ProvisioningTester.createConfig()).getFlavors(), nameResolver, 0);
+        ProvisioningTester tester = new ProvisioningTester.Builder().zone(zone).hostProvisioner(hostProvisioner).nameResolver(nameResolver).build();
+        tester.makeReadyHosts(2, new NodeResources(12, 12, 200, 12));
+        tester.flagSource().withJacksonFlag(PermanentFlags.SHARED_HOST.id(), new SharedHost(List.of(new HostResources(4.0, 16.0, 50.0, 0.3, "fast", "local", null, 10, "x86_64"))), SharedHost.class);
+
+        ApplicationId application = ProvisioningTester.applicationId();
+        ClusterSpec cluster = ClusterSpec.request(clusterType, ClusterSpec.Id.from("default")).vespaVersion("6.42").build();
+        tester.prepare(application, cluster, 2, 1, new NodeResources(2., 10., 20, 1));
+        assertEquals(expectShared ? 2 : 4, tester.nodeRepository().nodes().list().nodeType(NodeType.host).size());
+    }
+
+    @Test
     public void retires_on_exclusivity_violation() {
         var tester = tester(true);
         ApplicationId application1 = ProvisioningTester.applicationId();
@@ -207,7 +235,7 @@ public class DynamicProvisioningTest {
 
     @Test
     public void node_indices_are_unique_even_when_a_node_is_left_in_reserved_state() {
-        var tester = tester(false);
+        var tester = tester(true);
         NodeResources resources = new NodeResources(10, 10, 10, 10);
         ApplicationId app = ProvisioningTester.applicationId();