Store currentDockerImage in node in ZK

7 files changed, 57 insertions, 31 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java
index 0f84ac1d6a6..68a1cf5cd8a 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.provision.node;
 
 import com.yahoo.component.Version;
+import com.yahoo.config.provision.DockerImage;
 
 import javax.annotation.concurrent.Immutable;
 import java.time.Instant;
@@ -18,6 +19,7 @@ public class Status {
 
     private final Generation reboot;
     private final Optional<Version> vespaVersion;
+    private final Optional<DockerImage> dockerImage;
     private final int failCount;
     private final Optional<String> hardwareFailureDescription;
     private final boolean wantToRetire;
@@ -28,6 +30,7 @@ public class Status {
 
     public Status(Generation generation,
                   Optional<Version> vespaVersion,
+                  Optional<DockerImage> dockerImage,
                   int failCount,
                   Optional<String> hardwareFailureDescription,
                   boolean wantToRetire,
@@ -39,6 +42,7 @@ public class Status {
         hardwareDivergence.ifPresent(s -> requireNonEmptyString(s, "Hardware divergence must be non-empty"));
         this.reboot = Objects.requireNonNull(generation, "Generation must be non-null");
         this.vespaVersion = Objects.requireNonNull(vespaVersion, "Vespa version must be non-null").filter(v -> !Version.emptyVersion.equals(v));
+        this.dockerImage = Objects.requireNonNull(dockerImage, "Docker image must be non-null").filter(d -> !DockerImage.EMPTY.equals(d));
         this.failCount = failCount;
         this.hardwareFailureDescription = Objects.requireNonNull(hardwareFailureDescription, "Hardware failure description must be non-null");
         this.wantToRetire = wantToRetire;
@@ -49,34 +53,40 @@ public class Status {
     }
 
     /** Returns a copy of this with the reboot generation changed */
-    public Status withReboot(Generation reboot) { return new Status(reboot, vespaVersion, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
+    public Status withReboot(Generation reboot) { return new Status(reboot, vespaVersion, dockerImage, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
 
     /** Returns the reboot generation of this node */
     public Generation reboot() { return reboot; }
 
     /** Returns a copy of this with the vespa version changed */
-    public Status withVespaVersion(Version version) { return new Status(reboot, Optional.of(version), failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
+    public Status withVespaVersion(Version version) { return new Status(reboot, Optional.of(version), dockerImage, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
 
     /** Returns the Vespa version installed on the node, if known */
     public Optional<Version> vespaVersion() { return vespaVersion; }
 
-    public Status withIncreasedFailCount() { return new Status(reboot, vespaVersion, failCount + 1, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
+    /** Returns a copy of this with the docker image changed */
+    public Status withDockerImage(DockerImage dockerImage) { return new Status(reboot, vespaVersion, Optional.of(dockerImage), failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
 
-    public Status withDecreasedFailCount() { return new Status(reboot, vespaVersion, failCount - 1, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
+    /** Returns the docker image the node is running, if known */
+    public Optional<DockerImage> dockerImage() { return dockerImage; }
 
-    public Status setFailCount(Integer value) { return new Status(reboot, vespaVersion, value, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
+    public Status withIncreasedFailCount() { return new Status(reboot, vespaVersion, dockerImage, failCount + 1, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
+
+    public Status withDecreasedFailCount() { return new Status(reboot, vespaVersion, dockerImage, failCount - 1, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
+
+    public Status setFailCount(Integer value) { return new Status(reboot, vespaVersion, dockerImage, value, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
 
     /** Returns how many times this node has been moved to the failed state. */
     public int failCount() { return failCount; }
 
-    public Status withHardwareFailureDescription(Optional<String> hardwareFailureDescription) { return new Status(reboot, vespaVersion, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
+    public Status withHardwareFailureDescription(Optional<String> hardwareFailureDescription) { return new Status(reboot, vespaVersion, dockerImage, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt); }
 
     /** Returns the type of the last hardware failure detected on this node, or empty if none */
     public Optional<String> hardwareFailureDescription() { return hardwareFailureDescription; }
 
     /** Returns a copy of this with the want to retire flag changed */
     public Status withWantToRetire(boolean wantToRetire) {
-        return new Status(reboot, vespaVersion, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt);
+        return new Status(reboot, vespaVersion, dockerImage, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt);
     }
 
     /**
@@ -89,7 +99,7 @@ public class Status {
 
     /** Returns a copy of this with the want to de-provision flag changed */
     public Status withWantToDeprovision(boolean wantToDeprovision) {
-        return new Status(reboot, vespaVersion, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt);
+        return new Status(reboot, vespaVersion, dockerImage, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt);
     }
 
     /**
@@ -100,7 +110,7 @@ public class Status {
     }
 
     public Status withHardwareDivergence(Optional<String> hardwareDivergence) {
-        return new Status(reboot, vespaVersion, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt);
+        return new Status(reboot, vespaVersion, dockerImage, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, firmwareVerifiedAt);
     }
 
     /** Returns hardware divergence report as JSON string, if any */
@@ -108,7 +118,7 @@ public class Status {
 
     /** Returns a copy of this with the current OS version set to version */
     public Status withOsVersion(Version version) {
-        return new Status(reboot, vespaVersion, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, Optional.of(version), firmwareVerifiedAt);
+        return new Status(reboot, vespaVersion, dockerImage, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, Optional.of(version), firmwareVerifiedAt);
     }
 
     /** Returns the current OS version of this node, if any */
@@ -118,7 +128,7 @@ public class Status {
 
     /** Returns a copy of this with the firmwareVerifiedAt set to the given instant. */
     public Status withFirmwareVerifiedAt(Instant instant) {
-        return new Status(reboot, vespaVersion, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, Optional.of(instant));
+        return new Status(reboot, vespaVersion, dockerImage, failCount, hardwareFailureDescription, wantToRetire, wantToDeprovision, hardwareDivergence, osVersion, Optional.of(instant));
     }
 
     /** Returns the last time this node had firmware that was verified to be up to date. */
@@ -128,7 +138,7 @@ public class Status {
 
     /** Returns the initial status of a newly provisioned node */
     public static Status initial() {
-        return new Status(Generation.initial(), Optional.empty(), 0, Optional.empty(), false,
+        return new Status(Generation.initial(), Optional.empty(), Optional.empty(), 0, Optional.empty(), false,
                           false, Optional.empty(), Optional.empty(), Optional.empty());
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
index bb4dab3b97b..5ba56dfa8ed 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
@@ -6,12 +6,12 @@ import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.ClusterMembership;
+import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.InstanceName;
+import com.yahoo.config.provision.NetworkPortsSerializer;
 import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.NodeType;
-import com.yahoo.config.provision.NetworkPorts;
-import com.yahoo.config.provision.NetworkPortsSerializer;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.slime.ArrayTraverser;
 import com.yahoo.slime.Cursor;
@@ -57,6 +57,7 @@ public class NodeSerializer {
     private static final String rebootGenerationKey = "rebootGeneration";
     private static final String currentRebootGenerationKey = "currentRebootGeneration";
     private static final String vespaVersionKey = "vespaVersion";
+    private static final String currentDockerImageKey = "currentDockerImage";
     private static final String failCountKey = "failCount";
     private static final String hardwareFailureKey = "hardwareFailure";
     private static final String nodeTypeKey = "type";
@@ -117,6 +118,7 @@ public class NodeSerializer {
         object.setLong(rebootGenerationKey, node.status().reboot().wanted());
         object.setLong(currentRebootGenerationKey, node.status().reboot().current());
         node.status().vespaVersion().ifPresent(version -> object.setString(vespaVersionKey, version.toString()));
+        node.status().dockerImage().ifPresent(image -> object.setString(currentDockerImageKey, image.asString()));
         object.setLong(failCountKey, node.status().failCount());
         node.status().hardwareFailureDescription().ifPresent(failure -> object.setString(hardwareFailureKey, failure));
         object.setBool(wantToRetireKey, node.status().wantToRetire());
@@ -185,6 +187,7 @@ public class NodeSerializer {
     private Status statusFromSlime(Inspector object) {
         return new Status(generationFromSlime(object, rebootGenerationKey, currentRebootGenerationKey),
                           versionFromSlime(object.field(vespaVersionKey)),
+                          dockerImageFromSlime(object.field(currentDockerImageKey)),
                           (int)object.field(failCountKey).asLong(),
                           hardwareFailureDescriptionFromSlime(object),
                           object.field(wantToRetireKey).asBool(),
@@ -246,6 +249,11 @@ public class NodeSerializer {
         return Optional.of(Version.fromString(object.asString()));
     }
 
+    private Optional<DockerImage> dockerImageFromSlime(Inspector object) {
+        if ( ! object.valid()) return Optional.empty();
+        return Optional.of(DockerImage.fromString(object.asString()));
+    }
+
     private Optional<Instant> instantFromSlime(Inspector object) {
         if ( ! object.valid())
             return Optional.empty();
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java
index 2bf41e0685c..f9234af7fb4 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java
@@ -113,13 +113,7 @@ public class NodePatcher {
             case "currentDockerImage" :
                 if (node.flavor().getType() != Flavor.Type.DOCKER_CONTAINER)
                     throw new IllegalArgumentException("Docker image can only be set for docker containers");
-                Version versionFromImage = Optional.of(asString(value))
-                        .filter(s -> !s.isEmpty())
-                        .map(DockerImage::fromString)
-                        .map(image -> image.tag().map(Version::fromString).orElseThrow(() ->
-                                new IllegalArgumentException("tag components of docker image must be set")))
-                        .orElse(Version.emptyVersion);
-                return node.with(node.status().withVespaVersion(versionFromImage));
+                return node.with(node.status().withDockerImage(DockerImage.fromString(asString(value))));
             case "vespaVersion" :
             case "currentVespaVersion" :
                 return node.with(node.status().withVespaVersion(Version.fromString(asString(value))));
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
index 7858621b72d..a0ecb063618 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.restapi.v2;
 
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterMembership;
+import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.NetworkPortsSerializer;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.container.jdisc.HttpRequest;
@@ -173,11 +174,8 @@ class NodesResponse extends HttpResponse {
         node.status().firmwareVerifiedAt().ifPresent(instant -> object.setLong("currentFirmwareCheck", instant.toEpochMilli()));
         if (node.type().isDockerHost())
             nodeRepository.firmwareChecks().requiredAfter().ifPresent(after -> object.setLong("wantedFirmwareCheck", after.toEpochMilli()));
-        node.status().vespaVersion()
-                .ifPresent(version -> {
-                    object.setString("vespaVersion", version.toFullString());
-                    object.setString("currentDockerImage", nodeRepository.dockerImage().withTag(version).asString());
-                });
+        node.status().vespaVersion().ifPresent(version -> object.setString("vespaVersion", version.toFullString()));
+        currentDockerImage(node).ifPresent(dockerImage -> object.setString("currentDockerImage", dockerImage.asString()));
         object.setLong("failCount", node.status().failCount());
         object.setBool("hardwareFailure", node.status().hardwareFailureDescription().isPresent());
         node.status().hardwareFailureDescription().ifPresent(failure -> object.setString("hardwareFailureDescription", failure));
@@ -214,6 +212,17 @@ class NodesResponse extends HttpResponse {
         }
     }
 
+    // Hack: For docker hosts, return current docker image as default prefix + current Vespa version
+    // TODO: Remove current + wanted docker image from response for non-docker types
+    private Optional<DockerImage> currentDockerImage(Node node) {
+        return node.status().dockerImage()
+                .or(() -> Optional.of(node)
+                        .filter(n -> n.type().isDockerHost())
+                        .flatMap(n -> n.status().vespaVersion()
+                                .map(version -> nodeRepository.dockerImages().dockerImage().withTag(version))));
+    }
+
+
     /** maven-vespa-plugin @ v6 needs to deserialize nodes w/history. */
     private Agent normalizedAgentUntilV6IsGone(Agent agent) {
         return agent == Agent.NodeFailer ? Agent.system : agent;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
index c76d25bb48d..83f5d6bf783 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
@@ -73,11 +73,15 @@ public class MockNodeRepository extends NodeRepository {
         nodes.add(createNode("node3", "host3.yahoo.com", ipAddresses, Optional.empty(), flavors.getFlavorOrThrow("expensive"), NodeType.tenant));
 
         Node node4 = createNode("node4", "host4.yahoo.com", ipAddresses, Optional.of("dockerhost1.yahoo.com"), flavors.getFlavorOrThrow("docker"), NodeType.tenant);
-        node4 = node4.with(node4.status().withVespaVersion(new Version("6.41.0")));
+        node4 = node4.with(node4.status()
+                .withVespaVersion(new Version("6.41.0"))
+                .withDockerImage(DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa:6.41.0")));
         nodes.add(node4);
 
         Node node5 = createNode("node5", "host5.yahoo.com", ipAddresses, Optional.of("dockerhost2.yahoo.com"), flavors.getFlavorOrThrow("docker"), NodeType.tenant);
-        nodes.add(node5.with(node5.status().withVespaVersion(new Version("1.2.3"))));
+        nodes.add(node5.with(node5.status()
+                .withVespaVersion(new Version("1.2.3"))
+                .withDockerImage(DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa:1.2.3"))));
 
 
         nodes.add(createNode("node6", "host6.yahoo.com", ipAddresses, Optional.empty(), flavors.getFlavorOrThrow("default"), NodeType.tenant));
@@ -86,7 +90,8 @@ public class MockNodeRepository extends NodeRepository {
         Node node10 = createNode("node10", "host10.yahoo.com", ipAddresses, Optional.of("parent1.yahoo.com"), flavors.getFlavorOrThrow("default"), NodeType.tenant);
         Status node10newStatus = node10.status();
         node10newStatus = node10newStatus
-                .withVespaVersion(Version.fromString("5.104.142"));
+                .withVespaVersion(Version.fromString("5.104.142"))
+                .withDockerImage(DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa:5.104.142"));
         node10 = node10.with(node10newStatus);
         nodes.add(node10);
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
index 73e98303c14..e4b79071dde 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
@@ -214,7 +214,7 @@ public class RestApiTest {
                         Utf8.toBytes("{\"wantToDeprovision\": true}"), Request.Method.PATCH),
                 "{\"message\":\"Updated host4.yahoo.com\"}");
         assertResponse(new Request("http://localhost:8080/nodes/v2/node/host4.yahoo.com",
-                        Utf8.toBytes("{\"currentVespaVersion\": \"6.43.0\",\"currentDockerImage\": \"docker-registry.domain.tld:8080/dist/vespa:6.43.0\"}"), Request.Method.PATCH),
+                        Utf8.toBytes("{\"currentVespaVersion\": \"6.43.0\",\"currentDockerImage\": \"docker-registry.domain.tld:8080/dist/vespa:6.45.0\"}"), Request.Method.PATCH),
                         "{\"message\":\"Updated host4.yahoo.com\"}");
         assertResponse(new Request("http://localhost:8080/nodes/v2/node/host4.yahoo.com",
                         Utf8.toBytes("{\"openStackId\": \"patched-openstackid\"}"), Request.Method.PATCH),
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4-after-changes.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4-after-changes.json
index bba9134897a..e2a788cc886 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4-after-changes.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/node4-after-changes.json
@@ -35,7 +35,7 @@
   "rebootGeneration": 3,
   "currentRebootGeneration": 1,
   "vespaVersion": "6.43.0",
-  "currentDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.43.0",
+  "currentDockerImage": "docker-registry.domain.tld:8080/dist/vespa:6.45.0",
   "failCount": 1,
   "hardwareFailure": true,
   "hardwareFailureDescription": "memory_mcelog",