Simplify, and fix some bugs

5 files changed, 24 insertions, 23 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
index f0848f5caeb..5e01ba5b0a6 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
@@ -105,7 +105,7 @@ public class GroupPreparer {
                                                           indices::next, wantedGroups, allNodesAndHosts);
             NodeType hostType = allocation.nodeType().hostType();
             if (canProvisionDynamically(hostType) && allocation.hostDeficit().isPresent()) {
-                HostSharing sharing = hostSharing(requestedNodes, hostType);
+                HostSharing sharing = hostSharing(cluster, hostType);
                 Version osVersion = nodeRepository.osVersions().targetFor(hostType).orElse(Version.emptyVersion);
                 NodeAllocation.HostDeficit deficit = allocation.hostDeficit().get();
 
@@ -125,7 +125,8 @@ public class GroupPreparer {
                 try {
                     hostProvisioner.get().provisionHosts(
                             allocation.provisionIndices(deficit.count()), hostType, deficit.resources(), application,
-                            osVersion, sharing, Optional.of(cluster.type()), requestedNodes.cloudAccount(), provisionedHostsConsumer);
+                            osVersion, sharing, Optional.of(cluster.type()), requestedNodes.cloudAccount(),
+                            provisionedHostsConsumer);
                 } catch (NodeAllocationException e) {
                     // Mark the nodes that were written to ZK in the consumer for deprovisioning. While these hosts do
                     // not exist, we cannot remove them from ZK here because other nodes may already have been
@@ -173,12 +174,11 @@ public class GroupPreparer {
                (hostType == NodeType.host || hostType.isConfigServerHostLike());
     }
 
-    private static HostSharing hostSharing(NodeSpec spec, NodeType hostType) {
-        HostSharing sharing = spec.isExclusive() ? HostSharing.exclusive : HostSharing.any;
-        if (!hostType.isSharable() && sharing != HostSharing.any) {
-            throw new IllegalArgumentException(hostType + " does not support sharing requirement");
-        }
-        return sharing;
+    private HostSharing hostSharing(ClusterSpec cluster, NodeType hostType) {
+        if ( hostType.isSharable())
+            return nodeRepository.exclusiveAllocation(cluster) ? HostSharing.exclusive : HostSharing.any;
+        else
+            return HostSharing.any;
     }
 
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java
index 95d10557e3a..53df27c312b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java
@@ -10,6 +10,7 @@ import com.yahoo.config.provision.NodeAllocationException;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeRepository;
 
 import java.util.List;
 import java.util.Optional;
@@ -47,7 +48,8 @@ public interface HostProvisioner {
      * @param osVersion the OS version to use. If this version does not exist, implementations may choose a suitable
      *                  fallback version.
      * @param sharing puts requirements on sharing or exclusivity of the host to be provisioned.
-     * @param clusterType provision host exclusively for this cluster type
+     * @param cluster the cluster we are provisioning for, or empty if we are provisioning hosts
+     *                to be shared by multiple cluster nodes
      * @param cloudAccount the cloud account to use
      * @param provisionedHostConsumer consumer of {@link ProvisionedHost}s describing the provisioned nodes,
      *                                the {@link Node} returned from {@link ProvisionedHost#generateHost()} must be
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 21efd1b014c..8d350e304a2 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
@@ -127,9 +127,8 @@ class NodeAllocation {
 
                 if ((! saturated() && hasCompatibleResources(candidate) && requestedNodes.acceptable(candidate)) || acceptToRetire) {
                     candidate = candidate.withNode();
-                    if (candidate.isValid()) {
+                    if (candidate.isValid())
                         acceptNode(candidate, shouldRetire(candidate, candidates), resizeable);
-                    }
                 }
             }
             else if (! saturated() && hasCompatibleResources(candidate)) {
@@ -220,7 +219,7 @@ class NodeAllocation {
 
     /**
      * Returns whether this node should be accepted into the cluster even if it is not currently desired
-     * (already enough nodes, or wrong flavor).
+     * (already enough nodes, or wrong resources, etc.).
      * Such nodes will be marked retired during finalization of the list of accepted nodes.
      * The conditions for this are:
      *
@@ -262,8 +261,9 @@ class NodeAllocation {
                 || ! ( requestedNodes.needsResize(node) && node.allocation().get().membership().retired()))
                 acceptedWithoutResizingRetired++;
 
-            if (resizeable && ! ( node.allocation().isPresent() && node.allocation().get().membership().retired()))
+            if (resizeable && ! ( node.allocation().isPresent() && node.allocation().get().membership().retired())) {
                 node = resize(node);
+            }
 
             if (node.state() != Node.State.active) // reactivated node - wipe state that deactivated it
                 node = node.unretire().removable(false);
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 1525dbe1008..59c089943ab 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
@@ -154,6 +154,8 @@ public interface NodeSpec {
         @Override
         public boolean canResize(NodeResources currentNodeResources, NodeResources currentSpareHostResources,
                                  ClusterSpec.Type type, boolean hasTopologyChange, int currentClusterSize) {
+            if (exclusive) return false; // exclusive resources must match the host
+
             // Never allow in-place resize when also changing topology or decreasing cluster size
             if (hasTopologyChange || count < currentClusterSize) 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 e190f1c3bd2..15ee064b59f 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
@@ -46,21 +46,19 @@ public class MockHostProvisioner implements HostProvisioner {
     private int deprovisionedHosts = 0;
     private EnumSet<Behaviour> behaviours = EnumSet.noneOf(Behaviour.class);
     private Optional<Flavor> hostFlavor = Optional.empty();
-    private Cloud cloud;
 
-    public MockHostProvisioner(List<Flavor> flavors, MockNameResolver nameResolver, int memoryTaxGb, Cloud cloud) {
+    public MockHostProvisioner(List<Flavor> flavors, MockNameResolver nameResolver, int memoryTaxGb) {
         this.flavors = List.copyOf(flavors);
         this.nameResolver = nameResolver;
         this.memoryTaxGb = memoryTaxGb;
-        this.cloud = cloud;
     }
 
-    public MockHostProvisioner(List<Flavor> flavors, Cloud cloud) {
-        this(flavors, 0, cloud);
+    public MockHostProvisioner(List<Flavor> flavors) {
+        this(flavors, 0);
     }
 
-    public MockHostProvisioner(List<Flavor> flavors, int memoryTaxGb, Cloud cloud) {
-        this(flavors, new MockNameResolver().mockAnyLookup(), memoryTaxGb, cloud);
+    public MockHostProvisioner(List<Flavor> flavors, int memoryTaxGb) {
+        this(flavors, new MockNameResolver().mockAnyLookup(), memoryTaxGb);
     }
 
     @Override
@@ -68,10 +66,9 @@ public class MockHostProvisioner implements HostProvisioner {
                                ApplicationId applicationId, Version osVersion, HostSharing sharing,
                                Optional<ClusterSpec.Type> clusterType, CloudAccount cloudAccount,
                                Consumer<List<ProvisionedHost>> provisionedHostsConsumer) {
-        boolean exclusive = sharing == HostSharing.exclusive || ! cloud.allowHostSharing();
         Flavor hostFlavor = this.hostFlavor.orElseGet(() -> flavors.stream()
-                                                                   .filter(f -> exclusive ? compatible(f, resources)
-                                                                                          : f.resources().satisfies(resources))
+                                                                   .filter(f -> sharing == HostSharing.exclusive ? compatible(f, resources)
+                                                                                                                 : f.resources().satisfies(resources))
                                                                    .findFirst()
                                                                    .orElseThrow(() -> new NodeAllocationException("No host flavor matches " + resources, true)));
         List<ProvisionedHost> hosts = new ArrayList<>();