Merge with master

5 files changed, 40 insertions, 5 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/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
index 24d05f10cb8..001beabdb07 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
@@ -115,9 +115,6 @@ public class GroupPreparer {
                                                      " in " + application.toShortString() +
                                                      outOfCapacityDetails(allocation));
 
-                // Extend reservation for already reserved nodes
-                nodeRepository.reserve(nodeRepository.getNodes(application, Node.State.reserved));
-
                 // Carry out and return allocation
                 nodeRepository.reserve(allocation.reservableNodes());
                 nodeRepository.addDockerNodes(new LockedNodeList(allocation.newNodes(), allocationLock));
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
index 8b1afa42c50..ef6e4747f94 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
@@ -20,6 +20,7 @@ import java.time.Clock;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Comparator;
+import java.util.EnumSet;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -339,7 +340,9 @@ class NodeAllocation {
     }
 
     List<Node> reservableNodes() {
-        return nodesFilter(n -> n.node.state() == Node.State.inactive || n.node.state() == Node.State.ready);
+        // Include already reserved nodes to extend reservation period and to potentially update their cluster spec.
+        EnumSet<Node.State> reservableStates = EnumSet.of(Node.State.inactive, Node.State.ready, Node.State.reserved);
+        return nodesFilter(n -> !n.isNewNode && reservableStates.contains(n.node.state()));
     }
 
     List<Node> surplusNodes() {
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)) {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
index a54afd1c229..e8960849c3e 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
@@ -625,6 +625,23 @@ public class ProvisioningTest {
         tester.activate(application, state.allHosts);
     }
 
+    @Test
+    public void cluster_spec_update_for_already_reserved_nodes() {
+        ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.dev, RegionName.from("us-east"))).build();
+        ApplicationId application = tester.makeApplicationId();
+        Version version1 = Version.fromString("6.42");
+        Version version2 = Version.fromString("6.43");
+        tester.makeReadyNodes(2, defaultResources);
+
+        prepare(application, 1, 0, 1, 0, true, defaultResources, version1, tester);
+        tester.getNodes(application, Node.State.reserved).forEach(node ->
+                assertEquals(version1, node.allocation().get().membership().cluster().vespaVersion()));
+
+        prepare(application, 1, 0, 1, 0, true, defaultResources, version2, tester);
+        tester.getNodes(application, Node.State.reserved).forEach(node ->
+                assertEquals(version2, node.allocation().get().membership().cluster().vespaVersion()));
+    }
+
     private SystemState prepare(ApplicationId application, int container0Size, int container1Size, int content0Size,
                                 int content1Size, NodeResources flavor, ProvisioningTester tester) {
         return prepare(application, container0Size, container1Size, content0Size, content1Size, flavor,