Merge pull request #24880 from vespa-engine/bratseth/allow-host-sharing

Separate dynamic provisioning and host sharing

17 files changed, 62 insertions, 36 deletions

diff --git a/config-model/src/main/java/com/yahoo/schema/document/Matching.java b/config-model/src/main/java/com/yahoo/schema/document/Matching.java
index f70f31be0bd..1fe947d672b 100644
--- a/config-model/src/main/java/com/yahoo/schema/document/Matching.java
+++ b/config-model/src/main/java/com/yahoo/schema/document/Matching.java
@@ -24,12 +24,12 @@ public class Matching implements Cloneable, Serializable {
     private boolean algorithmUserSet = false;
 
     /** The gram size is the n in n-gram, or -1 if not set. Should only be set with gram matching. */
-    private int gramSize=-1;
+    private int gramSize = -1;
 
     /** Maximum number of characters to consider when searching in this field. Used for limiting resources, especially in streaming search. */
     private Integer maxLength;
 
-    private String exactMatchTerminator=null;
+    private String exactMatchTerminator = null;
 
     /** Creates a matching of type "text" */
     public Matching() {}
@@ -122,9 +122,8 @@ public class Matching implements Cloneable, Serializable {
 
     @Override
     public boolean equals(Object o) {
-        if (! (o instanceof Matching)) return false;
+        if (! (o instanceof Matching other)) return false;
 
-        Matching other=(Matching)o;
         if ( ! other.type.equals(this.type)) return false;
         if ( ! other.algorithm.equals(this.algorithm)) return false;
         if ( this.exactMatchTerminator == null && other.exactMatchTerminator != null) return false;
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Cloud.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Cloud.java
index b9f56ab02a7..801e6e3fe75 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/Cloud.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Cloud.java
@@ -13,12 +13,15 @@ public class Cloud {
     private final CloudName name;
 
     private final boolean dynamicProvisioning;
+    private final boolean allowHostSharing;
     private final boolean requireAccessControl;
     private final CloudAccount account;
 
-    private Cloud(CloudName name, boolean dynamicProvisioning, boolean requireAccessControl, CloudAccount account) {
+    private Cloud(CloudName name, boolean dynamicProvisioning, boolean allowHostSharing, boolean requireAccessControl,
+                  CloudAccount account) {
         this.name = Objects.requireNonNull(name);
         this.dynamicProvisioning = dynamicProvisioning;
+        this.allowHostSharing = allowHostSharing;
         this.requireAccessControl = requireAccessControl;
         this.account = Objects.requireNonNull(account);
         if (name.equals(CloudName.AWS) && account.isUnspecified()) {
@@ -36,6 +39,9 @@ public class Cloud {
         return dynamicProvisioning;
     }
 
+    /** Returns whether this allows host sharing */
+    public boolean allowHostSharing() { return allowHostSharing; }
+
     /** Returns whether to require access control for all clusters in this */
     public boolean requireAccessControl() {
         return requireAccessControl;
@@ -59,6 +65,7 @@ public class Cloud {
 
         private CloudName name = CloudName.DEFAULT;
         private boolean dynamicProvisioning = false;
+        private boolean allowHostSharing = true;
         private boolean requireAccessControl = false;
         private CloudAccount account = CloudAccount.empty;
 
@@ -74,6 +81,11 @@ public class Cloud {
             return this;
         }
 
+        public Builder allowHostSharing(boolean allowHostSharing) {
+            this.allowHostSharing = allowHostSharing;
+            return this;
+        }
+
         public Builder requireAccessControl(boolean requireAccessControl) {
             this.requireAccessControl = requireAccessControl;
             return this;
@@ -85,7 +97,7 @@ public class Cloud {
         }
 
         public Cloud build() {
-            return new Cloud(name, dynamicProvisioning, requireAccessControl, account);
+            return new Cloud(name, dynamicProvisioning, allowHostSharing, requireAccessControl, account);
         }
 
     }
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java
index b1a9ecac394..97234056705 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java
@@ -26,6 +26,7 @@ public class Zone {
         this(Cloud.builder()
                   .name(CloudName.from(configserverConfig.cloud()))
                   .dynamicProvisioning(cloudConfig.dynamicProvisioning())
+                  .allowHostSharing(cloudConfig.allowHostSharing())
                   .requireAccessControl(cloudConfig.requireAccessControl())
                   .account(CloudAccount.from(cloudConfig.account()))
                   .build(),
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 5e923f524f7..0bf32e534b7 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
@@ -197,7 +197,7 @@ public class NodeRepository extends AbstractComponent {
      * perfectly.
      */
     public boolean exclusiveAllocation(ClusterSpec clusterSpec) {
-        return clusterSpec.isExclusive() || zone().cloud().dynamicProvisioning();
+        return clusterSpec.isExclusive() || ! zone().cloud().allowHostSharing();
     }
 
     /**
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirer.java
index 517458afd00..8766dea3d61 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirer.java
@@ -27,7 +27,7 @@ public class DirtyExpirer extends Expirer {
 
     DirtyExpirer(NodeRepository nodeRepository, Duration dirtyTimeout, Metric metric) {
         super(Node.State.dirty, History.Event.Type.deallocated, nodeRepository, dirtyTimeout, metric);
-        // Deprovision hosts in dynamically provisioned on expiry
+        // Deprovision hosts on expiry if dynamically provisioned
         this.wantToDeprovisionOnExpiry = nodeRepository.zone().cloud().dynamicProvisioning();
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java
index 8f8a332c86a..c853bfa2abe 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java
@@ -37,7 +37,7 @@ public class Rebalancer extends NodeMover<Rebalancer.Move> {
     protected double maintain() {
         if ( ! nodeRepository().nodes().isWorking()) return 0.0;
 
-        if (nodeRepository().zone().cloud().dynamicProvisioning()) return 1.0; // Rebalancing not necessary
+        if ( ! nodeRepository().zone().cloud().allowHostSharing()) return 1.0; // Rebalancing not necessary
         if (nodeRepository().zone().environment().isTest()) return 1.0; // Short lived deployments; no need to rebalance
         if (nodeRepository().zone().system().isCd()) return 1.0; // CD tests assert on # of nodes, avoid rebalnacing as it make tests unstable
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
index 7a3acd18cd6..1e2ecb98a42 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
@@ -498,7 +498,7 @@ public class Nodes {
         try (Mutex lock = lockUnallocated()) {
             requireRemovable(node, false, force);
             NestedTransaction transaction = new NestedTransaction();
-            final List<Node> removed;
+            List<Node> removed;
             if (!node.type().isHost()) {
                 removed = List.of(node);
                 db.removeNodes(removed, transaction);
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 aded92fdb30..8d6c6b4bb62 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
@@ -69,7 +69,7 @@ public class CapacityPolicies {
         if (target.isUnspecified()) return target; // Cannot be modified
 
         // Dev does not cap the cpu or network of containers since usage is spotty: Allocate just a small amount exclusively
-        if (zone.environment() == Environment.dev && !zone.cloud().dynamicProvisioning())
+        if (zone.environment() == Environment.dev && zone.cloud().allowHostSharing())
             target = target.withVcpu(0.1).withBandwidthGbps(0.1);
 
         // Allow slow storage in zones which are not performance sensitive
@@ -120,7 +120,7 @@ public class CapacityPolicies {
 
     /** 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().dynamicProvisioning() && (exclusive || !sharedHosts.value().isEnabled(type.name()));
+        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. */
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 fe634a77997..46dd88b78d0 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
@@ -199,15 +199,16 @@ class NodeAllocation {
     private boolean violatesExclusivity(NodeCandidate candidate) {
         if (candidate.parentHostname().isEmpty()) return false;
 
-        // In dynamic provisioned zones, exclusivity is violated if...
-        if (nodeRepository.zone().cloud().dynamicProvisioning()) {
+        // 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));
         }
 
-        // In non-dynamic provisioned zones we require that if either of the nodes on the host requires exclusivity,
+        // 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 : allNodesAndHosts.childrenOf(candidate.parentHostname().get())) {
             if (nodeOnHost.allocation().isEmpty()) continue;
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 aa620630b3a..b4c01267d3b 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
@@ -260,7 +260,7 @@ public class NodeRepositoryProvisioner implements Provisioner {
     private IllegalArgumentException newNoAllocationPossible(ClusterSpec spec, Limits limits) {
         StringBuilder message = new StringBuilder("No allocation possible within ").append(limits);
 
-        boolean exclusiveHosts = spec.isExclusive() || nodeRepository.zone().cloud().dynamicProvisioning();
+        boolean exclusiveHosts = spec.isExclusive() || ! nodeRepository.zone().cloud().allowHostSharing();
         if (exclusiveHosts)
             message.append(". Nearest allowed node resources: ").append(findNearestNodeResources(limits));
 
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
index bff963c3fec..81dd852e2a1 100644
--- 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
@@ -62,7 +62,7 @@ public class NodeResourceLimits {
     }
 
     private double minAdvertisedVcpu(ClusterSpec.Type clusterType) {
-        if (zone().environment() == Environment.dev && !zone().cloud().dynamicProvisioning()) return 0.1;
+        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;
@@ -79,7 +79,7 @@ public class NodeResourceLimits {
 
     // Note: Assumes node type 'host'
     private long reservedDiskSpaceGb(NodeResources.StorageType storageType, boolean exclusive) {
-        if (storageType == NodeResources.StorageType.local && zone().cloud().dynamicProvisioning())
+        if (storageType == NodeResources.StorageType.local && ! zone().cloud().allowHostSharing())
             return nodeRepository.resourcesCalculator().reservedDiskSpaceInBase2Gb(NodeType.host, ! exclusive);
         else
             return 4;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java
index 4ff3342a6c6..a2f5eabf447 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java
@@ -25,10 +25,7 @@ public interface NodeSpec {
     /** The node type this requests */
     NodeType type();
 
-    /**
-     * Returns whether the physical hosts running the nodes of this application can
-     * also run nodes of other applications.
-     */
+    /** Returns whether the hosts running the nodes of this application can also run nodes of other applications. */
     boolean isExclusive();
 
     /** Returns whether the given flavor is compatible with this spec */
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 6b15063d40e..4f3a3fd5afa 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
@@ -14,11 +14,9 @@ import com.yahoo.config.provision.NodeResources.StorageType;
 import static com.yahoo.config.provision.NodeResources.StorageType.remote;
 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.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.Nodelike;
-import com.yahoo.vespa.hosted.provision.autoscale.awsnodes.AwsNodeTypes;
 import com.yahoo.vespa.hosted.provision.provisioning.CapacityPolicies;
 import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator;
 import org.junit.Test;
@@ -64,7 +62,7 @@ public class AutoscalingTest {
     /** Using too many resources for a short period is proof we should scale up regardless of the time that takes. */
     @Test
     public void test_no_autoscaling_with_no_measurements() {
-        var fixture = AutoscalingTester.fixture().awsProdSetup().build();
+        var fixture = AutoscalingTester.fixture().awsProdSetup(false).build();
         assertTrue(fixture.autoscale().target().isEmpty());
     }
 
@@ -117,7 +115,7 @@ public class AutoscalingTest {
         var max = new ClusterResources(4, 1, new NodeResources(16, 32, 50, 0.3));
         var fixture = AutoscalingTester.fixture(min, now, max)
                                        .clusterType(ClusterSpec.Type.container)
-                                       .awsProdSetup()
+                                       .awsProdSetup(false)
                                        .build();
         var duration = fixture.loader().addMeasurements(new Load(0.04, 0.39, 0.01), 20);
         fixture.tester().clock().advance(duration.negated());
@@ -289,6 +287,7 @@ public class AutoscalingTest {
         var remote = new NodeResources(3, 100,  50, 1, fast, StorageType.remote);
         var fixture = AutoscalingTester.fixture()
                                        .dynamicProvisioning(true)
+                                       .allowHostSharing(false)
                                        .clusterType(ClusterSpec.Type.container)
                                        .hostFlavors(local, remote)
                                        .capacity(Capacity.from(resources))
@@ -467,6 +466,7 @@ public class AutoscalingTest {
         ClusterResources max = new ClusterResources(20, 1, new NodeResources(100, 1000, 1000, 1));
         var fixture = AutoscalingTester.fixture()
                                        .dynamicProvisioning(true)
+                                       .allowHostSharing(false)
                                        .hostFlavors(new NodeResources(3, 200, 100, 1, fast, remote),
                                                     new NodeResources(3, 150, 100, 1, fast, remote),
                                                     new NodeResources(3, 100, 100, 1, fast, remote),
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
index 254b03194ad..0eb65511299 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
@@ -205,6 +205,7 @@ class AutoscalingTester {
 
     public MetricsDb nodeMetricsDb() { return nodeRepository().metricsDb(); }
 
+    // TODO: Discontinue use of this
     public static class MockHostResourcesCalculator implements HostResourcesCalculator {
 
         private final Zone zone;
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java
index 0fb22445ab5..d348df00b78 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java
@@ -139,13 +139,23 @@ public class Fixture {
             return this;
         }
 
-        /**
-         * Set to true to behave as if hosts are provisioned dynamically,
-         * and must therefore be allocated completely to one tenant node.
-         */
-        public Fixture. Builder dynamicProvisioning(boolean dynamic) {
+        /** Set to true to behave as if hosts are provisioned dynamically. */
+        public Fixture. Builder dynamicProvisioning(boolean dynamicProvisioning) {
             this.zone = new Zone(Cloud.builder()
-                                      .dynamicProvisioning(dynamic)
+                                      .dynamicProvisioning(dynamicProvisioning)
+                                      .allowHostSharing(zone.cloud().allowHostSharing())
+                                      .build(),
+                                 zone.system(),
+                                 zone.environment(),
+                                 zone.region());
+            return this;
+        }
+
+        /** Set to true to allow multiple nodes be provisioned on the same host. */
+        public Fixture. Builder allowHostSharing(boolean allowHostSharing) {
+            this.zone = new Zone(Cloud.builder()
+                                      .dynamicProvisioning(zone.cloud().dynamicProvisioning())
+                                      .allowHostSharing(allowHostSharing)
                                       .build(),
                                  zone.system(),
                                  zone.environment(),
@@ -158,10 +168,12 @@ public class Fixture {
             return this;
         }
 
-        public Fixture.Builder awsProdSetup() {
+        public Fixture.Builder awsProdSetup(boolean allowHostSharing) {
              return this.awsHostFlavors()
                         .awsResourceCalculator()
-                        .zone(new Zone(Cloud.builder().dynamicProvisioning(true).build(),
+                        .zone(new Zone(Cloud.builder().dynamicProvisioning(true)
+                                                      .allowHostSharing(allowHostSharing)
+                                            .build(),
                                        SystemName.Public,
                                        Environment.prod,
                                        RegionName.from("aws-eu-west-1a")));
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisionerTest.java
index 715aa82afb0..18a756d4411 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisionerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisionerTest.java
@@ -40,7 +40,10 @@ public class HostResumeProvisionerTest {
     private final MockNameResolver nameResolver = new MockNameResolver();
     private final MockHostProvisioner hostProvisioner = new MockHostProvisioner(flavors, nameResolver, 0);
     private final ProvisioningTester tester = new ProvisioningTester.Builder()
-            .zone(new Zone(Cloud.builder().dynamicProvisioning(true).build(), SystemName.defaultSystem(), Environment.dev, RegionName.defaultName()))
+            .zone(new Zone(Cloud.builder().dynamicProvisioning(true).allowHostSharing(false).build(),
+                           SystemName.defaultSystem(),
+                           Environment.dev,
+                           RegionName.defaultName()))
             .hostProvisioner(hostProvisioner)
             .nameResolver(nameResolver)
             .flavors(flavors)
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 bd36a8f011b..55c41cae1d4 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
@@ -60,7 +60,7 @@ import static org.mockito.Mockito.when;
 public class DynamicProvisioningTest {
 
     private static final Zone zone = new Zone(
-            Cloud.builder().dynamicProvisioning(true).build(),
+            Cloud.builder().dynamicProvisioning(true).allowHostSharing(false).build(),
             SystemName.main,
             Environment.prod,
             RegionName.from("us-east"));