Merge pull request #11791 from vespa-engine/mpolden/fix-upgrade

Only count version as changing if wanted version is set

2 files changed, 19 insertions, 1 deletions

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 b3c265124db..b06bbbb54b5 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
@@ -35,7 +35,7 @@ public class OsVersion {
 
     /** Returns whether this node is currently changing its version */
     public boolean changing() {
-        return !current.equals(wanted);
+        return wanted.isPresent() && !current.equals(wanted);
     }
 
     /** Returns whether current version matches given version */
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 2a3e59bee42..ebb64d650f1 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
@@ -10,6 +10,7 @@ import org.junit.Test;
 
 import java.util.Comparator;
 import java.util.List;
+import java.util.Optional;
 import java.util.function.Supplier;
 
 import static org.junit.Assert.assertEquals;
@@ -74,6 +75,9 @@ public class OsVersionsTest {
         tester.makeReadyNodes(totalNodes, "default", NodeType.host);
         Supplier<NodeList> hostNodes = () -> tester.nodeRepository().list().nodeType(NodeType.host);
 
+        // Some nodes have reported current version
+        setCurrentVersion(hostNodes.get().asList().subList(0, 2), Version.fromString("7.0"));
+
         // Set target
         var version1 = Version.fromString("7.1");
         versions.setTarget(NodeType.host, version1, false);
@@ -84,6 +88,10 @@ public class OsVersionsTest {
             versions.setActive(NodeType.host, true);
             var nodesUpgrading = hostNodes.get().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());
             completeUpgradeOf(nodesUpgrading.asList());
         }
 
@@ -94,6 +102,16 @@ public class OsVersionsTest {
                                         .min(Comparator.naturalOrder()).get());
     }
 
+    private void setCurrentVersion(List<Node> nodes, Version currentVersion) {
+        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);
+            }
+        }
+    }
+
     private void completeUpgradeOf(List<Node> nodes) {
         for (var node : nodes) {
             try (var lock = tester.nodeRepository().lock(node)) {