Mark new docker nodes as headroom violators on non-violated hosts

5 files changed, 69 insertions, 45 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java
index 77d91c7bea7..78ea258107b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java
@@ -65,12 +65,12 @@ public class DockerHostCapacity {
      * Checks the node capacity and free ip addresses to see
      * if we could allocate a flavor on the docker host.
      */
-    boolean hasCapacity(Node dockerHost, Flavor flavor) {
-        return freeCapacityOf(dockerHost, false).hasCapacityFor(flavor) && freeIPs(dockerHost) > 0;
+    boolean hasCapacity(Node dockerHost, ResourceCapacity requestedCapacity) {
+        return freeCapacityOf(dockerHost, false).hasCapacityFor(requestedCapacity) && freeIPs(dockerHost) > 0;
     }
 
-    boolean hasCapacityWhenRetiredAndInactiveNodesAreGone(Node dockerHost, Flavor flavor) {
-        return freeCapacityOf(dockerHost, true).hasCapacityFor(flavor) && freeIPs(dockerHost) > 0;
+    boolean hasCapacityWhenRetiredAndInactiveNodesAreGone(Node dockerHost, ResourceCapacity requestedCapacity) {
+        return freeCapacityOf(dockerHost, true).hasCapacityFor(requestedCapacity) && freeIPs(dockerHost) > 0;
     }
 
     /**
@@ -105,7 +105,7 @@ public class DockerHostCapacity {
     public long getNofHostsAvailableFor(Flavor flavor) {
         return allNodes.asList().stream()
                 .filter(n -> n.type().equals(NodeType.host))
-                .filter(n -> hasCapacity(n, flavor))
+                .filter(n -> hasCapacity(n, ResourceCapacity.of(flavor)))
                 .count();
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
index 960d0b9d729..eb7c8ab7f3c 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
@@ -39,10 +39,10 @@ public class NodePrioritizer {
     private final ApplicationId appId;
     private final ClusterSpec clusterSpec;
 
+    private final boolean isDocker;
     private final boolean isAllocatingForReplacement;
     private final Set<Node> spareHosts;
-    private final Map<Node, Boolean> headroomHosts;
-    private final boolean isDocker;
+    private final Map<Node, ResourceCapacity> headroomHosts;
 
     NodePrioritizer(List<Node> allNodes, ApplicationId appId, ClusterSpec clusterSpec, NodeSpec nodeSpec, NodeFlavors nodeFlavors, int spares) {
         this.allNodes = Collections.unmodifiableList(allNodes);
@@ -104,14 +104,14 @@ public class NodePrioritizer {
     }
 
     /**
-     * Headroom are the nodes with the least but sufficient space for the requested headroom.
+     * Headroom hosts are the host with the least but sufficient capacity for the requested headroom.
      *
-     * If not enough headroom - the headroom violating hosts are the once that are closest to fulfull
+     * If not enough headroom - the headroom violating hosts are the once that are closest to fulfill
      * a headroom request.
      */
-    private static Map<Node, Boolean> findHeadroomHosts(List<Node> nodes, Set<Node> spareNodes, NodeFlavors flavors) {
+    private static Map<Node, ResourceCapacity> findHeadroomHosts(List<Node> nodes, Set<Node> spareNodes, NodeFlavors flavors) {
         DockerHostCapacity capacity = new DockerHostCapacity(nodes);
-        Map<Node, Boolean> headroomNodesToViolation = new HashMap<>();
+        Map<Node, ResourceCapacity> headroomHosts = new HashMap<>();
 
         List<Node> hostsSortedOnLeastCapacity = nodes.stream()
                 .filter(n -> !spareNodes.contains(n))
@@ -121,20 +121,25 @@ public class NodePrioritizer {
                 .sorted((a, b) -> capacity.compareWithoutInactive(b, a))
                 .collect(Collectors.toList());
 
+        // For all flavors with ideal headroom - find which hosts this headroom should be allocated to
         for (Flavor flavor : flavors.getFlavors().stream().filter(f -> f.getIdealHeadroom() > 0).collect(Collectors.toList())) {
             Set<Node> tempHeadroom = new HashSet<>();
             Set<Node> notEnoughCapacity = new HashSet<>();
+
+            ResourceCapacity headroomCapacity = ResourceCapacity.of(flavor);
+
+            // Select hosts that has available capacity for both headroom and for new allocations
             for (Node host : hostsSortedOnLeastCapacity) {
-                if (headroomNodesToViolation.containsKey(host)) continue;
-                if (capacity.hasCapacityWhenRetiredAndInactiveNodesAreGone(host, flavor)) {
-                    headroomNodesToViolation.put(host, false);
+                if (headroomHosts.containsKey(host)) continue;
+                if (capacity.hasCapacityWhenRetiredAndInactiveNodesAreGone(host, headroomCapacity)) {
+                    headroomHosts.put(host, headroomCapacity);
                     tempHeadroom.add(host);
                 } else {
                     notEnoughCapacity.add(host);
                 }
 
                 if (tempHeadroom.size() == flavor.getIdealHeadroom()) {
-                    continue;
+                    break;
                 }
             }
 
@@ -145,14 +150,13 @@ public class NodePrioritizer {
                         .limit(flavor.getIdealHeadroom() - tempHeadroom.size())
                         .collect(Collectors.toList());
 
-                for (Node nodeViolatingHeadrom : violations) {
-                    headroomNodesToViolation.put(nodeViolatingHeadrom, true);
+                for (Node hostViolatingHeadrom : violations) {
+                    headroomHosts.put(hostViolatingHeadrom, headroomCapacity);
                 }
-
             }
         }
 
-        return headroomNodesToViolation;
+        return headroomHosts;
     }
 
     /**
@@ -197,14 +201,14 @@ public class NodePrioritizer {
                     }
                 }
 
-                if (!conflictingCluster && capacity.hasCapacity(node, getFlavor())) {
+                if (!conflictingCluster && capacity.hasCapacity(node, ResourceCapacity.of(getFlavor(requestedNodes)))) {
                     Set<String> ipAddresses = DockerHostCapacity.findFreeIps(node, allNodes);
                     if (ipAddresses.isEmpty()) continue;
                     String ipAddress = ipAddresses.stream().findFirst().get();
                     String hostname = lookupHostname(ipAddress);
                     if (hostname == null) continue;
                     Node newNode = Node.createDockerNode("fake-" + hostname, Collections.singleton(ipAddress),
-                            Collections.emptySet(), hostname, Optional.of(node.hostname()), getFlavor(), NodeType.tenant);
+                            Collections.emptySet(), hostname, Optional.of(node.hostname()), getFlavor(requestedNodes), NodeType.tenant);
                     PrioritizableNode nodePri = toNodePriority(newNode, false, true);
                     if (!nodePri.violatesSpares || isAllocatingForReplacement) {
                         nodes.put(newNode, nodePri);
@@ -249,7 +253,7 @@ public class NodePrioritizer {
         pri.node = node;
         pri.isSurplusNode = isSurplusNode;
         pri.isNewNode = isNewNode;
-        pri.preferredOnFlavor = requestedNodes.specifiesNonStockFlavor() && node.flavor().equals(getFlavor());
+        pri.preferredOnFlavor = requestedNodes.specifiesNonStockFlavor() && node.flavor().equals(getFlavor(requestedNodes));
         pri.parent = findParentNode(node);
 
         if (pri.parent.isPresent()) {
@@ -261,7 +265,13 @@ public class NodePrioritizer {
             }
 
             if (headroomHosts.containsKey(parent)) {
-                pri.violatesHeadroom = headroomHosts.get(parent);
+                ResourceCapacity neededCapacity = headroomHosts.get(parent);
+
+                // If the node is new then we need to check the headroom requirement after it has been added
+                if (isNewNode) {
+                    neededCapacity = ResourceCapacity.composite(neededCapacity, new ResourceCapacity(node));
+                }
+                pri.violatesHeadroom = !capacity.hasCapacity(parent, neededCapacity);
             }
         }
 
@@ -280,7 +290,7 @@ public class NodePrioritizer {
         return (wantedCount > nofNodesInCluster - nodeFailedNodes);
     }
 
-    private Flavor getFlavor() {
+    private static Flavor getFlavor(NodeSpec requestedNodes) {
         if (requestedNodes instanceof NodeSpec.CountNodeSpec) {
             NodeSpec.CountNodeSpec countSpec = (NodeSpec.CountNodeSpec) requestedNodes;
             return countSpec.getFlavor();
@@ -289,7 +299,7 @@ public class NodePrioritizer {
     }
 
     private boolean isDocker() {
-        Flavor flavor = getFlavor();
+        Flavor flavor = getFlavor(requestedNodes);
         return (flavor != null) && flavor.getType().equals(Flavor.Type.DOCKER_CONTAINER);
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java
index 06acd646ea7..807fbfae1c9 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java
@@ -23,7 +23,7 @@ class PrioritizableNode implements Comparable<PrioritizableNode> {
     /** True if the node is allocated to a host that should be dedicated as a spare */
     boolean violatesSpares;
 
-    /** True if the node is allocated on slots that should be dedicated to headroom */
+    /** True if the node is (or would be) allocated on slots that should be dedicated to headroom */
     boolean violatesHeadroom;
 
     /** True if this is a node that has been retired earlier in the allocation process */
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacity.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacity.java
index fdec29d5b97..362e83da80a 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacity.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacity.java
@@ -28,6 +28,14 @@ public class ResourceCapacity {
         disk = node.flavor().getMinDiskAvailableGb();
     }
 
+    static ResourceCapacity of(Flavor flavor) {
+        ResourceCapacity capacity = new ResourceCapacity();
+        capacity.memory = flavor.getMinMainMemoryAvailableGb();
+        capacity.cpu = flavor.getMinCpuCores();
+        capacity.disk = flavor.getMinDiskAvailableGb();
+        return capacity;
+    }
+
     public double getMemory() {
         return memory;
     }
@@ -40,6 +48,15 @@ public class ResourceCapacity {
         return disk;
     }
 
+    static ResourceCapacity composite(ResourceCapacity a, ResourceCapacity b) {
+        ResourceCapacity composite = new ResourceCapacity();
+        composite.memory = a.memory + b.memory;
+        composite.cpu -= a.cpu + b.cpu;
+        composite.disk -=  a.disk + b.disk;
+
+        return composite;
+    }
+
     void subtract(Node node) {
         memory -= node.flavor().getMinMainMemoryAvailableGb();
         cpu -= node.flavor().getMinCpuCores();
@@ -54,14 +71,18 @@ public class ResourceCapacity {
         return result;
     }
 
+    boolean hasCapacityFor(ResourceCapacity capacity) {
+        return memory >= capacity.memory &&
+                cpu >= capacity.cpu &&
+                disk >= capacity.disk;
+    }
+
     boolean hasCapacityFor(Flavor flavor) {
-        return memory >= flavor.getMinMainMemoryAvailableGb() &&
-                cpu >= flavor.getMinCpuCores() &&
-                disk >= flavor.getMinDiskAvailableGb();
+        return hasCapacityFor(ResourceCapacity.of(flavor));
     }
 
     int freeCapacityInFlavorEquivalence(Flavor flavor) {
-        if (!hasCapacityFor(flavor)) return 0;
+        if (!hasCapacityFor(ResourceCapacity.of(flavor))) return 0;
 
         double memoryFactor = Math.floor(memory/flavor.getMinMainMemoryAvailableGb());
         double cpuFactor = Math.floor(cpu/flavor.getMinCpuCores());
@@ -85,11 +106,4 @@ public class ResourceCapacity {
         if (cpu < that.cpu) return -1;
         return 0;
     }
-
-    Flavor asFlavor() {
-        FlavorConfigBuilder b = new FlavorConfigBuilder();
-        b.addFlavor("spareflavor", cpu, memory, disk, Flavor.Type.DOCKER_CONTAINER).idealHeadroom(1);
-        return new Flavor(b.build().flavor(0));
-    }
-
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java
index 55e1ff8de9f..dce9f694647 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java
@@ -72,20 +72,20 @@ public class DockerHostCapacityTest {
 
     @Test
     public void hasCapacity() {
-        assertTrue(capacity.hasCapacity(host1, flavorDocker));
-        assertTrue(capacity.hasCapacity(host1, flavorDocker2));
-        assertTrue(capacity.hasCapacity(host2, flavorDocker));
-        assertTrue(capacity.hasCapacity(host2, flavorDocker2));
-        assertFalse(capacity.hasCapacity(host3, flavorDocker));  // No ip available
-        assertFalse(capacity.hasCapacity(host3, flavorDocker2)); // No ip available
+        assertTrue(capacity.hasCapacity(host1, ResourceCapacity.of(flavorDocker)));
+        assertTrue(capacity.hasCapacity(host1, ResourceCapacity.of(flavorDocker2)));
+        assertTrue(capacity.hasCapacity(host2, ResourceCapacity.of(flavorDocker)));
+        assertTrue(capacity.hasCapacity(host2, ResourceCapacity.of(flavorDocker2)));
+        assertFalse(capacity.hasCapacity(host3, ResourceCapacity.of(flavorDocker)));  // No ip available
+        assertFalse(capacity.hasCapacity(host3, ResourceCapacity.of(flavorDocker2))); // No ip available
 
         // Add a new node to host1 to deplete the memory resource
         Node nodeF = Node.create("nodeF", Collections.singleton("::6"), Collections.emptySet(),
                 "nodeF", Optional.of("host1"), flavorDocker, NodeType.tenant);
         nodes.add(nodeF);
         capacity = new DockerHostCapacity(nodes);
-        assertFalse(capacity.hasCapacity(host1, flavorDocker));
-        assertFalse(capacity.hasCapacity(host1, flavorDocker2));
+        assertFalse(capacity.hasCapacity(host1, ResourceCapacity.of(flavorDocker)));
+        assertFalse(capacity.hasCapacity(host1, ResourceCapacity.of(flavorDocker2)));
     }
 
     @Test