Store version and commit details in ControllerVersion

10 files changed, 194 insertions, 61 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
index ff27a92de9a..f51465dd307 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
@@ -33,6 +33,7 @@ import com.yahoo.vespa.hosted.controller.deployment.JobController;
 import com.yahoo.vespa.hosted.controller.dns.NameServiceForwarder;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.security.AccessControl;
+import com.yahoo.vespa.hosted.controller.versions.ControllerVersion;
 import com.yahoo.vespa.hosted.controller.versions.OsVersion;
 import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus;
 import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
@@ -141,7 +142,7 @@ public class Controller extends AbstractComponent {
         auditLogger = new AuditLogger(curator, clock);
 
         // Record the version of this controller
-        curator().writeControllerVersion(this.hostname(), Vtag.currentVersion);
+        curator().writeControllerVersion(this.hostname(), ControllerVersion.CURRENT);
 
         jobController.updateStorage();
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ControllerVersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ControllerVersionSerializer.java
new file mode 100644
index 00000000000..58902619cf2
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ControllerVersionSerializer.java
@@ -0,0 +1,48 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.controller.persistence;
+
+import com.yahoo.component.Version;
+import com.yahoo.slime.Slime;
+import com.yahoo.vespa.hosted.controller.versions.ControllerVersion;
+
+import java.time.Instant;
+
+/**
+ * Serializer for {@link com.yahoo.vespa.hosted.controller.versions.ControllerVersion}.
+ *
+ * @author mpolden
+ */
+public class ControllerVersionSerializer {
+
+    // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one
+    //          (and rewrite all nodes on startup), changes to the serialized format must be made
+    //          such that what is serialized on version N+1 can be read by version N:
+    //          - ADDING FIELDS: Always ok
+    //          - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version.
+    //          - CHANGING THE FORMAT OF A FIELD: Don't do it bro.
+
+    private static final String VERSION_FIELD = "version";
+    private static final String COMMIT_SHA_FIELD = "commitSha";
+    private static final String COMMIT_DATE_FIELD = "commitDate";
+
+    public Slime toSlime(ControllerVersion controllerVersion) {
+        var slime = new Slime();
+        var root = slime.setObject();
+        root.setString(VERSION_FIELD, controllerVersion.version().toFullString());
+        root.setString(COMMIT_SHA_FIELD, controllerVersion.commitSha());
+        root.setLong(COMMIT_DATE_FIELD, controllerVersion.commitDate().toEpochMilli());
+        return slime;
+    }
+
+    public ControllerVersion fromSlime(Slime slime) {
+        var root = slime.get();
+        var version = Version.fromString(root.field(VERSION_FIELD).asString());
+        // TODO(mpolden): Make the following two fields non-optional after August 2019
+        var commitSha = Serializers.optionalString(root.field(COMMIT_SHA_FIELD))
+                                   .orElse("badc0ffee");
+        var commitDate = Serializers.optionalInstant(root.field(COMMIT_DATE_FIELD))
+                                    .orElse(Instant.EPOCH);
+        return new ControllerVersion(version, commitSha, commitDate);
+    }
+
+}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
index b7b64b9cda2..91bd486a18a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
@@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.controller.persistence;
 import com.google.common.util.concurrent.UncheckedTimeoutException;
 import com.google.inject.Inject;
 import com.yahoo.component.Version;
-import com.yahoo.component.Vtag;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.TenantName;
@@ -23,6 +22,7 @@ import com.yahoo.vespa.hosted.controller.deployment.Run;
 import com.yahoo.vespa.hosted.controller.deployment.Step;
 import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue;
 import com.yahoo.vespa.hosted.controller.tenant.Tenant;
+import com.yahoo.vespa.hosted.controller.versions.ControllerVersion;
 import com.yahoo.vespa.hosted.controller.versions.OsVersion;
 import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus;
 import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
@@ -78,7 +78,7 @@ public class CuratorDb {
 
     private final StringSetSerializer stringSetSerializer = new StringSetSerializer();
     private final VersionStatusSerializer versionStatusSerializer = new VersionStatusSerializer();
-    private final VersionSerializer versionSerializer = new VersionSerializer();
+    private final ControllerVersionSerializer controllerVersionSerializer = new ControllerVersionSerializer();
     private final ConfidenceOverrideSerializer confidenceOverrideSerializer = new ConfidenceOverrideSerializer();
     private final TenantSerializer tenantSerializer = new TenantSerializer();
     private final ApplicationSerializer applicationSerializer = new ApplicationSerializer();
@@ -272,14 +272,14 @@ public class CuratorDb {
                                                    .orElseGet(Collections::emptyMap);
     }
 
-    public void writeControllerVersion(HostName hostname, Version version) {
-        curator.set(controllerPath(hostname.value()), asJson(versionSerializer.toSlime(version)));
+    public void writeControllerVersion(HostName hostname, ControllerVersion version) {
+        curator.set(controllerPath(hostname.value()), asJson(controllerVersionSerializer.toSlime(version)));
     }
 
-    public Version readControllerVersion(HostName hostname) {
+    public ControllerVersion readControllerVersion(HostName hostname) {
         return readSlime(controllerPath(hostname.value()))
-                .map(versionSerializer::fromSlime)
-                .orElse(Vtag.currentVersion);
+                .map(controllerVersionSerializer::fromSlime)
+                .orElse(ControllerVersion.CURRENT);
     }
 
     // Infrastructure upgrades
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionSerializer.java
deleted file mode 100644
index e5897963254..00000000000
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionSerializer.java
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.controller.persistence;
-
-import com.yahoo.component.Version;
-import com.yahoo.slime.Cursor;
-import com.yahoo.slime.Inspector;
-import com.yahoo.slime.Slime;
-
-/**
- * Serializer for {@link Version}.
- *
- * @author mpolden
- */
-public class VersionSerializer {
-
-    // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one
-    //          (and rewrite all nodes on startup), changes to the serialized format must be made
-    //          such that what is serialized on version N+1 can be read by version N:
-    //          - ADDING FIELDS: Always ok
-    //          - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version.
-    //          - CHANGING THE FORMAT OF A FIELD: Don't do it bro.
-
-    private static final String versionField = "version";
-
-    public Slime toSlime(Version version) {
-        Slime slime = new Slime();
-        Cursor root = slime.setObject();
-        root.setString(versionField, version.toFullString());
-        return slime;
-    }
-
-    public Version fromSlime(Slime slime) {
-        Inspector root = slime.get();
-        return Version.fromString(root.field(versionField).asString());
-    }
-
-}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/ControllerVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/ControllerVersion.java
new file mode 100644
index 00000000000..a8bfd1ebf68
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/ControllerVersion.java
@@ -0,0 +1,70 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.controller.versions;
+
+import com.yahoo.component.Version;
+import com.yahoo.component.Vtag;
+import org.jetbrains.annotations.NotNull;
+
+import java.time.Instant;
+import java.util.Objects;
+
+/**
+ * A controller's Vespa version and commit details.
+ *
+ * @author mpolden
+ */
+public class ControllerVersion implements Comparable<ControllerVersion> {
+
+    /** The current version of this controller */
+    public static final ControllerVersion CURRENT = new ControllerVersion(Vtag.currentVersion, Vtag.commitSha,
+                                                                          Vtag.commitDate);
+
+    private final Version version;
+    private final String commitSha;
+    private final Instant commitDate;
+
+    public ControllerVersion(Version version, String commitSha, Instant commitDate) {
+        this.version = Objects.requireNonNull(version);
+        this.commitSha = Objects.requireNonNull(commitSha);
+        this.commitDate = Objects.requireNonNull(commitDate);
+    }
+
+    /** Vespa version */
+    public Version version() {
+        return version;
+    }
+
+    /** Commit SHA of this */
+    public String commitSha() {
+        return commitSha;
+    }
+
+    /** The time this was committed */
+    public Instant commitDate() {
+        return commitDate;
+    }
+
+    @Override
+    public String toString() {
+        return version + ", commit " + commitSha + " @ " + commitDate;
+    }
+
+    @Override
+    public int compareTo(@NotNull ControllerVersion o) {
+        return version.compareTo(o.version);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        ControllerVersion that = (ControllerVersion) o;
+        return version.equals(that.version);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(version);
+    }
+
+}
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 10b55c75ff0..aa75b3351d4 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
@@ -5,7 +5,6 @@ import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ListMultimap;
 import com.yahoo.component.Version;
-import com.yahoo.component.Vtag;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.log.LogLevel;
@@ -96,14 +95,19 @@ public class VersionStatus {
     /** Create a full, updated version status. This is expensive and should be done infrequently */
     public static VersionStatus compute(Controller controller) {
         ListMultimap<Version, HostName> systemApplicationVersions = findSystemApplicationVersions(controller);
-        ListMultimap<Version, HostName> controllerVersions = findControllerVersions(controller);
+        ListMultimap<ControllerVersion, HostName> controllerVersions = findControllerVersions(controller);
 
         ListMultimap<Version, HostName> infrastructureVersions = ArrayListMultimap.create();
-        infrastructureVersions.putAll(controllerVersions);
+        for (var kv : controllerVersions.asMap().entrySet()) {
+            infrastructureVersions.putAll(kv.getKey().version(), kv.getValue());
+        }
         infrastructureVersions.putAll(systemApplicationVersions);
 
         // The controller version is the lowest controller version of all controllers
-        Version controllerVersion = controllerVersions.keySet().stream().min(Comparator.naturalOrder()).get();
+        Version controllerVersion = controllerVersions.keySet().stream()
+                                                      .min(Comparator.naturalOrder())
+                                                      .map(ControllerVersion::version)
+                                                      .get();
 
         // The system version is the oldest infrastructure version, if that version is newer than the current system
         // version
@@ -177,10 +181,10 @@ public class VersionStatus {
         return versions;
     }
 
-    private static ListMultimap<Version, HostName> findControllerVersions(Controller controller) {
-        ListMultimap<Version, HostName> versions = ArrayListMultimap.create();
+    private static ListMultimap<ControllerVersion, HostName> findControllerVersions(Controller controller) {
+        ListMultimap<ControllerVersion, HostName> versions = ArrayListMultimap.create();
         if (controller.curator().cluster().isEmpty()) { // Use vtag if we do not have cluster
-            versions.put(Vtag.currentVersion, controller.hostname());
+            versions.put(ControllerVersion.CURRENT, controller.hostname());
         } else {
             for (HostName hostname : controller.curator().cluster()) {
                 versions.put(controller.curator().readControllerVersion(hostname), hostname);
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 887406ecba8..11ed95c3f15 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,7 +6,6 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.ZoneApi;
-import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.ApplicationController;
@@ -27,9 +26,11 @@ 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;
 import java.util.List;
 import java.util.Optional;
 import java.util.UUID;
@@ -118,7 +119,7 @@ public class DeploymentTester {
 
     /** Upgrade controller to given version */
     public void upgradeController(Version version) {
-        controller().curator().writeControllerVersion(controller().hostname(), version);
+        controller().curator().writeControllerVersion(controller().hostname(), new ControllerVersion(version, "badc0ffee", Instant.EPOCH));
         computeVersionStatus();
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ControllerVersionSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ControllerVersionSerializerTest.java
new file mode 100644
index 00000000000..4d2627f50c2
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ControllerVersionSerializerTest.java
@@ -0,0 +1,38 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.controller.persistence;
+
+import com.yahoo.component.Version;
+import com.yahoo.vespa.config.SlimeUtils;
+import com.yahoo.vespa.hosted.controller.versions.ControllerVersion;
+import org.junit.Test;
+
+import java.time.Instant;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author mpolden
+ */
+public class ControllerVersionSerializerTest {
+
+    private final ControllerVersionSerializer serializer = new ControllerVersionSerializer();
+
+    @Test
+    public void serialization() {
+        var version = new ControllerVersion(Version.fromString("7.42.1"), "badc0ffee", Instant.ofEpochSecond(1565876112));
+        var serialized = serializer.fromSlime(serializer.toSlime(version));
+        assertEquals(version.version(), serialized.version());
+        assertEquals(version.commitSha(), serialized.commitSha());
+        assertEquals(version.commitDate(), serialized.commitDate());
+    }
+
+    @Test // TODO(mpolden): Remove after August 2019
+    public void legacy_serialization() {
+        var slime = SlimeUtils.jsonToSlime("{\"version\":\"7.42.1\"}");
+        var serialized = serializer.fromSlime(slime);
+        assertEquals(Version.fromString("7.42.1"), serialized.version());
+        assertEquals("badc0ffee", serialized.commitSha());
+        assertEquals(Instant.EPOCH, serialized.commitDate());
+    }
+
+}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java
index b32cbbcb926..d920b5d5769 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java
@@ -13,6 +13,7 @@ import com.yahoo.jdisc.http.filter.SecurityRequestFilterChain;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
 import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock;
+import com.yahoo.vespa.hosted.controller.versions.ControllerVersion;
 import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
 import org.junit.ComparisonFailure;
 
@@ -22,6 +23,7 @@ import java.io.UncheckedIOException;
 import java.nio.charset.CharacterCodingException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
+import java.time.Instant;
 import java.util.Optional;
 import java.util.function.Supplier;
 import java.util.regex.Pattern;
@@ -58,10 +60,11 @@ public class ContainerTester {
     }
 
     public void upgradeSystem(Version version) {
-        controller().curator().writeControllerVersion(controller().hostname(), version);
+        var controllerVersion = new ControllerVersion(version, "badc0ffee", Instant.EPOCH);
+        controller().curator().writeControllerVersion(controller().hostname(), controllerVersion);
         for (ZoneApi zone : controller().zoneRegistry().zones().all().zones()) {
             for (SystemApplication application : SystemApplication.all()) {
-                configServer().setVersion(application.id(), zone.getId(), version);
+                configServer().setVersion(application.id(), zone.getId(), controllerVersion.version());
                 configServer().convergeServices(application.id(), zone.getId());
             }
         }
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 655c16ccceb..a9fdaac6ee7 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
@@ -11,15 +11,16 @@ import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
-import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
+import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence;
 import org.junit.Test;
 
+import java.time.Instant;
 import java.util.List;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -82,16 +83,16 @@ public class VersionStatusTest {
                                                    .collect(Collectors.joining(",")));
         ControllerTester tester = new ControllerTester(db);
 
-        db.writeControllerVersion(controller1, Version.fromString("6.2"));
-        db.writeControllerVersion(controller2, Version.fromString("6.1"));
-        db.writeControllerVersion(controller3, Version.fromString("6.2"));
+        writeControllerVersion(controller1, Version.fromString("6.2"), db);
+        writeControllerVersion(controller2, Version.fromString("6.1"), db);
+        writeControllerVersion(controller3, Version.fromString("6.2"), db);
 
         VersionStatus versionStatus = VersionStatus.compute(tester.controller());
         assertEquals("Controller version is oldest version", Version.fromString("6.1"),
                      versionStatus.controllerVersion().get().versionNumber());
 
         // Last controller upgrades
-        db.writeControllerVersion(controller2, Version.fromString("6.2"));
+        writeControllerVersion(controller2, Version.fromString("6.2"), db);
         versionStatus = VersionStatus.compute(tester.controller());
         assertEquals(Version.fromString("6.2"), versionStatus.controllerVersion().get().versionNumber());
     }
@@ -330,6 +331,10 @@ public class VersionStatusTest {
                                                     .keySet().contains(version0));
     }
 
+    private static void writeControllerVersion(HostName hostname, Version version, CuratorDb db) {
+        db.writeControllerVersion(hostname, new ControllerVersion(version, "badc0ffee", Instant.EPOCH));
+    }
+
     private Confidence confidence(Controller controller, Version version) {
         return controller.versionStatus().versions().stream()
                 .filter(v -> v.statistics().version().equals(version))