Merge pull request #23025 from vespa-engine/mpolden/versions-record

Use records in versions package

9 files changed, 60 insertions, 231 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java
index 5ff15307ac3..790f54b5e8c 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java
@@ -213,7 +213,7 @@ public class RoutingController {
                                                                  .map(com.yahoo.config.application.api.Endpoint.Target::region)
                                                                  .distinct()
                                                                  .map(region -> new DeploymentId(deployment.applicationId(), ZoneId.from(Environment.prod, region)))
-                                                                 .collect(Collectors.toUnmodifiableList());
+                                                                 .toList();
             TenantAndApplicationId application = TenantAndApplicationId.from(deployment.applicationId());
             for (var targetDeployment : deploymentTargets) {
                 builders.add(Endpoint.of(application).targetApplication(EndpointId.defaultId(), targetDeployment));
@@ -420,13 +420,12 @@ public class RoutingController {
     }
 
     private static String asString(Endpoint.Scope scope) {
-        switch (scope) {
-            case application: return "application";
-            case global: return "global";
-            case weighted: return "weighted";
-            case zone: return "zone";
-        }
-        throw new IllegalArgumentException("Unknown scope " + scope);
+        return switch (scope) {
+            case application -> "application";
+            case global -> "global";
+            case weighted -> "weighted";
+            case zone -> "zone";
+        };
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/DeploymentStatistics.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/DeploymentStatistics.java
index 179a64d9491..e3077cb232f 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/DeploymentStatistics.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/DeploymentStatistics.java
@@ -18,25 +18,29 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static java.util.Comparator.naturalOrder;
 import static java.util.function.Function.identity;
 
 /**
- * Statistics about deployments on a platform version. This is immutable.
+ * Statistics about deployments on a platform version.
+ *
+ * @param version             the version these statistics are for
+ * @param failingUpgrades     the runs on the version of this, for currently failing instances, where the failure may be because of the upgrade
+ * @param otherFailing        all other failing runs on the version of this, for currently failing instances
+ * @param productionSuccesses the production runs where the last success was on the version of this
+ * @param runningUpgrade      the currently running runs on the version of this, where an upgrade is attempted
+ * @param otherRunning        all other currently running runs on the version on this
  *
  * @author jonmv
  */
-public class DeploymentStatistics {
-    
-    private final Version version;
-    private final List<Run> failingUpgrades;
-    private final List<Run> otherFailing;
-    private final List<Run> productionSuccesses;
-    private final List<Run> runningUpgrade;
-    private final List<Run> otherRunning;
+public record DeploymentStatistics(Version version,
+                                   List<Run> failingUpgrades,
+                                   List<Run> otherFailing,
+                                   List<Run> productionSuccesses,
+                                   List<Run> runningUpgrade,
+                                   List<Run> otherRunning) {
 
     public DeploymentStatistics(Version version, List<Run> failingUpgrades, List<Run> otherFailing,
                                 List<Run> productionSuccesses, List<Run> runningUpgrade, List<Run> otherRunning) {
@@ -48,26 +52,7 @@ public class DeploymentStatistics {
         this.otherRunning = List.copyOf(otherRunning);
     }
 
-    /** Returns the version these statistics are for. */
-    public Version version() { return version; }
-
-    /** Returns the runs on the version of this, for currently failing instances, where the failure may be because of the upgrade. */
-    public List<Run> failingUpgrades() { return failingUpgrades; }
-
-    /** Returns all other failing runs on the version of this, for currently failing instances. */
-    public List<Run> otherFailing() { return otherFailing; }
-
-    /** Returns the production runs where the last success was on the version of this. */
-    public List<Run> productionSuccesses() { return productionSuccesses; }
-
-    /** Returns the currently running runs on the version of this, where an upgrade is attempted. */
-    public List<Run> runningUpgrade() { return runningUpgrade; }
-
-    /** Returns all other currently running runs on the version on this. */
-    public List<Run> otherRunning() { return otherRunning; }
-
     public static List<DeploymentStatistics> compute(Collection<Version> infrastructureVersions, DeploymentStatusList statuses) {
-
         Set<Version> allVersions = new HashSet<>(infrastructureVersions);
         Map<Version, List<Run>> failingUpgrade = new HashMap<>();
         Map<Version, List<Run>> otherFailing = new HashMap<>();
@@ -154,8 +139,7 @@ public class DeploymentStatistics {
                                                               productionSuccesses.getOrDefault(version, List.of()),
                                                               runningUpgrade.getOrDefault(version, List.of()),
                                                               otherRunning.getOrDefault(version, List.of())))
-                     .collect(Collectors.toUnmodifiableList());
-
+                     .toList();
     }
 
 }
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 69773597f37..04f5d9866f8 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
@@ -13,45 +13,20 @@ import java.util.Optional;
 /**
  * Version information for a node allocated to a {@link com.yahoo.vespa.hosted.controller.application.SystemApplication}.
  *
- * This is immutable.
- *
  * @author mpolden
  */
-public class NodeVersion {
-
-    private final HostName hostname;
-    private final ZoneId zone;
-    private final Version currentVersion;
-    private final Version wantedVersion;
-    private final Optional<Instant> suspendedAt;
-
-    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.suspendedAt = Objects.requireNonNull(suspendedAt, "suspendedAt must be non-null");
-    }
-
-    /** Hostname of this */
-    public HostName hostname() {
-        return hostname;
-    }
-
-    /** Zone of this */
-    public ZoneId zone() {
-        return zone;
-    }
-
-    /** Current version of this */
-    public Version currentVersion() {
-        return currentVersion;
-    }
+public record NodeVersion(HostName hostname,
+                          ZoneId zone,
+                          Version currentVersion,
+                          Version wantedVersion,
+                          Optional<Instant> suspendedAt) {
 
-    /** Wanted version of this */
-    public Version wantedVersion() {
-        return wantedVersion;
+    public NodeVersion {
+        Objects.requireNonNull(hostname, "hostname must be non-null");
+        Objects.requireNonNull(zone, "zone must be non-null");
+        Objects.requireNonNull(currentVersion, "version must be non-null");
+        Objects.requireNonNull(wantedVersion, "wantedVersion must be non-null");
+        Objects.requireNonNull(suspendedAt, "suspendedAt must be non-null");
     }
 
     /** Returns the duration of the change in this, measured relative to instant */
@@ -61,33 +36,11 @@ public class NodeVersion {
         return Duration.between(suspendedAt.get(), instant).abs();
     }
 
-    /** The most recent time the node referenced in this suspended. This is empty if the node is not suspended. */
-    public Optional<Instant> suspendedAt() {
-        return suspendedAt;
-    }
-
     @Override
     public String toString() {
         return hostname + ": " + currentVersion + " -> " + wantedVersion + " [zone=" + zone + ", suspendedAt=" + suspendedAt.map(Instant::toString).orElse("<not suspended>") + "]";
     }
 
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        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) &&
-               suspendedAt.equals(that.suspendedAt);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(hostname, zone, currentVersion, wantedVersion, suspendedAt);
-    }
-
     /** Returns whether this is upgrading */
     private boolean upgrading() {
         return currentVersion.isBefore(wantedVersion);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersion.java
index c240a7ddf35..30a88733ed3 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersion.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersion.java
@@ -12,41 +12,14 @@ import java.util.Objects;
  *
  * @author mpolden
  */
-public class OsVersion implements Comparable<OsVersion> {
+public record OsVersion(Version version, CloudName cloud) implements Comparable<OsVersion> {
 
     private static final Comparator<OsVersion> comparator = Comparator.comparing(OsVersion::cloud)
                                                                       .thenComparing(OsVersion::version);
 
-    private final Version version;
-    private final CloudName cloud;
-
-    public OsVersion(Version version, CloudName cloud) {
-        this.version = Objects.requireNonNull(version, "version must be non-null");
-        this.cloud = Objects.requireNonNull(cloud, "cloud must be non-null");
-    }
-
-    /** The version number of this */
-    public Version version() {
-        return version;
-    }
-
-    /** The cloud where this OS version is used */
-    public CloudName cloud() {
-        return cloud;
-    }
-
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-        OsVersion osVersion = (OsVersion) o;
-        return Objects.equals(version, osVersion.version) &&
-               Objects.equals(cloud, osVersion.cloud);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(version, cloud);
+    public OsVersion {
+        Objects.requireNonNull(version, "version must be non-null");
+        Objects.requireNonNull(cloud, "cloud must be non-null");
     }
 
     @Override
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 fc7fbe45767..8ee891ae8a6 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
@@ -26,22 +26,15 @@ import java.util.stream.Collectors;
  *
  * @author mpolden
  */
-public class OsVersionStatus {
+public record OsVersionStatus(Map<OsVersion, List<NodeVersion>> versions) {
 
     public static final OsVersionStatus empty = new OsVersionStatus(ImmutableMap.of());
 
-    private final Map<OsVersion, List<NodeVersion>> versions;
-
     /** Public for serialization purpose only. Use {@link OsVersionStatus#compute(Controller)} for an up-to-date status */
     public OsVersionStatus(Map<OsVersion, List<NodeVersion>> versions) {
         this.versions = ImmutableMap.copyOf(Objects.requireNonNull(versions, "versions must be non-null"));
     }
 
-    /** All known OS versions and their nodes */
-    public Map<OsVersion, List<NodeVersion>> versions() {
-        return versions;
-    }
-
     /** Returns nodes eligible for OS upgrades that exist in given cloud */
     public List<NodeVersion> nodesIn(CloudName cloud) {
         return versions.entrySet().stream()
@@ -91,7 +84,7 @@ public class OsVersionStatus {
         return controller.zoneRegistry().osUpgradePolicies().stream()
                          .flatMap(upgradePolicy -> upgradePolicy.steps().stream())
                          .flatMap(Collection::stream)
-                         .collect(Collectors.toUnmodifiableList());
+                         .toList();
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionTarget.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionTarget.java
index 1c27058a6ef..0a13244ce5e 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionTarget.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionTarget.java
@@ -12,54 +12,17 @@ import java.util.Objects;
  *
  * @author mpolden
  */
-public class OsVersionTarget implements VersionTarget, Comparable<OsVersionTarget> {
-
-    // 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 final OsVersion osVersion;
-    private final Duration upgradeBudget;
-    private final Instant scheduledAt;
-
-    public OsVersionTarget(OsVersion osVersion, Duration upgradeBudget, Instant scheduledAt) {
-        this.osVersion = Objects.requireNonNull(osVersion);
-        this.upgradeBudget = Objects.requireNonNull(upgradeBudget);
-        this.scheduledAt = Objects.requireNonNull(scheduledAt);
+public record OsVersionTarget(OsVersion osVersion,
+                              Duration upgradeBudget,
+                              Instant scheduledAt) implements VersionTarget, Comparable<OsVersionTarget> {
+
+    public OsVersionTarget {
+        Objects.requireNonNull(osVersion);
+        Objects.requireNonNull(upgradeBudget);
+        Objects.requireNonNull(scheduledAt);
         if (upgradeBudget.isNegative()) throw new IllegalArgumentException("upgradeBudget cannot be negative");
     }
 
-    /** The OS version contained in this target */
-    public OsVersion osVersion() {
-        return osVersion;
-    }
-
-    /** The total time budget across all zones for applying target, if any */
-    public Duration upgradeBudget() {
-        return upgradeBudget;
-    }
-
-    /** Returns when this target was scheduled */
-    public Instant scheduledAt() {
-        return scheduledAt;
-    }
-
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-        OsVersionTarget that = (OsVersionTarget) o;
-        return osVersion.equals(that.osVersion) && upgradeBudget.equals(that.upgradeBudget) && scheduledAt.equals(that.scheduledAt);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(osVersion, upgradeBudget, scheduledAt);
-    }
-
     @Override
     public int compareTo(OsVersionTarget o) {
         return osVersion.compareTo(o.osVersion);
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 117abd52193..e937e8af60d 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
@@ -27,16 +27,12 @@ import java.util.stream.Collectors;
  * The versions in use are the set of all versions running in current applications, versions
  * of config servers in all zones, and the version of this controller itself.
  * 
- * This is immutable.
- * 
  * @author bratseth
  * @author mpolden
  */
-public class VersionStatus {
+public record VersionStatus(List<VespaVersion> versions) {
 
     private static final Logger log = Logger.getLogger(VersionStatus.class.getName());
-
-    private final List<VespaVersion> versions;
     
     /** Create a version status. DO NOT USE: Public for testing and serialization only */
     public VersionStatus(List<VespaVersion> versions) {
@@ -172,7 +168,7 @@ public class VersionStatus {
                 var nodes = controller.serviceRegistry().configServer().nodeRepository()
                                       .list(zone.getId(), NodeFilter.all().applications(application.id())).stream()
                                       .filter(SystemUpgrader::eligibleForUpgrade)
-                                      .collect(Collectors.toList());
+                                      .toList();
                 if (nodes.isEmpty()) continue;
                 boolean configConverged = application.configConvergedIn(zone.getId(), controller, Optional.empty());
                 if (!configConverged) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java
index 162890ff74d..7f33f612cd0 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java
@@ -16,35 +16,17 @@ import static com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy;
 /**
  * Information about a particular Vespa version.
  * VespaVersions are identified by their version number and ordered by increasing version numbers.
- *
- * This is immutable.
  * 
  * @author bratseth
  */
-public class VespaVersion implements Comparable<VespaVersion> {
-
-    private final Version version;
-    private final String releaseCommit;
-    private final Instant committedAt;
-    private final boolean isControllerVersion;
-    private final boolean isSystemVersion;
-    private final boolean isReleased;
-    private final List<NodeVersion> nodeVersions;
-    private final Confidence confidence;
-
-    public VespaVersion(Version version, String releaseCommit, Instant committedAt,
-                        boolean isControllerVersion, boolean isSystemVersion, boolean isReleased,
-                        List<NodeVersion> nodeVersions,
-                        Confidence confidence) {
-        this.version = version;
-        this.releaseCommit = releaseCommit;
-        this.committedAt = committedAt;
-        this.isControllerVersion = isControllerVersion;
-        this.isSystemVersion = isSystemVersion;
-        this.isReleased = isReleased;
-        this.nodeVersions = nodeVersions;
-        this.confidence = confidence;
-    }
+public record VespaVersion(Version version,
+                           String releaseCommit,
+                           Instant committedAt,
+                           boolean isControllerVersion,
+                           boolean isSystemVersion,
+                           boolean isReleased,
+                           List<NodeVersion> nodeVersions,
+                           Confidence confidence) implements Comparable<VespaVersion> {
 
     public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller) {
         InstanceList all = InstanceList.from(controller.jobController().deploymentStatuses(ApplicationList.from(controller.applications().asList())
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersionTarget.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersionTarget.java
index 78d6d0ebf29..26890cfd8f8 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersionTarget.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersionTarget.java
@@ -10,24 +10,10 @@ import java.util.Objects;
  *
  * @author mpolden
  */
-public class VespaVersionTarget implements VersionTarget {
+public record VespaVersionTarget(Version version, boolean downgrade) implements VersionTarget {
 
-    private final Version version;
-    private final boolean downgrade;
-
-    public VespaVersionTarget(Version version, boolean downgrade) {
-        this.version = Objects.requireNonNull(version);
-        this.downgrade = downgrade;
-    }
-
-    @Override
-    public Version version() {
-        return version;
-    }
-
-    @Override
-    public boolean downgrade() {
-        return downgrade;
+    public VespaVersionTarget {
+        Objects.requireNonNull(version);
     }
 
     @Override