Log reason for redeployment

3 files changed, 46 insertions, 41 deletions

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 24160c19dfa..fb88644a654 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
@@ -9,7 +9,7 @@ import com.yahoo.vespa.hosted.provision.NodeRepository;
 
 import java.time.Duration;
 import java.time.Instant;
-import java.util.Set;
+import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.ThreadPoolExecutor;
@@ -62,28 +62,28 @@ public abstract class ApplicationMaintainer extends NodeRepositoryMaintainer {
      * The default implementation deploys asynchronously to make sure we do all applications timely
      * even when deployments are slow.
      */
-    protected void deploy(ApplicationId application) {
+    protected void deploy(ApplicationId application, String reason) {
         if (pendingDeployments.addIfAbsent(application)) { // Avoid queuing multiple deployments for same application
-            deploymentExecutor.execute(() -> deployWithLock(application));
+            deploymentExecutor.execute(() -> deployWithLock(application, reason));
         }
     }
 
     protected Deployer deployer() { return deployer; }
 
     /** Returns the applications that should be maintained by this now. */
-    protected abstract Set<ApplicationId> applicationsNeedingMaintenance();
+    protected abstract Map<ApplicationId, String> applicationsNeedingMaintenance();
 
     /**
      * Redeploy this application. A lock will be taken for the duration of the deployment activation
-     *
-     * @return whether it was successfully deployed
      */
-    protected final boolean deployWithLock(ApplicationId application) {
+    protected final void deployWithLock(ApplicationId application, String reason) {
         try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository())) {
-            if ( ! deployment.isValid()) return false; // this will be done at another config server
-            if ( ! canDeployNow(application)) return false; // redeployment is no longer needed
-            log.log(Level.INFO, application + " will be deployed, last deploy time " + getLastDeployTime(application));
-            return deployment.activate().isPresent();
+            if ( ! deployment.isValid()) return; // this will be done at another config server
+            if ( ! canDeployNow(application)) return; // redeployment is no longer needed
+            log.log(Level.INFO, () -> application + " will be redeployed" +
+                                      (reason == null || reason.isBlank() ? "" : " due to " + reason) +
+                                      ", last deploy time " + getLastDeployTime(application));
+            deployment.activate();
         } finally {
             pendingDeployments.remove(application);
         }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ExpeditedChangeApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ExpeditedChangeApplicationMaintainer.java
index 884d4ea7281..725ad2a5711 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ExpeditedChangeApplicationMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ExpeditedChangeApplicationMaintainer.java
@@ -9,14 +9,14 @@ 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.node.History;
 
 import java.time.Duration;
 import java.time.Instant;
-import java.util.HashSet;
+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;
 
 /**
  * This maintainer detects changes to nodes that must be expedited, and redeploys affected applications.
@@ -39,23 +39,24 @@ public class ExpeditedChangeApplicationMaintainer extends ApplicationMaintainer
     }
 
     @Override
-    protected Set<ApplicationId> applicationsNeedingMaintenance() {
-        var applications = new HashSet<ApplicationId>();
+    protected Map<ApplicationId, String> applicationsNeedingMaintenance() {
+        var applications = new HashMap<ApplicationId, String>();
 
         nodeRepository().nodes()
                         .list()
                         .nodeType(NodeType.tenant, NodeType.proxy)
                         .matching(node -> node.allocation().isPresent())
                         .groupingBy(node -> node.allocation().get().owner())
-                        .entrySet()
-                        .stream()
-                        .filter(entry -> hasNodesWithChanges(entry.getKey(), entry.getValue()))
-                        .map(Map.Entry::getKey)
-                        .forEach(applications::add);
+                        .forEach((applicationId, nodes) -> {
+                            hasNodesWithChanges(applicationId, nodes)
+                                    .ifPresent(reason -> applications.put(applicationId, reason));
+                        });
 
         // A ready proxy node should trigger a redeployment as it will activate the node.
         if (!nodeRepository().nodes().list(Node.State.ready, Node.State.reserved).nodeType(NodeType.proxy).isEmpty()) {
-            applications.add(ApplicationId.from("hosted-vespa", "routing", "default"));
+            applications.merge(ApplicationId.from("hosted-vespa", "routing", "default"),
+                               "nodes being ready",
+                               (oldValue, newValue) -> oldValue + ", " + newValue);
         }
 
         return applications;
@@ -66,22 +67,29 @@ public class ExpeditedChangeApplicationMaintainer extends ApplicationMaintainer
      * longer to deploy than the (short) maintenance interval of this
      */
     @Override
-    protected void deploy(ApplicationId application) {
-        boolean deployed = deployWithLock(application);
-        if (deployed)
-            log.info("Redeployed application " + application.toShortString() +
-                     " as an expedited change was made to its nodes");
+    protected void deploy(ApplicationId application, String reason) {
+        deployWithLock(application, reason);
     }
 
-    private boolean hasNodesWithChanges(ApplicationId applicationId, NodeList nodes) {
+    /** Returns the reason for doing an expedited deploy. */
+    private Optional<String> hasNodesWithChanges(ApplicationId applicationId, NodeList nodes) {
         Optional<Instant> lastDeployTime = deployer().lastDeployTime(applicationId);
-        if (lastDeployTime.isEmpty()) return false;
+        if (lastDeployTime.isEmpty()) return Optional.empty();
 
-        return nodes.stream()
-                    .flatMap(node -> node.history().events().stream())
-                    .filter(event -> expediteChangeBy(event.agent()))
-                    .map(History.Event::at)
-                    .anyMatch(e -> lastDeployTime.get().isBefore(e));
+        List<String> reasons = nodes.stream()
+                                    .flatMap(node -> node.history()
+                                                            .events()
+                                                            .stream()
+                                                            .filter(event -> expediteChangeBy(event.agent()))
+                                                            .filter(event -> lastDeployTime.get().isBefore(event.at()))
+                                                            .map(event -> event.type() + (event.agent() == Agent.system ? "" : " by " + event.agent())))
+                                    .sorted()
+                                    .distinct()
+                                    .collect(Collectors.toList());
+
+        return reasons.isEmpty() ?
+                Optional.empty() :
+                Optional.of("recent node events: [" + String.join(", ", reasons) + "]");
     }
 
     @Override
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 e0f6c9d78bb..bbefaf0f941 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
@@ -14,10 +14,7 @@ import com.yahoo.vespa.hosted.provision.NodeRepository;
 
 import java.time.Duration;
 import java.time.Instant;
-import java.util.LinkedHashSet;
-import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -52,8 +49,8 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer {
 
     // Returns the applications that need to be redeployed by this config server at this point in time.
     @Override
-    protected Set<ApplicationId> applicationsNeedingMaintenance() {
-        if (deployer().bootstrapping()) return Set.of();
+    protected Map<ApplicationId, String> applicationsNeedingMaintenance() {
+        if (deployer().bootstrapping()) return Map.of();
 
         // Collect all deployment times before sorting as deployments may happen while we build the set, breaking
         // the comparable contract. Stale times are fine as the time is rechecked in ApplicationMaintainer#deployWithLock
@@ -66,8 +63,8 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer {
         return deploymentTimes.entrySet().stream()
                               .sorted(Map.Entry.comparingByValue())
                               .map(Map.Entry::getKey)
-                              .filter(id -> shouldMaintain(id))
-                              .collect(Collectors.toCollection(LinkedHashSet::new));
+                              .filter(this::shouldMaintain)
+                              .collect(Collectors.toMap(applicationId -> applicationId, applicationId -> "current deployment being too old"));
     }
 
     private boolean shouldMaintain(ApplicationId id) {