Merge branch 'master' into hakonhall/support-node-reports-in-node-repository

22 files changed, 484 insertions, 30 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
index 79966c8c529..b2597e9cc50 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
@@ -73,7 +73,6 @@ public final class Node {
                 Flavor flavor, Status status, State state, Optional<Allocation> allocation, History history, NodeType type,
                 Reports reports) {
         Objects.requireNonNull(id, "A node must have an ID");
-        requireNonEmpty(ipAddresses, "A node must have at least one valid IP address");
         requireNonEmptyString(hostname, "A node must have a hostname");
         requireNonEmptyString(parentHostname, "A parent host name must be a proper value");
         Objects.requireNonNull(flavor, "A node must have a flavor");
@@ -84,6 +83,9 @@ public final class Node {
         Objects.requireNonNull(type, "A null node type is not permitted");
         Objects.requireNonNull(reports, "A null reports is not permitted");
 
+        if (state == State.active)
+            requireNonEmpty(ipAddresses, "An active node must have at least one valid IP address");
+
         this.ipAddresses = ImmutableSet.copyOf(ipAddresses);
         this.ipAddressPool = new IP.AddressPool(this, ipAddressPool);
         this.hostname = hostname;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
index cbce0f38c43..601ef555cab 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
@@ -7,6 +7,7 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.NodeType;
 
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Predicate;
@@ -20,7 +21,7 @@ import static java.util.stream.Collectors.collectingAndThen;
  * @author bratseth
  * @author mpolden
  */
-public class NodeList {
+public class NodeList implements Iterable<Node> {
 
     private final List<Node> nodes;
 
@@ -103,4 +104,8 @@ public class NodeList {
         return nodes.stream().filter(predicate).collect(collectingAndThen(Collectors.toList(), NodeList::new));
     }
 
+    @Override
+    public Iterator<Node> iterator() {
+        return nodes.iterator();
+    }
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainer.java
new file mode 100644
index 00000000000..1e4016f2112
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainer.java
@@ -0,0 +1,64 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.maintenance;
+
+import com.yahoo.config.provision.NodeType;
+import com.yahoo.transaction.Mutex;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeList;
+import com.yahoo.vespa.hosted.provision.NodeRepository;
+import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
+
+import java.time.Duration;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import java.util.stream.Collectors;
+
+/**
+ * @author freva
+ */
+public class HostDeprovisionMaintainer extends Maintainer {
+
+    private static final Logger log = Logger.getLogger(HostDeprovisionMaintainer.class.getName());
+
+    private final HostProvisioner hostProvisioner;
+
+    public HostDeprovisionMaintainer(
+            NodeRepository nodeRepository, Duration interval, JobControl jobControl, HostProvisioner hostProvisioner) {
+        super(nodeRepository, interval, jobControl);
+        this.hostProvisioner = hostProvisioner;
+    }
+
+    @Override
+    protected void maintain() {
+        try (Mutex lock = nodeRepository().lockUnallocated()) {
+            NodeList nodes = nodeRepository().list();
+
+            for (Node node : candidates(nodes)) {
+                try {
+                    hostProvisioner.deprovision(node);
+                    nodeRepository().removeRecursively(node.hostname());
+                } catch (RuntimeException e) {
+                    log.log(Level.WARNING, "Failed to deprovision " + node.hostname() + ", will retry in " + interval(), e);
+                }
+            }
+        }
+    }
+
+    /** @return Nodes of type host, in any state, that have no children with allocation */
+    static Set<Node> candidates(NodeList nodes) {
+        Map<String, Node> hostsByHostname = nodes.nodeType(NodeType.host)
+                .asList().stream()
+                .collect(Collectors.toMap(Node::hostname, Function.identity()));
+
+        nodes.asList().stream()
+                .filter(node -> node.allocation().isPresent())
+                .flatMap(node -> node.parentHostname().stream())
+                .distinct()
+                .forEach(hostsByHostname::remove);
+
+        return Set.copyOf(hostsByHostname.values());
+    }
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainer.java
new file mode 100644
index 00000000000..0479f20ba37
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainer.java
@@ -0,0 +1,70 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.maintenance;
+
+import com.yahoo.config.provision.NodeType;
+import com.yahoo.transaction.Mutex;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeList;
+import com.yahoo.vespa.hosted.provision.NodeRepository;
+import com.yahoo.vespa.hosted.provision.node.Agent;
+import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
+import com.yahoo.vespa.hosted.provision.provisioning.FatalProvisioningException;
+
+import java.time.Duration;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import java.util.stream.Collectors;
+
+/**
+ * @author freva
+ */
+public class HostProvisionMaintainer extends Maintainer {
+
+    private static final Logger log = Logger.getLogger(HostProvisionMaintainer.class.getName());
+
+    private final HostProvisioner hostProvisioner;
+
+    public HostProvisionMaintainer(
+            NodeRepository nodeRepository, Duration interval, JobControl jobControl, HostProvisioner hostProvisioner) {
+        super(nodeRepository, interval, jobControl);
+        this.hostProvisioner = hostProvisioner;
+    }
+
+    @Override
+    protected void maintain() {
+        try (Mutex lock = nodeRepository().lockUnallocated()) {
+            NodeList nodes = nodeRepository().list();
+
+            candidates(nodes).forEach((host, children) -> {
+                try {
+                    List<Node> updatedNodes = hostProvisioner.provision(host, children);
+                    nodeRepository().write(updatedNodes);
+                } catch (FatalProvisioningException e) {
+                    log.log(Level.SEVERE, "Failed to provision " + host.hostname() + ", failing out the host recursively", e);
+                    // Fail out as operator to force a quick redeployment
+                    nodeRepository().failRecursively(
+                            host.hostname(), Agent.operator, "Failed by HostProvisioner due to provisioning failure");
+                } catch (RuntimeException e) {
+                    log.log(Level.WARNING, "Failed to provision " + host.hostname() + ", will retry in " + interval(), e);
+                }
+            });
+        }
+    }
+
+    /** @return map of set of children by parent Node, where parent is of type host and in state provisioned */
+    static Map<Node, Set<Node>> candidates(NodeList nodes) {
+        Map<String, Node> provisionedHostsByHostname = nodes.state(Node.State.provisioned).nodeType(NodeType.host)
+                .asList().stream()
+                .collect(Collectors.toMap(Node::hostname, Function.identity()));
+
+        return nodes.asList().stream()
+                .filter(node -> node.parentHostname().map(parent -> provisionedHostsByHostname.keySet().contains(parent)).orElse(false))
+                .collect(Collectors.groupingBy(
+                        node -> provisionedHostsByHostname.get(node.parentHostname().get()),
+                        Collectors.toSet()));
+    }
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
index 17351ab3c10..80f6f5ccea5 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
@@ -298,7 +298,7 @@ public class NodeFailer extends Maintainer {
             // the children nodes running on it before we fail the host
             boolean allTenantNodesFailedOutSuccessfully = true;
             String reasonForChildFailure = "Failing due to parent host " + node.hostname() + " failure: " + reason;
-            for (Node failingTenantNode : nodeRepository().list().childrenOf(node).asList()) {
+            for (Node failingTenantNode : nodeRepository().list().childrenOf(node)) {
                 if (failingTenantNode.state() == Node.State.active) {
                     allTenantNodesFailedOutSuccessfully &= failActive(failingTenantNode, reasonForChildFailure);
                 } else {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
index bf2e2dd5e74..5774176956a 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
@@ -52,6 +52,8 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
     private final MetricsReporter metricsReporter;
     private final InfrastructureProvisioner infrastructureProvisioner;
     private final Optional<LoadBalancerExpirer> loadBalancerExpirer;
+    private final Optional<HostProvisionMaintainer> hostProvisionMaintainer;
+    private final Optional<HostDeprovisionMaintainer> hostDeprovisionMaintainer;
 
     private final JobControl jobControl;
     private final InfrastructureVersions infrastructureVersions;
@@ -90,6 +92,10 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
         infrastructureProvisioner = new InfrastructureProvisioner(provisioner, nodeRepository, infrastructureVersions, durationFromEnv("infrastructure_provision_interval").orElse(defaults.infrastructureProvisionInterval), jobControl, duperModelInfraApi);
         loadBalancerExpirer = provisionServiceProvider.getLoadBalancerService().map(lbService ->
                 new LoadBalancerExpirer(nodeRepository, durationFromEnv("load_balancer_expiry").orElse(defaults.loadBalancerExpiry), jobControl, lbService));
+        hostProvisionMaintainer = provisionServiceProvider.getHostProvisioner().map(hostProvisioner ->
+                new HostProvisionMaintainer(nodeRepository, durationFromEnv("host_provisioner_interval").orElse(defaults.hostProvisionerInterval), jobControl, hostProvisioner));
+        hostDeprovisionMaintainer = provisionServiceProvider.getHostProvisioner().map(hostProvisioner ->
+                new HostDeprovisionMaintainer(nodeRepository, durationFromEnv("host_deprovisioner_interval").orElse(defaults.hostDeprovisionerInterval), jobControl, hostProvisioner));
 
         // The DuperModel is filled with infrastructure applications by the infrastructure provisioner, so explicitly run that now
         infrastructureProvisioner.maintain();
@@ -115,7 +121,9 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
         provisionedExpirer.deconstruct();
         metricsReporter.deconstruct();
         infrastructureProvisioner.deconstruct();
-        loadBalancerExpirer.ifPresent(LoadBalancerExpirer::deconstruct);
+        loadBalancerExpirer.ifPresent(Maintainer::deconstruct);
+        hostProvisionMaintainer.ifPresent(Maintainer::deconstruct);
+        hostDeprovisionMaintainer.ifPresent(Maintainer::deconstruct);
     }
 
     public JobControl jobControl() { return jobControl; }
@@ -164,6 +172,8 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
         private final Duration retiredInterval;
         private final Duration infrastructureProvisionInterval;
         private final Duration loadBalancerExpiry;
+        private final Duration hostProvisionerInterval;
+        private final Duration hostDeprovisionerInterval;
 
         private final NodeFailer.ThrottlePolicy throttlePolicy;
 
@@ -181,6 +191,8 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
             throttlePolicy = NodeFailer.ThrottlePolicy.hosted;
             loadBalancerExpiry = Duration.ofHours(1);
             reservationExpiry = Duration.ofMinutes(20); // Need to be long enough for deployment to be finished for all config model versions
+            hostProvisionerInterval = Duration.ofMinutes(5);
+            hostDeprovisionerInterval = Duration.ofMinutes(5);
 
             if (zone.environment().equals(Environment.prod) && zone.system() != SystemName.cd) {
                 inactiveExpiry = Duration.ofHours(4); // enough time for the application owner to discover and redeploy
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Generation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Generation.java
index e29d10f4c1f..77d0edd9d2e 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Generation.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Generation.java
@@ -43,6 +43,6 @@ public class Generation {
     }
 
     /** Returns the initial generation (0, 0) */
-    public static Generation inital() { return new Generation(0, 0); }
+    public static Generation initial() { return new Generation(0, 0); }
 
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java
index 14609809f54..1b438ef2bd6 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java
@@ -207,14 +207,10 @@ public class IP {
 
     /** Validates and returns the given set of IP addresses */
     public static Set<String> requireAddresses(Set<String> addresses) {
-        String message = "A node must have at least one valid IP address";
-        if (addresses.isEmpty()) {
-            throw new IllegalArgumentException(message);
-        }
         try {
             addresses.forEach(InetAddresses::forString);
         } catch (IllegalArgumentException e) {
-            throw new IllegalArgumentException(message, e);
+            throw new IllegalArgumentException("A node must have at least one valid IP address", e);
         }
         return addresses;
     }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java
index 82470787f5b..ff74e92b722 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java
@@ -128,7 +128,7 @@ public class Status {
 
     /** Returns the initial status of a newly provisioned node */
     public static Status initial() {
-        return new Status(Generation.inital(), Optional.empty(), 0, Optional.empty(), false,
+        return new Status(Generation.initial(), Optional.empty(), 0, Optional.empty(), false,
                           false, Optional.empty(), Optional.empty(), Optional.empty());
     }
 
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 c7bfb9178ce..2fcb26ea25b 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
@@ -117,7 +117,7 @@ public class DockerHostCapacity {
         if (!dockerHost.type().equals(NodeType.host)) return new ResourceCapacity();
 
         ResourceCapacity hostCapacity = new ResourceCapacity(dockerHost);
-        for (Node container : allNodes.childrenOf(dockerHost).asList()) {
+        for (Node container : allNodes.childrenOf(dockerHost)) {
             boolean isUsedCapacity = !(treatInactiveOrRetiredAsUnusedCapacity && isInactiveOrRetired(container));
             if (isUsedCapacity) {
                 hostCapacity.subtract(container);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FatalProvisioningException.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FatalProvisioningException.java
new file mode 100644
index 00000000000..1f38ef9b3e2
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FatalProvisioningException.java
@@ -0,0 +1,19 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.provisioning;
+
+/**
+ * Thrown by {@link HostProvisioner} to indicate that the provisioning of a host has
+ * irrecoverably failed. The host should be deprovisioned and (together with its children)
+ * removed from the node-repository.
+ *
+ * @author freva
+ */
+public class FatalProvisioningException extends RuntimeException {
+    public FatalProvisioningException(String message) {
+        super(message);
+    }
+
+    public FatalProvisioningException(String message, Exception cause) {
+        super(message, cause);
+    }
+}
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 d905282ef0b..d82e3900e76 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
@@ -5,15 +5,46 @@ import com.yahoo.config.provision.Flavor;
 import com.yahoo.vespa.hosted.provision.Node;
 
 import java.util.List;
+import java.util.Set;
 
 /**
+ * Service for provisioning physical docker tenant hosts inside the zone.
+ *
  * @author freva
  */
 public interface HostProvisioner {
 
-    List<Node> provisionHosts(int numHosts, Flavor nodeFlavor, int numNodesOnHost);
+    /**
+     * Schedule provisioning of a given number of hosts.
+     *
+     * @param numHosts number of hosts to provision
+     * @param nodeFlavor Vespa flavor of the node that will run on this host. The resulting provisioned host
+     *                   will be of a flavor that is at least as big or bigger than this.
+     * @return list of nodes that should be added to the node-repo, the list will contain exactly 2 elements:
+     * the provisioned host, and a docker container on that host with flavor {@code nodeFlavor}
+     */
+    List<Node> provisionHosts(int numHosts, Flavor nodeFlavor);
 
-    void provisioning(Node node);
+    /**
+     * Continue provisioning of given list of Nodes.
+     *
+     * @param host the host to provision
+     * @param children list of all the nodes that run on the given host
+     * @return a subset of {@code host} and {@code children} where the values have been modified and should
+     * be written back to node-repository.
+     * @throws FatalProvisioningException if the provisioning has irrecoverably failed and the input nodes
+     * should be deleted from node-repo.
+     */
+    List<Node> provision(Node host, Set<Node> children) throws FatalProvisioningException;
 
-    void deprovision(Node node);
+    /**
+     * Deprovisions a given host and resources associated with it and its children (such as DNS entries).
+     * This method will only perform the actual deprovisioning of the host and does NOT:
+     *  - verify whether it is safe to do
+     *  - clean up config server references to this node or any of its children
+     * Therefore, this method should probably only be called for hosts that have no children.
+     *
+     * @param host host to deprovision.
+     */
+    void deprovision(Node host);
 }
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 5e98466fd7c..990cc575178 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
@@ -161,7 +161,7 @@ class NodeAllocation {
      */
     private boolean exclusiveTo(TenantName tenant, Optional<String> parentHostname) {
         if ( ! parentHostname.isPresent()) return true;
-        for (Node nodeOnHost : nodeRepository.list().childrenOf(parentHostname.get()).asList()) {
+        for (Node nodeOnHost : nodeRepository.list().childrenOf(parentHostname.get())) {
             if ( ! nodeOnHost.allocation().isPresent()) continue;
 
             if ( nodeOnHost.allocation().get().membership().cluster().isExclusive() &&
@@ -174,7 +174,7 @@ class NodeAllocation {
     private boolean hostsOnly(TenantName tenant, Optional<String> parentHostname) {
         if ( ! parentHostname.isPresent()) return true; // yes, as host is exclusive
 
-        for (Node nodeOnHost : nodeRepository.list().childrenOf(parentHostname.get()).asList()) {
+        for (Node nodeOnHost : nodeRepository.list().childrenOf(parentHostname.get())) {
             if ( ! nodeOnHost.allocation().isPresent()) continue;
             if ( ! nodeOnHost.allocation().get().owner().tenant().equals(tenant))
                 return false;
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainerTest.java
new file mode 100644
index 00000000000..95c857527a1
--- /dev/null
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainerTest.java
@@ -0,0 +1,98 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.maintenance;
+
+import com.yahoo.config.provision.NodeType;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeList;
+import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
+import org.junit.Test;
+
+import java.time.Duration;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+
+import static com.yahoo.vespa.hosted.provision.maintenance.HostProvisionMaintainerTest.HostProvisionerTester;
+import static com.yahoo.vespa.hosted.provision.maintenance.HostProvisionMaintainerTest.HostProvisionerTester.createNode;
+import static com.yahoo.vespa.hosted.provision.maintenance.HostProvisionMaintainerTest.HostProvisionerTester.proxyApp;
+import static com.yahoo.vespa.hosted.provision.maintenance.HostProvisionMaintainerTest.HostProvisionerTester.proxyHostApp;
+import static com.yahoo.vespa.hosted.provision.maintenance.HostProvisionMaintainerTest.HostProvisionerTester.tenantApp;
+import static com.yahoo.vespa.hosted.provision.maintenance.HostProvisionMaintainerTest.HostProvisionerTester.tenantHostApp;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+/**
+ * @author freva
+ */
+public class HostDeprovisionMaintainerTest {
+    private final HostProvisionerTester tester = new HostProvisionerTester();
+    private final HostProvisioner hostProvisioner = mock(HostProvisioner.class);
+    private final HostDeprovisionMaintainer maintainer = new HostDeprovisionMaintainer(
+            tester.nodeRepository(), Duration.ofDays(1), tester.jobControl(), hostProvisioner);
+
+    @Test
+    public void removes_nodes_if_successful() {
+        tester.addNode("host1", Optional.empty(), NodeType.host, Node.State.active, Optional.of(tenantHostApp));
+        tester.addNode("host1-1", Optional.of("host1"), NodeType.tenant, Node.State.reserved, Optional.of(tenantApp));
+        tester.addNode("host1-2", Optional.of("host1"), NodeType.tenant, Node.State.failed, Optional.empty());
+        tester.addNode("host4", Optional.empty(), NodeType.host, Node.State.provisioned, Optional.empty());
+        tester.addNode("host4-1", Optional.of("host4"), NodeType.tenant, Node.State.reserved, Optional.of(tenantApp));
+
+        Node host2 = tester.addNode("host2", Optional.empty(), NodeType.host, Node.State.failed, Optional.of(tenantApp));
+        tester.addNode("host2-1", Optional.of("host2"), NodeType.tenant, Node.State.failed, Optional.empty());
+
+        assertEquals(7, tester.nodeRepository().getNodes().size());
+
+        maintainer.maintain();
+
+        assertEquals(5, tester.nodeRepository().getNodes().size());
+        verify(hostProvisioner).deprovision(eq(host2));
+        verifyNoMoreInteractions(hostProvisioner);
+
+        assertTrue(tester.nodeRepository().getNode("host2").isEmpty());
+        assertTrue(tester.nodeRepository().getNode("host2-1").isEmpty());
+    }
+
+    @Test
+    public void does_not_remove_if_failed() {
+        Node host2 = tester.addNode("host2", Optional.empty(), NodeType.host, Node.State.failed, Optional.of(tenantApp));
+        doThrow(new RuntimeException()).when(hostProvisioner).deprovision(eq(host2));
+
+        maintainer.maintain();
+
+        assertEquals(1, tester.nodeRepository().getNodes().size());
+        verify(hostProvisioner).deprovision(eq(host2));
+        verifyNoMoreInteractions(hostProvisioner);
+    }
+
+    @Test
+    public void finds_nodes_that_need_deprovisioning() {
+        Node host2 = createNode("host2", Optional.empty(), NodeType.host, Node.State.failed, Optional.of(tenantApp));
+        Node host21 = createNode("host2-1", Optional.of("host2"), NodeType.tenant, Node.State.failed, Optional.empty());
+        Node host3 = createNode("host3", Optional.empty(), NodeType.host, Node.State.provisioned, Optional.empty());
+
+        List<Node> nodes = List.of(
+                createNode("host1", Optional.empty(), NodeType.host, Node.State.active, Optional.of(tenantHostApp)),
+                createNode("host1-1", Optional.of("host1"), NodeType.tenant, Node.State.reserved, Optional.of(tenantApp)),
+                createNode("host1-2", Optional.of("host1"), NodeType.tenant, Node.State.failed, Optional.empty()),
+
+                host2, host21, host3,
+
+                createNode("host4", Optional.empty(), NodeType.host, Node.State.provisioned, Optional.empty()),
+                createNode("host4-1", Optional.of("host4"), NodeType.tenant, Node.State.reserved, Optional.of(tenantApp)),
+
+                createNode("proxyhost1", Optional.empty(), NodeType.proxyhost, Node.State.provisioned, Optional.empty()),
+
+                createNode("proxyhost2", Optional.empty(), NodeType.proxyhost, Node.State.active, Optional.of(proxyHostApp)),
+                createNode("proxy2", Optional.of("proxyhost2"), NodeType.proxy, Node.State.active, Optional.of(proxyApp)));
+
+        Set<Node> expected = Set.of(host2, host3);
+        Set<Node> actual = HostDeprovisionMaintainer.candidates(new NodeList(nodes));
+        assertEquals(expected, actual);
+    }
+}
\ No newline at end of file
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainerTest.java
new file mode 100644
index 00000000000..01c83bbeff4
--- /dev/null
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainerTest.java
@@ -0,0 +1,157 @@
+package com.yahoo.vespa.hosted.provision.maintenance;
+
+import com.yahoo.component.Version;
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ClusterMembership;
+import com.yahoo.config.provision.DockerImage;
+import com.yahoo.config.provision.Flavor;
+import com.yahoo.config.provision.NodeFlavors;
+import com.yahoo.config.provision.NodeType;
+import com.yahoo.config.provision.Zone;
+import com.yahoo.test.ManualClock;
+import com.yahoo.vespa.curator.mock.MockCurator;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeList;
+import com.yahoo.vespa.hosted.provision.NodeRepository;
+import com.yahoo.vespa.hosted.provision.node.Allocation;
+import com.yahoo.vespa.hosted.provision.node.Generation;
+import com.yahoo.vespa.hosted.provision.node.History;
+import com.yahoo.vespa.hosted.provision.node.Status;
+import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder;
+import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
+import com.yahoo.vespa.hosted.provision.provisioning.FatalProvisioningException;
+import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver;
+import org.junit.Test;
+
+import java.time.Duration;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static com.yahoo.vespa.hosted.provision.maintenance.HostProvisionMaintainerTest.HostProvisionerTester.createNode;
+import static com.yahoo.vespa.hosted.provision.maintenance.HostProvisionMaintainerTest.HostProvisionerTester.proxyApp;
+import static com.yahoo.vespa.hosted.provision.maintenance.HostProvisionMaintainerTest.HostProvisionerTester.proxyHostApp;
+import static com.yahoo.vespa.hosted.provision.maintenance.HostProvisionMaintainerTest.HostProvisionerTester.tenantApp;
+import static com.yahoo.vespa.hosted.provision.maintenance.HostProvisionMaintainerTest.HostProvisionerTester.tenantHostApp;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+/**
+ * @author freva
+ */
+public class HostProvisionMaintainerTest {
+
+    private final HostProvisionerTester tester = new HostProvisionerTester();
+    private final HostProvisioner hostProvisioner = mock(HostProvisioner.class);
+    private final HostProvisionMaintainer maintainer = new HostProvisionMaintainer(
+            tester.nodeRepository(), Duration.ofDays(1), tester.jobControl(), hostProvisioner);
+
+    @Test
+    public void delegates_to_host_provisioner_and_writes_back_result() {
+        tester.addNode("host1", Optional.empty(), NodeType.host, Node.State.active, Optional.of(tenantHostApp));
+        tester.addNode("host1-1", Optional.of("host1"), NodeType.tenant, Node.State.reserved, Optional.of(tenantApp));
+        tester.addNode("host1-2", Optional.of("host1"), NodeType.tenant, Node.State.failed, Optional.empty());
+        tester.addNode("host2", Optional.empty(), NodeType.host, Node.State.failed, Optional.of(tenantApp));
+
+        Node host4 = tester.addNode("host4", Optional.empty(), NodeType.host, Node.State.provisioned, Optional.empty());
+        Node host41 = tester.addNode("host4-1", Optional.of("host4"), NodeType.tenant, Node.State.reserved, Optional.of(tenantApp));
+
+        Node host4new = host4.withIpAddresses(Set.of("::2"));
+        Node host41new = host41.withIpAddresses(Set.of("::4", "10.0.0.1"));
+        assertTrue(Stream.of(host4, host41).map(Node::ipAddresses).allMatch(Set::isEmpty));
+        when(hostProvisioner.provision(eq(host4), eq(Set.of(host41)))).thenReturn(List.of(host4new, host41new));
+
+        maintainer.maintain();
+        verify(hostProvisioner).provision(eq(host4), eq(Set.of(host41)));
+        verifyNoMoreInteractions(hostProvisioner);
+
+        assertEquals(Optional.of(host4new), tester.nodeRepository().getNode("host4"));
+        assertEquals(Optional.of(host41new), tester.nodeRepository().getNode("host4-1"));
+    }
+
+    @Test
+    public void correctly_fails_if_irrecoverable_failure() {
+        Node host4 = tester.addNode("host4", Optional.empty(), NodeType.host, Node.State.provisioned, Optional.empty());
+        Node host41 = tester.addNode("host4-1", Optional.of("host4"), NodeType.tenant, Node.State.reserved, Optional.of(tenantApp));
+
+        assertTrue(Stream.of(host4, host41).map(Node::ipAddresses).allMatch(Set::isEmpty));
+        when(hostProvisioner.provision(eq(host4), eq(Set.of(host41)))).thenThrow(new FatalProvisioningException("Fatal"));
+
+        maintainer.maintain();
+
+        assertEquals(Set.of("host4", "host4-1"),
+                tester.nodeRepository().getNodes(Node.State.failed).stream().map(Node::hostname).collect(Collectors.toSet()));
+    }
+
+    @Test
+    public void finds_nodes_that_need_provisioning() {
+        Node host4 = createNode("host4", Optional.empty(), NodeType.host, Node.State.provisioned, Optional.empty());
+        Node host41 = createNode("host4-1", Optional.of("host4"), NodeType.tenant, Node.State.reserved, Optional.of(tenantApp));
+
+        List<Node> nodes = List.of(
+                createNode("host1", Optional.empty(), NodeType.host, Node.State.active, Optional.of(tenantHostApp)),
+                createNode("host1-1", Optional.of("host1"), NodeType.tenant, Node.State.reserved, Optional.of(tenantApp)),
+                createNode("host1-2", Optional.of("host1"), NodeType.tenant, Node.State.failed, Optional.empty()),
+
+                createNode("host2", Optional.empty(), NodeType.host, Node.State.failed, Optional.of(tenantApp)),
+
+                createNode("host3", Optional.empty(), NodeType.host, Node.State.provisioned, Optional.empty()),
+
+                host4, host41,
+
+                createNode("proxyhost1", Optional.empty(), NodeType.proxyhost, Node.State.provisioned, Optional.empty()),
+
+                createNode("proxyhost2", Optional.empty(), NodeType.proxyhost, Node.State.active, Optional.of(proxyHostApp)),
+                createNode("proxy2", Optional.of("proxyhost2"), NodeType.proxy, Node.State.active, Optional.of(proxyApp)));
+
+        Map<Node, Set<Node>> expected = Map.of(host4, Set.of(host41));
+        Map<Node, Set<Node>> actual = HostProvisionMaintainer.candidates(new NodeList(nodes));
+        assertEquals(expected, actual);
+    }
+
+    static class HostProvisionerTester {
+        private static final NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default", "docker");
+        static final ApplicationId tenantApp = ApplicationId.from("mytenant", "myapp", "default");
+        static final ApplicationId tenantHostApp = ApplicationId.from("vespa", "tenant-host", "default");
+        static final ApplicationId proxyHostApp = ApplicationId.from("vespa", "proxy-host", "default");
+        static final ApplicationId proxyApp = ApplicationId.from("vespa", "proxy", "default");
+
+        private final ManualClock clock = new ManualClock();
+        private final NodeRepository nodeRepository = new NodeRepository(
+                nodeFlavors, new MockCurator(), clock, Zone.defaultZone(), new MockNameResolver().mockAnyLookup(), new DockerImage("docker-image"), true);
+
+        Node addNode(String hostname, Optional<String> parentHostname, NodeType nodeType, Node.State state, Optional<ApplicationId> application) {
+            Node node = createNode(hostname, parentHostname, nodeType, state, application);
+            return nodeRepository.database().addNodesInState(List.of(node), node.state()).get(0);
+        }
+
+        static Node createNode(String hostname, Optional<String> parentHostname, NodeType nodeType, Node.State state, Optional<ApplicationId> application) {
+            Flavor flavor = nodeFlavors.getFlavor(parentHostname.isPresent() ? "docker" : "default").orElseThrow();
+            Optional<Allocation> allocation = application
+                    .map(app -> new Allocation(
+                            app,
+                            ClusterMembership.from("container/default/0/0", Version.fromString("7.3")),
+                            Generation.initial(),
+                            false));
+            Set<String> ips = state == Node.State.active ? Set.of("::1") : Set.of();
+            return new Node("fake-id-" + hostname, ips, Set.of(), hostname,
+                    parentHostname, flavor, Status.initial(), state, allocation, History.empty(), nodeType);
+        }
+
+        NodeRepository nodeRepository() {
+            return nodeRepository;
+        }
+
+        JobControl jobControl() {
+            return new JobControl(nodeRepository.database());
+        }
+    }
+}
\ No newline at end of file
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/monitoring/MetricsReporterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/monitoring/MetricsReporterTest.java
index de181c2d7e1..6b45352adba 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/monitoring/MetricsReporterTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/monitoring/MetricsReporterTest.java
@@ -198,7 +198,7 @@ public class MetricsReporterTest {
         if (tenant.isPresent()) {
             Allocation allocation = new Allocation(app(tenant.get()),
                                                    ClusterMembership.from("container/id1/0/3", new Version()),
-                                                   Generation.inital(),
+                                                   Generation.initial(),
                                                    false);
             return Optional.of(allocation);
         }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationSimulator.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationSimulator.java
index 134448971fb..dc936b0f29f 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationSimulator.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationSimulator.java
@@ -96,7 +96,7 @@ public class AllocationSimulator {
         if (tenant.isPresent()) {
             Allocation allocation = new Allocation(app(tenant.get()),
                                                    ClusterMembership.from("container/id1/3", new Version()),
-                                                   Generation.inital(),
+                                                   Generation.initial(),
                                                    false);
             return Optional.of(allocation);
         }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationVisualizer.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationVisualizer.java
index 24011bd4b49..61e14be53d8 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationVisualizer.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationVisualizer.java
@@ -76,14 +76,14 @@ public class AllocationVisualizer extends JPanel {
         // Draw the docker hosts - and color each container according to application
         AllocationSnapshot simStep = steps.get(step);
         NodeList hosts = simStep.nodes.nodeType(NodeType.host);
-        for (Node host : hosts.asList()) {
+        for (Node host : hosts) {
 
             // Paint the host
             paintNode(host, g, nodeX, nodeY, true);
 
             // Paint containers
             NodeList containers = simStep.nodes.childrenOf(host);
-            for (Node container : containers.asList()) {
+            for (Node container : containers) {
                 nodeY = paintNode(container, g, nodeX, nodeY, false);
             }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java
index 85ecf9c5073..0003e4f23cc 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java
@@ -166,17 +166,17 @@ public class DockerProvisioningTest {
 
         ApplicationId application1 = tester.makeApplicationId();
         prepareAndActivate(application1, 2, false, tester);
-        for (Node node : tester.getNodes(application1, Node.State.active).asList())
+        for (Node node : tester.getNodes(application1, Node.State.active))
             assertFalse(node.allocation().get().membership().cluster().isExclusive());
 
         prepareAndActivate(application1, 2, true, tester);
         assertEquals(setOf("host1", "host2"), hostsOf(tester.getNodes(application1, Node.State.active)));
-        for (Node node : tester.getNodes(application1, Node.State.active).asList())
+        for (Node node : tester.getNodes(application1, Node.State.active))
             assertTrue(node.allocation().get().membership().cluster().isExclusive());
 
         prepareAndActivate(application1, 2, false, tester);
         assertEquals(setOf("host1", "host2"), hostsOf(tester.getNodes(application1, Node.State.active)));
-        for (Node node : tester.getNodes(application1, Node.State.active).asList())
+        for (Node node : tester.getNodes(application1, Node.State.active))
             assertFalse(node.allocation().get().membership().cluster().isExclusive());
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java
index 86daf636875..308127c7f39 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java
@@ -166,7 +166,7 @@ public class MultigroupProvisioningTest {
 
         // Check invariants for all nodes
         Set<Integer> allIndexes = new HashSet<>();
-        for (Node node : tester.getNodes(application, Node.State.active).asList()) {
+        for (Node node : tester.getNodes(application, Node.State.active)) {
             // Node indexes must be unique
             int index = node.allocation().get().membership().index();
             assertFalse("Node indexes are unique", allIndexes.contains(index));
@@ -178,7 +178,7 @@ public class MultigroupProvisioningTest {
         // Count unretired nodes and groups of the requested flavor
         Set<Integer> indexes = new HashSet<>();
         Map<ClusterSpec.Group, Integer> nonretiredGroups = new HashMap<>();
-        for (Node node : tester.getNodes(application, Node.State.active).nonretired().flavor(flavor).asList()) {
+        for (Node node : tester.getNodes(application, Node.State.active).nonretired().flavor(flavor)) {
             indexes.add(node.allocation().get().membership().index());
 
             ClusterSpec.Group group = node.allocation().get().membership().cluster().group().get();
@@ -193,7 +193,7 @@ public class MultigroupProvisioningTest {
             assertEquals("Group size", (long)nodeCount / wantedGroups, (long)groupSize);
 
         Map<ClusterSpec.Group, Integer> allGroups = new HashMap<>();
-        for (Node node : tester.getNodes(application, Node.State.active).flavor(flavor).asList()) {
+        for (Node node : tester.getNodes(application, Node.State.active).flavor(flavor)) {
             ClusterSpec.Group group = node.allocation().get().membership().cluster().group().get();
             allGroups.put(group, nonretiredGroups.getOrDefault(group, 0) + 1);
         }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
index 0f86b072d4d..fabc842c377 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
@@ -279,8 +279,8 @@ public class ProvisioningTest {
         SystemState state2 = prepare(application1, 2, 2, 4, 4, "dockerSmall", tester);
         tester.activate(application1, state2.allHosts);
 
-        assertEquals(12, tester.getNodes(application1, Node.State.active).asList().size());
-        for (Node node : tester.getNodes(application1, Node.State.active).asList())
+        assertEquals(12, tester.getNodes(application1, Node.State.active).size());
+        for (Node node : tester.getNodes(application1, Node.State.active))
             assertEquals("Node changed flavor in place", "dockerSmall", node.flavor().name());
         assertEquals("No nodes are retired",
                      0, tester.getNodes(application1, Node.State.active).retired().size());
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/NodeIdentifierTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/NodeIdentifierTest.java
index 624845cc5e0..c3d320119f4 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/NodeIdentifierTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/NodeIdentifierTest.java
@@ -245,7 +245,7 @@ public class NodeIdentifierTest {
                                                 Version.emptyVersion,
                                                 false),
                                         clusterIndex),
-                                Generation.inital(),
+                                Generation.initial(),
                                 false));
 
     }