Merge pull request #10901 from vespa-engine/mpolden/os-upgrade-failing-metric

Report metric for nodes failing OS upgrade

19 files changed, 399 insertions, 216 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 9253e249765..361cc43da50 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
@@ -12,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.NodeVersions;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 
 import java.time.Clock;
@@ -20,6 +21,7 @@ import java.time.Instant;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 /**
@@ -36,6 +38,7 @@ public class MetricsReporter extends Maintainer {
     public static final String DEPLOYMENT_BUILD_AGE_SECONDS = "deployment.buildAgeSeconds";
     public static final String DEPLOYMENT_WARNINGS = "deployment.warnings";
     public static final String NODES_FAILING_SYSTEM_UPGRADE = "deployment.nodesFailingSystemUpgrade";
+    public static final String NODES_FAILING_OS_UPGRADE = "deployment.nodesFailingOsUpgrade";
     public static final String REMAINING_ROTATIONS = "remaining_rotations";
     public static final String NAME_SERVICE_REQUESTS_QUEUED = "dns.queuedRequests";
 
@@ -56,6 +59,7 @@ public class MetricsReporter extends Maintainer {
         reportRemainingRotations();
         reportQueuedNameServiceRequests();
         reportNodesFailingSystemUpgrade();
+        reportNodesFailingOsUpgrade();
     }
 
     private void reportRemainingRotations() {
@@ -103,13 +107,31 @@ public class MetricsReporter extends Maintainer {
         metric.set(NODES_FAILING_SYSTEM_UPGRADE, nodesFailingSystemUpgrade(), metric.createContext(Map.of()));
     }
 
+    private void reportNodesFailingOsUpgrade() {
+        metric.set(NODES_FAILING_OS_UPGRADE, nodesFailingOsUpgrade(), metric.createContext(Map.of()));
+    }
+
     private int nodesFailingSystemUpgrade() {
         if (!controller().versionStatus().isUpgrading()) return 0;
+        return nodesFailingUpgrade(controller().versionStatus().versions(), (vespaVersion) -> {
+            if (vespaVersion.confidence() == VespaVersion.Confidence.broken) return NodeVersions.EMPTY;
+            return vespaVersion.nodeVersions();
+        });
+    }
+
+    private int nodesFailingOsUpgrade() {
+        return nodesFailingUpgrade(controller().osVersionStatus().versions().entrySet(), (kv) -> {
+            var osVersion = kv.getKey();
+            if (osVersion.version().isEmpty()) return NodeVersions.EMPTY;
+            return kv.getValue();
+        });
+    }
+
+    private <V> int nodesFailingUpgrade(Collection<V> collection, Function<V, NodeVersions> nodeVersionsFunction) {
         var nodesFailingUpgrade = 0;
         var acceptableInstant = clock.instant().minus(NODE_UPGRADE_TIMEOUT);
-        for (var vespaVersion : controller().versionStatus().versions()) {
-            if (vespaVersion.confidence() == VespaVersion.Confidence.broken) continue;
-            for (var nodeVersion : vespaVersion.nodeVersions().asMap().values()) {
+        for (var object : collection) {
+            for (var nodeVersion : nodeVersionsFunction.apply(object).asMap().values()) {
                 if (!nodeVersion.changing()) continue;
                 if (nodeVersion.changedAt().isBefore(acceptableInstant)) nodesFailingUpgrade++;
             }
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 60bc3d15ec6..93d1dac7382 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
@@ -61,7 +61,7 @@ public class OsUpgrader extends InfrastructureUpgrader {
         // Return target if we have nodes in this cloud on a lower version
         return controller().osVersion(cloud)
                            .filter(target -> controller().osVersionStatus().nodesIn(cloud).stream()
-                                                         .anyMatch(node -> node.version().isBefore(target.version())))
+                                                         .anyMatch(node -> node.currentVersion().isBefore(target.version())))
                            .map(OsVersion::version);
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
index 357dbb37b27..dbd52fc6d02 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
@@ -6,7 +6,6 @@ import com.google.inject.Inject;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
-import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.path.Path;
@@ -82,14 +81,15 @@ public class CuratorDb {
     private static final Path applicationCertificateRoot = root.append("applicationCertificates");
 
     private final StringSetSerializer stringSetSerializer = new StringSetSerializer();
-    private final VersionStatusSerializer versionStatusSerializer = new VersionStatusSerializer();
+    private final NodeVersionSerializer nodeVersionSerializer = new NodeVersionSerializer();
+    private final VersionStatusSerializer versionStatusSerializer = new VersionStatusSerializer(nodeVersionSerializer);
     private final ControllerVersionSerializer controllerVersionSerializer = new ControllerVersionSerializer();
     private final ConfidenceOverrideSerializer confidenceOverrideSerializer = new ConfidenceOverrideSerializer();
     private final TenantSerializer tenantSerializer = new TenantSerializer();
     private final ApplicationSerializer applicationSerializer = new ApplicationSerializer();
     private final RunSerializer runSerializer = new RunSerializer();
     private final OsVersionSerializer osVersionSerializer = new OsVersionSerializer();
-    private final OsVersionStatusSerializer osVersionStatusSerializer = new OsVersionStatusSerializer(osVersionSerializer);
+    private final OsVersionStatusSerializer osVersionStatusSerializer = new OsVersionStatusSerializer(osVersionSerializer, nodeVersionSerializer);
     private final RoutingPolicySerializer routingPolicySerializer = new RoutingPolicySerializer();
     private final AuditLogSerializer auditLogSerializer = new AuditLogSerializer();
     private final NameServiceQueueSerializer nameServiceQueueSerializer = new NameServiceQueueSerializer();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java
new file mode 100644
index 00000000000..4b6e997241d
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java
@@ -0,0 +1,78 @@
+// 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.HostName;
+import com.yahoo.config.provision.zone.ZoneId;
+import com.yahoo.slime.ArrayTraverser;
+import com.yahoo.slime.Cursor;
+import com.yahoo.slime.Inspector;
+import com.yahoo.vespa.hosted.controller.versions.NodeVersion;
+import com.yahoo.vespa.hosted.controller.versions.NodeVersions;
+
+import java.time.Instant;
+
+/**
+ * Serializer for {@link com.yahoo.vespa.hosted.controller.versions.NodeVersion}.
+ *
+ * @author mpolden
+ */
+public class NodeVersionSerializer {
+
+    // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one
+    //          (and rewrite all nodes on startup), changes to the serialized format must be made
+    //          such that what is serialized on version N+1 can be read by version N:
+    //          - ADDING FIELDS: Always ok
+    //          - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version.
+    //          - CHANGING THE FORMAT OF A FIELD: Don't do it bro.
+
+    private static final String hostnameField = "hostname";
+    private static final String zoneField = "zone";
+    private static final String wantedVersionField = "wantedVersion";
+    private static final String changedAtField = "changedAt";
+
+    // Legacy fields
+    private static final String environmentField = "environment";
+    private static final String regionField = "region";
+
+    public void nodeVersionsToSlime(NodeVersions nodeVersions, Cursor array) {
+        for (var nodeVersion : nodeVersions.asMap().values()) {
+            var nodeVersionObject = array.addObject();
+            nodeVersionObject.setString(hostnameField, nodeVersion.hostname().value());
+            nodeVersionObject.setString(zoneField, nodeVersion.zone().value());
+            nodeVersionObject.setString(wantedVersionField, nodeVersion.wantedVersion().toFullString());
+            nodeVersionObject.setLong(changedAtField, nodeVersion.changedAt().toEpochMilli());
+        }
+    }
+
+    public NodeVersions nodeVersionsFromSlime(Inspector array, Version version) {
+        var nodeVersions = ImmutableMap.<HostName, NodeVersion>builder();
+        array.traverse((ArrayTraverser) (i, entry) -> {
+            var hostname = HostName.from(entry.field(hostnameField).asString());
+            var zone = zoneFromSlime(entry);
+            // TODO(mpolden): Make the following fields non-optional after September 2019
+            var wantedVersion = Serializers.optionalString(entry.field(wantedVersionField))
+                                           .map(Version::fromString)
+                                           .orElse(Version.emptyVersion);
+            var changedAt = Serializers.optionalInstant(entry.field(changedAtField)).orElse(Instant.EPOCH);
+            nodeVersions.put(hostname, new NodeVersion(hostname, zone, version, wantedVersion, changedAt));
+        });
+        return new NodeVersions(nodeVersions.build());
+    }
+
+    // TODO(mpolden): Simplify and in-line after September 2019
+    private ZoneId zoneFromSlime(Inspector object) {
+        var zoneInspector = object.field(zoneField);
+        if (zoneInspector.valid()) {
+            return ZoneId.from(zoneInspector.asString());
+        }
+        var regionInspector = object.field(regionField);
+        var environmentInspector = object.field(environmentField);
+        if (regionInspector.valid() && environmentInspector.valid()) {
+            return ZoneId.from(environmentInspector.asString(), regionInspector.asString());
+        }
+        return ZoneId.defaultId();
+    }
+
+}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java
index 88805f54d65..fa29969f166 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java
@@ -1,23 +1,19 @@
 // Copyright 2018 Yahoo Holdings. 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.google.common.collect.ImmutableSortedMap;
 import com.yahoo.component.Version;
