Measure platform and OS change duration per host

13 files changed, 291 insertions, 213 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 9c414ce8348..d70c65f343a 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
@@ -13,6 +13,7 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatusList;
 import com.yahoo.vespa.hosted.controller.deployment.JobList;
 import com.yahoo.vespa.hosted.controller.rotation.RotationLock;
+import com.yahoo.vespa.hosted.controller.versions.NodeVersion;
 import com.yahoo.vespa.hosted.controller.versions.NodeVersions;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 
@@ -20,6 +21,7 @@ import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
@@ -38,18 +40,21 @@ public class MetricsReporter extends Maintainer {
     public static final String DEPLOYMENT_FAILING_UPGRADES = "deployment.failingUpgrades";
     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";
+    // TODO(mpolden): Remove these two metrics
+    public static final String NODES_FAILING_PLATFORM_UPGRADE = "deployment.nodesFailingSystemUpgrade";
     public static final String NODES_FAILING_OS_UPGRADE = "deployment.nodesFailingOsUpgrade";
+    public static final String OS_CHANGE_DURATION = "deployment.osChangeDuration";
+    public static final String PLATFORM_CHANGE_DURATION = "deployment.platformChangeDuration";
     public static final String REMAINING_ROTATIONS = "remaining_rotations";
     public static final String NAME_SERVICE_REQUESTS_QUEUED = "dns.queuedRequests";
 
-    // The time a node belonging to a system application can spend from being told to upgrade until the upgrade is
-    // completed. Nodes exceeding this time are counted as failures.
-    private static final Duration NODE_UPGRADE_TIMEOUT = Duration.ofMinutes(90);
+    // The time a system application node can spend after suspending for Vespa upgrade until the upgrade is completed.
+    // Nodes exceeding this budget are counted as failures.
+    private static final Duration PLATFORM_UPGRADE_BUDGET = Duration.ofMinutes(30);
 
-    // The time a single node can spend performing an OS upgrade after being told to upgrade. Nodes exceeding this time
-    // multiplied by the number of nodes upgrading are counted as failures.
-    private static final Duration OS_UPGRADE_TIME_ALLOWANCE_PER_NODE = Duration.ofMinutes(30);
+    // The time a system application node can spend after suspending foor OS upgrade until the upgrade is completed.
+    // Nodes exceeding this budget are counted as failures.
+    private static final Duration OS_UPGRADE_BUDGET = Duration.ofMinutes(30);
 
     private final Metric metric;
     private final Clock clock;
@@ -65,7 +70,7 @@ public class MetricsReporter extends Maintainer {
         reportDeploymentMetrics();
         reportRemainingRotations();
         reportQueuedNameServiceRequests();
-        reportNodesFailingUpgrade();
+        reportChangeDurations();
     }
 
     private void reportRemainingRotations() {
@@ -107,43 +112,44 @@ public class MetricsReporter extends Maintainer {
                    metric.createContext(Map.of()));
     }
 
-    private void reportNodesFailingUpgrade() {
-        metric.set(NODES_FAILING_SYSTEM_UPGRADE, nodesFailingSystemUpgrade(), metric.createContext(Map.of()));
-        metric.set(NODES_FAILING_OS_UPGRADE, nodesFailingOsUpgrade(), metric.createContext(Map.of()));
+    private void reportChangeDurations() {
+        var platformChangeDurations = platformChangeDurations();
+        var osChangeDurations = osChangeDurations();
+        var nodesFailingSystemUpgrade = platformChangeDurations.values().stream()
+                                                               .filter(duration -> duration.compareTo(PLATFORM_UPGRADE_BUDGET) > 0)
+                                                               .count();
+        var nodesFailingOsUpgrade = osChangeDurations.values().stream()
+                                                     .filter(duration -> duration.compareTo(OS_UPGRADE_BUDGET) > 0)
+                                                     .count();
+        metric.set(NODES_FAILING_PLATFORM_UPGRADE, nodesFailingSystemUpgrade, metric.createContext(Map.of()));
+        metric.set(NODES_FAILING_OS_UPGRADE, nodesFailingOsUpgrade, metric.createContext(Map.of()));
+        platformChangeDurations.forEach((nodeVersion, duration) -> {
+            metric.set(PLATFORM_CHANGE_DURATION, duration.toSeconds(), metric.createContext(dimensions(nodeVersion)));
+        });
+        osChangeDurations.forEach((nodeVersion, duration) -> {
+            metric.set(OS_CHANGE_DURATION, duration.toSeconds(), metric.createContext(dimensions(nodeVersion)));
+        });
     }
 
-    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();
-        }, NODE_UPGRADE_TIMEOUT);
+    private Map<NodeVersion, Duration> platformChangeDurations() {
+        return changeDurations(controller().versionStatus().versions(), VespaVersion::nodeVersions);
     }
 
