Merge pull request #2132 from yahoo/bratseth/fast-deploy-on-manual-changes-2

Bratseth/fast deploy on manual changes 2

8 files changed, 343 insertions, 54 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
index 819a4acfb1b..3bc9b84003b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
@@ -13,6 +13,7 @@ import com.yahoo.config.provision.Zone;
 import com.yahoo.transaction.Mutex;
 import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.curator.Curator;
+import com.yahoo.vespa.hosted.provision.maintenance.PeriodicApplicationMaintainer;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.NodeAcl;
 import com.yahoo.vespa.hosted.provision.node.filter.NodeFilter;
@@ -50,7 +51,7 @@ import java.util.stream.Collectors;
  * Changes to the set of active nodes must be accompanied by changes to the config model of the application.
  * Such changes are not handled by the node repository but by the classes calling it - see
  * {@link com.yahoo.vespa.hosted.provision.provisioning.NodeRepositoryProvisioner} for such changes initiated
- * by the application package and {@link com.yahoo.vespa.hosted.provision.maintenance.ApplicationMaintainer}
+ * by the application package and {@link PeriodicApplicationMaintainer}
  * for changes initiated by the node repository.
  * Refer to {@link com.yahoo.vespa.hosted.provision.maintenance.NodeRepositoryMaintenance} for timing details
  * of the node state transitions.
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
index 28d6297ca18..84679cf972a 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
@@ -1,4 +1,3 @@
-// Copyright 2016 Yahoo 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.ApplicationId;
@@ -15,33 +14,26 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
-import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
 
 /**
- * The application maintainer regularly redeploys all applications.
- * This is necessary because applications may gain and lose active nodes due to nodes being moved to and from the
- * failed state. This is corrected by redeploying the applications periodically.
- * It can not (at this point) be done reliably synchronously as part of the fail/reactivate call due to the need for this
- * to happen at a node having the deployer.
- *
  * @author bratseth
  */
