Add unit tests and fix errors along the way

14 files changed, 406 insertions, 187 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 1ca624df01e..77d91c7bea7 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
@@ -49,7 +49,7 @@ public class DockerHostCapacity {
         return comp;
     }
 
-    int compareWithoutRetired(Node hostA, Node hostB) {
+    int compareWithoutInactive(Node hostA, Node hostB) {
         int comp = freeCapacityOf(hostB,  true).compare(freeCapacityOf(hostA, true));
         if (comp == 0) {
             comp = freeCapacityOf(hostB, true).compare(freeCapacityOf(hostA, true));
@@ -69,6 +69,10 @@ public class DockerHostCapacity {
         return freeCapacityOf(dockerHost, false).hasCapacityFor(flavor) && freeIPs(dockerHost) > 0;
     }
 
+    boolean hasCapacityWhenRetiredAndInactiveNodesAreGone(Node dockerHost, Flavor flavor) {
+        return freeCapacityOf(dockerHost, true).hasCapacityFor(flavor) && freeIPs(dockerHost) > 0;
+    }
+
     /**
      * Number of free (not allocated) IP addresses assigned to the dockerhost.
      */
@@ -117,19 +121,30 @@ public class DockerHostCapacity {
      *
      * @return A default (empty) capacity if not a docker host, otherwise the free/unallocated/rest capacity
      */
-    public ResourceCapacity freeCapacityOf(Node dockerHost, boolean retiredAsFreeCapacity) {
+    public ResourceCapacity freeCapacityOf(Node dockerHost, boolean treatInactiveOrRetiredAsUnusedCapacity) {
         // Only hosts have free capacity
         if (!dockerHost.type().equals(NodeType.host)) return new ResourceCapacity();
 
         ResourceCapacity hostCapacity = new ResourceCapacity(dockerHost);
         for (Node container : allNodes.childNodes(dockerHost).asList()) {
-            if (retiredAsFreeCapacity && container.allocation().isPresent()
-                    && container.allocation().get().membership().retired()) continue;
-            hostCapacity.subtract(container);
+            boolean isUsedCapacity = !(treatInactiveOrRetiredAsUnusedCapacity && isInactiveOrRetired(container));
+            if (isUsedCapacity) {
+                hostCapacity.subtract(container);
+            }
         }
         return hostCapacity;
     }
 
+    private boolean isInactiveOrRetired(Node node) {
+        boolean isInactive = node.state().equals(Node.State.inactive);
+        boolean isRetired = false;
+        if (node.allocation().isPresent()) {
+            isRetired = node.allocation().get().membership().retired();
+        }
+
+        return isInactive || isRetired;
+    }
+
     /**
      * Compare the additional ip addresses against the set of used addresses on
      * child nodes.
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java
index cff62508ec6..b52506c268c 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java
@@ -19,7 +19,7 @@ public class FlavorConfigBuilder {
         return new FlavorsConfig(builder);
     }
 
-    public FlavorsConfig.Flavor.Builder addFlavor(String flavorName, double cpu, double mem, double disk, Flavor.Type type) {
+    public FlavorsConfig.Flavor.Builder addFlavor(String flavorName, double cpu, double mem, double disk, Flavor.Type type, int headRoom) {
         FlavorsConfig.Flavor.Builder flavor = new FlavorsConfig.Flavor.Builder();
         flavor.name(flavorName);
         flavor.description("Flavor-name-is-" + flavorName);
@@ -27,10 +27,15 @@ public class FlavorConfigBuilder {
         flavor.minCpuCores(cpu);
         flavor.minMainMemoryAvailableGb(mem);
         flavor.environment(type.name());
+        flavor.idealHeadroom(headRoom);
         builder.flavor(flavor);
         return flavor;
     }
 
+    public FlavorsConfig.Flavor.Builder addFlavor(String flavorName, double cpu, double mem, double disk, Flavor.Type type) {
+        return addFlavor(flavorName, cpu, mem, disk, type, 0);
+    }
+
     public FlavorsConfig.Flavor.Builder addNonStockFlavor(String flavorName, double cpu, double mem, double disk, Flavor.Type type) {
         FlavorsConfig.Flavor.Builder flavor = new FlavorsConfig.Flavor.Builder();
         flavor.name(flavorName);
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 e987ac891c8..1733554365b 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
@@ -19,7 +19,6 @@ import java.util.function.BiConsumer;
  * @author bratseth
  */
 class GroupPreparer {
-    private static final boolean canChangeGroup = true;
     private final NodeRepository nodeRepository;
     private final Clock clock;
 
@@ -59,7 +58,6 @@ class GroupPreparer {
                         cluster,
                         requestedNodes,
                         nodeRepository.getAvailableFlavors(),
-                        1,
                         nofSpares);
 
                 prioritizer.addApplicationNodes();
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 76230cfa680..64093d69907 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
@@ -14,6 +14,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -35,25 +36,21 @@ public class NodePrioritizer {
     private final DockerHostCapacity capacity;
     private final NodeSpec requestedNodes;
     private final ApplicationId appId;
-    private final int maxRetires;
     private final ClusterSpec clusterSpec;
 
     private final boolean isAllocatingForReplacement;
-    private final List<Node> spareHosts;
-    private final List<Node> headroomViolatedHosts;
+    private final Set<Node> spareHosts;
+    private final Map<Node, Boolean> headroomHosts;
     private final boolean isDocker;
 
-    int nofViolations = 0;
-
-    NodePrioritizer(List<Node> allNodes, ApplicationId appId, ClusterSpec clusterSpec, NodeSpec nodeSpec, NodeFlavors nodeFlavors, int maxRetires, int spares) {
+    NodePrioritizer(List<Node> allNodes, ApplicationId appId, ClusterSpec clusterSpec, NodeSpec nodeSpec, NodeFlavors nodeFlavors, int spares) {
         this.allNodes = Collections.unmodifiableList(allNodes);
         this.requestedNodes = nodeSpec;
-        this.maxRetires = maxRetires;
         this.clusterSpec = clusterSpec;
         this.appId = appId;
 
         spareHosts = findSpareHosts(allNodes, spares);
-        headroomViolatedHosts = findHeadroomHosts(allNodes, spareHosts, nodeFlavors);
+        headroomHosts = findHeadroomHosts(allNodes, spareHosts, nodeFlavors);
 
         this.capacity = new DockerHostCapacity(allNodes);
 
@@ -74,6 +71,78 @@ public class NodePrioritizer {
         isDocker = isDocker();
     }
 
+    /**
+     * From ipAddress - get hostname
+     *
+     * @return hostname or null if not able to do the loopup
+     */
+    private static String lookupHostname(String ipAddress) {
+        try {
+            return InetAddress.getByName(ipAddress).getHostName();
+        } catch (UnknownHostException e) {
+            e.printStackTrace();
+        }
+        return null;
+    }
+
+    private static Set<Node> findSpareHosts(List<Node> nodes, int spares) {
+        DockerHostCapacity capacity = new DockerHostCapacity(new ArrayList<>(nodes));
+        return nodes.stream()
+                .filter(node -> node.type().equals(NodeType.host))
+                .filter(dockerHost -> dockerHost.state().equals(Node.State.active))
+                .filter(dockerHost -> capacity.freeIPs(dockerHost) > 0)
+                .sorted(capacity::compareWithoutInactive)
+                .limit(spares)
+                .collect(Collectors.toSet());
+    }
+
+    private static Map<Node, Boolean> findHeadroomHosts(List<Node> nodes, Set<Node> spareNodes, NodeFlavors flavors) {
+        DockerHostCapacity capacity = new DockerHostCapacity(nodes);
+        Map<Node, Boolean> headroomNodesToViolation = new HashMap<>();
+
+        List<Node> hostsSortedOnLeastCapacity = nodes.stream()
+                .filter(n -> !spareNodes.contains(n))
+                .filter(node -> node.type().equals(NodeType.host))
+                .filter(dockerHost -> dockerHost.state().equals(Node.State.active))
+                .filter(dockerHost -> capacity.freeIPs(dockerHost) > 0)
+                .sorted((a, b) -> capacity.compareWithoutInactive(b, a))
+                .collect(Collectors.toList());
+
+        for (Flavor flavor : flavors.getFlavors().stream().filter(f -> f.getIdealHeadroom() > 0).collect(Collectors.toList())) {
+            Set<Node> tempHeadroom = new HashSet<>();
+            Set<Node> notEnoughCapacity = new HashSet<>();
+            for (Node host : hostsSortedOnLeastCapacity) {
+                if (headroomNodesToViolation.containsKey(host)) continue;
+                if (capacity.hasCapacityWhenRetiredAndInactiveNodesAreGone(host, flavor)) {
+                    headroomNodesToViolation.put(host, false);
+                    tempHeadroom.add(host);
+                } else {
+                    notEnoughCapacity.add(host);
+                }
+
+                if (tempHeadroom.size() == flavor.getIdealHeadroom()) {
+                    continue;
+                }
+            }
+
+            // Now check if we have enough headroom - if not choose the nodes that almost has it
+            if (tempHeadroom.size() < flavor.getIdealHeadroom()) {
+                List<Node> violations = notEnoughCapacity.stream()
+                        .sorted((a, b) -> capacity.compare(b, a))
+                        .limit(flavor.getIdealHeadroom() - tempHeadroom.size())
+                        .collect(Collectors.toList());
+
+                // TODO should we be selective on which application on the node that violates the headroom?
+                for (Node nodeViolatingHeadrom : violations) {
+                    headroomNodesToViolation.put(nodeViolatingHeadrom, true);
+                }
+
+            }
+        }
+
+        return headroomNodesToViolation;
+    }
+
     List<NodePriority> prioritize() {
         List<NodePriority> priorityList = new ArrayList<>(nodes.values());
         Collections.sort(priorityList, (a, b) -> NodePriority.compare(a, b));
@@ -131,7 +200,6 @@ public class NodePrioritizer {
                 .filter(node -> node.allocation().isPresent())
                 .filter(node -> node.allocation().get().owner().equals(appId))
                 .map(node -> toNodePriority(node, false, false))
-                .filter(n -> !n.violatesSpares || isAllocatingForReplacement)
                 .forEach(nodePriority -> nodes.put(nodePriority.node, nodePriority));
     }
 
@@ -160,38 +228,16 @@ public class NodePrioritizer {
             Node parent = pri.parent.get();
             pri.freeParentCapacity = capacity.freeCapacityOf(parent, false);
 
-            /**
-             * To be conservative we have a restriction of how many nodes we can retire for each cluster,
-             * pr. allocation iteration. TODO also account for previously retired nodes? (thus removing the pr iteration restriction)
-             */
-            if (nofViolations <= maxRetires) {
-                if (spareHosts.contains(parent)) {
-                    pri.violatesSpares = true;
-                    nofViolations++;
-                }
+            if (spareHosts.contains(parent)) {
+                pri.violatesSpares = true;
+            }
 
-                // Headroom violation
-                if (headroomViolatedHosts.contains(parent)) {
-                    pri.violatesHeadroom = true;
-                    nofViolations++;
-                }
+            if (headroomHosts.containsKey(parent)) {
+                pri.violatesHeadroom = headroomHosts.get(parent);
             }
         }
-        return pri;
-    }
 
-    /**
-     * From ipAddress - get hostname
-     *
-     * @return hostname or null if not able to do the loopup
-     */
-    private static String lookupHostname(String ipAddress) {
-        try {
-            return InetAddress.getByName(ipAddress).getHostName();
-        } catch (UnknownHostException e) {
-            e.printStackTrace();
-        }
-        return null;
+        return pri;
     }
 
     private boolean isReplacement(long nofNodesInCluster, long nodeFailedNodes) {
@@ -216,10 +262,7 @@ public class NodePrioritizer {
 
     private boolean isDocker() {
         Flavor flavor = getFlavor();
-        if (flavor != null) {
-            return flavor.getType().equals(Flavor.Type.DOCKER_CONTAINER);
-        }
-        return false;
+        return (flavor != null) && flavor.getType().equals(Flavor.Type.DOCKER_CONTAINER);
     }
 
     private Optional<Node> findParentNode(Node node) {
@@ -228,47 +271,4 @@ public class NodePrioritizer {
                 .filter(n -> n.hostname().equals(node.parentHostname().orElse(" NOT A NODE")))
                 .findAny();
     }
-
-    private static List<Node> findSpareHosts(List<Node> nodes, int spares) {
-        DockerHostCapacity capacity = new DockerHostCapacity(nodes);
-        return nodes.stream()
-                .filter(node -> node.type().equals(NodeType.host))
-                .filter(dockerHost -> dockerHost.state().equals(Node.State.active))
-                .filter(dockerHost -> capacity.freeIPs(dockerHost) > 0)
-                .sorted(capacity::compareWithoutRetired)
-                .limit(spares)
-                .collect(Collectors.toList());
-    }
-
-    private static List<Node> findHeadroomHosts(List<Node> nodes, List<Node> spareNodes, NodeFlavors flavors) {
-        DockerHostCapacity capacity = new DockerHostCapacity(nodes);
-        List<Node> headroomNodes = new ArrayList<>();
-
-        List<Node> hostsSortedOnLeastCapacity = nodes.stream()
-                .filter(n -> !spareNodes.contains(n))
-                .filter(node -> node.type().equals(NodeType.host))
-                .filter(dockerHost -> dockerHost.state().equals(Node.State.active))
-                .filter(dockerHost -> capacity.freeIPs(dockerHost) > 0)
-                .sorted((a,b) -> capacity.compareWithoutRetired(b,a))
-                .collect(Collectors.toList());
-
-        for (Flavor flavor : flavors.getFlavors()) {
-            for (int i = 0; i < flavor.getIdealHeadroom(); i++) {
-                Node lastNode = null;
-                for (Node potentialHeadroomHost : hostsSortedOnLeastCapacity) {
-                    if (headroomNodes.contains(potentialHeadroomHost)) continue;
-                    lastNode = potentialHeadroomHost;
-                    if (capacity.hasCapacity(potentialHeadroomHost, flavor)) {
-                        headroomNodes.add(potentialHeadroomHost);
-                        continue;
-                    }
-                }
-                if (lastNode != null) {
-                    headroomNodes.add(lastNode);
-                }
-            }
-        }
-
-        return headroomNodes;
-    }
 }
\ No newline at end of file
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisioningTest.java
index e273d110b04..290e8436b1a 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisioningTest.java
@@ -5,6 +5,7 @@ import com.google.common.collect.ImmutableSet;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Capacity;
+import com.yahoo.config.provision.ClusterMembership;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.Flavor;
@@ -15,11 +16,19 @@ import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.config.provisioning.FlavorsConfig;
 import com.yahoo.path.Path;
+import com.yahoo.transaction.NestedTransaction;
+import com.yahoo.vespa.curator.transaction.CuratorTransaction;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
+import org.junit.Assert;
 import org.junit.Test;
 
+import java.time.Instant;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import static org.hamcrest.CoreMatchers.is;
@@ -32,6 +41,146 @@ import static org.junit.Assert.fail;
  */
 public class DynamicDockerProvisioningTest {
 
+    /**
+     * Test reloaction of nodes that violates headroom.
+     *
+     * Setup 4 docker hosts and allocate one container on each (from two different applications)
+     * No spares - only headroom (4xd-2)
+     *
+     * One application is now violating headroom and need relocation
+     *
+     *  Initial allocation of app 1 and 2 --> final allocation (headroom marked as H):
+     *
+     * | H  |  H  | H   | H   |        |    |    |    |    |
+     * | H  |  H  | H1a | H1b |   -->  |    |    |    |    |
+     * |    |     | 2a  | 2b  |        | 1a | 1b | 2a | 2b |
+     *
+     */
+    @Test
+    public void relocate_nodes_from_headroom_hosts() {
+        ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.perf, RegionName.from("us-east")), flavorsConfig(true));
+        enableDynamicAllocation(tester);
+        tester.makeReadyNodes(4, "host", "host-small", NodeType.host, 32);
+        deployZoneApp(tester);
+        List<Node> dockerHosts = tester.nodeRepository().getNodes(NodeType.host, Node.State.active);
+        Flavor flavor = tester.nodeRepository().getAvailableFlavors().getFlavorOrThrow("d-1");
+
+        // Application 1
+        ApplicationId application1 = makeApplicationId("t1", "a1");
+        ClusterSpec clusterSpec1 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent"), Version.fromString("6.100"));
+        addAndAssignNode(application1, "1a", dockerHosts.get(2).hostname(), flavor, 0, tester);
+        addAndAssignNode(application1, "1b", dockerHosts.get(3).hostname(), flavor, 1, tester);
+
+        // Application 2
+        ApplicationId application2 = makeApplicationId("t2", "a2");
+        ClusterSpec clusterSpec2 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent"), Version.fromString("6.100"));
+        addAndAssignNode(application2, "2a", dockerHosts.get(2).hostname(), flavor, 0, tester);
+        addAndAssignNode(application2, "2b", dockerHosts.get(3).hostname(), flavor, 1, tester);
+
+        // Redeploy one of the applications
+        redeply(application1, clusterSpec1, flavor, tester);
+
+        // Assert that the nodes are spread across all hosts (to allow headroom)
+        Set<String> hostsWithChildren = new HashSet<>();
+        for (Node node : tester.nodeRepository().getNodes(NodeType.tenant, Node.State.active)) {
+            if (!isInactiveOrRetired(node)) {
+                hostsWithChildren.add(node.parentHostname().get());
+            }
+        }
+        Assert.assertEquals(4, hostsWithChildren.size());
+    }
+
+    /**
+     * Test relocation of nodes from spare hosts.
+     *
+     * Setup 4 docker hosts and allocate one container on each (from two different applications)
+     * No headroom defined - only 2 spares.
+     *
+     * Check that it relocates containers away from the 2 spares
+     *
+     *  Initial allocation of app 1 and 2 --> final allocation:
+     *
+     * |    |    |    |    |        |    |    |    |    |
+     * |    |    |    |    |   -->  | 2a | 2b |    |    |
+     * | 1a | 1b | 2a | 2b |        | 1a | 1b |    |    |
+     *
+     */
+    @Test
+    public void relocate_nodes_from_spare_hosts() {
+        ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.prod, RegionName.from("us-east")), flavorsConfig());
+        enableDynamicAllocation(tester);
+        tester.makeReadyNodes(4, "host", "host-small", NodeType.host, 32);
+        deployZoneApp(tester);
+        List<Node> dockerHosts = tester.nodeRepository().getNodes(NodeType.host, Node.State.active);
+        Flavor flavor = tester.nodeRepository().getAvailableFlavors().getFlavorOrThrow("d-1");
+
+        // Application 1
+        ApplicationId application1 = makeApplicationId("t1", "a1");
+        ClusterSpec clusterSpec1 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent"), Version.fromString("6.100"));
+        addAndAssignNode(application1, "1a", dockerHosts.get(0).hostname(), flavor, 0, tester);
+        addAndAssignNode(application1, "1b", dockerHosts.get(1).hostname(), flavor, 1, tester);
+
+        // Application 2
+        ApplicationId application2 = makeApplicationId("t2", "a2");
+        ClusterSpec clusterSpec2 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent"), Version.fromString("6.100"));
+        addAndAssignNode(application2, "2a", dockerHosts.get(2).hostname(), flavor, 0, tester);
+        addAndAssignNode(application2, "2b", dockerHosts.get(3).hostname(), flavor, 1, tester);
+
+        // Redeploy both applications (to be agnostic on which hosts are picked as spares)
+        redeply(application1, clusterSpec1, flavor, tester);
+        redeply(application2, clusterSpec2, flavor, tester);
+
+        // Assert that we have two spare nodes (two hosts that are don't have allocations)
+        Set<String> hostsWithChildren = new HashSet<>();
+        for (Node node : tester.nodeRepository().getNodes(NodeType.tenant, Node.State.active)) {
+            if (!isInactiveOrRetired(node)) {
+                hostsWithChildren.add(node.parentHostname().get());
+            }
+        }
+        Assert.assertEquals(2, hostsWithChildren.size());
+    }
+
+    /**
+     * Test redeployment of nodes that violates spare headroom - but without alternatives
+     *
+     * Setup 2 docker hosts and allocate one app with a container on each
+     * No headroom defined - only 2 spares.
+     *
+     * Initial allocation of app 1 --> final allocation:
+     *
+     * |    |    |        |    |    |
+     * |    |    |   -->  |    |    |
+     * | 1a | 1b |        | 1a | 1b |
+     *
+     */
+    @Test
+    public void do_not_relocate_nodes_from_spare_if_no_where_to_reloacte_them() {
+        ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.prod, RegionName.from("us-east")), flavorsConfig());
+        enableDynamicAllocation(tester);
+        tester.makeReadyNodes(2, "host", "host-small", NodeType.host, 32);
+        deployZoneApp(tester);
+        List<Node> dockerHosts = tester.nodeRepository().getNodes(NodeType.host, Node.State.active);
+        Flavor flavor = tester.nodeRepository().getAvailableFlavors().getFlavorOrThrow("d-1");
+
+        // Application 1
+        ApplicationId application1 = makeApplicationId("t1", "a1");
+        ClusterSpec clusterSpec1 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent"), Version.fromString("6.100"));
+        addAndAssignNode(application1, "1a", dockerHosts.get(0).hostname(), flavor, 0, tester);
+        addAndAssignNode(application1, "1b", dockerHosts.get(1).hostname(), flavor, 1, tester);
+
+        // Redeploy both applications (to be agnostic on which hosts are picked as spares)
+        redeply(application1, clusterSpec1, flavor, tester);
+
+        // Assert that we have two spare nodes (two hosts that are don't have allocations)
+        Set<String> hostsWithChildren = new HashSet<>();
+        for (Node node : tester.nodeRepository().getNodes(NodeType.tenant, Node.State.active)) {
+            if (!isInactiveOrRetired(node)) {
+                hostsWithChildren.add(node.parentHostname().get());
+            }
+        }
+        Assert.assertEquals(2, hostsWithChildren.size());
+    }
+
     @Test(expected = OutOfCapacityException.class)
     public void multiple_groups_are_on_separate_parent_hosts() {
         ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.prod, RegionName.from("us-east")), flavorsConfig());
@@ -93,9 +242,6 @@ public class DynamicDockerProvisioningTest {
         List<Node> finalSpareCapacity = findSpareCapacity(tester);
 
         assertThat(finalSpareCapacity.size(), is(1));
-
-        // Uncomment the statement below to walk through the allocation events visually
-        //AllocationVisualizer.visualize(tester.getAllocationSnapshots());
     }
 
     @Test
@@ -116,6 +262,29 @@ public class DynamicDockerProvisioningTest {
         assertThat(initialSpareCapacity.size(), is(0));
     }
 
+    private ApplicationId makeApplicationId(String tenant, String appName) {
+        return ApplicationId.from(tenant, appName, "default");
+    }
+
+    private void redeply(ApplicationId id, ClusterSpec spec, Flavor flavor, ProvisioningTester tester) {
+        List<HostSpec> hostSpec = tester.prepare(id, spec, 2,1, flavor.canonicalName());
+        tester.activate(id, new HashSet<>(hostSpec));
+    }
+
+    private Node addAndAssignNode(ApplicationId id, String hostname, String parentHostname, Flavor flavor, int index, ProvisioningTester tester) {
+        Node node1a = Node.create("open1", Collections.singleton("127.0.0.100"), new HashSet<>(), hostname, Optional.of(parentHostname), flavor, NodeType.tenant);
+        ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent"), Version.fromString("6.100")).changeGroup(Optional.of(ClusterSpec.Group.from(0)));
+        ClusterMembership clusterMembership1 = ClusterMembership.from(clusterSpec,index);
+        Node node1aAllocation = node1a.allocate(id,clusterMembership1, Instant.now());
+
+        tester.nodeRepository().addNodes(Collections.singletonList(node1aAllocation));
+        NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(tester.getCurator()));
+        tester.nodeRepository().activate(Collections.singletonList(node1aAllocation), transaction);
+        transaction.commit();
+
+        return node1aAllocation;
+    }
+
     private List<Node> findSpareCapacity(ProvisioningTester tester) {
         List<Node> nodes = tester.nodeRepository().getNodes(Node.State.values());
         NodeList nl = new NodeList(nodes);
@@ -125,6 +294,22 @@ public class DynamicDockerProvisioningTest {
                 .collect(Collectors.toList());
     }
 
+    private FlavorsConfig flavorsConfig(boolean includeHeadroom) {
+        FlavorConfigBuilder b = new FlavorConfigBuilder();
+        b.addFlavor("host-large", 6., 6., 6, Flavor.Type.BARE_METAL);
+        b.addFlavor("host-small", 3., 3., 3, Flavor.Type.BARE_METAL);
+        b.addFlavor("d-1", 1, 1., 1, Flavor.Type.DOCKER_CONTAINER);
+        b.addFlavor("d-2", 2, 2., 2, Flavor.Type.DOCKER_CONTAINER);
+        if (includeHeadroom) {
+            b.addFlavor("d-2-4", 2, 2., 2, Flavor.Type.DOCKER_CONTAINER, 4);
+        }
+        b.addFlavor("d-3", 3, 3., 3, Flavor.Type.DOCKER_CONTAINER);
+        b.addFlavor("d-3-disk", 3, 3., 5, Flavor.Type.DOCKER_CONTAINER);
+        b.addFlavor("d-3-mem", 3, 5., 3, Flavor.Type.DOCKER_CONTAINER);
+        b.addFlavor("d-3-cpu", 5, 3., 3, Flavor.Type.DOCKER_CONTAINER);
+        return b.build();
+    }
+
     private FlavorsConfig flavorsConfig() {
         FlavorConfigBuilder b = new FlavorConfigBuilder();
         b.addFlavor("host-large", 6., 6., 6, Flavor.Type.BARE_METAL);
@@ -153,4 +338,14 @@ public class DynamicDockerProvisioningTest {
     private void enableDynamicAllocation(ProvisioningTester tester) {
         tester.getCurator().set(Path.fromString("/provision/v1/dynamicDockerAllocation"), new byte[0]);
     }
+
+    private boolean isInactiveOrRetired(Node node) {
+        boolean isInactive = node.state().equals(Node.State.inactive);
+        boolean isRetired = false;
+        if (node.allocation().isPresent()) {
+            isRetired = node.allocation().get().membership().retired();
+        }
+
+        return isInactive || isRetired;
+    }
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
index 3bade354ef3..73ef95ca4c8 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
@@ -226,6 +226,10 @@ public class ProvisioningTester implements AutoCloseable {
     }
 
     public List<Node> makeReadyNodes(int n, String flavor, NodeType type, int additionalIps) {
+        return makeReadyNodes(n, UUID.randomUUID().toString(), flavor, type, additionalIps);
+    }
+
+    public List<Node> makeReadyNodes(int n, String prefix, String flavor, NodeType type, int additionalIps) {
         List<Node> nodes = new ArrayList<>(n);
         for (int i = 0; i < n; i++) {
             Set<String> ips = IntStream.range(additionalIps * i, additionalIps * (i+1))
@@ -233,7 +237,7 @@ public class ProvisioningTester implements AutoCloseable {
                     .collect(Collectors.toSet());
 
             nodes.add(nodeRepository.createNode(UUID.randomUUID().toString(),
-                    UUID.randomUUID().toString(),
+                    prefix + i,
                     Collections.emptySet(),
                     ips,
                     Optional.empty(),
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
index 205c8d40bc5..2a820308874 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
@@ -389,7 +389,7 @@ public class RestApiTest {
                        "{\"message\":\"Moved host2.yahoo.com to parked\"}");
         assertResponse(new Request("http://localhost:8080/nodes/v2/state/ready/host2.yahoo.com",
                                    new byte[0], Request.Method.PUT),
-                       400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Can not set parked node host2.yahoo.com allocated to tenant1.application1.instance1 as 'container/id1/0/1' ready. It is not dirty.\"}");
+                       400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Can not set parked node host2.yahoo.com allocated to tenant2.application2.instance2 as 'content/id2/0/0' ready. It is not dirty.\"}");
         // (... while dirty then ready works (the ready move will be initiated by node maintenance))
         assertResponse(new Request("http://localhost:8080/nodes/v2/state/dirty/host2.yahoo.com",
                                    new byte[0], Request.Method.PUT),
@@ -446,18 +446,18 @@ public class RestApiTest {
     @Test
     public void test_hardware_patching_of_docker_host() throws Exception {
         assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/host5.yahoo.com"), Optional.of(false));
-        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/parent1.yahoo.com"), Optional.of(false));
+        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/dockerhost2.yahoo.com"), Optional.of(false));
 
-        assertResponse(new Request("http://localhost:8080/nodes/v2/node/parent1.yahoo.com",
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node/dockerhost2.yahoo.com",
                         Utf8.toBytes("{" +
                                 "\"hardwareFailureType\": \"memory_mcelog\"" +
                                 "}"
                         ),
                         Request.Method.PATCH),
-                "{\"message\":\"Updated parent1.yahoo.com\"}");
+                "{\"message\":\"Updated dockerhost2.yahoo.com\"}");
 
         assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/host5.yahoo.com"), Optional.of(true));
-        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/parent1.yahoo.com"), Optional.of(true));
+        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/dockerhost2.yahoo.com"), Optional.of(true));
     }
 
     @Test
@@ -562,10 +562,10 @@ public class RestApiTest {
 
     private void assertHardwareFailure(Request request, Optional<Boolean> expectedHardwareFailure) throws CharacterCodingException {
         Response response = container.handleRequest(request);
-        assertEquals(response.getStatus(), 200);
         String json = response.getBodyAsString();
         Optional<Boolean> actualHardwareFailure = getHardwareFailure(json);
         assertEquals(expectedHardwareFailure, actualHardwareFailure);
+        assertEquals(200, response.getStatus());
     }
 
     /** Asserts a particular response and 200 as response status */
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node10.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node10.json
index b49f4fc2960..120d6286634 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node10.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node10.json
@@ -1,7 +1,7 @@
 {
   "url": "http://localhost:8080/nodes/v2/node/host10.yahoo.com",
   "id": "host10.yahoo.com",
-  "state": "active",
+  "state": "reserved",
   "type": "tenant",
   "hostname": "host10.yahoo.com",
   "parentHostname": "parent1.yahoo.com",
@@ -15,15 +15,15 @@
   "fastDisk": true,
   "environment": "BARE_METAL",
   "owner": {
-    "tenant": "tenant3",
-    "application": "application3",
-    "instance": "instance3"
+    "tenant": "tenant1",
+    "application": "application1",
+    "instance": "instance1"
   },
   "membership": {
-    "clustertype": "content",
-    "clusterid": "id3",
+    "clustertype": "container",
+    "clusterid": "id1",
     "group": "0",
-    "index": 0,
+    "index": 1,
     "retired": false
   },
   "restartGeneration": 0,
@@ -50,11 +50,6 @@
       "event": "reserved",
       "at": 123,
       "agent": "application"
-    },
-    {
-      "event": "activated",
-      "at": 123,
-      "agent": "application"
     }
   ],
   "ipAddresses": [
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node2.json
index abc758a4562..52864fc165c 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node2.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node2.json
@@ -1,7 +1,7 @@
 {
   "url": "http://localhost:8080/nodes/v2/node/host2.yahoo.com",
   "id": "host2.yahoo.com",
-  "state": "reserved",
+  "state": "active",
   "type": "tenant",
   "hostname": "host2.yahoo.com",
   "openStackId": "node2",
@@ -14,15 +14,15 @@
   "fastDisk": true,
   "environment": "BARE_METAL",
   "owner": {
-    "tenant": "tenant1",
-    "application": "application1",
-    "instance": "instance1"
+    "tenant": "tenant2",
+    "application": "application2",
+    "instance": "instance2"
   },
   "membership": {
-    "clustertype": "container",
-    "clusterid": "id1",
+    "clustertype": "content",
+    "clusterid": "id2",
     "group": "0",
-    "index": 1,
+    "index": 0,
     "retired": false
   },
   "restartGeneration": 0,
@@ -45,6 +45,11 @@
       "event": "reserved",
       "at": 123,
       "agent": "application"
+    },
+    {
+      "event": "activated",
+      "at": 123,
+      "agent": "application"
     }
   ],
   "ipAddresses": [
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node3.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node3.json
index 35ac924b4cb..7782cf15e50 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node3.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node3.json
@@ -1,7 +1,7 @@
 {
   "url": "http://localhost:8080/nodes/v2/node/host3.yahoo.com",
   "id": "host3.yahoo.com",
-  "state": "active",
+  "state": "ready",
   "type": "tenant",
   "hostname": "host3.yahoo.com",
   "openStackId": "node3",
@@ -11,22 +11,6 @@
   "cost": 200,
   "fastDisk": true,
   "environment": "BARE_METAL",
-  "owner": {
-    "tenant": "tenant2",
-    "application": "application2",
-    "instance": "instance2"
-  },
-  "membership": {
-    "clustertype": "content",
-    "clusterid": "id2",
-    "group": "0",
-    "index": 1,
-    "retired": false
-  },
-  "restartGeneration": 0,
-  "currentRestartGeneration": 0,
-  "wantedDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.42.0",
-  "wantedVespaVersion": "6.42.0",
   "rebootGeneration": 1,
   "currentRebootGeneration": 0,
   "failCount": 0,
@@ -38,16 +22,6 @@
       "event": "readied",
       "at": 123,
       "agent": "system"
-    },
-    {
-      "event": "reserved",
-      "at": 123,
-      "agent": "application"
-    },
-    {
-      "event": "activated",
-      "at": 123,
-      "agent": "application"
     }
   ],
   "ipAddresses": [
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4.json
index e2b61d4b27b..10b5689f8ce 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4.json
@@ -4,16 +4,16 @@
   "state": "active",
   "type": "tenant",
   "hostname": "host4.yahoo.com",
-  "parentHostname": "dockerhost4",
+  "parentHostname": "dockerhost1.yahoo.com",
   "openStackId": "node4",
-  "flavor": "default",
-  "canonicalFlavor": "default",
-  "minDiskAvailableGb": 400.0,
-  "minMainMemoryAvailableGb": 16.0,
-  "description": "Flavor-name-is-default",
-  "minCpuCores": 2.0,
+  "flavor": "docker",
+  "canonicalFlavor": "docker",
+  "minDiskAvailableGb": 100.0,
+  "minMainMemoryAvailableGb": 0.5,
+  "description": "Flavor-name-is-docker",
+  "minCpuCores": 0.2,
   "fastDisk": true,
-  "environment": "BARE_METAL",
+  "environment": "DOCKER_CONTAINER",
   "owner": {
     "tenant": "tenant3",
     "application": "application3",
@@ -23,7 +23,7 @@
     "clustertype": "content",
     "clusterid": "id3",
     "group": "0",
-    "index": 1,
+    "index": 0,
     "retired": false
   },
   "restartGeneration": 0,
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node5-after-changes.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node5-after-changes.json
index 0d0fda0b594..bf81509b79a 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node5-after-changes.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node5-after-changes.json
@@ -4,23 +4,37 @@
   "state": "failed",
   "type": "tenant",
   "hostname": "host5.yahoo.com",
-  "parentHostname":"parent1.yahoo.com",
+  "parentHostname": "dockerhost2.yahoo.com",
   "openStackId": "node5",
-  "flavor": "default",
-  "canonicalFlavor": "default",
-  "minDiskAvailableGb":400.0,
-  "minMainMemoryAvailableGb":16.0,
-  "description":"Flavor-name-is-default",
-  "minCpuCores":2.0,
-  "fastDisk":true,
-  "environment":"BARE_METAL",
+  "flavor": "docker",
+  "canonicalFlavor": "docker",
+  "minDiskAvailableGb": 100.0,
+  "minMainMemoryAvailableGb": 0.5,
+  "description": "Flavor-name-is-docker",
+  "minCpuCores": 0.2,
+  "fastDisk": true,
+  "environment": "DOCKER_CONTAINER",
   "rebootGeneration": 1,
   "currentRebootGeneration": 0,
   "failCount": 1,
   "hardwareFailure": false,
   "wantToRetire": false,
-  "wantToDeprovision" : false,
-  "history":[{"event":"readied","at":123,"agent":"system"},{"event":"failed","at":123,"agent":"system"}],
-  "ipAddresses":["::1", "127.0.0.1"],
-  "additionalIpAddresses":[]
-}
+  "wantToDeprovision": false,
+  "history": [
+    {
+      "event": "readied",
+      "at": 123,
+      "agent": "system"
+    },
+    {
+      "event": "failed",
+      "at": 123,
+      "agent": "system"
+    }
+  ],
+  "ipAddresses": [
+    "::1",
+    "127.0.0.1"
+  ],
+  "additionalIpAddresses": []
+}
\ No newline at end of file
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node5.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node5.json
index 35805e3b20f..1fc001fa224 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node5.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node5.json
@@ -4,16 +4,16 @@
   "state": "failed",
   "type": "tenant",
   "hostname": "host5.yahoo.com",
-  "parentHostname":"parent1.yahoo.com",
+  "parentHostname": "dockerhost2.yahoo.com",
   "openStackId": "node5",
-  "flavor": "default",
-  "canonicalFlavor": "default",
-  "minDiskAvailableGb":400.0,
-  "minMainMemoryAvailableGb":16.0,
-  "description":"Flavor-name-is-default",
-  "minCpuCores":2.0,
-  "fastDisk":true,
-  "environment":"BARE_METAL",
+  "flavor": "docker",
+  "canonicalFlavor": "docker",
+  "minDiskAvailableGb": 100.0,
+  "minMainMemoryAvailableGb": 0.5,
+  "description": "Flavor-name-is-docker",
+  "minCpuCores": 0.2,
+  "fastDisk": true,
+  "environment": "DOCKER_CONTAINER",
   "rebootGeneration": 1,
   "currentRebootGeneration": 0,
   "vespaVersion": "1.2.3",
@@ -21,10 +21,24 @@
   "hostedVersion": "1.2.3",
   "convergedStateVersion": "1.2.3",
   "failCount": 1,
-  "hardwareFailure" : false,
-  "wantToRetire" : false,
-  "wantToDeprovision" : false,
-  "history":[{"event":"readied","at":123,"agent":"system"},{"event":"failed","at":123,"agent":"system"}],
-  "ipAddresses":["::1", "127.0.0.1"],
-  "additionalIpAddresses":[]
+  "hardwareFailure": false,
+  "wantToRetire": false,
+  "wantToDeprovision": false,
+  "history": [
+    {
+      "event": "readied",
+      "at": 123,
+      "agent": "system"
+    },
+    {
+      "event": "failed",
+      "at": 123,
+      "agent": "system"
+    }
+  ],
+  "ipAddresses": [
+    "::1",
+    "127.0.0.1"
+  ],
+  "additionalIpAddresses": []
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node6.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node6.json
index b13ae2ffad6..750ebbd695e 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node6.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node6.json
@@ -22,7 +22,7 @@
     "clustertype": "content",
     "clusterid": "id2",
     "group": "0",
-    "index": 0,
+    "index": 1,
     "retired": false
   },
   "restartGeneration": 0,