Ignore nodes currently deferring upgrade

2 files changed, 27 insertions, 10 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
index 0c146179f34..8155476f139 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
@@ -62,7 +62,7 @@ public class OsUpgrader extends InfrastructureUpgrader<OsVersionTarget> {
     protected boolean expectUpgradeOf(Node node, SystemApplication application, ZoneApi zone) {
         return cloud.equals(zone.getCloudName()) && // Cloud is managed by this upgrader
                application.shouldUpgradeOs() &&     // Application should upgrade in this cloud
-               canUpgrade(node);                    // Node is in an upgradable state
+               canUpgrade(node);
     }
 
     @Override
@@ -101,9 +101,9 @@ public class OsUpgrader extends InfrastructureUpgrader<OsVersionTarget> {
         return !controller().zoneRegistry().systemZone().getVirtualId().equals(zone.getVirtualId()); // Do not spend budget on controller zone
     }
 
-    /** Returns whether node is in a state where it can be upgraded */
+    /** Returns whether node currently allows upgrades */
     public static boolean canUpgrade(Node node) {
-        return upgradableNodeStates.contains(node.state());
+        return !node.deferOsUpgrade() && upgradableNodeStates.contains(node.state());
     }
 
     private static String name(CloudName cloud) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
index 8adf55155a5..3a5b4a90baa 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
@@ -19,7 +19,9 @@ import java.time.Duration;
 import java.util.Collection;
 import java.util.List;
 import java.util.function.Function;
+import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
@@ -52,18 +54,19 @@ public class OsUpgraderTest {
         OsUpgrader osUpgrader = osUpgrader(upgradePolicy, cloud1, false);
 
         // Bootstrap system
-        List<ZoneId> nonControllerZones = List.of(zone1, zone2, zone3, zone4, zone5).stream()
-                                              .map(ZoneApi::getVirtualId)
-                                              .collect(Collectors.toList());
+        List<ZoneId> nonControllerZones = Stream.of(zone1, zone2, zone3, zone4, zone5)
+                                                .map(ZoneApi::getVirtualId)
+                                                .collect(Collectors.toList());
         tester.configServer().bootstrap(nonControllerZones, List.of(SystemApplication.tenantHost));
         tester.configServer().addNodes(List.of(zone0.getVirtualId()), List.of(SystemApplication.controllerHost));
 
         // Add system application that exists in a real system, but isn't eligible for OS upgrades
         tester.configServer().addNodes(nonControllerZones, List.of(SystemApplication.configServer));
 
-        // Fail a few nodes. Failed nodes should not affect versions
+        // Change state of a few nodes. These should not affect convergence
         failNodeIn(zone1, SystemApplication.tenantHost);
         failNodeIn(zone3, SystemApplication.tenantHost);
+        Node nodeDeferringOsUpgrade = deferOsUpgradeIn(zone2, SystemApplication.tenantHost);
 
         // New OS version released
         Version version1 = Version.fromString("7.1");
@@ -91,7 +94,7 @@ public class OsUpgraderTest {
         completeUpgrade(version1, SystemApplication.tenantHost, zone1);
         statusUpdater.maintain();
         assertEquals(5, nodesOn(version1).size());
-        assertEquals(11, nodesOn(Version.emptyVersion).size());
+        assertEquals(10, nodesOn(Version.emptyVersion).size());
 
         // zone 2 and 3: begins upgrading
         osUpgrader.maintain();
@@ -102,6 +105,10 @@ public class OsUpgraderTest {
 
         // zone 2 and 3: completes upgrade
         completeUpgrade(version1, SystemApplication.tenantHost, zone2, zone3);
+        assertEquals("Current version is unchanged for node deferring OS upgrade", Version.emptyVersion,
+                     nodeRepository().list(zone2.getVirtualId(), NodeFilter.all().hostnames(nodeDeferringOsUpgrade.hostname()))
+                                     .get(0)
+                                     .currentOsVersion());
 
         // zone 4: begins upgrading
         osUpgrader.maintain();
@@ -271,13 +278,23 @@ public class OsUpgraderTest {
                                .collect(Collectors.toList());
     }
 
-    private void failNodeIn(ZoneApi zone, SystemApplication application) {
+    private Node failNodeIn(ZoneApi zone, SystemApplication application) {
+        return patchOneNodeIn(zone, application, (node) -> Node.builder(node).state(Node.State.failed).build());
+    }
+
+    private Node deferOsUpgradeIn(ZoneApi zone, SystemApplication application) {
+        return patchOneNodeIn(zone, application, (node) -> Node.builder(node).deferOsUpgrade(true).build());
+    }
+
+    private Node patchOneNodeIn(ZoneApi zone, SystemApplication application, UnaryOperator<Node> patcher) {
         List<Node> nodes = nodeRepository().list(zone.getVirtualId(), NodeFilter.all().applications(application.id()));
         if (nodes.isEmpty()) {
             throw new IllegalArgumentException("No nodes allocated to " + application.id());
         }
         Node node = nodes.get(0);
-        nodeRepository().putNodes(zone.getVirtualId(), Node.builder(node).state(Node.State.failed).build());
+        Node newNode = patcher.apply(node);
+        nodeRepository().putNodes(zone.getVirtualId(), newNode);
+        return newNode;
     }
 
     /** Simulate OS upgrade of nodes allocated to application. In a real system this is done by the node itself */