-public class ApplicationMaintainer extends Maintainer {
+public abstract class ApplicationMaintainer extends Maintainer {
 
     private final Deployer deployer;
 
     private final Executor deploymentExecutor = Executors.newCachedThreadPool();
-    
-    public ApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Duration interval) {
+
+    protected ApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Duration interval) {
         super(nodeRepository, interval);
         this.deployer = deployer;
     }
 
     @Override
     protected void maintain() {
-        Set<ApplicationId> applications = activeApplications();
+        Set<ApplicationId> applications = applicationsNeedingMaintenance();
         for (ApplicationId application : applications) {
             try {
                 // An application might change it's state between the time the set of applications is retrieved and the
@@ -53,8 +45,7 @@ public class ApplicationMaintainer extends Maintainer {
                     Optional<Deployment> deployment = deployer.deployFromLocalActive(application, Duration.ofMinutes(30));
                     if ( ! deployment.isPresent()) continue; // this will be done at another config server
 
-                    // deploy asynchronously to make sure we do all applications even when deployments are slow
-                    deployAsynchronously(deployment.get()); 
+                    deploy(application, deployment.get());
                 }
                 throttle(applications.size());
             }
@@ -63,8 +54,13 @@ public class ApplicationMaintainer extends Maintainer {
             }
         }
     }
-    
-    protected void deployAsynchronously(Deployment deployment) {
+
+    /**
+     * Redeploy this application using the provided deployer.
+     * The default implementation deploys asynchronously to make sure we do all applications timely 
+     * even when deployments are slow.
+     */
+    protected void deploy(ApplicationId applicationId, Deployment deployment) {
         deploymentExecutor.execute(() -> {
             try {
                 deployment.activate();
@@ -74,23 +70,24 @@ public class ApplicationMaintainer extends Maintainer {
             }
         });
     }
-    
-    protected void throttle(int applicationCount) {
-        // Sleep for a length of time that will spread deployment evenly over the maintenance period
-        try { Thread.sleep(interval().toMillis() / applicationCount); } catch (InterruptedException e) { return; }
-    }
 
-    protected Set<ApplicationId> activeApplications() {
-        return nodeRepository().getNodes(Node.State.active).stream()
+    /** Block in this method until the next application should be maintained */
+    protected abstract void throttle(int applicationCount);
+
+    private Set<ApplicationId> applicationsNeedingMaintenance() {
+        return nodesNeedingMaintenance().stream()
                 .map(node -> node.allocation().get().owner())
                 .collect(Collectors.toCollection(LinkedHashSet::new));
     }
 
+    /** 
+     * Returns the nodes whose applications should be maintained by this now. 
+     * This should be some subset of the allocated nodes. 
+     */
+    protected abstract List<Node> nodesNeedingMaintenance();
+
     private boolean isActive(ApplicationId application) {
         return ! nodeRepository().getNodes(application, Node.State.active).isEmpty();
     }
 
-    @Override
-    public String toString() { return "Periodic application redeployer"; }
-
 }
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 12b309bd7e3..8ef423fa6da 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
@@ -30,7 +30,8 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
     private static final String envPrefix = "vespa_node_repository__";
 
     private final NodeFailer nodeFailer;
-    private final ApplicationMaintainer applicationMaintainer;
+    private final PeriodicApplicationMaintainer periodicApplicationMaintainer;
+    private final OperatorChangeApplicationMaintainer operatorChangeApplicationMaintainer;
     private final ZooKeeperAccessMaintainer zooKeeperAccessMaintainer;
     private final ReservationExpirer reservationExpirer;
     private final InactiveExpirer inactiveExpirer;
@@ -52,7 +53,8 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
                                      Zone zone, Clock clock, Orchestrator orchestrator, Metric metric) {
         DefaultTimes defaults = new DefaultTimes(zone.environment());
         nodeFailer = new NodeFailer(deployer, hostLivenessTracker, serviceMonitor, nodeRepository, durationFromEnv("fail_grace").orElse(defaults.failGrace), clock, orchestrator, throttlePolicyFromEnv("throttle_policy").orElse(defaults.throttlePolicy));
-        applicationMaintainer = new ApplicationMaintainer(deployer, nodeRepository, durationFromEnv("redeploy_frequency").orElse(defaults.redeployFrequency));
+        periodicApplicationMaintainer = new PeriodicApplicationMaintainer(deployer, nodeRepository, durationFromEnv("periodic_redeploy_interval").orElse(defaults.periodicRedeployInterval));
+        operatorChangeApplicationMaintainer = new OperatorChangeApplicationMaintainer(deployer, nodeRepository, clock, durationFromEnv("operator_change_redeploy_interval").orElse(defaults.operatorChangeRedeployInterval));
         zooKeeperAccessMaintainer = new ZooKeeperAccessMaintainer(nodeRepository, curator, durationFromEnv("zookeeper_access_maintenance_interval").orElse(defaults.zooKeeperAccessMaintenanceInterval));
         reservationExpirer = new ReservationExpirer(nodeRepository, clock, durationFromEnv("reservation_expiry").orElse(defaults.reservationExpiry));
         retiredExpirer = new RetiredExpirer(nodeRepository, deployer, clock, durationFromEnv("retired_expiry").orElse(defaults.retiredExpiry));
@@ -66,7 +68,8 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
     @Override
     public void deconstruct() {
         nodeFailer.deconstruct();
-        applicationMaintainer.deconstruct();
+        periodicApplicationMaintainer.deconstruct();
+        operatorChangeApplicationMaintainer.deconstruct();
         zooKeeperAccessMaintainer.deconstruct();
         reservationExpirer.deconstruct();
         inactiveExpirer.deconstruct();
@@ -94,8 +97,10 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
 
     private static class DefaultTimes {
 
-        /** All applications are redeployed with this frequency */
-        private final Duration redeployFrequency;
+        /** All applications are redeployed with this period */
+        private final Duration periodicRedeployInterval;
+        /** Applications are redeployed after manual operator changes within this time period */
+        private final Duration operatorChangeRedeployInterval;
 
         /** The time a node must be continuously nonresponsive before it is failed */
         private final Duration failGrace;
@@ -117,7 +122,8 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
                 // These values are to avoid losing data (retired), and to be able to return an application
                 // back to a previous state fast (inactive)
                 failGrace = Duration.ofMinutes(60);
-                redeployFrequency = Duration.ofMinutes(30);
+                periodicRedeployInterval = Duration.ofMinutes(30);
+                operatorChangeRedeployInterval = Duration.ofMinutes(1);
                 zooKeeperAccessMaintenanceInterval = Duration.ofMinutes(1);
                 reservationExpiry = Duration.ofMinutes(20); // same as deployment timeout
                 inactiveExpiry = Duration.ofHours(4); // enough time for the application owner to discover and redeploy
@@ -131,7 +137,8 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
                 // These values ensure tests and development is not delayed due to nodes staying around
                 // Use non-null values as these also determine the maintenance interval
                 failGrace = Duration.ofMinutes(60);
-                redeployFrequency = Duration.ofMinutes(30);
+                periodicRedeployInterval = Duration.ofMinutes(30);
+                operatorChangeRedeployInterval = Duration.ofMinutes(1);
                 zooKeeperAccessMaintenanceInterval = Duration.ofSeconds(10);
                 reservationExpiry = Duration.ofMinutes(10); // Need to be long enough for deployment to be finished for all config model versions
                 inactiveExpiry = Duration.ofSeconds(2); // support interactive wipe start over
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java
new file mode 100644
index 00000000000..45dadb0e09a
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java
@@ -0,0 +1,81 @@
+// Copyright 2016 Yahoo 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.ApplicationId;
+import com.yahoo.config.provision.Deployer;
+import com.yahoo.config.provision.Deployment;
+import com.yahoo.transaction.Mutex;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeRepository;
+import com.yahoo.vespa.hosted.provision.node.Agent;
+import com.yahoo.vespa.hosted.provision.node.History;
+
+import java.time.Clock;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
+import java.util.logging.Level;
+import java.util.stream.Collectors;
+
+/**
+ * The application maintainer detects manual operator changes to nodes and redeploys affected applications.
+ * The purpose of this is to redeploy affected applications faster than achieved by the regular application
+ * maintenance to reduce the time period where the node repository and the application model is out of sync.
+ * 
+ * Why can't the manual change directly make the application redeployment?
+ * Because the redeployment must run at the right config server, while the node state change may be running
+ * at any config server.
+ *
+ * @author bratseth
+ */
+public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer {
+
+    private final Clock clock;
+    
+    private Instant previousRun;
+    
+    public OperatorChangeApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Clock clock,
+                                               Duration interval) {
+        super(deployer, nodeRepository, interval);
+        this.clock = clock;
+        previousRun = clock.instant(); // Changes before this will be caught by the first PeriodicApplicationMaintainer run
+    }
+
+    @Override
+    protected List<Node> nodesNeedingMaintenance() {
+        Instant windowEnd = clock.instant();
+        Instant windowStart = previousRun;
+        previousRun = windowEnd;
+        return nodeRepository().getNodes().stream()
+                               .filter(node -> node.allocation().isPresent())
+                               .filter(node -> hasManualStateChangeSince(windowStart, node))
+                               .collect(Collectors.toList());
+    }
+    
+    private boolean hasManualStateChangeSince(Instant instant, Node node) {
+        return node.history().events().stream()
+                .anyMatch(event -> event.agent() == Agent.operator && event.at().isAfter(instant));
+    }
+
+    protected void throttle(int applicationCount) { }
+
+    /** 
+     * Deploy in the maintenance thread to avoid scheduling multiple deployments of the same application if it takes
+     * longer to deploy than the (short) maintenance interval of this
+     */
+    @Override
+    protected void deploy(ApplicationId applicationId, Deployment deployment) {
+        deployment.activate();
+        log.info("Redeployed application " + applicationId.toShortString() + 
+                 " as a manual change was made to its nodes");
+    }
+
+    @Override
+    public String toString() { return "Operator change application redeployer"; }
+
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java
new file mode 100644
index 00000000000..b106d739f64
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java
@@ -0,0 +1,48 @@
+// Copyright 2016 Yahoo 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.ApplicationId;
+import com.yahoo.config.provision.Deployer;
+import com.yahoo.config.provision.Deployment;
+import com.yahoo.transaction.Mutex;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeRepository;
+
+import java.time.Duration;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
+import java.util.function.Function;
+import java.util.logging.Level;
+import java.util.stream.Collectors;
+
+/**
+ * The application maintainer regularly redeploys all applications to make sure the node repo and application
+ * model is in sync and to trigger background node allocation changes such as allocation optimizations and
+ * flavor retirement.
+ *
+ * @author bratseth
+ */
+public class PeriodicApplicationMaintainer extends ApplicationMaintainer {
+
+    public PeriodicApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Duration interval) {
+        super(deployer, nodeRepository, interval);
+    }
+
+    protected void throttle(int applicationCount) {
+        // Sleep for a length of time that will spread deployment evenly over the maintenance period
+        try { Thread.sleep(interval().toMillis() / applicationCount); } catch (InterruptedException e) { return; }
+    }
+
+    @Override
+    protected List<Node> nodesNeedingMaintenance() {
+        return nodeRepository().getNodes(Node.State.active);
+    }
+
+    @Override
+    public String toString() { return "Periodic application redeployer"; }
+
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java
index cb271f90e6e..49c077f3a05 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java
@@ -148,7 +148,7 @@ public class History {
         public Instant at() { return at; }
 
         @Override
-        public String toString() { return "'" + type + "' event at " + at; }
+        public String toString() { return "'" + type + "' event at " + at + " by " + agent; }
 
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java
new file mode 100644
index 00000000000..c4135b19a63
--- /dev/null
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java
@@ -0,0 +1,159 @@
+// Copyright 2016 Yahoo 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.ApplicationId;
+import com.yahoo.config.provision.ApplicationName;
+import com.yahoo.config.provision.Capacity;
+import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.Deployer;
+import com.yahoo.config.provision.Deployment;
+import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.HostSpec;
+import com.yahoo.config.provision.InstanceName;
+import com.yahoo.config.provision.NodeFlavors;
+import com.yahoo.config.provision.NodeType;
+import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.TenantName;
+import com.yahoo.config.provision.Zone;
+import com.yahoo.test.ManualClock;
+import com.yahoo.transaction.NestedTransaction;
+import com.yahoo.vespa.curator.Curator;
+import com.yahoo.vespa.curator.mock.MockCurator;
+import com.yahoo.vespa.curator.transaction.CuratorTransaction;
+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.NodeRepositoryProvisioner;
+import com.yahoo.vespa.hosted.provision.testutils.FlavorConfigBuilder;
+import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author bratseth
+ */
+public class OperatorChangeApplicationMaintainerTest {
+
+    private static final NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default");
+
+    private NodeRepository nodeRepository;
+    private Fixture fixture;
+
+    @Test
+    public void test_application_maintenance() throws InterruptedException {
+        ManualClock clock = new ManualClock();
+        Curator curator = new MockCurator();
+        Zone zone = new Zone(Environment.prod, RegionName.from("us-east"));
+        this.nodeRepository = new NodeRepository(nodeFlavors, curator, clock, zone,
+                                                 new MockNameResolver().mockAnyLookup());
+        this.fixture = new Fixture(zone, nodeRepository, nodeFlavors, curator);
+
+        createReadyNodes(15, nodeRepository, nodeFlavors);
+        createHostNodes(2, nodeRepository, nodeFlavors);
+
+        // Create applications
+        fixture.activate();
+        OperatorChangeApplicationMaintainer maintainer = new OperatorChangeApplicationMaintainer(fixture.deployer, nodeRepository, clock, Duration.ofMinutes(1));
+        
+        clock.advance(Duration.ofMinutes(2));
+        maintainer.maintain();
+        assertEquals("No changes -> no redeployments", 0, fixture.deployer.redeployments);
+
+        nodeRepository.fail(nodeRepository.getNodes(fixture.app1).get(3).hostname(), Agent.system, "Failing to unit test");
+        clock.advance(Duration.ofMinutes(2));
+        maintainer.maintain();
+        assertEquals("System change -> no redeployments", 0, fixture.deployer.redeployments);
+
+        clock.advance(Duration.ofSeconds(1));
+        nodeRepository.fail(nodeRepository.getNodes(fixture.app2).get(4).hostname(), Agent.operator, "Manual node failing");
+        clock.advance(Duration.ofMinutes(2));
+        maintainer.maintain();
+        assertEquals("Operator change -> redeployment", 1, fixture.deployer.redeployments);
+
+        clock.advance(Duration.ofMinutes(2));
+        maintainer.maintain();
+        assertEquals("No further operator changes -> no (new) redeployments", 1, fixture.deployer.redeployments);
+    }
+
+    private void createReadyNodes(int count, NodeRepository nodeRepository, NodeFlavors nodeFlavors) {
+        List<Node> nodes = new ArrayList<>(count);
+        for (int i = 0; i < count; i++)
+            nodes.add(nodeRepository.createNode("node" + i, "host" + i, Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.tenant));
+        nodes = nodeRepository.addNodes(nodes);
+        nodes = nodeRepository.setDirty(nodes);
+        nodeRepository.setReady(nodes);
+    }
+
+    private void createHostNodes(int count, NodeRepository nodeRepository, NodeFlavors nodeFlavors) {
+        List<Node> nodes = new ArrayList<>(count);
+        for (int i = 0; i < count; i++)
+            nodes.add(nodeRepository.createNode("hostNode" + i, "realHost" + i, Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.host));
+        nodes = nodeRepository.addNodes(nodes);
+        nodes = nodeRepository.setDirty(nodes);
+        nodeRepository.setReady(nodes);
+    }
+
+    private class Fixture {
+
+        final NodeRepository nodeRepository;
+        final NodeRepositoryProvisioner provisioner;
+        final Curator curator;
+
+        final ApplicationId app1 = ApplicationId.from(TenantName.from("foo1"), ApplicationName.from("bar"), InstanceName.from("fuz"));
+        final ApplicationId app2 = ApplicationId.from(TenantName.from("foo2"), ApplicationName.from("bar"), InstanceName.from("fuz"));
+        final ClusterSpec clusterApp1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("test"), Optional.empty());
+        final ClusterSpec clusterApp2 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("test"), Optional.empty());
+        final int wantedNodesApp1 = 5;
+        final int wantedNodesApp2 = 7;
+        MockDeployer deployer; // created on activation
+
+        Fixture(Zone zone, NodeRepository nodeRepository, NodeFlavors flavors, Curator curator) {
+            this.nodeRepository = nodeRepository;
+            this.curator = curator;
+            this.provisioner =  new NodeRepositoryProvisioner(nodeRepository, flavors, zone);
+        }
+
+        void activate() {
+            activate(app1, clusterApp1, wantedNodesApp1, provisioner);
+            activate(app2, clusterApp2, wantedNodesApp2, provisioner);
+            assertEquals(wantedNodesApp1, nodeRepository.getNodes(app1, Node.State.active).size());
+            assertEquals(wantedNodesApp2, nodeRepository.getNodes(app2, Node.State.active).size());
+
+            Map<ApplicationId, MockDeployer.ApplicationContext> apps = new HashMap<>();
+            apps.put(app1, new MockDeployer.ApplicationContext(app1, clusterApp1,
+                                                               Capacity.fromNodeCount(wantedNodesApp1, Optional.of("default")), 1));
+            apps.put(app2, new MockDeployer.ApplicationContext(app2, clusterApp2,
+                                                               Capacity.fromNodeCount(wantedNodesApp2, Optional.of("default")), 1));
+            this.deployer = new MockDeployer(provisioner, apps);
+        }
+
+        private void activate(ApplicationId applicationId, ClusterSpec cluster, int nodeCount, NodeRepositoryProvisioner provisioner) {
+            List<HostSpec> hosts = provisioner.prepare(applicationId, cluster, Capacity.fromNodeCount(nodeCount), 1, null);
+            NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator));
+            provisioner.activate(transaction, applicationId, hosts);
+            transaction.commit();
+        }
+
+        void remove(ApplicationId application) {
+            NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator));
+            provisioner.remove(transaction, application);
+            transaction.commit();
+        }
+
+        NodeList getNodes(Node.State ... states) {
+            return new NodeList(nodeRepository.getNodes(NodeType.tenant, states));
+        }
+
+    }
+    
+}
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java
index 535dc3bd655..c8ca4777ff9 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java
@@ -36,15 +36,13 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
-import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
 
 /**
  * @author bratseth
  */