-import com.yahoo.config.provision.Environment;
-import com.yahoo.config.provision.HostName;
-import com.yahoo.config.provision.RegionName;
 import com.yahoo.slime.ArrayTraverser;
 import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.Slime;
+import com.yahoo.vespa.hosted.controller.versions.NodeVersion;
+import com.yahoo.vespa.hosted.controller.versions.NodeVersions;
 import com.yahoo.vespa.hosted.controller.versions.OsVersion;
 import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
 import java.util.Objects;
-import java.util.TreeMap;
 
 /**
  * Serializer for {@link OsVersionStatus}.
@@ -39,11 +35,14 @@ public class OsVersionStatusSerializer {
     private static final String hostnameField = "hostname";
     private static final String regionField = "region";
     private static final String environmentField = "environment";
+    private static final String nodeVersionsField = "nodeVersions";
 
     private final OsVersionSerializer osVersionSerializer;
+    private final NodeVersionSerializer nodeVersionSerializer;
 
-    public OsVersionStatusSerializer(OsVersionSerializer osVersionSerializer) {
+    public OsVersionStatusSerializer(OsVersionSerializer osVersionSerializer, NodeVersionSerializer nodeVersionSerializer) {
         this.osVersionSerializer = Objects.requireNonNull(osVersionSerializer, "osVersionSerializer must be non-null");
+        this.nodeVersionSerializer = Objects.requireNonNull(nodeVersionSerializer, "nodeVersionSerializer must be non-null");
     }
 
     public Slime toSlime(OsVersionStatus status) {
@@ -53,6 +52,8 @@ public class OsVersionStatusSerializer {
         status.versions().forEach((version, nodes) -> {
             Cursor object = versions.addObject();
             osVersionSerializer.toSlime(version, object);
+            nodeVersionSerializer.nodeVersionsToSlime(nodes, object.setArray(nodeVersionsField));
+            // TODO(mpolden): Stop writing this after September 2019
             nodesToSlime(nodes, object.setArray(nodesField));
         });
         return slime;
@@ -62,40 +63,33 @@ public class OsVersionStatusSerializer {
         return new OsVersionStatus(osVersionsFromSlime(slime.get().field(versionsField)));
     }
 
-    private void nodesToSlime(List<OsVersionStatus.Node> nodes, Cursor array) {
-        nodes.forEach(node -> nodeToSlime(node, array.addObject()));
+    private void nodesToSlime(NodeVersions nodeVersions, Cursor array) {
+        nodeVersions.asMap().values().forEach(node -> nodeToSlime(node, array.addObject()));
     }
 
-    private void nodeToSlime(OsVersionStatus.Node node, Cursor object) {
+    private void nodeToSlime(NodeVersion node, Cursor object) {
         object.setString(hostnameField, node.hostname().value());
-        object.setString(versionField, node.version().toFullString());
-        object.setString(regionField, node.region().value());
-        object.setString(environmentField, node.environment().value());
+        object.setString(versionField, node.currentVersion().toFullString());
+        object.setString(regionField, node.zone().region().value());
+        object.setString(environmentField, node.zone().environment().value());
     }
 
-    private Map<OsVersion, List<OsVersionStatus.Node>> osVersionsFromSlime(Inspector array) {
-        Map<OsVersion, List<OsVersionStatus.Node>> versions = new TreeMap<>();
+    private ImmutableMap<OsVersion, NodeVersions> osVersionsFromSlime(Inspector array) {
+        var versions = ImmutableSortedMap.<OsVersion, NodeVersions>naturalOrder();
         array.traverse((ArrayTraverser) (i, object) -> {
             OsVersion osVersion = osVersionSerializer.fromSlime(object);
-            List<OsVersionStatus.Node> nodes = nodesFromSlime(object.field(nodesField));
-            versions.put(osVersion, nodes);
+            versions.put(osVersion, nodesFromSlime(object, osVersion.version()));
         });
-        return Collections.unmodifiableMap(versions);
+        return versions.build();
     }
 
-    private List<OsVersionStatus.Node> nodesFromSlime(Inspector array) {
-        List<OsVersionStatus.Node> nodes = new ArrayList<>();
-        array.traverse((ArrayTraverser) (i, object) -> nodes.add(nodeFromSlime(object)));
-        return Collections.unmodifiableList(nodes);
-    }
-
-    private OsVersionStatus.Node nodeFromSlime(Inspector object) {
-        return new OsVersionStatus.Node(
-                HostName.from(object.field(hostnameField).asString()),
-                Version.fromString(object.field(versionField).asString()),
-                Environment.from(object.field(environmentField).asString()),
-                RegionName.from(object.field(regionField).asString())
-        );
+    // TODO(mpolden): Simplify and in-line after September 2019
+    private NodeVersions nodesFromSlime(Inspector object, Version version) {
+        var newField = object.field(nodeVersionsField);
+        if (newField.valid()) {
+            return nodeVersionSerializer.nodeVersionsFromSlime(newField, version);
+        }
+        return nodeVersionSerializer.nodeVersionsFromSlime(object.field(nodesField), version);
     }
 
 }
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 5061f32da68..366e2c9af4b 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,16 +1,13 @@
 // 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;
 import com.yahoo.slime.ArrayTraverser;
 import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.Slime;
 import com.yahoo.vespa.hosted.controller.versions.DeploymentStatistics;
-import com.yahoo.vespa.hosted.controller.versions.NodeVersion;
 import com.yahoo.vespa.hosted.controller.versions.NodeVersions;
 import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
@@ -19,9 +16,8 @@ import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
+import java.util.Objects;
 
 /**
  * Serializer for {@link VersionStatus}.
@@ -53,17 +49,18 @@ public class VersionStatusSerializer {
     // NodeVersions fields
     private static final String nodeVersionsField = "nodeVersions";
 
-    // NodeVersion fields
-    private static final String hostnameField = "hostname";
-    private static final String wantedVersionField = "wantedVersion";
-    private static final String changedAtField = "changedAt";
-
     // DeploymentStatistics fields
     private static final String versionField = "version";
     private static final String failingField = "failing";
     private static final String productionField = "production";
     private static final String deployingField = "deploying";
 
+    private final NodeVersionSerializer nodeVersionSerializer;
+
+    public VersionStatusSerializer(NodeVersionSerializer nodeVersionSerializer) {
+        this.nodeVersionSerializer = Objects.requireNonNull(nodeVersionSerializer, "nodeVersionSerializer must be non-null");
+    }
+
     public Slime toSlime(VersionStatus status) {
         Slime slime = new Slime();
         Cursor root = slime.setObject();
@@ -88,22 +85,11 @@ public class VersionStatusSerializer {
         object.setBool(isReleasedField, version.isReleased());
         deploymentStatisticsToSlime(version.statistics(), object.setObject(deploymentStatisticsField));
         object.setString(confidenceField, version.confidence().name());
-        configServersToSlime(version.nodeVersions().hostnames(), object.setArray(configServersField));
         nodeVersionsToSlime(version.nodeVersions(), object.setArray(nodeVersionsField));
     }
 
     private void nodeVersionsToSlime(NodeVersions nodeVersions, Cursor array) {
-        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());
-        }
-    }
-
-    // TODO(mpolden): Remove after October 2019
-    private void configServersToSlime(Set<HostName> configServerHostnames, Cursor array) {
-        configServerHostnames.stream().map(HostName::value).forEach(array::addString);
+        nodeVersionSerializer.nodeVersionsToSlime(nodeVersions, array);
     }
 
     private void deploymentStatisticsToSlime(DeploymentStatistics statistics, Cursor object) {
@@ -131,37 +117,11 @@ public class VersionStatusSerializer {
                                 object.field(isControllerVersionField).asBool(),
                                 object.field(isSystemVersionField).asBool(),
                                 object.field(isReleasedField).asBool(),
-                                nodeVersionsFromSlime(object, deploymentStatistics.version()),
+                                nodeVersionSerializer.nodeVersionsFromSlime(object.field(nodeVersionsField), deploymentStatistics.version()),
                                 VespaVersion.Confidence.valueOf(object.field(confidenceField).asString())
         );
     }
 
-    private NodeVersions nodeVersionsFromSlime(Inspector root, Version version) {
-        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, wantedVersion, changedAt));
-            });
-        } else {
-            // TODO(mpolden): Remove after October 2019
-            var configServerHostnames = configServersFromSlime(root.field(configServersField));
-            for (var hostname : configServerHostnames) {
-                nodeVersions.put(hostname, NodeVersion.empty(hostname));
-            }
-        }
-        return new NodeVersions(nodeVersions.build());
-    }
-
-    private Set<HostName> configServersFromSlime(Inspector array) {
-        Set<HostName> configServerHostnames = new LinkedHashSet<>();
-        array.traverse((ArrayTraverser) (i, entry) -> configServerHostnames.add(HostName.from(entry.asString())));
-        return Collections.unmodifiableSet(configServerHostnames);
-    }
-
     private DeploymentStatistics deploymentStatisticsFromSlime(Inspector object) {
         return new DeploymentStatistics(Version.fromString(object.field(versionField).asString()),
                                         applicationsFromSlime(object.field(failingField)),
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java
index 450f4481c5f..c168a057bfb 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java
@@ -160,17 +160,17 @@ public class OsApiHandler extends AuditLoggingRequestHandler {
         Set<OsVersion> osVersions = controller.osVersions();
 
         Cursor versions = root.setArray("versions");
-        controller.osVersionStatus().versions().forEach((osVersion, nodes) -> {
+        controller.osVersionStatus().versions().forEach((osVersion, nodeVersions) -> {
             Cursor currentVersionObject = versions.addObject();
             currentVersionObject.setString("version", osVersion.version().toFullString());
             currentVersionObject.setBool("targetVersion", osVersions.contains(osVersion));
             currentVersionObject.setString("cloud", osVersion.cloud().value());
             Cursor nodesArray = currentVersionObject.setArray("nodes");
-            nodes.forEach(node -> {
+            nodeVersions.asMap().values().forEach(nodeVersion -> {
                 Cursor nodeObject = nodesArray.addObject();
-                nodeObject.setString("hostname", node.hostname().value());
-                nodeObject.setString("environment", node.environment().value());
-                nodeObject.setString("region", node.region().value());
+                nodeObject.setString("hostname", nodeVersion.hostname().value());
+                nodeObject.setString("environment", nodeVersion.zone().environment().value());
+                nodeObject.setString("region", nodeVersion.zone().region().value());
             });
         });
 
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 0a690b90410..8d0232afa58 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
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.versions;
 
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.HostName;
+import com.yahoo.config.provision.zone.ZoneId;
 
 import java.time.Instant;
 import java.util.Objects;
@@ -17,12 +18,14 @@ import java.util.Objects;
 public class NodeVersion {
 
     private final HostName hostname;
+    private final ZoneId zone;
     private final Version currentVersion;
     private final Version wantedVersion;
     private final Instant changedAt;
 
-    public NodeVersion(HostName hostname, Version currentVersion, Version wantedVersion, Instant changedAt) {
+    public NodeVersion(HostName hostname, ZoneId zone, Version currentVersion, Version wantedVersion, Instant changedAt) {
         this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null");
+        this.zone = Objects.requireNonNull(zone, "zone 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");
@@ -33,6 +36,11 @@ public class NodeVersion {
         return hostname;
     }
 
+    /** Zone of this */
+    public ZoneId zone() {
+        return zone;
+    }
+
     /** Current version of this */
     public Version currentVersion() {
         return currentVersion;
@@ -56,18 +64,18 @@ public class NodeVersion {
     /** 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);
+        return new NodeVersion(hostname, zone, 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);
+        return new NodeVersion(hostname, zone, currentVersion, version, changedAt);
     }
 
     @Override
     public String toString() {
-        return hostname + ": " + currentVersion + " -> " + wantedVersion + " [changedAt=" + changedAt + "]";
+        return hostname + ": " + currentVersion + " -> " + wantedVersion + " [zone=" + zone + ", changedAt=" + changedAt + "]";
     }
 
     @Override
@@ -76,6 +84,7 @@ public class NodeVersion {
         if (o == null || getClass() != o.getClass()) return false;
         NodeVersion that = (NodeVersion) o;
         return hostname.equals(that.hostname) &&
+               zone.equals(that.zone) &&
                currentVersion.equals(that.currentVersion) &&
                wantedVersion.equals(that.wantedVersion) &&
                changedAt.equals(that.changedAt);
@@ -83,11 +92,11 @@ public class NodeVersion {
 
     @Override
     public int hashCode() {
-        return Objects.hash(hostname, currentVersion, wantedVersion, changedAt);
+        return Objects.hash(hostname, zone, currentVersion, wantedVersion, changedAt);
     }
 
     public static NodeVersion empty(HostName hostname) {
-        return new NodeVersion(hostname, Version.emptyVersion, Version.emptyVersion, Instant.EPOCH);
+        return new NodeVersion(hostname, ZoneId.defaultId(), Version.emptyVersion, Version.emptyVersion, Instant.EPOCH);
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java
index a73a20198f0..d5e83d99cdd 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java
@@ -4,9 +4,7 @@ package com.yahoo.vespa.hosted.controller.versions;
 import com.google.common.collect.ImmutableMap;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.CloudName;
-import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostName;
-import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
@@ -14,11 +12,11 @@ import com.yahoo.vespa.hosted.controller.maintenance.OsUpgrader;
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -29,25 +27,25 @@ import java.util.stream.Collectors;
  */
 public class OsVersionStatus {
 
-    public static final OsVersionStatus empty = new OsVersionStatus(Collections.emptyMap());
+    public static final OsVersionStatus empty = new OsVersionStatus(ImmutableMap.of());
 
-    private final Map<OsVersion, List<Node>> versions;
+    private final Map<OsVersion, NodeVersions> versions;
 
     /** Public for serialization purpose only. Use {@link OsVersionStatus#compute(Controller)} for an up-to-date status */
-    public OsVersionStatus(Map<OsVersion, List<Node>> versions) {
+    public OsVersionStatus(ImmutableMap<OsVersion, NodeVersions> versions) {
         this.versions = ImmutableMap.copyOf(Objects.requireNonNull(versions, "versions must be non-null"));
     }
 
     /** All known OS versions and their nodes */
-    public Map<OsVersion, List<Node>> versions() {
+    public Map<OsVersion, NodeVersions> versions() {
         return versions;
     }
 
     /** Returns nodes eligible for OS upgrades that exist in given cloud */
-    public List<Node> nodesIn(CloudName cloud) {
+    public List<NodeVersion> nodesIn(CloudName cloud) {
         return versions.entrySet().stream()
                        .filter(entry -> entry.getKey().cloud().equals(cloud))
-                       .flatMap(entry -> entry.getValue().stream())
+                       .flatMap(entry -> entry.getValue().asMap().values().stream())
                        .collect(Collectors.toUnmodifiableList());
     }
 
@@ -61,28 +59,52 @@ public class OsVersionStatus {
 
     /** Compute the current OS versions in this system. This is expensive and should be called infrequently */
     public static OsVersionStatus compute(Controller controller) {
-        Map<OsVersion, List<Node>> versions = new HashMap<>();
-
-        // Always include all target versions
-        controller.osVersions().forEach(osVersion -> versions.put(osVersion, new ArrayList<>()));
-
-        for (SystemApplication application : SystemApplication.all()) {
-            if (!application.isEligibleForOsUpgrades()) {
-                continue; // Avoid querying applications that are not eligible for OS upgrades
-            }
-            for (ZoneApi zone : zonesToUpgrade(controller)) {
-                controller.serviceRegistry().configServer().nodeRepository().list(zone.getId(), application.id()).stream()
+        var osVersionStatus = controller.osVersionStatus();
+        var osVersions = new HashMap<OsVersion, List<NodeVersion>>();
+        var now = controller.clock().instant();
+        controller.osVersions().forEach(osVersion -> osVersions.put(osVersion, new ArrayList<>()));
+
+        for (var application : SystemApplication.all()) {
+            if (!application.isEligibleForOsUpgrades()) continue;
+            for (var zone : zonesToUpgrade(controller)) {
+                var targetOsVersion = controller.serviceRegistry().configServer().nodeRepository()
+                                                .targetVersionsOf(zone.getId())
+                                                .osVersion(application.nodeType())
+                                                .orElse(Version.emptyVersion);
+                controller.serviceRegistry().configServer().nodeRepository()
+                          .list(zone.getId(), application.id()).stream()
                           .filter(node -> OsUpgrader.eligibleForUpgrade(node, application))
-                          .map(node -> new Node(node.hostname(), node.currentOsVersion(), zone.getEnvironment(), zone.getRegionName()))
-                          .forEach(node -> {
-                              var version = new OsVersion(node.version(), zone.getCloudName());
-                              versions.putIfAbsent(version, new ArrayList<>());
-                              versions.get(version).add(node);
+                          .map(node -> new NodeVersion(node.hostname(), zone.getId(), node.currentOsVersion(), targetOsVersion, now))
+                          .forEach(nodeVersion -> {
+                              var newNodeVersion = osVersionStatus.of(nodeVersion.hostname())
+                                                                  .map(nv -> nv.withCurrentVersion(nodeVersion.currentVersion(), now)
+                                                                               .withWantedVersion(nodeVersion.wantedVersion()))
+                                                                  .orElse(nodeVersion);
+                              var version = new OsVersion(newNodeVersion.currentVersion(), zone.getCloudName());
+                              osVersions.putIfAbsent(version, new ArrayList<>());
+                              osVersions.get(version).add(newNodeVersion);
                           });
             }
         }
 
-        return new OsVersionStatus(versions);
+        var newOsVersions = ImmutableMap.<OsVersion, NodeVersions>builder();
+        for (var osVersion : osVersions.entrySet()) {
+            var nodeVersions = ImmutableMap.<HostName, NodeVersion>builder();
+            for (var nodeVersion : osVersion.getValue()) {
+                nodeVersions.put(nodeVersion.hostname(), nodeVersion);
+            }
+            newOsVersions.put(osVersion.getKey(), new NodeVersions(nodeVersions.build()));
+        }
+        return new OsVersionStatus(newOsVersions.build());
+    }
+
+    /** Returns version of node identified by given host name */
+    private Optional<NodeVersion> of(HostName hostname) {
+        return versions.values().stream()
+                       .map(nodeVersions -> nodeVersions.asMap().get(hostname))
+                       .map(Optional::ofNullable)
+                       .flatMap(Optional::stream)
+                       .findFirst();
     }
 
     private static List<ZoneApi> zonesToUpgrade(Controller controller) {
@@ -92,52 +114,4 @@ public class OsVersionStatus {
                          .collect(Collectors.toUnmodifiableList());
     }
 
-    /** A node in this system and its current OS version */
-    public static class Node {
-
-        private final HostName hostname;
-        private final Version version;
-        private final Environment environment;
-        private final RegionName region;
-
-        public Node(HostName hostname, Version version, Environment environment, RegionName region) {
-            this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null");
-            this.version = Objects.requireNonNull(version, "version must be non-null");
-            this.environment = Objects.requireNonNull(environment, "environment must be non-null");
-            this.region = Objects.requireNonNull(region, "region must be non-null");
-        }
-
-        public HostName hostname() {
-            return hostname;
-        }
-
-        public Version version() {
-            return version;
-        }
-
-        public Environment environment() {
-            return environment;
-        }
-
-        public RegionName region() {
-            return region;
-        }
-
-        @Override
-        public boolean equals(Object o) {
-            if (this == o) return true;
-            if (o == null || getClass() != o.getClass()) return false;
-            Node node = (Node) o;
-            return Objects.equals(hostname, node.hostname) &&
-                   Objects.equals(version, node.version) &&
-                   environment == node.environment &&
-                   Objects.equals(region, node.region);
-        }
-
-        @Override
-        public int hashCode() {
-            return Objects.hash(hostname, version, environment, region);
-        }
-    }
-
 }
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 bb43ec20234..ab445de5a7f 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
@@ -172,7 +172,7 @@ public class VersionStatus {
                 for (var node : nodes) {
                     // Only use current node version if config has converged
                     Version version = configConverged ? node.currentVersion() : controller.systemVersion();
-                    newNodeVersions.add(new NodeVersion(node.hostname(), version, node.wantedVersion(), now));
+                    newNodeVersions.add(new NodeVersion(node.hostname(), zone.getId(), version, node.wantedVersion(), now));
                 }
             }
         }
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 6da77a967f1..6e7a50b5f81 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,15 +151,44 @@ 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);
+        setVersion(application, zone, version, -1, false);
     }
 
     /** Set version for nodeCount number of nodes in application in a given zone */
     public void setVersion(ApplicationId application, ZoneId zone, Version version, int nodeCount) {
+        setVersion(application, zone, version, nodeCount, false);
+    }
+
+    /** Set OS version for an application in a given zone */
+    public void setOsVersion(ApplicationId application, ZoneId zone, Version version) {
+        setOsVersion(application, zone, version, -1);
+    }
+
+    /** Set OS version for an application in a given zone */
+    public void setOsVersion(ApplicationId application, ZoneId zone, Version version, int nodeCount) {
+        setVersion(application, zone, version, nodeCount, true);
+    }
+
+    private void setVersion(ApplicationId application, ZoneId zone, Version version, int nodeCount, boolean osVersion) {
         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));
+            Node newNode;
+            if (osVersion) {
+                newNode = new Node(node.hostname(), node.state(), node.type(), node.owner(), node.currentVersion(),
+                                   node.wantedVersion(), version, version, node.serviceState(),
+                                   node.restartGeneration(), node.wantedRestartGeneration(), node.rebootGeneration(),
+                                   node.wantedRebootGeneration(), node.vcpu(), node.memoryGb(), node.diskGb(),
+                                   node.bandwidthGbps(), node.fastDisk(), node.cost(), node.canonicalFlavor(),
+                                   node.clusterId(), node.clusterType());
+            } else {
+                newNode = new Node(node.hostname(), node.state(), node.type(), node.owner(), version,
+                                   version, node.currentOsVersion(), node.wantedOsVersion(), node.serviceState(),
+                                   node.restartGeneration(), node.wantedRestartGeneration(), node.rebootGeneration(),
+                                   node.wantedRebootGeneration(), node.vcpu(), node.memoryGb(), node.diskGb(),
+                                   node.bandwidthGbps(), node.fastDisk(), node.cost(), node.canonicalFlavor(),
+                                   node.clusterId(), node.clusterType());
+            }
+            nodeRepository().putByHostname(zone, newNode);
             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 9cb40d60677..44785407874 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
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.zone.UpgradePolicy;
 import com.yahoo.config.provision.zone.ZoneId;
@@ -262,6 +263,57 @@ public class MetricsReporterTest {
         }
     }
 
+    @Test
+    public void test_nodes_failing_os_upgrade() {
+        var tester = new DeploymentTester();
+        var reporter = createReporter(tester.controller());
+        var zone = ZoneApiMock.fromId("prod.eu-west-1");
+        var cloud = CloudName.defaultName();
+        tester.controllerTester().zoneRegistry().setOsUpgradePolicy(cloud, UpgradePolicy.create().upgrade(zone));
+        var osUpgrader = new OsUpgrader(tester.controller(), Duration.ofDays(1),
+                                        new JobControl(tester.controllerTester().curator()), CloudName.defaultName());;
+        var statusUpdater = new OsVersionStatusUpdater(tester.controller(), Duration.ofDays(1),
+                                                       new JobControl(tester.controller().curator()));
+        tester.configServer().bootstrap(List.of(zone.getId()), SystemApplication.tenantHost);
+
+        // All nodes upgrade to initial OS version
+        var version0 = Version.fromString("8.0");
+        tester.controller().upgradeOsIn(cloud, version0, false);
+        osUpgrader.maintain();
+        tester.configServer().setOsVersion(SystemApplication.tenantHost.id(), zone.getId(), version0);
+        statusUpdater.maintain();
+        reporter.maintain();
+        assertEquals(0, getNodesFailingOsUpgrade());
+
+        for (var version : List.of(Version.fromString("8.1"), Version.fromString("8.2"))) {
+            // System starts upgrading to next OS version
+            tester.controller().upgradeOsIn(cloud, version, false);
+            osUpgrader.maintain();
+            statusUpdater.maintain();
+            reporter.maintain();
+            assertEquals(0, getNodesFailingOsUpgrade());
+
+            // 30 minutes pass and nothing happens
+            tester.clock().advance(Duration.ofMinutes(30));
+            statusUpdater.maintain();
+            reporter.maintain();
+            assertEquals(0, getNodesFailingOsUpgrade());
+
+            // 1/3 nodes upgrade within timeout
+            tester.configServer().setOsVersion(SystemApplication.tenantHost.id(), zone.getId(), version, 1);
+            tester.clock().advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1)));
+            statusUpdater.maintain();
+            reporter.maintain();
+            assertEquals(2, getNodesFailingOsUpgrade());
+
+            // 3/3 nodes upgrade
+            tester.configServer().setOsVersion(SystemApplication.tenantHost.id(), zone.getId(), version);
+            statusUpdater.maintain();
+            reporter.maintain();
+            assertEquals(0, getNodesFailingOsUpgrade());
+        }
+    }
+
     private Duration getAverageDeploymentDuration(ApplicationId id) {
         return Duration.ofSeconds(getMetric(MetricsReporter.DEPLOYMENT_AVERAGE_DURATION, id).longValue());
     }
@@ -278,6 +330,10 @@ public class MetricsReporterTest {
         return metrics.getMetric(MetricsReporter.NODES_FAILING_SYSTEM_UPGRADE).intValue();
     }
 
+    private int getNodesFailingOsUpgrade() {
+        return metrics.getMetric(MetricsReporter.NODES_FAILING_OS_UPGRADE).intValue();
+    }
+
     private Number getMetric(String name, ApplicationId id) {
         return metrics.getMetric((dimensions) -> id.tenant().value().equals(dimensions.get("tenant")) &&
                                                  appDimension(id).equals(dimensions.get("app")),
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 1af5fafbb79..5e92112d465 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
@@ -12,7 +12,7 @@ import com.yahoo.vespa.hosted.controller.application.SystemApplication;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
 import com.yahoo.vespa.hosted.controller.integration.NodeRepositoryMock;
 import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock;
-import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus;
+import com.yahoo.vespa.hosted.controller.versions.NodeVersion;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -111,13 +111,13 @@ public class OsUpgraderTest {
         assertWanted(version1, SystemApplication.tenantHost, zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId());
         statusUpdater.maintain();
         assertTrue("All nodes on target version", tester.controller().osVersionStatus().nodesIn(cloud).stream()
-                                                        .allMatch(node -> node.version().equals(version1)));
+                                                        .allMatch(node -> node.currentVersion().equals(version1)));
     }
 
-    private List<OsVersionStatus.Node> nodesOn(Version version) {
+    private List<NodeVersion> nodesOn(Version version) {
         return tester.controller().osVersionStatus().versions().entrySet().stream()
                      .filter(entry -> entry.getKey().version().equals(version))
-                     .flatMap(entry -> entry.getValue().stream())
+                     .flatMap(entry -> entry.getValue().asMap().values().stream())
                      .collect(Collectors.toList());
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java
index fe7f39fd66d..e51fcff33d1 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java
@@ -3,18 +3,15 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.CloudName;
+import com.yahoo.config.provision.zone.UpgradePolicy;
 import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
-import com.yahoo.config.provision.zone.UpgradePolicy;
-import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
 import com.yahoo.vespa.hosted.controller.versions.OsVersion;
 import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus;
 import org.junit.Test;
 
 import java.time.Duration;
-import java.util.List;
-import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -47,10 +44,10 @@ public class OsVersionStatusUpdaterTest {
         tester.controller().upgradeOsIn(cloud, version1, false);
         statusUpdater.maintain();
 
-        Map<OsVersion, List<OsVersionStatus.Node>> osVersions = tester.controller().osVersionStatus().versions();
+        var osVersions = tester.controller().osVersionStatus().versions();
         assertEquals(2, osVersions.size());
-        assertFalse("All nodes on unknown version", osVersions.get(new OsVersion(Version.emptyVersion, cloud)).isEmpty());
-        assertTrue("No nodes on current target", osVersions.get(new OsVersion(version1, cloud)).isEmpty());
+        assertFalse("All nodes on unknown version", osVersions.get(new OsVersion(Version.emptyVersion, cloud)).asMap().isEmpty());
+        assertTrue("No nodes on current target", osVersions.get(new OsVersion(version1, cloud)).asMap().isEmpty());
     }
 
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java
index 5073f651fd3..ba771d70d26 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java
@@ -1,18 +1,23 @@
 // Copyright 2018 Yahoo Holdings. 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.CloudName;
-import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostName;
-import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.zone.ZoneId;
+import com.yahoo.vespa.config.SlimeUtils;
+import com.yahoo.vespa.hosted.controller.versions.NodeVersion;
+import com.yahoo.vespa.hosted.controller.versions.NodeVersions;
 import com.yahoo.vespa.hosted.controller.versions.OsVersion;
 import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus;
 import org.junit.Test;
 
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.time.Instant;
 import java.util.List;
-import java.util.Map;
-import java.util.TreeMap;
 
 import static org.junit.Assert.assertEquals;
 
@@ -25,22 +30,41 @@ public class OsVersionStatusSerializerTest {
     public void test_serialization() {
         Version version1 = Version.fromString("7.1");
         Version version2 = Version.fromString("7.2");
-        Map<OsVersion, List<OsVersionStatus.Node>> versions = new TreeMap<>();
+        var versions = ImmutableMap.<OsVersion, NodeVersions>builder();
 
-        versions.put(new OsVersion(version1, CloudName.defaultName()), List.of(
-                new OsVersionStatus.Node(HostName.from("node1"), version1, Environment.prod, RegionName.from("us-west")),
-                new OsVersionStatus.Node(HostName.from("node2"), version1, Environment.prod, RegionName.from("us-east"))
-        ));
-        versions.put(new OsVersion(version2, CloudName.defaultName()), List.of(
-                new OsVersionStatus.Node(HostName.from("node3"), version2, Environment.prod, RegionName.from("us-west")),
-                new OsVersionStatus.Node(HostName.from("node4"), version2, Environment.prod, RegionName.from("us-east"))
+        versions.put(new OsVersion(version1, CloudName.defaultName()), NodeVersions.EMPTY.with(List.of(
+                new NodeVersion(HostName.from("node1"), ZoneId.from("prod", "us-west"), version1, version2, Instant.ofEpochMilli(1)),
+                new NodeVersion(HostName.from("node2"), ZoneId.from("prod", "us-east"), version1, version2, Instant.ofEpochMilli(2))
+        )));
+        versions.put(new OsVersion(version2, CloudName.defaultName()), NodeVersions.EMPTY.with(List.of(
+                new NodeVersion(HostName.from("node3"), ZoneId.from("prod", "us-west"), version2, version2, Instant.ofEpochMilli(3)),
+                new NodeVersion(HostName.from("node4"), ZoneId.from("prod", "us-east"), version2, version2, Instant.ofEpochMilli(4))
+        )));
 
-        ));
-
-        OsVersionStatusSerializer serializer = new OsVersionStatusSerializer(new OsVersionSerializer());
-        OsVersionStatus status = new OsVersionStatus(versions);
+        OsVersionStatusSerializer serializer = new OsVersionStatusSerializer(new OsVersionSerializer(), new NodeVersionSerializer());
+        OsVersionStatus status = new OsVersionStatus(versions.build());
         OsVersionStatus serialized = serializer.fromSlime(serializer.toSlime(status));
         assertEquals(status.versions(), serialized.versions());
     }
 
+    @Test
+    public void testLegacySerialization() throws Exception {
+        var data = Files.readAllBytes(Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/os-version-status-legacy-format.json"));
+        var serializer = new OsVersionStatusSerializer(new OsVersionSerializer(), new NodeVersionSerializer());
+        var versions = ImmutableMap.of(
+                new OsVersion(Version.fromString("7.42"), CloudName.from("yahoo")),
+                NodeVersions.EMPTY.with(List.of(new NodeVersion(HostName.from("node1"), ZoneId.from("prod", "us-north-1"),
+                                                                Version.fromString("7.42"), Version.emptyVersion, Instant.EPOCH),
+                                                new NodeVersion(HostName.from("node2"), ZoneId.from("test", "us-north-2"),
+                                                                Version.fromString("7.42"), Version.emptyVersion, Instant.EPOCH))));
+
+        var deserialized = serializer.fromSlime(SlimeUtils.jsonToSlime(data));
+        assertEquals(versions, deserialized.versions());
+
+
+        var serialized = new String(SlimeUtils.toJsonBytes(serializer.toSlime(new OsVersionStatus(versions))), StandardCharsets.UTF_8);
+        assertEquals("{\"versions\":[{\"version\":\"7.42.0\",\"cloud\":\"yahoo\",\"nodeVersions\":[{\"hostname\":\"node1\",\"zone\":\"prod.us-north-1\",\"wantedVersion\":\"0.0.0\",\"changedAt\":0},{\"hostname\":\"node2\",\"zone\":\"test.us-north-2\",\"wantedVersion\":\"0.0.0\",\"changedAt\":0}],\"nodes\":[{\"hostname\":\"node1\",\"version\":\"7.42.0\",\"region\":\"us-north-1\",\"environment\":\"prod\"},{\"hostname\":\"node2\",\"version\":\"7.42.0\",\"region\":\"us-north-2\",\"environment\":\"test\"}]}]}",
+                     serialized);
+    }
+
 }
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 5d65cf0381e..a80dcc118dc 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
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.persistence;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
+import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.config.SlimeUtils;
 import com.yahoo.vespa.hosted.controller.versions.DeploymentStatistics;
 import com.yahoo.vespa.hosted.controller.versions.NodeVersion;
@@ -45,7 +46,7 @@ public class VersionStatusSerializerTest {
                                            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();
+        VersionStatusSerializer serializer = new VersionStatusSerializer(new NodeVersionSerializer());
         VersionStatus deserialized = serializer.fromSlime(serializer.toSlime(status));
 
         assertEquals(status.versions().size(), deserialized.versions().size());
@@ -67,7 +68,7 @@ public class VersionStatusSerializerTest {
     @Test
     public void testLegacySerialization() throws Exception {
         var data = Files.readAllBytes(Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/version-status-legacy-format.json"));
-        var serializer = new VersionStatusSerializer();
+        var serializer = new VersionStatusSerializer(new NodeVersionSerializer());
         var deserializedStatus = serializer.fromSlime(SlimeUtils.jsonToSlime(data));
 
         var statistics = new DeploymentStatistics(
@@ -76,11 +77,16 @@ public class VersionStatusSerializerTest {
                 List.of(),
                 List.of()
         );
+        var nodeVersions = List.of(new NodeVersion(HostName.from("cfg1"), ZoneId.defaultId(), Version.fromString("7.0"),
+                                                   Version.fromString("7.1"), Instant.ofEpochMilli(1111)),
+                                   new NodeVersion(HostName.from("cfg2"), ZoneId.defaultId(), Version.fromString("7.0"),
+                                                   Version.fromString("7.1"), Instant.ofEpochMilli(2222)),
+                                   new NodeVersion(HostName.from("cfg3"), ZoneId.defaultId(), Version.fromString("7.0"),
+                                                   Version.fromString("7.1"), Instant.ofEpochMilli(3333)));
         var vespaVersion = new VespaVersion(statistics, "badc0ffee",
                                             Instant.ofEpochMilli(123), true,
                                             true, true,
-                                            nodeVersions(Version.emptyVersion, Version.emptyVersion,
-                                                         Instant.EPOCH, "cfg1", "cfg2", "cfg3"),
+                                            NodeVersions.EMPTY.with(nodeVersions),
                                             VespaVersion.Confidence.normal);
 
         VespaVersion deserialized = deserializedStatus.versions().get(0);
@@ -97,7 +103,7 @@ public class VersionStatusSerializerTest {
     private static NodeVersions nodeVersions(Version version, Version wantedVersion, Instant changedAt, String... hostnames) {
         var nodeVersions = new ArrayList<NodeVersion>();
         for (var hostname : hostnames) {
-            nodeVersions.add(new NodeVersion(HostName.from(hostname), version, wantedVersion, changedAt));
+            nodeVersions.add(new NodeVersion(HostName.from(hostname), ZoneId.from("prod", "us-north-1"), version, wantedVersion, changedAt));
         }
         return NodeVersions.EMPTY.with(nodeVersions);
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/os-version-status-legacy-format.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/os-version-status-legacy-format.json
new file mode 100644
index 00000000000..5a6a864cbf8
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/os-version-status-legacy-format.json
@@ -0,0 +1,22 @@
+{
+  "versions": [
+    {
+      "version": "7.42",
+      "cloud": "yahoo",
+      "nodes": [
+        {
+          "hostname": "node1",
+          "version": "7.42",
+          "region": "us-north-1",
+          "environment": "prod"
+        },
+        {
+          "hostname": "node2",
+          "version": "7.42",
+          "region": "us-north-2",
+          "environment": "test"
+        }
+      ]
+    }
+  ]
+}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/version-status-legacy-format.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/version-status-legacy-format.json
index 96ca22e1c1a..08463ed7cb4 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/version-status-legacy-format.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/version-status-legacy-format.json
@@ -13,10 +13,22 @@
         "deploying": []
       },
       "confidence": "normal",
-      "configServerHostnames": [
-        "cfg1",
-        "cfg2",
-        "cfg3"
+      "nodeVersions": [
+        {
+          "hostname": "cfg1",
+          "wantedVersion": "7.1",
+          "changedAt": 1111
+        },
+        {
+          "hostname": "cfg2",
+          "wantedVersion": "7.1",
+          "changedAt": 2222
+        },
+        {
+          "hostname": "cfg3",
+          "wantedVersion": "7.1",
+          "changedAt": 3333
+        }
       ]
     }
   ]
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 0a4d046e318..bb1e6b6256a 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(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))),
+                                           NodeVersions.EMPTY.with(List.of(new NodeVersion(HostName.from("config1.test"), ZoneId.defaultId(), version.versionNumber(), version.versionNumber(), Instant.EPOCH),
+                                                                           new NodeVersion(HostName.from("config2.test"), ZoneId.defaultId(), version.versionNumber(), version.versionNumber(), Instant.EPOCH))),
                                            VespaVersion.confidenceFrom(version.statistics(), controller)
                 );
             }