-    private int nodesFailingOsUpgrade() {
-        var allNodeVersions = controller().osVersionStatus().versions().values();
-        var totalTimeout = 0L;
-        for (var nodeVersions : allNodeVersions) {
-            for (var nodeVersion : nodeVersions.asMap().values()) {
-                if (!nodeVersion.changing()) continue;
-                totalTimeout += OS_UPGRADE_TIME_ALLOWANCE_PER_NODE.toMillis();
-            }
-        }
-        return nodesFailingUpgrade(allNodeVersions, Function.identity(), Duration.ofMillis(totalTimeout));
+    private Map<NodeVersion, Duration> osChangeDurations() {
+        return changeDurations(controller().osVersionStatus().versions().values(), Function.identity());
     }
 
-    private <V> int nodesFailingUpgrade(Collection<V> collection, Function<V, NodeVersions> nodeVersionsFunction, Duration timeout) {
-        var nodesFailingUpgrade = 0;
-        var acceptableInstant = clock.instant().minus(timeout);
-        for (var object : collection) {
-            for (var nodeVersion : nodeVersionsFunction.apply(object).asMap().values()) {
-                if (!nodeVersion.changing()) continue;
-                if (nodeVersion.changedAt().isBefore(acceptableInstant)) nodesFailingUpgrade++;
+    private <V> Map<NodeVersion, Duration> changeDurations(Collection<V> versions, Function<V, NodeVersions> versionsGetter) {
+        var now = clock.instant();
+        var durations = new HashMap<NodeVersion, Duration>();
+        for (var version : versions) {
+            for (var nodeVersion : versionsGetter.apply(version).asMap().values()) {
+                durations.put(nodeVersion, nodeVersion.changeDuration(now));
             }
         }
-        return nodesFailingUpgrade;
+        return durations;
     }
-    
+
     private static double deploymentFailRatio(DeploymentStatusList statuses) {
         return statuses.asList().stream()
                        .mapToInt(status -> status.hasFailures() ? 1 : 0)
@@ -198,6 +204,11 @@ public class MetricsReporter extends Maintainer {
                       "app",application.application().value() + "." + application.instance().value());
     }
 
+    private static Map<String, String> dimensions(NodeVersion nodeVersion) {
+        return Map.of("host", nodeVersion.hostname().value(),
+                      "zone", nodeVersion.zone().value());
+    }
+
 }
 
 
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
index 6e9def40e44..f9f8de96591 100644
--- 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
@@ -11,8 +11,6 @@ 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}.
  *
@@ -30,7 +28,7 @@ public class NodeVersionSerializer {
     private static final String hostnameField = "hostname";
     private static final String zoneField = "zone";
     private static final String wantedVersionField = "wantedVersion";
-    private static final String changedAtField = "changedAt";
+    private static final String suspendedAtField = "suspendedAt";
 
     public void nodeVersionsToSlime(NodeVersions nodeVersions, Cursor array) {
         for (var nodeVersion : nodeVersions.asMap().values()) {
@@ -38,7 +36,8 @@ public class NodeVersionSerializer {
             nodeVersionObject.setString(hostnameField, nodeVersion.hostname().value());
             nodeVersionObject.setString(zoneField, nodeVersion.zone().value());
             nodeVersionObject.setString(wantedVersionField, nodeVersion.wantedVersion().toFullString());
-            nodeVersionObject.setLong(changedAtField, nodeVersion.changedAt().toEpochMilli());
+            nodeVersion.suspendedAt().ifPresent(suspendedAt -> nodeVersionObject.setLong(suspendedAtField,
+                                                                                         suspendedAt.toEpochMilli()));
         }
     }
 
@@ -48,8 +47,8 @@ public class NodeVersionSerializer {
             var hostname = HostName.from(entry.field(hostnameField).asString());
             var zone = ZoneId.from(entry.field(zoneField).asString());
             var wantedVersion = Version.fromString(entry.field(wantedVersionField).asString());
-            var changedAt = Instant.ofEpochMilli(entry.field(changedAtField).asLong());
-            nodeVersions.put(hostname, new NodeVersion(hostname, zone, version, wantedVersion, changedAt));
+            var suspendedAt = Serializers.optionalInstant(entry.field(suspendedAtField));
+            nodeVersions.put(hostname, new NodeVersion(hostname, zone, version, wantedVersion, suspendedAt));
         });
         return new NodeVersions(nodeVersions.build());
     }
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 d82cddb4779..4a00add411f 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
@@ -5,8 +5,10 @@ import com.yahoo.component.Version;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.zone.ZoneId;
 
+import java.time.Duration;
 import java.time.Instant;
 import java.util.Objects;
+import java.util.Optional;
 
 /**
  * Version information for a node allocated to a {@link com.yahoo.vespa.hosted.controller.application.SystemApplication}.
@@ -21,14 +23,15 @@ public class NodeVersion {
     private final ZoneId zone;
     private final Version currentVersion;
     private final Version wantedVersion;
-    private final Instant changedAt;
+    private final Optional<Instant> suspendedAt;
 
-    public NodeVersion(HostName hostname, ZoneId zone, Version currentVersion, Version wantedVersion, Instant changedAt) {
+    public NodeVersion(HostName hostname, ZoneId zone, Version currentVersion, Version wantedVersion,
+                       Optional<Instant> suspendedAt) {
         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");
+        this.suspendedAt = Objects.requireNonNull(suspendedAt, "suspendedAt must be non-null");
     }
 
     /** Hostname of this */
@@ -51,31 +54,39 @@ public class NodeVersion {
         return wantedVersion;
     }
 
-    /** Returns whether this is changing (upgrading or downgrading) */
-    public boolean changing() {
-        return !currentVersion.equals(wantedVersion);
+    /** Returns the duration of the change in this, measured relative to instant */
+    public Duration changeDuration(Instant instant) {
+        if (!changing()) return Duration.ZERO;
+        if (suspendedAt.isEmpty()) return Duration.ZERO; // Node hasn't suspended to apply the change yet
+        return Duration.between(suspendedAt.get(), instant).abs();
     }
 
-    /** The most recent time the version of this changed */
-    public Instant changedAt() {
-        return changedAt;
+    /** The most recent time the node referenced by this suspended. This is empty if the node is not suspended. */
+    public Optional<Instant> suspendedAt() {
+        return suspendedAt;
     }
 
     /** Returns a copy of this with current version set to given version */
-    public NodeVersion withCurrentVersion(Version version, Instant changedAt) {
+    public NodeVersion withCurrentVersion(Version version) {
         if (currentVersion.equals(version)) return this;
-        return new NodeVersion(hostname, zone, version, wantedVersion, changedAt);
+        return new NodeVersion(hostname, zone, version, wantedVersion, suspendedAt);
     }
 
     /** 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, zone, currentVersion, version, changedAt);
+        return new NodeVersion(hostname, zone, currentVersion, version, suspendedAt);
+    }
+
+    /** Returns a copy of this with wanted version set to given version */
+    public NodeVersion withSuspendedAt(Optional<Instant> suspendedAt) {
+        if (suspendedAt.equals(this.suspendedAt)) return this;
+        return new NodeVersion(hostname, zone, currentVersion, wantedVersion, suspendedAt);
     }
 
     @Override
     public String toString() {
-        return hostname + ": " + currentVersion + " -> " + wantedVersion + " [zone=" + zone + ", changedAt=" + changedAt + "]";
+        return hostname + ": " + currentVersion + " -> " + wantedVersion + " [zone=" + zone + ", suspendedAt=" + suspendedAt.map(Instant::toString).orElse("<not suspended>") + "]";
     }
 
     @Override
@@ -87,12 +98,17 @@ public class NodeVersion {
                zone.equals(that.zone) &&
                currentVersion.equals(that.currentVersion) &&
                wantedVersion.equals(that.wantedVersion) &&
-               changedAt.equals(that.changedAt);
+               suspendedAt.equals(that.suspendedAt);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(hostname, zone, currentVersion, wantedVersion, changedAt);
+        return Objects.hash(hostname, zone, currentVersion, wantedVersion, suspendedAt);
+    }
+
+    /** Returns whether this is changing (upgrading or downgrading) */
+    private boolean changing() {
+        return !currentVersion.equals(wantedVersion);
     }
 
 }
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 3ab96e03bcd..4ce0a35e96f 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
@@ -7,6 +7,7 @@ import com.google.common.collect.ListMultimap;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.HostName;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -20,12 +21,10 @@ import java.util.function.Predicate;
  */
 public class NodeVersions {
 
-    public static final NodeVersions EMPTY = new NodeVersions(ImmutableMap.of());
-
     private final ImmutableMap<HostName, NodeVersion> nodeVersions;
 
-    public NodeVersions(ImmutableMap<HostName, NodeVersion> nodeVersions) {
-        this.nodeVersions = Objects.requireNonNull(nodeVersions);
+    public NodeVersions(Map<HostName, NodeVersion> nodeVersions) {
+        this.nodeVersions = ImmutableMap.copyOf(Objects.requireNonNull(nodeVersions));
     }
 
     public Map<HostName, NodeVersion> asMap() {
@@ -48,7 +47,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.currentVersion().equals(version));
+        return copyOf(nodeVersions.values(), nodeVersion -> nodeVersion.currentVersion().equals(version));
     }
 
     /** Returns number of node versions in this */
@@ -56,31 +55,6 @@ public class NodeVersions {
         return nodeVersions.size();
     }
 
-    /** 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) {
-        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
     public boolean equals(Object o) {
         if (this == o) return true;
@@ -94,4 +68,21 @@ public class NodeVersions {
         return Objects.hash(nodeVersions);
     }
 
+    public static NodeVersions copyOf(List<NodeVersion> nodeVersions) {
+        return copyOf(nodeVersions, (ignored) -> true);
+    }
+
+    public static NodeVersions copyOf(Map<HostName, NodeVersion> nodeVersions) {
+        return new NodeVersions(nodeVersions);
+    }
+
+    private static NodeVersions copyOf(Collection<NodeVersion> nodeVersions, Predicate<NodeVersion> predicate) {
+        var newNodeVersions = ImmutableMap.<HostName, NodeVersion>builder();
+        for (var nodeVersion : nodeVersions) {
+            if (!predicate.test(nodeVersion)) continue;
+            newNodeVersions.put(nodeVersion.hostname(), nodeVersion);
+        }
+        return new NodeVersions(newNodeVersions.build());
+    }
+
 }
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 1773a9c122e..1dffd1383bd 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
@@ -16,7 +16,6 @@ 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;
 
@@ -59,9 +58,7 @@ 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) {
-        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()) {
@@ -71,19 +68,16 @@ public class OsVersionStatus {
                                                 .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 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);
-                          });
+
+                for (var node : controller.serviceRegistry().configServer().nodeRepository().list(zone.getId(), application.id())) {
+                    if (!OsUpgrader.eligibleForUpgrade(node, application)) continue;
+                    var suspendedAt = node.suspendedSince();
+                    var nodeVersion = new NodeVersion(node.hostname(), zone.getId(), node.currentOsVersion(),
+                                                      targetOsVersion, suspendedAt);
+                    var osVersion = new OsVersion(nodeVersion.currentVersion(), zone.getCloudName());
+                    osVersions.putIfAbsent(osVersion, new ArrayList<>());
+                    osVersions.get(osVersion).add(nodeVersion);
+                }
             }
         }
 
@@ -98,15 +92,6 @@ public class OsVersionStatus {
         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) {
         return controller.zoneRegistry().osUpgradePolicies().stream()
                          .flatMap(upgradePolicy -> upgradePolicy.asList().stream())
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 0868d7ca695..7618e8210e3 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
@@ -8,23 +8,15 @@ import com.yahoo.component.Version;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.hosted.controller.Controller;
-import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.application.ApplicationList;
-import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
-import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus;
-import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatusList;
-import com.yahoo.vespa.hosted.controller.deployment.JobStatus;
-import com.yahoo.vespa.hosted.controller.deployment.RunStatus;
 import com.yahoo.vespa.hosted.controller.maintenance.SystemUpgrader;
 
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.logging.Level;
@@ -149,10 +141,7 @@ public class VersionStatus {
     }
 
     private static NodeVersions findSystemApplicationVersions(Controller controller) {
-        var nodeVersions = controller.versionStatus().systemVersion()
-                                     .map(VespaVersion::nodeVersions)
-                                     .orElse(NodeVersions.EMPTY);
-        var newNodeVersions = new ArrayList<NodeVersion>();
+        var nodeVersions = new LinkedHashMap<HostName, NodeVersion>();
         for (var zone : controller.zoneRegistry().zones().controllerUpgraded().zones()) {
             for (var application : SystemApplication.all()) {
                 var nodes = controller.serviceRegistry().configServer().nodeRepository()
@@ -165,15 +154,16 @@ public class VersionStatus {
                     log.log(LogLevel.WARNING, "Config for " + application.id() + " in " + zone.getId() +
                                               " has not converged");
                 }
-                var now = controller.clock().instant();
                 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(), zone.getId(), version, node.wantedVersion(), now));
+                    var version = configConverged ? node.currentVersion() : controller.systemVersion();
+                    var nodeVersion = new NodeVersion(node.hostname(), zone.getId(), version, node.wantedVersion(),
+                                                      node.suspendedSince());
+                    nodeVersions.put(nodeVersion.hostname(), nodeVersion);
                 }
             }
         }
-        return nodeVersions.with(newNodeVersions);
+        return NodeVersions.copyOf(nodeVersions);
     }
 
     private static ListMultimap<ControllerVersion, HostName> findControllerVersions(Controller controller) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
index 775eb2a4d75..189bd5156e3 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
@@ -223,7 +223,7 @@ public final class ControllerTester {
                 if (!application.hasApplicationPackage()) {
                     configServer().nodeRepository().upgrade(zone.getId(), application.nodeType(), version);
                 }
-                configServer().setVersion(application.id(), zone.getId(), version);
+                configServer().setVersion(version, application.id(), zone.getId());
                 configServer().convergeServices(application.id(), zone.getId());
             }
         }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index d86d2960c74..07c643070a0 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -201,9 +201,9 @@ public class InternalStepRunnerTest {
 
         // Node is down too long in system test, and no nodes go down in staging.
         tester.runner().run();
-        tester.configServer().setVersion(app.testerId().id(), JobType.systemTest.zone(system()), tester.controller().systemVersion());
+        tester.configServer().setVersion(tester.controller().systemVersion(), app.testerId().id(), JobType.systemTest.zone(system()));
         tester.configServer().convergeServices(app.testerId().id(), JobType.systemTest.zone(system()));
-        tester.configServer().setVersion(app.testerId().id(), JobType.stagingTest.zone(system()), tester.controller().systemVersion());
+        tester.configServer().setVersion(tester.controller().systemVersion(), app.testerId().id(), JobType.stagingTest.zone(system()));
         tester.configServer().convergeServices(app.testerId().id(), JobType.stagingTest.zone(system()));
         tester.runner().run();
         assertEquals(succeeded, tester.jobs().last(app.instanceId(), JobType.systemTest).get().stepStatuses().get(Step.installTester));
@@ -377,7 +377,7 @@ public class InternalStepRunnerTest {
         assertEquals(applicationPackage.hash(), tester.configServer().application(app.instanceId(), zone).get().applicationPackage().hash());
         assertEquals(otherPackage.hash(), tester.configServer().application(app.instanceId(), JobType.perfUsEast3.zone(system())).get().applicationPackage().hash());
 
-        tester.configServer().setVersion(app.instanceId(), zone, version);
+        tester.configServer().setVersion(version, app.instanceId(), zone);
         tester.runner().run();
         assertEquals(1, tester.jobs().active().size());
         assertEquals(version, tester.instance(app.instanceId()).deployments().get(zone).version());
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 d5a71d5adca..0fb2451ccba 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
@@ -178,28 +178,27 @@ 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, false);
+    public void setVersion(Version version, ApplicationId application, ZoneId zone) {
+        setVersion(zone, nodeRepository.list(zone, application), version, 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);
+    public void setVersion(Version version, List<Node> nodes, ZoneId zone) {
+        setVersion(zone, nodes, version, 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);
+    public void setOsVersion(Version version, ApplicationId application, ZoneId zone) {
+        setVersion(zone, nodeRepository.list(zone, application), version, true);
     }
 
     /** 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);
+    public void setOsVersion(Version version, List<Node> nodes, ZoneId zone) {
+        setVersion(zone, nodes, version, true);
     }
 
-    private void setVersion(ApplicationId application, ZoneId zone, Version version, int nodeCount, boolean osVersion) {
-        int n = 0;
-        for (Node node : nodeRepository().list(zone, application)) {
+    private void setVersion(ZoneId zone, List<Node> nodes, Version version, boolean osVersion) {
+        for (var node : nodes) {
             Node newNode;
             if (osVersion) {
                 newNode = new Node.Builder(node).currentOsVersion(version).wantedOsVersion(version).build();
@@ -207,7 +206,6 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
                 newNode = new Node.Builder(node).currentVersion(version).wantedVersion(version).build();
             }
             nodeRepository().putNodes(zone, newNode);
-            if (++n == nodeCount) break;
         }
     }
 
@@ -397,8 +395,8 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
     @Override
     public void deactivate(DeploymentId deployment) throws NotFoundException {
         ApplicationId applicationId = deployment.applicationId();
-        nodeRepository().removeByHostname(deployment.zoneId(),
-                                          nodeRepository().list(deployment.zoneId(), applicationId));
+        nodeRepository().removeNodes(deployment.zoneId(),
+                                     nodeRepository().list(deployment.zoneId(), applicationId));
         if ( ! applications.containsKey(deployment))
             throw new NotFoundException("No application with id " + applicationId + " exists, cannot deactivate");
         applications.remove(deployment);
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 c00705149e9..e2482fc9174 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
@@ -5,6 +5,7 @@ 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.HostName;
 import com.yahoo.config.provision.zone.UpgradePolicy;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.Controller;
@@ -22,12 +23,15 @@ import org.junit.Test;
 import java.time.Duration;
 import java.util.Comparator;
 import java.util.List;
+import java.util.function.UnaryOperator;
+import java.util.stream.Collectors;
 
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.productionUsWest1;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 /**
  * @author mortent
@@ -226,11 +230,11 @@ public class MetricsReporterTest {
     public void nodes_failing_system_upgrade() {
         var tester = new ControllerTester();
         var reporter = createReporter(tester.controller());
-        var zone1 = ZoneApiMock.fromId("prod.eu-west-1");
-        tester.zoneRegistry().setUpgradePolicy(UpgradePolicy.create().upgrade(zone1));
+        var zone = ZoneId.from("prod.eu-west-1");
+        tester.zoneRegistry().setUpgradePolicy(UpgradePolicy.create().upgrade(ZoneApiMock.from(zone)));
         var systemUpgrader = new SystemUpgrader(tester.controller(), Duration.ofDays(1),
                                                 new JobControl(tester.curator()));
-        tester.configServer().bootstrap(List.of(zone1.getId()), SystemApplication.configServer);
+        tester.configServer().bootstrap(List.of(zone), SystemApplication.configServer);
 
         // System on initial version
         var version0 = Version.fromString("7.0");
@@ -247,25 +251,37 @@ public class MetricsReporterTest {
 
             // 30 minutes pass and nothing happens
             tester.clock().advance(Duration.ofMinutes(30));
-            tester.computeVersionStatus();
-            reporter.maintain();
+            runAll(tester::computeVersionStatus, reporter);
             assertEquals(0, getNodesFailingUpgrade());
 
             // 1/3 nodes upgrade within timeout
-            assertEquals("Wanted version is raised for all nodes", version,
-                         tester.configServer().nodeRepository().list(zone1.getId(), SystemApplication.configServer.id()).stream()
-                               .map(Node::wantedVersion).min(Comparator.naturalOrder()).get());
-            tester.configServer().setVersion(SystemApplication.configServer.id(), zone1.getId(), version, 1);
-            tester.clock().advance(Duration.ofMinutes(60).plus(Duration.ofSeconds(1)));
-            tester.computeVersionStatus();
-            reporter.maintain();
+            var hosts = tester.configServer().nodeRepository().list(zone, SystemApplication.configServer.id());
+            assertEquals("Wanted version is raised for all nodes", version, hosts.stream()
+                                                                                 .map(Node::wantedVersion)
+                                                                                 .min(Comparator.naturalOrder())
+                                                                                 .get());
+            suspend(hosts, zone, tester);
+            var firstHost = hosts.get(0);
+            upgradeTo(version, List.of(firstHost), zone, tester);
+
+            // 2/3 spend their budget and are reported as failures
+            tester.clock().advance(Duration.ofHours(1));
+            runAll(tester::computeVersionStatus, reporter);
             assertEquals(2, getNodesFailingUpgrade());
-
-            // 3/3 nodes upgrade
-            tester.configServer().setVersion(SystemApplication.configServer.id(), zone1.getId(), version);
-            tester.computeVersionStatus();
-            reporter.maintain();
+            assertEquals("Change duration is reset for completed upgrade", Duration.ZERO,
+                         getChangeDuration(MetricsReporter.PLATFORM_CHANGE_DURATION, firstHost.hostname()));
+            for (var host : hosts.subList(1, hosts.size())) {
+                assertEquals(Duration.ofHours(1), getChangeDuration(MetricsReporter.PLATFORM_CHANGE_DURATION,
+                                                                    host.hostname()));
+            }
+
+            // Remaining nodes eventually upgrade
+            upgradeTo(version, hosts.subList(1, 3), zone, tester);
+            runAll(tester::computeVersionStatus, reporter);
             assertEquals(0, getNodesFailingUpgrade());
+            for (var host : hosts) {
+                assertEquals(Duration.ZERO, getChangeDuration(MetricsReporter.PLATFORM_CHANGE_DURATION, host.hostname()));
+            }
             assertEquals(version, tester.controller().systemVersion());
         }
     }
@@ -274,64 +290,131 @@ public class MetricsReporterTest {
     public void nodes_failing_os_upgrade() {
         var tester = new ControllerTester();
         var reporter = createReporter(tester.controller());
-        var zone = ZoneApiMock.fromId("prod.eu-west-1");
+        var zone = ZoneId.from("prod.eu-west-1");
         var cloud = CloudName.defaultName();
-        tester.zoneRegistry().setOsUpgradePolicy(cloud, UpgradePolicy.create().upgrade(zone));
+        tester.zoneRegistry().setOsUpgradePolicy(cloud, UpgradePolicy.create().upgrade(ZoneApiMock.from(zone)));
         var osUpgrader = new OsUpgrader(tester.controller(), Duration.ofDays(1),
                                         new JobControl(tester.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.configServerHost, SystemApplication.tenantHost);
+        tester.configServer().bootstrap(List.of(zone), SystemApplication.configServerHost, 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);
-        tester.configServer().setOsVersion(SystemApplication.configServerHost.id(), zone.getId(), version0);
-        statusUpdater.maintain();
-        reporter.maintain();
+        tester.configServer().setOsVersion(version0, SystemApplication.tenantHost.id(), zone);
+        tester.configServer().setOsVersion(version0, SystemApplication.configServerHost.id(), zone);
+        runAll(statusUpdater, reporter);
         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();
+            runAll(osUpgrader, statusUpdater, reporter);
             assertEquals(0, getNodesFailingOsUpgrade());
 
-            // 30 minutes pass and nothing happens
-            tester.clock().advance(Duration.ofMinutes(30));
-            statusUpdater.maintain();
-            reporter.maintain();
+            // Over 30 minutes pass and nothing happens
+            tester.clock().advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1)));
+            runAll(statusUpdater, reporter);
             assertEquals(0, getNodesFailingOsUpgrade());
 
-            // 2/6 nodes upgrade within timeout
+            // Nodes are told to upgrade, but do not suspend yet
             assertEquals("Wanted OS version is raised for all nodes", version,
-                         tester.configServer().nodeRepository().list(zone.getId(), SystemApplication.tenantHost.id()).stream()
+                         tester.configServer().nodeRepository().list(zone, SystemApplication.tenantHost.id()).stream()
                                .map(Node::wantedOsVersion).min(Comparator.naturalOrder()).get());
-            tester.configServer().setOsVersion(SystemApplication.tenantHost.id(), zone.getId(), version, 2);
-            tester.clock().advance(Duration.ofMinutes(30 * 3 /* time allowance * node count */).plus(Duration.ofSeconds(1)));
-            statusUpdater.maintain();
-            reporter.maintain();
-            assertEquals(4, getNodesFailingOsUpgrade());
+            assertTrue("No nodes are suspended", tester.controller().serviceRegistry().configServer()
+                                                       .nodeRepository().list(zone).stream()
+                                                       .noneMatch(node -> node.serviceState() == Node.ServiceState.allowedDown));
 
-            // 5/6 nodes upgrade
-            tester.configServer().setOsVersion(SystemApplication.tenantHost.id(), zone.getId(), version);
-            tester.configServer().setOsVersion(SystemApplication.configServerHost.id(), zone.getId(), version, 2);
-            statusUpdater.maintain();
-            reporter.maintain();
+            // Another 30 minutes pass
+            tester.clock().advance(Duration.ofMinutes(30));
+            runAll(statusUpdater, reporter);
+            assertEquals(0, getNodesFailingOsUpgrade());
+
+            // 3/6 hosts suspend
+            var hosts = tester.configServer().nodeRepository().list(zone);
+            var suspendedHosts = hosts.subList(0, 3);
+            suspend(suspendedHosts, zone, tester);
+            runAll(statusUpdater, reporter);
+            assertEquals(0, getNodesFailingOsUpgrade());
+
+            // Two hosts upgrade after 20 minutes
+            var hostsUpgraded = suspendedHosts.subList(0, 2);
+            tester.clock().advance(Duration.ofMinutes(20));
+            runAll(statusUpdater, reporter);
+            for (var host : suspendedHosts) {
+                assertEquals(Duration.ofMinutes(20), getChangeDuration(MetricsReporter.OS_CHANGE_DURATION, host.hostname()));
+            }
+            upgradeOsTo(version, hostsUpgraded, zone, tester);
+            runAll(statusUpdater, reporter);
+            assertEquals(0, getNodesFailingOsUpgrade());
+            for (var host : hostsUpgraded) {
+                assertEquals(Duration.ZERO, getChangeDuration(MetricsReporter.OS_CHANGE_DURATION, host.hostname()));
+            }
+
+            // One host consumes budget without upgrading
+            var brokenHost = suspendedHosts.get(2);
+            tester.clock().advance(Duration.ofMinutes(15));
+            runAll(statusUpdater, reporter);
             assertEquals(1, getNodesFailingOsUpgrade());
+            assertEquals(Duration.ofMinutes(35), getChangeDuration(MetricsReporter.OS_CHANGE_DURATION,
+                                                                   brokenHost.hostname()));
 
-            // Final node upgrades
-            tester.configServer().setOsVersion(SystemApplication.configServerHost.id(), zone.getId(), version);
-            statusUpdater.maintain();
-            reporter.maintain();
+            // Host eventually upgrades and is no longer reported
+            upgradeOsTo(version, List.of(brokenHost), zone, tester);
+            runAll(statusUpdater, reporter);
+            assertEquals(0, getNodesFailingOsUpgrade());
+            assertEquals(Duration.ZERO, getChangeDuration(MetricsReporter.OS_CHANGE_DURATION, brokenHost.hostname()));
+
+            // Remaining hosts suspend and upgrade successfully
+            var remainingHosts = hosts.subList(3, hosts.size());
+            suspend(remainingHosts, zone, tester);
+            upgradeOsTo(version, remainingHosts, zone, tester);
+            runAll(statusUpdater, reporter);
             assertEquals(0, getNodesFailingOsUpgrade());
+            for (var host : hosts) {
+                assertEquals(Duration.ZERO, getChangeDuration(MetricsReporter.OS_CHANGE_DURATION, host.hostname()));
+            }
         }
     }
 
+    private void runAll(Runnable... runnables) {
+        for (var r : runnables) r.run();
+    }
+
+    private void upgradeTo(Version version, List<Node> nodes, ZoneId zone, ControllerTester tester) {
+        tester.configServer().setVersion(version, nodes, zone);
+        resume(nodes, zone, tester);
+    }
+
+    private void upgradeOsTo(Version version, List<Node> nodes, ZoneId zone, ControllerTester tester) {
+        tester.configServer().setOsVersion(version, nodes, zone);
+        resume(nodes, zone, tester);
+    }
+
+    private void resume(List<Node> nodes, ZoneId zone, ControllerTester tester) {
+        updateNodes(nodes, (builder) -> builder.serviceState(Node.ServiceState.expectedUp).suspendedSince(null),
+                    zone, tester);
+    }
+
+    private void suspend(List<Node> nodes, ZoneId zone, ControllerTester tester) {
+        var now = tester.clock().instant();
+        updateNodes(nodes, (builder) -> builder.serviceState(Node.ServiceState.allowedDown).suspendedSince(now),
+                    zone, tester);
+    }
+
+    private void updateNodes(List<Node> nodes, UnaryOperator<Node.Builder> builderOps, ZoneId zone,
+                             ControllerTester tester) {
+        var currentNodes = tester.configServer().nodeRepository().list(zone, nodes.stream()
+                                                                                  .map(Node::hostname)
+                                                                                  .collect(Collectors.toList()));
+        var updatedNodes = currentNodes.stream()
+                                       .map(node -> builderOps.apply(new Node.Builder(node)).build())
+                                       .collect(Collectors.toList());
+        tester.configServer().nodeRepository().putNodes(zone, updatedNodes);
+    }
+
     private Duration getAverageDeploymentDuration(ApplicationId id) {
         return Duration.ofSeconds(getMetric(MetricsReporter.DEPLOYMENT_AVERAGE_DURATION, id).longValue());
     }
@@ -345,7 +428,13 @@ public class MetricsReporterTest {
     }
 
     private int getNodesFailingUpgrade() {
-        return metrics.getMetric(MetricsReporter.NODES_FAILING_SYSTEM_UPGRADE).intValue();
+        return metrics.getMetric(MetricsReporter.NODES_FAILING_PLATFORM_UPGRADE).intValue();
+    }
+
+    private Duration getChangeDuration(String metric, HostName hostname) {
+        return metrics.getMetric((dimensions) -> hostname.value().equals(dimensions.get("host")), metric)
+                      .map(n -> Duration.ofSeconds(n.longValue()))
+                      .orElseThrow(() -> new IllegalArgumentException("Expected to find metric for " + hostname));
     }
 
     private int getNodesFailingOsUpgrade() {
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 e585547e921..efdf412ab77 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
@@ -14,6 +14,7 @@ import org.junit.Test;
 
 import java.time.Instant;
 import java.util.List;
+import java.util.Optional;
 
 import static org.junit.Assert.assertEquals;
 
@@ -23,18 +24,18 @@ import static org.junit.Assert.assertEquals;
 public class OsVersionStatusSerializerTest {
 
     @Test
-    public void test_serialization() {
+    public void serialization() {
         Version version1 = Version.fromString("7.1");
         Version version2 = Version.fromString("7.2");
         var versions = ImmutableMap.<OsVersion, NodeVersions>builder();
 
-        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(version1, CloudName.defaultName()), NodeVersions.copyOf(List.of(
+                new NodeVersion(HostName.from("node1"), ZoneId.from("prod", "us-west"), version1, version2, Optional.of(Instant.ofEpochMilli(11))),
+                new NodeVersion(HostName.from("node2"), ZoneId.from("prod", "us-east"), version1, version2, Optional.of(Instant.ofEpochMilli(22)))
         )));
-        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))
+        versions.put(new OsVersion(version2, CloudName.defaultName()), NodeVersions.copyOf(List.of(
+                new NodeVersion(HostName.from("node3"), ZoneId.from("prod", "us-west"), version2, version2, Optional.of(Instant.ofEpochMilli(33))),
+                new NodeVersion(HostName.from("node4"), ZoneId.from("prod", "us-east"), version2, version2, Optional.of(Instant.ofEpochMilli(44)))
         )));
 
         OsVersionStatusSerializer serializer = new OsVersionStatusSerializer(new OsVersionSerializer(), new NodeVersionSerializer());
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 0c1f94d90cf..41d59239321 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
@@ -2,10 +2,8 @@
 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.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;
@@ -14,8 +12,8 @@ import org.junit.Test;
 
 import java.time.Instant;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 
 import static java.time.temporal.ChronoUnit.MILLIS;
 import static org.junit.Assert.assertEquals;
@@ -31,10 +29,10 @@ public class VersionStatusSerializerTest {
         Version version = Version.fromString("5.0");
         vespaVersions.add(new VespaVersion(version, "dead", Instant.now(), false, false,
                                            true, nodeVersions(Version.fromString("5.0"), Version.fromString("5.1"),
-                                                              Instant.ofEpochMilli(123), "cfg1", "cfg2", "cfg3"), VespaVersion.Confidence.normal));
+                                                              "cfg1", "cfg2", "cfg3"), VespaVersion.Confidence.normal));
         vespaVersions.add(new VespaVersion(version, "cafe", Instant.now(), true, true,
                                            false, nodeVersions(Version.fromString("5.0"), Version.fromString("5.1"),
-                                                               Instant.ofEpochMilli(456), "cfg1", "cfg2", "cfg3"), VespaVersion.Confidence.normal));
+                                                               "cfg1", "cfg2", "cfg3"), VespaVersion.Confidence.normal));
         VersionStatus status = new VersionStatus(vespaVersions);
         VersionStatusSerializer serializer = new VersionStatusSerializer(new NodeVersionSerializer());
         VersionStatus deserialized = serializer.fromSlime(serializer.toSlime(status));
