Only count actively upgrading nodes

10 files changed, 136 insertions, 97 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java
index 650b8400298..9253e249765 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java
@@ -1,7 +1,6 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.maintenance;
 
-import com.google.common.collect.ImmutableMap;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.hosted.controller.Controller;
@@ -13,6 +12,7 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics;
 import com.yahoo.vespa.hosted.controller.application.JobList;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
 import com.yahoo.vespa.hosted.controller.rotation.RotationLock;
+import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 
 import java.time.Clock;
 import java.time.Duration;
@@ -23,6 +23,8 @@ import java.util.Map;
 import java.util.stream.Collectors;
 
 /**
+ * This calculates and reports system-wide metrics based on data from a {@link Controller}.
+ *
  * @author mortent
  * @author mpolden
  */
@@ -104,9 +106,13 @@ public class MetricsReporter extends Maintainer {
     private int nodesFailingSystemUpgrade() {
         if (!controller().versionStatus().isUpgrading()) return 0;
         var nodesFailingUpgrade = 0;
-        var timeoutInstant = clock.instant().minus(NODE_UPGRADE_TIMEOUT);
+        var acceptableInstant = clock.instant().minus(NODE_UPGRADE_TIMEOUT);
         for (var vespaVersion : controller().versionStatus().versions()) {
-            nodesFailingUpgrade += vespaVersion.nodeVersions().changedBefore(timeoutInstant).size();
+            if (vespaVersion.confidence() == VespaVersion.Confidence.broken) continue;
+            for (var nodeVersion : vespaVersion.nodeVersions().asMap().values()) {
+                if (!nodeVersion.changing()) continue;
+                if (nodeVersion.changedAt().isBefore(acceptableInstant)) nodesFailingUpgrade++;
+            }
         }
         return nodesFailingUpgrade;
     }
@@ -166,10 +172,8 @@ public class MetricsReporter extends Maintainer {
     }
 
     private static Map<String, String> dimensions(ApplicationId application) {
-        return ImmutableMap.of(
-                "tenant", application.tenant().value(),
-                "app",application.application().value() + "." + application.instance().value()
-        );
+        return Map.of("tenant", application.tenant().value(),
+                      "app",application.application().value() + "." + application.instance().value());
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java
index f4a9900c9ec..5061f32da68 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java
@@ -1,6 +1,7 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.persistence;
 
+import com.google.common.collect.ImmutableMap;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
@@ -18,7 +19,6 @@ import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
@@ -55,6 +55,7 @@ public class VersionStatusSerializer {
 
     // NodeVersion fields
     private static final String hostnameField = "hostname";
+    private static final String wantedVersionField = "wantedVersion";
     private static final String changedAtField = "changedAt";
 
     // DeploymentStatistics fields
@@ -95,6 +96,7 @@ public class VersionStatusSerializer {
         for (NodeVersion nodeVersion : nodeVersions.asMap().values()) {
             var nodeVersionObject = array.addObject();
             nodeVersionObject.setString(hostnameField, nodeVersion.hostname().value());
+            nodeVersionObject.setString(wantedVersionField, nodeVersion.wantedVersion().toFullString());
             nodeVersionObject.setLong(changedAtField, nodeVersion.changedAt().toEpochMilli());
         }
     }
@@ -135,13 +137,14 @@ public class VersionStatusSerializer {
     }
 
     private NodeVersions nodeVersionsFromSlime(Inspector root, Version version) {
-        var nodeVersions = new LinkedHashMap<HostName, NodeVersion>();
+        var nodeVersions = ImmutableMap.<HostName, NodeVersion>builder();
         var nodeVersionsRoot = root.field(nodeVersionsField);
         if (nodeVersionsRoot.valid()) {
             nodeVersionsRoot.traverse((ArrayTraverser) (i, entry) -> {
                 var hostname = HostName.from(entry.field(hostnameField).asString());
+                var wantedVersion = Version.fromString(entry.field(wantedVersionField).asString());
                 var changedAt = Instant.ofEpochMilli(entry.field(changedAtField).asLong());
-                nodeVersions.put(hostname, new NodeVersion(hostname, version, changedAt));
+                nodeVersions.put(hostname, new NodeVersion(hostname, version, wantedVersion, changedAt));
             });
         } else {
             // TODO(mpolden): Remove after October 2019
@@ -150,7 +153,7 @@ public class VersionStatusSerializer {
                 nodeVersions.put(hostname, NodeVersion.empty(hostname));
             }
         }
-        return new NodeVersions(nodeVersions);
+        return new NodeVersions(nodeVersions.build());
     }
 
     private Set<HostName> configServersFromSlime(Inspector array) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java
index 3ba1e4800db..0a690b90410 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java
@@ -17,12 +17,14 @@ import java.util.Objects;
 public class NodeVersion {
 
     private final HostName hostname;
-    private final Version version;
+    private final Version currentVersion;
+    private final Version wantedVersion;
     private final Instant changedAt;
 
-    public NodeVersion(HostName hostname, Version version, Instant changedAt) {
+    public NodeVersion(HostName hostname, Version currentVersion, Version wantedVersion, Instant changedAt) {
         this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null");
-        this.version = Objects.requireNonNull(version, "version must be non-null");
+        this.currentVersion = Objects.requireNonNull(currentVersion, "version must be non-null");
+        this.wantedVersion = Objects.requireNonNull(wantedVersion, "wantedVersion must be non-null");
         this.changedAt = Objects.requireNonNull(changedAt, "changedAt must be non-null");
     }
 
@@ -32,8 +34,18 @@ public class NodeVersion {
     }
 
     /** Current version of this */
-    public Version version() {
-        return version;
+    public Version currentVersion() {
+        return currentVersion;
+    }
+
+    /** Wanted version of this */
+    public Version wantedVersion() {
+        return wantedVersion;
+    }
+
+    /** Returns whether this is changing (upgrading or downgrading) */
+    public boolean changing() {
+        return !currentVersion.equals(wantedVersion);
     }
 
     /** The most recent time the version of this changed */
@@ -41,9 +53,21 @@ public class NodeVersion {
         return changedAt;
     }
 
+    /** Returns a copy of this with current version set to given version */
+    public NodeVersion withCurrentVersion(Version version, Instant changedAt) {
+        if (currentVersion.equals(version)) return this;
+        return new NodeVersion(hostname, version, wantedVersion, changedAt);
+    }
+
+    /** Returns a copy of this with wanted version set to given version */
+    public NodeVersion withWantedVersion(Version version) {
+        if (wantedVersion.equals(version)) return this;
+        return new NodeVersion(hostname, currentVersion, version, changedAt);
+    }
+
     @Override
     public String toString() {
-        return hostname + " on " + version + " [changed at " + changedAt + "]";
+        return hostname + ": " + currentVersion + " -> " + wantedVersion + " [changedAt=" + changedAt + "]";
     }
 
     @Override
@@ -52,17 +76,18 @@ public class NodeVersion {
         if (o == null || getClass() != o.getClass()) return false;
         NodeVersion that = (NodeVersion) o;
         return hostname.equals(that.hostname) &&
-               version.equals(that.version) &&
+               currentVersion.equals(that.currentVersion) &&
+               wantedVersion.equals(that.wantedVersion) &&
                changedAt.equals(that.changedAt);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(hostname, version, changedAt);
+        return Objects.hash(hostname, currentVersion, wantedVersion, changedAt);
     }
 
     public static NodeVersion empty(HostName hostname) {
-        return new NodeVersion(hostname, Version.emptyVersion, Instant.EPOCH);
+        return new NodeVersion(hostname, Version.emptyVersion, Version.emptyVersion, Instant.EPOCH);
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersions.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersions.java
index 80ecc2dd9bb..3ab96e03bcd 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersions.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersions.java
@@ -2,21 +2,17 @@
 package com.yahoo.vespa.hosted.controller.versions;
 
 import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.ImmutableSortedMap;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ListMultimap;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.HostName;
 
-import java.time.Instant;
-import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.function.Predicate;
 
-import static java.util.stream.Collectors.collectingAndThen;
-import static java.util.stream.Collectors.toMap;
-
 /**
  * A filterable list of {@link NodeVersion}s. This is immutable.
  *
@@ -24,12 +20,12 @@ import static java.util.stream.Collectors.toMap;
  */
 public class NodeVersions {
 
-    public static final NodeVersions EMPTY = new NodeVersions(Map.of());
+    public static final NodeVersions EMPTY = new NodeVersions(ImmutableMap.of());
 
-    private final Map<HostName, NodeVersion> nodeVersions;
+    private final ImmutableMap<HostName, NodeVersion> nodeVersions;
 
-    public NodeVersions(Map<HostName, NodeVersion> nodeVersions) {
-        this.nodeVersions = ImmutableSortedMap.copyOf(Objects.requireNonNull(nodeVersions));
+    public NodeVersions(ImmutableMap<HostName, NodeVersion> nodeVersions) {
+        this.nodeVersions = Objects.requireNonNull(nodeVersions);
     }
 
     public Map<HostName, NodeVersion> asMap() {
@@ -40,7 +36,7 @@ public class NodeVersions {
     public ListMultimap<Version, HostName> asVersionMap() {
         var versions = ImmutableListMultimap.<Version, HostName>builder();
         for (var kv : nodeVersions.entrySet()) {
-            versions.put(kv.getValue().version(), kv.getKey());
+            versions.put(kv.getValue().currentVersion(), kv.getKey());
         }
         return versions.build();
     }
@@ -52,21 +48,7 @@ public class NodeVersions {
 
     /** Returns a copy of this containing only node versions of given version */
     public NodeVersions matching(Version version) {
-        return filter(nodeVersion -> nodeVersion.version().equals(version));
-    }
-
-    /** Returns a copy of this containing only node versions that last changed before given instant */
-    public NodeVersions changedBefore(Instant instant) {
-        return filter(nodeVersion -> nodeVersion.changedAt().isBefore(instant));
-    }
-
-    /** Returns a copy of this retaining only node versions for the given host names */
-    public NodeVersions retainAll(Set<HostName> hostnames) {
-        if (this.nodeVersions.keySet().equals(hostnames)) return this;
-
-        var nodeVersions = new LinkedHashMap<>(this.nodeVersions);
-        nodeVersions.keySet().retainAll(hostnames);
-        return new NodeVersions(nodeVersions);
+        return filter(nodeVersion -> nodeVersion.currentVersion().equals(version));
     }
 
     /** Returns number of node versions in this */
@@ -74,21 +56,29 @@ public class NodeVersions {
         return nodeVersions.size();
     }
 
-    /** Returns a copy of this with a new node version added. Duplicate node versions are ignored */
-    public NodeVersions with(NodeVersion nodeVersion) {
-        var existing = nodeVersions.get(nodeVersion.hostname());
-        if (existing != null && existing.version().equals(nodeVersion.version())) return this;
-
-        var nodeVersions = new LinkedHashMap<>(this.nodeVersions);
-        nodeVersions.put(nodeVersion.hostname(), nodeVersion);
-        return new NodeVersions(nodeVersions);
+    /** Returns a copy of this containing only the given node versions */
+    public NodeVersions with(List<NodeVersion> nodeVersions) {
+        var newNodeVersions = ImmutableMap.<HostName, NodeVersion>builder();
+        for (var nodeVersion : nodeVersions) {
+            var existing = this.nodeVersions.get(nodeVersion.hostname());
+            if (existing != null) {
+                newNodeVersions.put(nodeVersion.hostname(), existing.withCurrentVersion(nodeVersion.currentVersion(),
+                                                                                        nodeVersion.changedAt())
+                                                                    .withWantedVersion(nodeVersion.wantedVersion()));
+            } else {
+                newNodeVersions.put(nodeVersion.hostname(), nodeVersion);
+            }
+        }
+        return new NodeVersions(newNodeVersions.build());
     }
 
     private NodeVersions filter(Predicate<NodeVersion> predicate) {
-        return nodeVersions.entrySet().stream()
-                           .filter(kv -> predicate.test(kv.getValue()))
-                           .collect(collectingAndThen(toMap(Map.Entry::getKey, Map.Entry::getValue),
-                                                      NodeVersions::new));
+        var newNodeVersions = ImmutableMap.<HostName, NodeVersion>builder();
+        for (var kv : nodeVersions.entrySet()) {
+            if (!predicate.test(kv.getValue())) continue;
+            newNodeVersions.put(kv.getKey(), kv.getValue());
+        }
+        return new NodeVersions(newNodeVersions.build());
     }
 
     @Override
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
index 8c7e539dee4..bb43ec20234 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
@@ -21,7 +21,6 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -156,7 +155,7 @@ public class VersionStatus {
         var nodeVersions = controller.versionStatus().systemVersion()
                                      .map(VespaVersion::nodeVersions)
                                      .orElse(NodeVersions.EMPTY);
-        var hostnames = new HashSet<HostName>();
+        var newNodeVersions = new ArrayList<NodeVersion>();
         for (var zone : controller.zoneRegistry().zones().controllerUpgraded().zones()) {
             for (var application : SystemApplication.all()) {
                 var nodes = controller.serviceRegistry().configServer().nodeRepository()
@@ -173,12 +172,11 @@ public class VersionStatus {
                 for (var node : nodes) {
                     // Only use current node version if config has converged
                     Version version = configConverged ? node.currentVersion() : controller.systemVersion();
-                    nodeVersions = nodeVersions.with(new NodeVersion(node.hostname(), version, now));
-                    hostnames.add(node.hostname());
+                    newNodeVersions.add(new NodeVersion(node.hostname(), version, node.wantedVersion(), now));
                 }
             }
         }
-        return nodeVersions.retainAll(hostnames);
+        return nodeVersions.with(newNodeVersions);
     }
 
     private static ListMultimap<ControllerVersion, HostName> findControllerVersions(Controller controller) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index 6d2090b652b..6da77a967f1 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -151,9 +151,16 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
 
     /** Set version for an application in a given zone */
     public void setVersion(ApplicationId application, ZoneId zone, Version version) {
+        setVersion(application, zone, version, -1);
+    }
+
+    /** Set version for nodeCount number of nodes in application in a given zone */
+    public void setVersion(ApplicationId application, ZoneId zone, Version version, int nodeCount) {
+        int n = 0;
         for (Node node : nodeRepository().list(zone, application)) {
             nodeRepository().putByHostname(zone, new Node(node.hostname(), node.state(), node.type(), node.owner(),
                                                           version, version));
+            if (++n == nodeCount) break;
         }
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
index f285e771a23..9cb40d60677 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.zone.UpgradePolicy;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
@@ -16,6 +17,7 @@ import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
 import com.yahoo.vespa.hosted.controller.deployment.InternalDeploymentTester;
 import com.yahoo.vespa.hosted.controller.integration.MetricsMock;
+import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock;
 import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
 import org.junit.Test;
 
@@ -219,6 +221,11 @@ public class MetricsReporterTest {
     public void test_nodes_failing_system_upgrade() {
         var tester = new DeploymentTester();
         var reporter = createReporter(tester.controller());
+        var zone1 = ZoneApiMock.fromId("prod.eu-west-1");
+        tester.controllerTester().zoneRegistry().setUpgradePolicy(UpgradePolicy.create().upgrade(zone1));
+        var systemUpgrader = new SystemUpgrader(tester.controller(), Duration.ofDays(1),
+                                                new JobControl(tester.controllerTester().curator()));
+        tester.configServer().bootstrap(List.of(zone1.getId()), SystemApplication.configServer);
 
         // System on initial version
         var version0 = Version.fromString("7.0");
@@ -226,30 +233,33 @@ public class MetricsReporterTest {
         reporter.maintain();
         assertEquals(0, getNodesFailingUpgrade());
 
-        // System starts upgrading to next version
-        var version1 = Version.fromString("7.1");
-        tester.upgradeController(version1);
-        reporter.maintain();
-        assertEquals(0, getNodesFailingUpgrade());
-
-        // Some time passes, but only a subset of nodes upgrade within timeout
-        tester.clock().advance(Duration.ofMinutes(30));
-        tester.upgradeSystemApplications(version1, List.of(SystemApplication.configServerHost));
-        reporter.maintain();
-        assertEquals(0, getNodesFailingUpgrade());
-        tester.clock().advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1)));
-        reporter.maintain();
-        assertEquals(48, getNodesFailingUpgrade());
-
-        // Some nodes are repaired and upgrade
-        tester.upgradeSystemApplications(version1, List.of(SystemApplication.configServer));
-        reporter.maintain();
-        assertEquals(36, getNodesFailingUpgrade());
-
-        // All nodes are repaired and system upgrades
-        tester.upgradeSystem(version1);
-        reporter.maintain();
-        assertEquals(0, getNodesFailingUpgrade());
+        for (var version : List.of(Version.fromString("7.1"), Version.fromString("7.2"))) {
+            // System starts upgrading to next version
+            tester.upgradeController(version);
+            reporter.maintain();
+            assertEquals(0, getNodesFailingUpgrade());
+            systemUpgrader.maintain();
+
+            // 30 minutes pass and nothing happens
+            tester.clock().advance(Duration.ofMinutes(30));
+            tester.computeVersionStatus();
+            reporter.maintain();
+            assertEquals(0, getNodesFailingUpgrade());
+
+            // 1/3 nodes upgrade within timeout
+            tester.configServer().setVersion(SystemApplication.configServer.id(), zone1.getId(), version, 1);
+            tester.clock().advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1)));
+            tester.computeVersionStatus();
+            reporter.maintain();
+            assertEquals(2, getNodesFailingUpgrade());
+
+            // 3/3 nodes upgrade
+            tester.configServer().setVersion(SystemApplication.configServer.id(), zone1.getId(), version);
+            tester.computeVersionStatus();
+            reporter.maintain();
+            assertEquals(0, getNodesFailingUpgrade());
+            assertEquals(version, tester.controller().systemVersion());
+        }
     }
 
     private Duration getAverageDeploymentDuration(ApplicationId id) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java
index 82837c20020..5d65cf0381e 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java
@@ -39,9 +39,11 @@ public class VersionStatusSerializerTest {
                         ApplicationId.from("tenant2", "success2", "default"))
         );
         vespaVersions.add(new VespaVersion(statistics, "dead", Instant.now(), false, false,
-                                           true, nodeVersions(Version.fromString("5.0"), Instant.ofEpochMilli(123), "cfg1", "cfg2", "cfg3"), VespaVersion.Confidence.normal));
+                                           true, nodeVersions(Version.fromString("5.0"), Version.fromString("5.1"),
+                                                              Instant.ofEpochMilli(123), "cfg1", "cfg2", "cfg3"), VespaVersion.Confidence.normal));
         vespaVersions.add(new VespaVersion(statistics, "cafe", Instant.now(), true, true,
-                                           false, nodeVersions(Version.fromString("5.0"), Instant.ofEpochMilli(456), "cfg1", "cfg2", "cfg3"), VespaVersion.Confidence.normal));
+                                           false, nodeVersions(Version.fromString("5.0"), Version.fromString("5.1"),
+                                                               Instant.ofEpochMilli(456), "cfg1", "cfg2", "cfg3"), VespaVersion.Confidence.normal));
         VersionStatus status = new VersionStatus(vespaVersions);
         VersionStatusSerializer serializer = new VersionStatusSerializer();
         VersionStatus deserialized = serializer.fromSlime(serializer.toSlime(status));
@@ -77,7 +79,8 @@ public class VersionStatusSerializerTest {
         var vespaVersion = new VespaVersion(statistics, "badc0ffee",
                                             Instant.ofEpochMilli(123), true,
                                             true, true,
-                                            nodeVersions(Version.emptyVersion, Instant.EPOCH, "cfg1", "cfg2", "cfg3"),
+                                            nodeVersions(Version.emptyVersion, Version.emptyVersion,
+                                                         Instant.EPOCH, "cfg1", "cfg2", "cfg3"),
                                             VespaVersion.Confidence.normal);
 
         VespaVersion deserialized = deserializedStatus.versions().get(0);
@@ -91,12 +94,12 @@ public class VersionStatusSerializerTest {
         assertEquals(vespaVersion.confidence(), deserialized.confidence());
     }
 
-    private static NodeVersions nodeVersions(Version version, Instant changedAt, String... hostnames) {
-        var nodeVersions = NodeVersions.EMPTY;
+    private static NodeVersions nodeVersions(Version version, Version wantedVersion, Instant changedAt, String... hostnames) {
+        var nodeVersions = new ArrayList<NodeVersion>();
         for (var hostname : hostnames) {
-            nodeVersions = nodeVersions.with(new NodeVersion(HostName.from(hostname), version, changedAt));
+            nodeVersions.add(new NodeVersion(HostName.from(hostname), version, wantedVersion, changedAt));
         }
-        return nodeVersions;
+        return NodeVersions.EMPTY.with(nodeVersions);
     }
 
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
index 7140702ad60..0a4d046e318 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
@@ -76,8 +76,8 @@ public class DeploymentApiTest extends ControllerContainerTest {
                                            version.isControllerVersion(),
                                            version.isSystemVersion(),
                                            version.isReleased(),
-                                           NodeVersions.EMPTY.with(new NodeVersion(HostName.from("config1.test"), version.versionNumber(), Instant.EPOCH))
-                                                             .with(new NodeVersion(HostName.from("config2.test"), version.versionNumber(), Instant.EPOCH)),
+                                           NodeVersions.EMPTY.with(List.of(new NodeVersion(HostName.from("config1.test"), version.versionNumber(), version.versionNumber(), Instant.EPOCH),
+                                                                           new NodeVersion(HostName.from("config2.test"), version.versionNumber(), version.versionNumber(), Instant.EPOCH))),
                                            VespaVersion.confidenceFrom(version.statistics(), controller)
                 );
             }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
index ba8309de286..83223b0e041 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
@@ -8,7 +8,6 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.vespa.hosted.controller.Application;
-import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;