Move violatesExclusivity to NodeCandidate

3 files changed, 93 insertions, 29 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 87e3bea1af9..9f6d4f159f6 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
@@ -117,21 +117,21 @@ class NodeAllocation {
                 ClusterMembership membership = allocation.membership();
                 if ( ! allocation.owner().equals(application)) continue; // wrong application
                 if ( ! membership.cluster().satisfies(cluster)) continue; // wrong cluster id/type
-                if ( candidate.state() == Node.State.active && allocation.removable()) continue; // don't accept; causes removal
-                if ( candidate.state() == Node.State.active && candidate.wantToFail()) continue; // don't accept; causes failing
-                if ( indexes.contains(membership.index())) continue; // duplicate index (just to be sure)
+                if (candidate.state() == Node.State.active && allocation.removable()) continue; // don't accept; causes removal
+                if (candidate.state() == Node.State.active && candidate.wantToFail()) continue; // don't accept; causes failing
+                if (indexes.contains(membership.index())) continue; // duplicate index (just to be sure)
                 if (nodeRepository.zone().cloud().allowEnclave() && candidate.parent.isPresent() && ! candidate.parent.get().cloudAccount().equals(requested.cloudAccount())) continue; // wrong account
 
                 boolean resizeable = requested.considerRetiring() && candidate.isResizable;
 
-                if ((! saturated() && hasCompatibleResources(candidate) && requested.acceptable(candidate)) || acceptIncompatible(candidate)) {
+                if (( ! saturated() && hasCompatibleResources(candidate) && requested.acceptable(candidate)) || acceptIncompatible(candidate)) {
                     candidate = candidate.withNode();
                     if (candidate.isValid())
                         acceptNode(candidate, shouldRetire(candidate, candidates), resizeable);
                 }
             }
-            else if (! saturated() && hasCompatibleResources(candidate)) {
-                if (! nodeRepository.nodeResourceLimits().isWithinRealLimits(candidate, application, cluster)) {
+            else if ( ! saturated() && hasCompatibleResources(candidate)) {
+                if ( ! nodeRepository.nodeResourceLimits().isWithinRealLimits(candidate, application, cluster)) {
                     ++rejectedDueToInsufficientRealResources;
                     continue;
                 }
@@ -196,29 +196,8 @@ class NodeAllocation {
     }
 
     private boolean violatesExclusivity(NodeCandidate candidate) {
-        if (candidate.parentHostname().isEmpty()) return false;
-        if (requested.type() != NodeType.tenant) return false;
-
-        // 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 the parent is dedicated to a different application
-                    ! candidate.parent.flatMap(Node::exclusiveToApplicationId).map(application::equals).orElse(true) ||
-                    // or this cluster requires exclusivity, but the host is not exclusive
-                    (nodeRepository.exclusiveAllocation(cluster) && candidate.parent.flatMap(Node::exclusiveToApplicationId).isEmpty());
-        }
-
-        // 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 (nodeRepository.exclusiveAllocation(cluster) || nodeOnHost.allocation().get().membership().cluster().isExclusive()) {
-                if ( ! nodeOnHost.allocation().get().owner().equals(application)) return true;
-            }
-        }
-        return false;
+        return candidate.violatesExclusivity(cluster, application, nodeRepository.exclusiveAllocation(cluster),
+                                             nodeRepository.zone().cloud().allowHostSharing(), allNodes);
     }
 
     /**
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
index 1efc35b416d..5ab7e032dc5 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
@@ -4,11 +4,13 @@ package com.yahoo.vespa.hosted.provision.provisioning;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterMembership;
+import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.provision.LockedNodeList;
 import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeList;
 import com.yahoo.vespa.hosted.provision.Nodelike;
 import com.yahoo.vespa.hosted.provision.node.Allocation;
 import com.yahoo.vespa.hosted.provision.node.IP;
@@ -20,6 +22,8 @@ import java.util.Objects;
 import java.util.Optional;
 import java.util.logging.Logger;
 
+import static com.yahoo.collections.Optionals.emptyOrEqual;
+
 /**
  * A node candidate containing the details required to prioritize it for allocation. This is immutable.
  *
@@ -559,4 +563,31 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat
 
     }
 
+    public boolean violatesExclusivity(ClusterSpec cluster, ApplicationId application,
+                                       boolean exclusiveCluster, boolean hostSharing, NodeList allNodes) {
+        if (parentHostname().isEmpty()) return false;
+        if (type() != NodeType.tenant) return false;
+
+        // In zones which do not allow host sharing, exclusivity is violated if...
+        if ( ! hostSharing) {
+            // If either the parent is dedicated to a cluster type different from this cluster
+            return ! emptyOrEqual(parent.flatMap(Node::exclusiveToClusterType), cluster.type()) ||
+                   // or the parent is dedicated to a different application
+                   ! emptyOrEqual(parent.flatMap(Node::exclusiveToApplicationId), application) ||
+                   // or this cluster requires exclusivity, but the host is not exclusive (to this, implicitly by the above).
+                   exclusiveCluster && parent.flatMap(Node::exclusiveToApplicationId).isEmpty();
+        }
+
+        // In zones with shared hosts we require that if any node on the host requires exclusivity,
+        // then all the nodes on the host must have the same owner.
+        for (Node nodeOnHost : allNodes.childrenOf(parentHostname().get())) {
+            if (nodeOnHost.allocation().isEmpty()) continue;
+            if (exclusiveCluster || nodeOnHost.allocation().get().membership().cluster().isExclusive()) {
+                if ( ! nodeOnHost.allocation().get().owner().equals(application)) return true;
+            }
+        }
+        return false;
+
+    }
+
 }
diff --git a/vespajlib/src/main/java/com/yahoo/collections/Optionals.java b/vespajlib/src/main/java/com/yahoo/collections/Optionals.java
new file mode 100644
index 00000000000..27a1e837900
--- /dev/null
+++ b/vespajlib/src/main/java/com/yahoo/collections/Optionals.java
@@ -0,0 +1,54 @@
+package com.yahoo.collections;
+
+import com.google.common.collect.Comparators;
+
+import java.util.Optional;
+import java.util.function.BinaryOperator;
+
+/**
+ * @author jonmv
+ */
+public class Optionals {
+
+    private Optionals() { }
+
+    /** Returns the first non-empty optional, or empty if all are empty. */
+    @SafeVarargs
+    public static <T> Optional<T> firstNonEmpty(Optional<T>... optionals) {
+        for (Optional<T> optional : optionals)
+            if (optional.isPresent())
+                return optional;
+        return Optional.empty();
+    }
+
+    /** Returns the non-empty optional with the lowest value, or empty if all are empty. */
+    @SafeVarargs
+    public static <T extends Comparable<T>> Optional<T> min(Optional<T>... optionals) {
+        Optional<T> best = Optional.empty();
+        for (Optional<T> optional : optionals)
+            if (best.isEmpty() || optional.isPresent() && optional.get().compareTo(best.get()) < 0)
+                best = optional;
+        return best;
+    }
+
+    /** Returns the non-empty optional with the highest value, or empty if all are empty. */
+    @SafeVarargs
+    public static <T extends Comparable<T>> Optional<T> max(Optional<T>... optionals) {
+        Optional<T> best = Optional.empty();
+        for (Optional<T> optional : optionals)
+            if (best.isEmpty() || optional.isPresent() && optional.get().compareTo(best.get()) > 0)
+                best = optional;
+        return best;
+    }
+
+    /** Returns whether either optional is empty, or both are present and equal. */
+    public static <T> boolean equalIfBothPresent(Optional<T> first, Optional<T> second) {
+        return first.isEmpty() || second.isEmpty() || first.equals(second);
+    }
+
+    /** Returns whether the optional is empty, or present and equal to the given value. */
+    public static <T> boolean emptyOrEqual(Optional<T> optional, T value) {
+        return optional.isEmpty() || optional.equals(Optional.of(value));
+    }
+
+}