@@ -55,12 +53,12 @@ public class VersionStatusSerializerTest {
 
     }
 
-    private static NodeVersions nodeVersions(Version version, Version wantedVersion, Instant changedAt, String... hostnames) {
+    private static NodeVersions nodeVersions(Version version, Version wantedVersion, String... hostnames) {
         var nodeVersions = new ArrayList<NodeVersion>();
         for (var hostname : hostnames) {
-            nodeVersions.add(new NodeVersion(HostName.from(hostname), ZoneId.from("prod", "us-north-1"), version, wantedVersion, changedAt));
+            nodeVersions.add(new NodeVersion(HostName.from(hostname), ZoneId.from("prod", "us-north-1"), version, wantedVersion, Optional.empty()));
         }
-        return NodeVersions.EMPTY.with(nodeVersions);
+        return NodeVersions.copyOf(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 c8036676fa9..32a77137182 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
@@ -19,9 +19,9 @@ import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 import org.junit.Test;
 
 import java.io.File;
-import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 
 /**
  * @author bratseth
@@ -87,8 +87,8 @@ public class DeploymentApiTest extends ControllerContainerTest {
                                            version.isControllerVersion(),
                                            version.isSystemVersion(),
                                            version.isReleased(),
-                                           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))),
+                                           NodeVersions.copyOf(List.of(new NodeVersion(HostName.from("config1.test"), ZoneId.defaultId(), version.versionNumber(), version.versionNumber(), Optional.empty()),
+                                                                       new NodeVersion(HostName.from("config2.test"), ZoneId.defaultId(), version.versionNumber(), version.versionNumber(), Optional.empty()))),
                                            version.confidence()
                 );
             }