-public class ApplicationMaintainerTest {
+public class PeriodicApplicationMaintainerTest {
 
     private static final NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default");
 
@@ -115,16 +113,14 @@ public class ApplicationMaintainerTest {
         fixture.activate();
 
         // Freeze active nodes to simulate an application being deleted during a maintenance run
-        Set<ApplicationId> frozenActiveApplications = nodeRepository.getNodes(Node.State.active).stream()
-                .map(n -> n.allocation().get().owner())
-                .collect(Collectors.toSet());
+        List<Node> frozenActiveNodes = nodeRepository.getNodes(Node.State.active);
 
         // Remove one application without letting the application maintainer know about it
         fixture.remove(fixture.app2);
         assertEquals(fixture.wantedNodesApp2, nodeRepository.getNodes(fixture.app2, Node.State.inactive).size());
 
         // Nodes belonging to app2 are inactive after maintenance
-        fixture.runApplicationMaintainer(Optional.of(frozenActiveApplications));
+        fixture.runApplicationMaintainer(Optional.of(frozenActiveNodes));
         assertEquals("Inactive nodes were incorrectly activated after maintenance", fixture.wantedNodesApp2,
                      nodeRepository.getNodes(fixture.app2, Node.State.inactive).size());
     }
@@ -190,14 +186,14 @@ public class ApplicationMaintainerTest {
             runApplicationMaintainer(Optional.empty());
         }
 
-        void runApplicationMaintainer(Optional<Set<ApplicationId>> overriddenActiveApplications) {
+        void runApplicationMaintainer(Optional<List<Node>> overriddenNodesNeedingMaintenance) {
             Map<ApplicationId, MockDeployer.ApplicationContext> apps = new HashMap<>();
             apps.put(app1, new MockDeployer.ApplicationContext(app1, clusterApp1, 
                                                                Capacity.fromNodeCount(wantedNodesApp1, Optional.of("default")), 1));
             apps.put(app2, new MockDeployer.ApplicationContext(app2, clusterApp2, 
                                                                Capacity.fromNodeCount(wantedNodesApp2, Optional.of("default")), 1));
             MockDeployer deployer = new MockDeployer(provisioner, apps);
-            new TestableApplicationMaintainer(deployer, nodeRepository, Duration.ofMinutes(30), overriddenActiveApplications).run();
+            new TestablePeriodicApplicationMaintainer(deployer, nodeRepository, Duration.ofMinutes(30), overriddenNodesNeedingMaintenance).run();
         }
 
         NodeList getNodes(Node.State ... states) {
@@ -206,28 +202,28 @@ public class ApplicationMaintainerTest {
 
     }
     
-    private static class TestableApplicationMaintainer extends ApplicationMaintainer {
+    private static class TestablePeriodicApplicationMaintainer extends PeriodicApplicationMaintainer {
 
-        private Optional<Set<ApplicationId>> overriddenActiveApplications;
+        private Optional<List<Node>> overriddenNodesNeedingMaintenance;
         
-        TestableApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Duration interval,
-                                      Optional<Set<ApplicationId>> overriddenActiveApplications) {
+        TestablePeriodicApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Duration interval,
+                                              Optional<List<Node>> overriddenNodesNeedingMaintenance) {
             super(deployer, nodeRepository, interval);
-            this.overriddenActiveApplications = overriddenActiveApplications;
+            this.overriddenNodesNeedingMaintenance = overriddenNodesNeedingMaintenance;
         }
 
         @Override
-        protected void deployAsynchronously(Deployment deployment) {
+        protected void deploy(ApplicationId application, Deployment deployment) {
             deployment.activate();
         }
 
         protected void throttle(int applicationCount) { }
 
         @Override
-        protected Set<ApplicationId> activeApplications() {
-            if (overriddenActiveApplications.isPresent())
-                return overriddenActiveApplications.get();
-            return super.activeApplications();
+        protected List<Node> nodesNeedingMaintenance() {
+            if (overriddenNodesNeedingMaintenance.isPresent())
+                return overriddenNodesNeedingMaintenance.get();
+            return super.nodesNeedingMaintenance();
         }
 
     }