Merge pull request #11137 from vespa-engine/mpolden/use-commit-details-from-latest

Store commit details of latest version

5 files changed, 38 insertions, 32 deletions

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 ab445de5a7f..97506d3f6ea 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
@@ -99,11 +99,6 @@ public class VersionStatus {
         }
         infrastructureVersions.putAll(systemApplicationVersions.asVersionMap());
 
-        // The controller version is the lowest controller version of all controllers
-        ControllerVersion controllerVersion = controllerVersions.keySet().stream()
-                                                                .min(Comparator.naturalOrder())
-                                                                .get();
-
         // The system version is the oldest infrastructure version, if that version is newer than the current system
         // version
         Version newSystemVersion = infrastructureVersions.keySet().stream().min(Comparator.naturalOrder()).get();
@@ -134,7 +129,7 @@ public class VersionStatus {
             try {
                 boolean isReleased = Collections.binarySearch(releasedVersions, statistics.version()) >= 0;
                 VespaVersion vespaVersion = createVersion(statistics,
-                                                          controllerVersion,
+                                                          controllerVersions.keySet(),
                                                           systemVersion,
                                                           isReleased,
                                                           systemApplicationVersions.matching(statistics.version()),
@@ -240,11 +235,13 @@ public class VersionStatus {
     }
 
     private static VespaVersion createVersion(DeploymentStatistics statistics,
-                                              ControllerVersion controllerVersion,
+                                              Set<ControllerVersion> controllerVersions,
                                               Version systemVersion,
                                               boolean isReleased,
                                               NodeVersions nodeVersions,
                                               Controller controller) {
+        var latestVersion = controllerVersions.stream().max(Comparator.naturalOrder()).get();
+        var controllerVersion = controllerVersions.stream().min(Comparator.naturalOrder()).get();
         var isSystemVersion = statistics.version().equals(systemVersion);
         var isControllerVersion = statistics.version().equals(controllerVersion.version());
         var confidence = controller.curator().readConfidenceOverrides().get(statistics.version());
@@ -263,8 +260,8 @@ public class VersionStatus {
         }
 
         // Preserve existing commit details if we've previously computed status for this version
-        var commitSha = controllerVersion.commitSha();
-        var commitDate = controllerVersion.commitDate();
+        var commitSha = latestVersion.commitSha();
+        var commitDate = latestVersion.commitDate();
         if (previousStatus != null) {
             commitSha = previousStatus.releaseCommit();
             commitDate = previousStatus.committedAt();
@@ -275,6 +272,7 @@ public class VersionStatus {
                 confidence = previousStatus.confidence();
             }
         }
+
         return new VespaVersion(statistics,
                                 commitSha,
                                 commitDate,
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 53eb30bb2f3..bd2868aeb0e 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
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.ZoneApi;
@@ -192,7 +193,14 @@ public final class ControllerTester {
 
     /** Upgrade controller to given version */
     public void upgradeController(Version version, String commitSha, Instant commitDate) {
-        controller().curator().writeControllerVersion(controller().hostname(), new ControllerVersion(version, commitSha, commitDate));
+        for (var hostname : controller().curator().cluster()) {
+            upgradeController(hostname, version, commitSha, commitDate);
+        }
+    }
+
+    /** Upgrade controller to given version */
+    public void upgradeController(HostName hostname, Version version, String commitSha, Instant commitDate) {
+        controller().curator().writeControllerVersion(hostname, new ControllerVersion(version, commitSha, commitDate));
         computeVersionStatus();
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
index e5127264b47..fdfe348cacb 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
@@ -6,13 +6,12 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.TenantName;
-import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.hosted.controller.Application;
-import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.ApplicationController;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
+import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockBuildService;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
@@ -27,8 +26,6 @@ import com.yahoo.vespa.hosted.controller.maintenance.NameServiceDispatcher;
 import com.yahoo.vespa.hosted.controller.maintenance.OutstandingChangeDeployer;
 import com.yahoo.vespa.hosted.controller.maintenance.ReadyJobsTrigger;
 import com.yahoo.vespa.hosted.controller.maintenance.Upgrader;
-import com.yahoo.vespa.hosted.controller.versions.ControllerVersion;
-import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
 
 import java.time.Duration;
 import java.time.Instant;
@@ -134,40 +131,32 @@ public class DeploymentTester {
 
     // TODO(mpolden): Change callers to use ControllerTester#computeVersionStatus and remove this
     public void computeVersionStatus() {
-        controller().updateVersionStatus(VersionStatus.compute(controller()));
+        tester.computeVersionStatus();
     }
 
     // TODO(mpolden): Change callers to use ControllerTester#upgradeController and remove this
     public void upgradeController(Version version) {
-        upgradeController(version, "badc0ffee", Instant.EPOCH);
+        tester.upgradeController(version);
     }
 
     // TODO(mpolden): Change callers to use ControllerTester#upgradeController and remove this
     public void upgradeController(Version version, String commitSha, Instant commitDate) {
-        controller().curator().writeControllerVersion(controller().hostname(), new ControllerVersion(version, commitSha, commitDate));
-        computeVersionStatus();
+        tester.upgradeController(version, commitSha, commitDate);
     }
 
     // TODO(mpolden): Change callers to use ControllerTester#upgradeSystemApplications and remove this
     public void upgradeSystemApplications(Version version) {
-        upgradeSystemApplications(version, SystemApplication.all());
+        tester.upgradeSystemApplications(version);
     }
 
     // TODO(mpolden): Change callers to use ControllerTester#upgradeSystemApplications and remove this
     public void upgradeSystemApplications(Version version, List<SystemApplication> systemApplications) {
-        for (ZoneApi zone : tester.zoneRegistry().zones().all().zones()) {
-            for (SystemApplication application : systemApplications) {
-                tester.configServer().setVersion(application.id(), zone.getId(), version);
-                tester.configServer().convergeServices(application.id(), zone.getId());
-            }
-        }
-        computeVersionStatus();
+        tester.upgradeSystemApplications(version, systemApplications);
     }
 
     // TODO(mpolden): Change callers to use ControllerTester#upgradeSystem and remove this
     public void upgradeSystem(Version version) {
-        upgradeController(version);
-        upgradeSystemApplications(version);
+        tester.upgradeSystem(version);
         upgrader().maintain();
         readyJobTrigger().maintain();
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
index fb9367cc020..0b5e44aad70 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
@@ -82,7 +82,11 @@ public class InternalDeploymentTester {
     public Instance instance(ApplicationId id) { return tester.instance(id); }
 
     public InternalDeploymentTester() {
-        tester = new DeploymentTester();
+        this(new ControllerTester());
+    }
+
+    public InternalDeploymentTester(ControllerTester controllerTester) {
+        tester = new DeploymentTester(controllerTester);
         jobs = tester.controller().jobController();
         routing = tester.controllerTester().serviceRegistry().routingGeneratorMock();
         cloud = (MockTesterCloud) tester.controller().jobController().cloud();
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
index c3bb5cb568f..380f1c6dd48 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
@@ -386,9 +386,16 @@ public class VersionStatusTest {
 
     @Test
     public void testCommitDetailsPreservation() {
-        InternalDeploymentTester tester = new InternalDeploymentTester();
+        HostName controller1 = HostName.from("controller-1");
+        HostName controller2 = HostName.from("controller-2");
+        HostName controller3 = HostName.from("controller-3");
+        MockCuratorDb db = new MockCuratorDb(Stream.of(controller1, controller2, controller3)
+                                                   .map(hostName -> hostName.value() + ":2222")
+                                                   .collect(Collectors.joining(",")));
+        InternalDeploymentTester tester = new InternalDeploymentTester(new ControllerTester(db));
+
         // Commit details are set for initial version
-        var version0 = new Version("6.2");
+        var version0 = new Version("7.2");
         var commitSha0 = "badc0ffee";
         var commitDate0 = Instant.EPOCH;
         tester.controllerTester().upgradeSystem(version0);
@@ -400,7 +407,7 @@ public class VersionStatusTest {
         tester.deploymentContext().submit().deploy();
 
         // Commit details are updated for new version
-        var version1 = new Version("6.3");
+        var version1 = new Version("7.3");
         var commitSha1 = "deadbeef";
         var commitDate1 = Instant.ofEpochMilli(123);
         tester.controllerTester().upgradeController(version1, commitSha1, commitDate1);