Merge pull request #18209 from vespa-engine/mpolden/expedited-changes

Expedite RebuildingOsUpgrader and HostEncrypter changes

4 files changed, 32 insertions, 20 deletions

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/ExpeditedChangeApplicationMaintainer.java
index 47337518a65..39183688340 100644
--- 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/ExpeditedChangeApplicationMaintainer.java
@@ -20,19 +20,22 @@ import java.util.Set;
 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.
+ * This maintainer detects changes to nodes that must be expedited, and redeploys affected applications.
+ *
+ * The purpose of this is to redeploy affected applications faster than achieved by
+ * {@link PeriodicApplicationMaintainer}, 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.
+ *
+ * Because we want to queue redeployments to avoid overloading config servers.
  *
  * @author bratseth
+ * @author mpolden
  */
-public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer {
+public class ExpeditedChangeApplicationMaintainer extends ApplicationMaintainer {
     
-    OperatorChangeApplicationMaintainer(Deployer deployer, Metric metric, NodeRepository nodeRepository, Duration interval) {
+    ExpeditedChangeApplicationMaintainer(Deployer deployer, Metric metric, NodeRepository nodeRepository, Duration interval) {
         super(deployer, metric, nodeRepository, interval);
     }
 
@@ -57,7 +60,7 @@ public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer {
         boolean deployed = deployWithLock(application);
         if (deployed)
             log.info("Redeployed application " + application.toShortString() +
-                     " as a manual change was made to its nodes");
+                     " as an expedited change was made to its nodes");
     }
 
     private boolean hasNodesWithChanges(ApplicationId applicationId, NodeList nodes) {
@@ -66,7 +69,7 @@ public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer {
 
         return nodes.stream()
                     .flatMap(node -> node.history().events().stream())
-                    .filter(event -> event.agent() == Agent.operator)
+                    .filter(event -> expediteChangeBy(event.agent()))
                     .map(History.Event::at)
                     .anyMatch(e -> lastDeployTime.get().isBefore(e));
     }
@@ -84,5 +87,14 @@ public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer {
                                .groupingBy(node -> node.allocation().get().owner());
     }
 
+    /** Returns whether to expedite changes performed by agent */
+    private boolean expediteChangeBy(Agent agent) {
+        switch (agent) {
+            case operator:
+            case RebuildingOsUpgrader:
+            case HostEncrypter: return true;
+        }
+        return false;
+    }
 
 }
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 40f9d17519c..cdb5202603a 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
@@ -48,7 +48,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
 
         maintainers.add(new NodeFailer(deployer, nodeRepository, defaults.failGrace, defaults.nodeFailerInterval, orchestrator, defaults.throttlePolicy, metric));
         maintainers.add(new NodeHealthTracker(hostLivenessTracker, serviceMonitor, nodeRepository, defaults.nodeFailureStatusUpdateInterval, metric));
-        maintainers.add(new OperatorChangeApplicationMaintainer(deployer, metric, nodeRepository, defaults.operatorChangeRedeployInterval));
+        maintainers.add(new ExpeditedChangeApplicationMaintainer(deployer, metric, nodeRepository, defaults.expeditedChangeRedeployInterval));
         maintainers.add(new ReservationExpirer(nodeRepository, defaults.reservationExpiry, metric));
         maintainers.add(new RetiredExpirer(nodeRepository, orchestrator, deployer, metric, defaults.retiredInterval, defaults.retiredExpiry));
         maintainers.add(new InactiveExpirer(nodeRepository, defaults.inactiveExpiry, Map.of(NodeType.config, defaults.inactiveConfigServerExpiry,
@@ -92,7 +92,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
         /** Time between each run of maintainer that does periodic redeployment */
         private final Duration redeployMaintainerInterval;
         /** Applications are redeployed after manual operator changes within this time period */
-        private final Duration operatorChangeRedeployInterval;
+        private final Duration expeditedChangeRedeployInterval;
 
         /** The time a node must be continuously unresponsive before it is failed */
         private final Duration failGrace;
@@ -135,7 +135,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
             nodeFailerInterval = Duration.ofMinutes(15);
             nodeFailureStatusUpdateInterval = Duration.ofMinutes(2);
             nodeMetricsCollectionInterval = Duration.ofMinutes(1);
-            operatorChangeRedeployInterval = Duration.ofMinutes(3);
+            expeditedChangeRedeployInterval = Duration.ofMinutes(3);
             // Vespa upgrade frequency is higher in CD so (de)activate OS upgrades more frequently as well
             osUpgradeActivatorInterval = zone.system().isCd() ? Duration.ofSeconds(30) : Duration.ofMinutes(5);
             periodicRedeployInterval = Duration.ofMinutes(60);
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/ExpeditedChangeApplicationMaintainerTest.java
index db6aebacddc..62d09c99f16 100644
--- 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/ExpeditedChangeApplicationMaintainerTest.java
@@ -26,7 +26,7 @@ import static org.junit.Assert.assertEquals;
 /**
  * @author bratseth
  */
-public class OperatorChangeApplicationMaintainerTest {
+public class ExpeditedChangeApplicationMaintainerTest {
 
     @Test
     public void test_application_maintenance() {
@@ -42,10 +42,10 @@ public class OperatorChangeApplicationMaintainerTest {
         // Create applications
         fixture.activate();
         assertEquals("Initial applications are deployed", 3, fixture.deployer.redeployments);
-        OperatorChangeApplicationMaintainer maintainer = new OperatorChangeApplicationMaintainer(fixture.deployer,
-                                                                                                 new TestMetric(),
-                                                                                                 nodeRepository,
-                                                                                                 Duration.ofMinutes(1));
+        ExpeditedChangeApplicationMaintainer maintainer = new ExpeditedChangeApplicationMaintainer(fixture.deployer,
+                                                                                                   new TestMetric(),
+                                                                                                   nodeRepository,
+                                                                                                   Duration.ofMinutes(1));
         
         clock.advance(Duration.ofMinutes(2));
         maintainer.maintain();
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json
index 73ac692e37b..26d711945c6 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json
@@ -7,6 +7,9 @@
       "name": "DirtyExpirer"
     },
     {
+      "name": "ExpeditedChangeApplicationMaintainer"
+    },
+    {
       "name": "FailedExpirer"
     },
     {
@@ -37,9 +40,6 @@
       "name": "NodeRebooter"
     },
     {
-      "name": "OperatorChangeApplicationMaintainer"
-    },
-    {
       "name": "OsUpgradeActivator"
     },
     {