Nodes allowed to upgrade always target latest version

4 files changed, 57 insertions, 11 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
index 21bde5900eb..bdae658f76e 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
@@ -75,6 +75,11 @@ public class NodeList implements Iterable<Node> {
                               !node.status().vespaVersion().get().equals(node.allocation().get().membership().cluster().vespaVersion()));
     }
 
+    /** Returns the subset of nodes that are currently changing their OS version to given version */
+    public NodeList changingOsVersionTo(Version version) {
+        return filter(node -> node.status().osVersion().changingTo(version));
+    }
+
     /** Returns the subset of nodes that are currently changing their OS version */
     public NodeList changingOsVersion() {
         return filter(node -> node.status().osVersion().changing());
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/OsVersion.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/OsVersion.java
index b06bbbb54b5..0622862f5ab 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/OsVersion.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/OsVersion.java
@@ -33,6 +33,11 @@ public class OsVersion {
         return wanted;
     }
 
+    /** Returns whether this node is currently changing its version to the given version */
+    public boolean changingTo(Version version) {
+        return changing() && wanted.get().equals(version);
+    }
+
     /** Returns whether this node is currently changing its version */
     public boolean changing() {
         return wanted.isPresent() && !current.equals(wanted);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java
index 9b21d2aceba..e10ff3d24cd 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java
@@ -120,8 +120,8 @@ public class OsVersions {
     /** Trigger upgrade of nodes of given type*/
     private void upgrade(NodeType type, Version version) {
         var nodes = nodeRepository.list().nodeType(type);
-        var numberToUpgrade = Math.max(0, maxActiveUpgrades - nodes.changingOsVersion().size());
-        var nodesToUpgrade = nodes.not().changingOsVersion()
+        var numberToUpgrade = Math.max(0, maxActiveUpgrades - nodes.changingOsVersionTo(version).size());
+        var nodesToUpgrade = nodes.not().changingOsVersionTo(version)
                                   .not().onOsVersion(version)
                                   .byIncreasingOsVersion()
                                   .first(numberToUpgrade);
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 477319a8bbb..5e859cd3d25 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
@@ -5,13 +5,17 @@ import com.yahoo.component.Version;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
+import com.yahoo.vespa.hosted.provision.node.OsVersion;
+import com.yahoo.vespa.hosted.provision.node.Status;
 import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester;
 import org.junit.Test;
 
 import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
+import java.util.function.Function;
 import java.util.function.Supplier;
+import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
@@ -94,9 +98,7 @@ public class OsVersionsTest {
             var nodesUpgrading = nodes.changingOsVersion();
             assertEquals("Target is changed for a subset of nodes", maxActiveUpgrades, nodesUpgrading.size());
             assertEquals("Wanted version is set for nodes upgrading", version1,
-                         nodesUpgrading.stream()
-                                       .map(node -> node.status().osVersion().wanted().get())
-                                       .min(Comparator.naturalOrder()).get());
+                         minVersion(nodesUpgrading, OsVersion::wanted));
             var nodesOnLowestVersion = nodes.asList().stream()
                                             .sorted(Comparator.comparing(node -> node.status().osVersion().current().orElse(Version.emptyVersion)))
                                             .collect(Collectors.toList())
@@ -108,18 +110,52 @@ public class OsVersionsTest {
 
         // Activating again after all nodes have upgraded does nothing
         versions.setActive(NodeType.host, true);
-        assertEquals("All nodes upgraded", version1,
-                     hostNodes.get().stream()
-                              .map(n -> n.status().osVersion().current().get())
-                              .min(Comparator.naturalOrder()).get());
+        assertEquals("All nodes upgraded", version1, minVersion(hostNodes.get(), OsVersion::current));
+    }
+
+    @Test
+    public void test_newer_upgrade_aborts_upgrade_to_stale_version() {
+        var versions = new OsVersions(tester.nodeRepository(), Integer.MAX_VALUE);
+        tester.makeReadyNodes(10, "default", NodeType.host);
+        Supplier<NodeList> hostNodes = () -> tester.nodeRepository().list().nodeType(NodeType.host);
+
+        // Some nodes are targeting an older version
+        var version1 = Version.fromString("7.1");
+        setWantedVersion(hostNodes.get().asList().subList(0, 5), version1);
+
+        // Trigger upgrade to next version
+        var version2 = Version.fromString("7.2");
+        versions.setTarget(NodeType.host, version2, false);
+        versions.setActive(NodeType.host, true);
+
+        // Wanted version is changed to newest target for all nodes
+        assertEquals(version2, minVersion(hostNodes.get(), OsVersion::wanted));
+    }
+
+    private Version minVersion(NodeList nodes, Function<OsVersion, Optional<Version>> versionField) {
+        return nodes.asList().stream()
+                    .map(Node::status)
+                    .map(Status::osVersion)
+                    .map(versionField)
+                    .flatMap(Optional::stream)
+                    .min(Comparator.naturalOrder())
+                    .orElse(Version.emptyVersion);
+
+    }
+
+    private void setWantedVersion(List<Node> nodes, Version wantedVersion) {
+        writeNode(nodes, node -> node.with(node.status().withOsVersion(node.status().osVersion().withWanted(Optional.of(wantedVersion)))));
     }
 
     private void setCurrentVersion(List<Node> nodes, Version currentVersion) {
+        writeNode(nodes, node -> node.with(node.status().withOsVersion(node.status().osVersion().withCurrent(Optional.of(currentVersion)))));
+    }
+
+    private void writeNode(List<Node> nodes, UnaryOperator<Node> updateFunc) {
         for (var node : nodes) {
             try (var lock = tester.nodeRepository().lock(node)) {
                 node = tester.nodeRepository().getNode(node.hostname()).get();
-                node = node.with(node.status().withOsVersion(node.status().osVersion().withCurrent(Optional.of(currentVersion))));
-                tester.nodeRepository().write(node, lock);
+                tester.nodeRepository().write(updateFunc.apply(node), lock);
             }
         }
     }