Use deploy time (prepare time) when considering applications for maintenance deployment

2 files changed, 31 insertions, 16 deletions

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
index 28679b504aa..ba71bbf0d42 100644
--- 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
@@ -16,7 +16,8 @@ import java.time.Duration;
 import java.time.Instant;
 import java.util.Map;
 import java.util.function.Function;
-import java.util.stream.Collectors;
+
+import static java.util.stream.Collectors.toMap;
 
 /**
  * The application maintainer regularly redeploys all applications to make sure the node repo and application
@@ -39,7 +40,7 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer {
 
     @Override
     protected boolean canDeployNow(ApplicationId application) {
-        return deployer().activationTime(application)
+        return deployer().deployTime(application)
                 // Don't deploy if a regular deploy just happened
                 .map(lastDeployTime -> lastDeployTime.isBefore(nodeRepository().clock().instant().minus(minTimeBetweenRedeployments)))
                 // We only know last deploy time for applications that were deployed on this config server,
@@ -57,13 +58,17 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer {
                                                                                .map(node -> node.allocation().get().owner())
                                                                                .distinct()
                                                                                .filter(this::canDeployNow)
-                                                                               .collect(Collectors.toMap(Function.identity(), this::activationTime));
+                                                                               .collect(toMap(Function.identity(), this::deployTime));
 
         return deploymentTimes.entrySet().stream()
                               .sorted(Map.Entry.comparingByValue())
                               .map(Map.Entry::getKey)
                               .filter(this::shouldMaintain)
-                              .collect(Collectors.toMap(applicationId -> applicationId, applicationId -> "current deployment being too old"));
+                              .collect(toMap(applicationId -> applicationId, applicationId -> "current deployment being too old"));
+    }
+
+    private Instant deployTime(ApplicationId applicationId) {
+        return deployer().deployTime(applicationId).orElse(Instant.EPOCH);
     }
 
     private boolean shouldMaintain(ApplicationId id) {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
index 3ed01e00ee6..4b4e9121be4 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
@@ -40,10 +40,11 @@ public class MockDeployer implements Deployer {
     // For mock deploy anything, changing wantToRetire to retired only
     private final NodeRepository nodeRepository;
 
-    /** The number of redeployments done to this, which is also the config generation */
-    public int redeployments = 0;
+    /** The number of activations done to this, which is also the config generation */
+    public int activations = 0;
 
-    private final Map<ApplicationId, Instant> lastDeployTimes = new HashMap<>();
+    private final Map<ApplicationId, Instant> lastPrepareTimes = new HashMap<>();
+    private final Map<ApplicationId, Instant> lastActivationTimes = new HashMap<>();
     private final Clock clock;
     private final ReentrantLock lock = new ReentrantLock();
 
@@ -121,7 +122,12 @@ public class MockDeployer implements Deployer {
 
     @Override
     public Optional<Instant> activationTime(ApplicationId application) {
-        return Optional.ofNullable(lastDeployTimes.get(application));
+        return Optional.ofNullable(lastActivationTimes.get(application));
+    }
+
+    @Override
+    public Optional<Instant> deployTime(ApplicationId application) {
+        return Optional.ofNullable(lastPrepareTimes.get(application));
     }
 
     @Override
@@ -136,7 +142,8 @@ public class MockDeployer implements Deployer {
         new MockDeployment(provisioner, new ApplicationContext(applicationId, List.of())).activate();
 
         applications.remove(applicationId);
-        lastDeployTimes.remove(applicationId);
+        lastPrepareTimes.remove(applicationId);
+        lastActivationTimes.remove(applicationId);
     }
 
     public class MockDeployment implements Deployment {
@@ -155,6 +162,7 @@ public class MockDeployer implements Deployer {
         @Override
         public void prepare() {
             preparedHosts = application.prepare(provisioner);
+            lastPrepareTimes.put(application.id, clock.instant());
         }
 
         @Override
@@ -164,15 +172,15 @@ public class MockDeployer implements Deployer {
             if (failActivate)
                 throw new IllegalStateException("failActivate is true");
 
-            redeployments++;
+            activations++;
             try (var lock = provisioner.lock(application.id)) {
                 try (NestedTransaction t = new NestedTransaction()) {
-                    provisioner.activate(preparedHosts, new ActivationContext(redeployments), new ApplicationTransaction(lock, t));
+                    provisioner.activate(preparedHosts, new ActivationContext(activations), new ApplicationTransaction(lock, t));
                     t.commit();
-                    lastDeployTimes.put(application.id, clock.instant());
+                    lastActivationTimes.put(application.id, clock.instant());
                 }
             }
-            return redeployments;
+            return activations;
         }
 
         @Override
@@ -191,18 +199,20 @@ public class MockDeployer implements Deployer {
         }
 
         @Override
-        public void prepare() { }
+        public void prepare() {
+            lastPrepareTimes.put(applicationId, clock.instant());
+        }
 
         @Override
         public long activate() {
-            lastDeployTimes.put(applicationId, clock.instant());
+            lastActivationTimes.put(applicationId, clock.instant());
 
             for (Node node : nodeRepository.nodes().list().owner(applicationId).state(Node.State.active).retiring()) {
                 try (NodeMutex lock = nodeRepository.nodes().lockAndGetRequired(node)) {
                     nodeRepository.nodes().write(lock.node().retire(nodeRepository.clock().instant()), lock);
                 }
             }
-            return redeployments++;
+            return activations++;
         }
 
         @Override