Add a grace period before upgrading new nodes

10 files changed, 72 insertions, 6 deletions

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 ac804f99cd3..c2d4506a28c 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
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.provision.node;
 import com.google.common.collect.ImmutableMap;
 import com.yahoo.vespa.hosted.provision.Node;
 
+import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -50,6 +51,12 @@ public class History {
         return builder.build();
     }
 
+    /** Returns the age of this node as best as we can determine: The time since the first event registered for it */
+    public Duration age(Instant now) {
+        Instant oldestEventTime = events.values().stream().map(event -> event.at()).sorted().findFirst().orElse(now);
+        return Duration.between(oldestEventTime, now);
+    }
+
     /** Returns the last event of given type, if it is present in this history */
     public Optional<Event> event(Event.Type type) { return Optional.ofNullable(events.get(type)); }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/filter/NodeListFilter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/filter/NodeListFilter.java
index 8578e3eb5ec..2b790ff7392 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/filter/NodeListFilter.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/filter/NodeListFilter.java
@@ -30,4 +30,5 @@ public class NodeListFilter {
     public static Predicate<Node> from(List<Node> nodes) {
         return makePredicate(nodes);
     }
+
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/DelegatingOsUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/DelegatingOsUpgrader.java
index 30fd2713017..9c938dd87ec 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/DelegatingOsUpgrader.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/DelegatingOsUpgrader.java
@@ -41,6 +41,7 @@ public class DelegatingOsUpgrader implements OsUpgrader {
         int numberToUpgrade = Math.max(0, maxActiveUpgrades - activeNodes.changingOsVersionTo(target.version()).size());
         NodeList nodesToUpgrade = activeNodes.not().changingOsVersionTo(target.version())
                                              .osVersionIsBefore(target.version())
+                                             .matching(node -> shouldUpgrade(node, nodeRepository.clock().instant()))
                                              .byIncreasingOsVersion()
                                              .first(numberToUpgrade);
         if (nodesToUpgrade.size() == 0) return;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsUpgrader.java
index 5310ef339ed..9abb0a29628 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsUpgrader.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsUpgrader.java
@@ -2,6 +2,11 @@
 package com.yahoo.vespa.hosted.provision.os;
 
 import com.yahoo.config.provision.NodeType;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.node.History;
+
+import java.time.Duration;
+import java.time.Instant;
 
 /**
  * Interface for an OS upgrader.
@@ -10,10 +15,17 @@ import com.yahoo.config.provision.NodeType;
  */
 public interface OsUpgrader {
 
+    /** The duration we should leave new nodes along before scheduling OS upgrades */
+    Duration gracePeriod = Duration.ofDays(30);
+
     /** Trigger upgrade to given target */
     void upgradeTo(OsVersionTarget target);
 
     /** Disable OS upgrade for all nodes of given type */
     void disableUpgrade(NodeType type);
 
+    default boolean shouldUpgrade(Node node, Instant now) {
+        return node.history().age(now).toSeconds() > gracePeriod.toSeconds();
+    }
+
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java
index efc377e6cc3..ffedbd241d7 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java
@@ -76,6 +76,7 @@ public class RebuildingOsUpgrader implements OsUpgrader {
         NodeList candidates = hostsOfTargetType.state(Node.State.active)
                                                .not().rebuilding()
                                                .osVersionIsBefore(target.version())
+                                               .matching(node -> shouldUpgrade(node, nodeRepository.clock().instant()))
                                                .byIncreasingOsVersion();
         for (Node host : candidates) {
             if (hostsToRebuild.size() == rebuildLimit) break;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringOsUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringOsUpgrader.java
index d923c78a929..dba79cd70ca 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringOsUpgrader.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringOsUpgrader.java
@@ -38,6 +38,7 @@ public class RetiringOsUpgrader implements OsUpgrader {
         Instant now = nodeRepository.clock().instant();
         NodeList candidates = candidates(now, target, allNodes);
         candidates.not().deprovisioning()
+                  .matching(node -> shouldUpgrade(node, nodeRepository.clock().instant()))
                   .byIncreasingOsVersion()
                   .first(1)
                   .forEach(node -> deprovision(node, target.version(), now));
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivatorTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivatorTest.java
index ff2ab44ad4a..f9b5e383ea0 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivatorTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivatorTest.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.Allocation;
 import com.yahoo.vespa.hosted.provision.node.Status;
+import com.yahoo.vespa.hosted.provision.os.OsUpgrader;
 import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester;
 import org.junit.Test;
 
@@ -49,6 +50,8 @@ public class OsUpgradeActivatorTest {
         var allNodes = new ArrayList<>(configHostNodes);
         allNodes.addAll(tenantHostNodes);
 
+        tester.clock().advance(OsUpgrader.gracePeriod.plusDays(1));
+
         // All nodes are on initial version
         assertEquals(version0, minWantedVersion(allNodes));
         completeUpgradeOf(configHostNodes);
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
index 97a8ac0d655..c3a5d7efd2f 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
@@ -65,6 +65,13 @@ public class OsVersionsTest {
         // Resume upgrade
         versions.resumeUpgradeOf(NodeType.host, true);
         NodeList allHosts = hostNodes.get();
+        assertFalse("No upgrades of new hosts", allHosts.except(hostOnLaterVersion).stream()
+                                                        .anyMatch(node -> node.status().osVersion().wanted().isPresent()));
+
+        // Resume upgrade after sufficient time
+        tester.clock().advance(OsUpgrader.gracePeriod.plusDays(1));
+        versions.resumeUpgradeOf(NodeType.host, true);
+        allHosts = hostNodes.get();
         assertTrue("Wanted version is set", allHosts.except(hostOnLaterVersion).stream()
                                                     .allMatch(node -> node.status().osVersion().wanted().isPresent()));
         assertTrue("Wanted version is not set for host on later version",
@@ -99,6 +106,8 @@ public class OsVersionsTest {
         provisionInfraApplication(totalNodes);
         Supplier<NodeList> hostNodes = () -> tester.nodeRepository().nodes().list().state(Node.State.active).hosts();
 
+        tester.clock().advance(OsUpgrader.gracePeriod.plusDays(1));
+
         // 5 nodes have no version. The other 15 are spread across different versions
         var hostNodesList = hostNodes.get().asList();
         for (int i = totalNodes - maxActiveUpgrades - 1; i >= 0; i--) {
@@ -144,6 +153,7 @@ public class OsVersionsTest {
         var versions = new OsVersions(tester.nodeRepository());
         provisionInfraApplication(10);
         Supplier<NodeList> hostNodes = () -> tester.nodeRepository().nodes().list().hosts();
+        tester.clock().advance(OsUpgrader.gracePeriod.plusDays(1));
 
         // Some nodes are targeting an older version
         var version1 = Version.fromString("7.1");
@@ -172,6 +182,7 @@ public class OsVersionsTest {
         Supplier<NodeList> hostNodes = () -> tester.nodeRepository().nodes().list()
                                                    .hosts()
                                                    .not().state(Node.State.deprovisioned);
+        tester.clock().advance(OsUpgrader.gracePeriod.plusDays(1));
 
         // Target is set and upgrade started
         var version1 = Version.fromString("7.1");
@@ -233,6 +244,7 @@ public class OsVersionsTest {
         Supplier<NodeList> hostNodes = () -> tester.nodeRepository().nodes().list()
                                                    .nodeType(NodeType.confighost)
                                                    .not().state(Node.State.deprovisioned);
+        tester.clock().advance(OsUpgrader.gracePeriod.plusDays(1));
 
         // Target is set with zero budget and upgrade started
         var version1 = Version.fromString("7.1");
@@ -255,6 +267,7 @@ public class OsVersionsTest {
         int hostCount = 10;
         provisionInfraApplication(hostCount + 1);
         Supplier<NodeList> hostNodes = () -> tester.nodeRepository().nodes().list().nodeType(NodeType.host);
+        tester.clock().advance(OsUpgrader.gracePeriod.plusDays(1));
 
         // All hosts upgrade to first version. Upgrades are delegated
         var version0 = Version.fromString("7.0");
@@ -302,6 +315,7 @@ public class OsVersionsTest {
         assertEquals(0, hostNodes.get().rebuilding().size());
 
         // Next version is within same major. Upgrade mechanism switches to delegated
+        tester.clock().advance(OsUpgrader.gracePeriod.plusDays(1));
         var version2 = Version.fromString("8.1");
         versions.setTarget(NodeType.host, version2, Duration.ZERO, false);
         versions.resumeUpgradeOf(NodeType.host, true);
@@ -332,6 +346,8 @@ public class OsVersionsTest {
         Supplier<NodeList> hosts = () -> tester.nodeRepository().nodes().list().nodeType(NodeType.host,
                                                                                          NodeType.confighost);
 
+        tester.clock().advance(OsUpgrader.gracePeriod.plusDays(1));
+
         // All hosts upgrade to first version. Upgrades are delegated
         var version0 = Version.fromString("7.0");
         versions.setTarget(NodeType.host, version0, Duration.ZERO, false);
@@ -366,6 +382,7 @@ public class OsVersionsTest {
         deployApplication(app1);
         deployApplication(app2);
         Supplier<NodeList> hosts = () -> tester.nodeRepository().nodes().list().nodeType(NodeType.host);
+        tester.clock().advance(OsUpgrader.gracePeriod.plusDays(1));
 
         // All hosts are on initial version
         var version0 = Version.fromString("7.0");
@@ -439,6 +456,7 @@ public class OsVersionsTest {
         var versions = new OsVersions(tester.nodeRepository(), false, Integer.MAX_VALUE);
         provisionInfraApplication(hostCount, infraApplication, NodeType.proxyhost);
         Supplier<NodeList> hosts = () -> tester.nodeRepository().nodes().list().nodeType(NodeType.proxyhost);
+        tester.clock().advance(OsUpgrader.gracePeriod.plusDays(1));
 
         // All hosts are on initial version
         var version0 = Version.fromString("7.0");
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java
index 6b1853b3893..eb540426ce0 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java
@@ -6,11 +6,17 @@ import com.yahoo.application.container.handler.Response;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.TenantName;
+import com.yahoo.test.ManualClock;
 import com.yahoo.text.Utf8;
 import com.yahoo.vespa.applicationmodel.HostName;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeMutex;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.maintenance.OsUpgradeActivator;
 import com.yahoo.vespa.hosted.provision.maintenance.TestMetric;
+import com.yahoo.vespa.hosted.provision.node.Agent;
+import com.yahoo.vespa.hosted.provision.node.History;
+import com.yahoo.vespa.hosted.provision.os.OsUpgrader;
 import com.yahoo.vespa.hosted.provision.testutils.MockNodeRepository;
 import com.yahoo.vespa.hosted.provision.testutils.OrchestratorMock;
 import org.junit.After;
@@ -770,8 +776,19 @@ public class NodesV2ApiTest {
                                    Request.Method.PATCH),
                        "{\"message\":\"Set osVersion to 7.5.2, upgradeBudget to PT0S for nodes of type host\"}");
 
-        // Activate target
         var nodeRepository = (NodeRepository)tester.container().components().getComponent(MockNodeRepository.class.getName());
+
+        // Age nodes to pass OS upgrade grace period for new nodes
+        for (Node node : nodeRepository.nodes().list()) {
+            try (NodeMutex lockedNode = nodeRepository.nodes().lockAndGet(node).orElseThrow()) {
+                Node agedNode = node.with(node.history().with(new History.Event(History.Event.Type.provisioned,
+                                                                                Agent.system,
+                                                                                nodeRepository.clock().instant().minus(OsUpgrader.gracePeriod.plusDays(1)))));
+                nodeRepository.nodes().write(agedNode, lockedNode);
+            }
+        }
+
+        // Activate target
         var osUpgradeActivator = new OsUpgradeActivator(nodeRepository, Duration.ofDays(1), new TestMetric());
         osUpgradeActivator.run();
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-node1-os-upgrade-complete.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-node1-os-upgrade-complete.json
index b885f7bd7fc..322f7cd0cad 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-node1-os-upgrade-complete.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-node1-os-upgrade-complete.json
@@ -38,11 +38,6 @@
   "down": false,
   "history": [
     {
-      "event": "provisioned",
-      "at": 123,
-      "agent": "system"
-    },
-    {
       "event": "readied",
       "at": 123,
       "agent": "system"
@@ -56,11 +51,21 @@
       "event": "activated",
       "at": 123,
       "agent": "application"
+    },
+    {
+      "event": "provisioned",
+      "at": -2678399877,
+      "agent": "system"
     }
   ],
   "log": [
     {
       "event": "provisioned",
+      "at": -2678399877,
+      "agent": "system"
+    },
+    {
+      "event": "provisioned",
       "at": 123,
       "agent": "system"
     },