Use DockerImage instead of String, part 2

10 files changed, 34 insertions, 23 deletions

diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java
index 4b882023e08..4604eea62c1 100644
--- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java
+++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java
@@ -54,7 +54,7 @@ public class IdentityDocumentGeneratorTest {
         ApplicationId appid = ApplicationId.from(
                 TenantName.from("tenant"), ApplicationName.from("application"), InstanceName.from("default"));
         Allocation allocation = new Allocation(appid,
-                                               ClusterMembership.from("container/default/0/0", Version.fromString("1.2.3")),
+                                               ClusterMembership.from("container/default/0/0", Version.fromString("1.2.3"), Optional.empty()),
                                                new NodeResources(1, 1, 1, 1),
                                                Generation.initial(),
                                                false);
diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java
index fdf2bbfccff..19ad9df2d4d 100644
--- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java
+++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java
@@ -240,7 +240,7 @@ public class InstanceValidatorTest {
     private List<Node> allocateNode(List<Node> nodeList, Node node, ApplicationId applicationId) {
         nodeList.removeIf(n -> n.id().equals(node.id()));
         nodeList.add(node.allocate(applicationId,
-                                   ClusterMembership.from("container/default/0/0", Version.fromString("6.123.4")),
+                                   ClusterMembership.from("container/default/0/0", Version.fromString("6.123.4"), Optional.empty()),
                                    new NodeResources(1, 1, 1, 1),
                                    Instant.now()));
         return nodeList;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
index ad6eebe1ca5..5418ba1a6ee 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
@@ -9,6 +9,7 @@ import com.yahoo.config.provision.Capacity;
 import com.yahoo.config.provision.ClusterMembership;
 import com.yahoo.config.provision.ClusterResources;
 import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.text.XML;
 import com.yahoo.vespa.model.HostResource;
@@ -21,7 +22,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.function.Function;
-import java.util.regex.Pattern;
 
 /**
  * A common utility class to represent a requirement for nodes during model building.
@@ -209,7 +209,7 @@ public class NodesSpecification {
                 .vespaVersion(version)
                 .exclusive(exclusive)
                 .combinedId(combinedId.map(ClusterSpec.Id::from))
-                .dockerImageRepo(dockerImageRepo)
+                .dockerImageRepository(dockerImageRepo.map(DockerImage::fromString))
                 .build();
         return hostSystem.allocateHosts(cluster, Capacity.from(min, max, required, canFail), logger);
     }
@@ -217,7 +217,7 @@ public class NodesSpecification {
     private static Pair<NodeResources, NodeResources> nodeResources(ModelElement nodesElement) {
         ModelElement resources = nodesElement.child("resources");
         if (resources != null) {
-            return nodeResourcesFromResorcesElement(resources);
+            return nodeResourcesFromResourcesElement(resources);
         }
         else if (nodesElement.stringAttribute("flavor") != null) { // legacy fallback
             var flavorResources = NodeResources.fromLegacyName(nodesElement.stringAttribute("flavor"));
@@ -228,7 +228,7 @@ public class NodesSpecification {
         }
     }
 
-    private static Pair<NodeResources, NodeResources> nodeResourcesFromResorcesElement(ModelElement element) {
+    private static Pair<NodeResources, NodeResources> nodeResourcesFromResourcesElement(ModelElement element) {
         Pair<Double, Double> vcpu       = toRange(element.requiredStringAttribute("vcpu"),   .0, Double::parseDouble);
         Pair<Double, Double> memory     = toRange(element.requiredStringAttribute("memory"), .0, s -> parseGbAmount(s, "B"));
         Pair<Double, Double> disk       = toRange(element.requiredStringAttribute("disk"),   .0, s -> parseGbAmount(s, "B"));
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
index 4bd9f5fa8b0..8c8e8be5a30 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
@@ -25,6 +25,7 @@ import com.yahoo.config.provision.Capacity;
 import com.yahoo.config.provision.ClusterMembership;
 import com.yahoo.config.provision.ClusterResources;
 import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.NodeResources;
@@ -659,7 +660,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
             ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.container,
                                                           ClusterSpec.Id.from(cluster.getName()))
                                                  .vespaVersion(deployState.getWantedNodeVespaVersion())
-                                                 .dockerImageRepo(deployState.getWantedDockerImageRepo())
+                                                 .dockerImageRepository(deployState.getWantedDockerImageRepo().map(DockerImage::fromString))
                                                  .build();
             int nodeCount = deployState.zone().environment().isProduction() ? 2 : 1;
             Capacity capacity = Capacity.from(new ClusterResources(nodeCount, 1, NodeResources.unspecified),
@@ -691,7 +692,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         NodeType type = NodeType.valueOf(nodesElement.getAttribute("type"));
         ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(cluster.getName()))
                 .vespaVersion(context.getDeployState().getWantedNodeVespaVersion())
-                .dockerImageRepo(context.getDeployState().getWantedDockerImageRepo())
+                .dockerImageRepository(context.getDeployState().getWantedDockerImageRepo().map(DockerImage::fromString))
                 .build();
         Map<HostResource, ClusterMembership> hosts = 
                 cluster.getRoot().hostSystem().allocateHosts(clusterSpec,
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java
index 178bbac9e64..779b6854e21 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java
@@ -20,7 +20,7 @@ public class ClusterMembership {
 
     protected ClusterMembership() {}
 
-    private ClusterMembership(String stringValue, Version vespaVersion, Optional<String> dockerImageRepo) {
+    private ClusterMembership(String stringValue, Version vespaVersion, Optional<DockerImage> dockerImageRepo) {
         String[] components = stringValue.split("/");
         if (components.length < 4)
             throw new RuntimeException("Could not parse '" + stringValue + "' to a cluster membership. " +
@@ -44,7 +44,7 @@ public class ClusterMembership {
                 .vespaVersion(vespaVersion)
                 .exclusive(exclusive)
                 .combinedId(combinedId.map(ClusterSpec.Id::from))
-                .dockerImageRepo(dockerImageRepo)
+                .dockerImageRepository(dockerImageRepo)
                 .build();
         this.index = Integer.parseInt(components[3]);
         this.stringValue = toStringValue();
@@ -110,11 +110,12 @@ public class ClusterMembership {
     @Override
     public String toString() { return stringValue(); }
 
+    // TODO: Remove when 7.208 is the latest model in use
     public static ClusterMembership from(String stringValue, Version vespaVersion) {
         return new ClusterMembership(stringValue, vespaVersion, Optional.empty());
     }
 
-    public static ClusterMembership from(String stringValue, Version vespaVersion, Optional<String> dockerImageRepo) {
+    public static ClusterMembership from(String stringValue, Version vespaVersion, Optional<DockerImage> dockerImageRepo) {
         return new ClusterMembership(stringValue, vespaVersion, dockerImageRepo);
     }
 
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java
index f7aacbc757b..97549e851ad 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java
@@ -22,10 +22,10 @@ public final class ClusterSpec {
     private final Version vespaVersion;
     private boolean exclusive;
     private final Optional<Id> combinedId;
-    private final Optional<String> dockerImageRepo;
+    private final Optional<DockerImage> dockerImageRepo;
 
     private ClusterSpec(Type type, Id id, Optional<Group> groupId, Version vespaVersion, boolean exclusive,
-                        Optional<Id> combinedId, Optional<String> dockerImageRepo) {
+                        Optional<Id> combinedId, Optional<DockerImage> dockerImageRepo) {
         this.type = type;
         this.id = id;
         this.groupId = groupId;
@@ -36,6 +36,8 @@ public final class ClusterSpec {
             throw new IllegalArgumentException("combinedId must be empty for cluster of type " + type);
         }
         this.combinedId = combinedId;
+        if (dockerImageRepo.isPresent() && dockerImageRepo.get().tag().isPresent())
+            throw new IllegalArgumentException("dockerimageRepo is not allowed to have a tag");
         this.dockerImageRepo = dockerImageRepo;
     }
 
@@ -46,7 +48,7 @@ public final class ClusterSpec {
     public Id id() { return id; }
 
     /** Returns the docker image repository part of a docker image we want this cluster to run */
-    public Optional<String> dockerImageRepo() { return dockerImageRepo; }
+    public Optional<DockerImage> dockerImageRepo() { return dockerImageRepo; }
 
     /** Returns the docker image (repository + vespa version) we want this cluster to run */
     public Optional<String> dockerImage() { return dockerImageRepo.map(repo -> repo + ":" + vespaVersion.toFullString()); }
@@ -94,7 +96,7 @@ public final class ClusterSpec {
         private final boolean specification;
 
         private Optional<Group> groupId = Optional.empty();
-        private Optional<String> dockerImageRepo = Optional.empty();
+        private Optional<DockerImage> dockerImageRepo = Optional.empty();
         private Version vespaVersion;
         private boolean exclusive = false;
         private Optional<Id> combinedId = Optional.empty();
@@ -139,7 +141,14 @@ public final class ClusterSpec {
             return this;
         }
 
+        @Deprecated
+        // TODO: Remove after 7.208 is oldest version in use
         public Builder dockerImageRepo(Optional<String> dockerImageRepo) {
+            this.dockerImageRepo = dockerImageRepo.map(DockerImage::fromString);
+            return this;
+        }
+
+        public Builder dockerImageRepository(Optional<DockerImage> dockerImageRepo) {
             this.dockerImageRepo = dockerImageRepo;
             return this;
         }
@@ -163,7 +172,7 @@ public final class ClusterSpec {
         if ( ! other.id.equals(this.id)) return false;
         if ( ! other.groupId.equals(this.groupId)) return false;
         if ( ! other.vespaVersion.equals(this.vespaVersion)) return false;
-        if ( ! other.dockerImageRepo.orElse("").equals(this.dockerImageRepo.orElse(""))) return false;
+        if ( ! other.dockerImageRepo.equals(this.dockerImageRepo)) return false;
         return true;
     }
 
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java
index 56c5544f6d7..9ba26be072c 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java
@@ -87,7 +87,7 @@ public class AllocatedHostsSerializer {
             object.setString(hostSpecMembershipKey, membership.stringValue());
             object.setString(hostSpecVespaVersionKey, membership.cluster().vespaVersion().toFullString());
             membership.cluster().dockerImageRepo().ifPresent(repo -> {
-                object.setString(hostSpecDockerImageRepoKey, repo);
+                object.setString(hostSpecDockerImageRepoKey, repo.repository());
             });
         });
         host.flavor().ifPresent(flavor -> toSlime(flavor, object));
@@ -208,7 +208,7 @@ public class AllocatedHostsSerializer {
         return ClusterMembership.from(object.field(hostSpecMembershipKey).asString(),
                                       com.yahoo.component.Version.fromString(object.field(hostSpecVespaVersionKey).asString()),
                                       object.field(hostSpecDockerImageRepoKey).valid()
-                                              ? Optional.of(object.field(hostSpecDockerImageRepoKey).asString())
+                                              ? Optional.of(DockerImage.fromString(object.field(hostSpecDockerImageRepoKey).asString()))
                                               : Optional.empty());
     }
 
diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java
index c38d67ddc81..71e039f6e8e 100644
--- a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java
+++ b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java
@@ -42,7 +42,7 @@ public class ClusterMembershipTest {
             assertTrue(instance.cluster().dockerImageRepo().isEmpty());
         }
         {
-            Optional<String> dockerImageRepo = Optional.of("docker.foo.com:4443/vespa/bar");
+            Optional<DockerImage> dockerImageRepo = Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar"));
             ClusterMembership instance = ClusterMembership.from("combined/id1/4/37/exclusive/containerId1", Vtag.currentVersion, dockerImageRepo);
             ClusterMembership serialized = ClusterMembership.from(instance.stringValue(), Vtag.currentVersion, dockerImageRepo);
             assertEquals(instance, serialized);
diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java
index f2163608050..33157a01c07 100644
--- a/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java
+++ b/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java
@@ -41,7 +41,7 @@ public class AllocatedHostsSerializerTest {
                                List.of(),
                                Optional.empty(),
                                Optional.of(ClusterMembership.from("container/test/0/0", Version.fromString("6.73.1"),
-                                                                  Optional.of("docker.foo.com:4443/vespa/bar"))),
+                                                                  Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar")))),
                                Optional.empty(), Optional.empty(), Optional.empty(),
                                Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar"))));
         hosts.add(new HostSpec("flavor-from-resources-1",
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 94f4dab1245..2f834ab289f 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
@@ -187,7 +187,7 @@ public class NodeSerializer {
         object.setLong(currentRestartGenerationKey, allocation.restartGeneration().current());
         object.setBool(removableKey, allocation.isRemovable());
         object.setString(wantedVespaVersionKey, allocation.membership().cluster().vespaVersion().toString());
-        allocation.membership().cluster().dockerImageRepo().ifPresent(repo -> object.setString(wantedDockerImageRepoKey, repo));
+        allocation.membership().cluster().dockerImageRepo().ifPresent(repo -> object.setString(wantedDockerImageRepoKey, repo.repository()));
         allocation.networkPorts().ifPresent(ports -> NetworkPortsSerializer.toSlime(ports, object.setArray(networkPortsKey)));
     }
 
@@ -317,9 +317,9 @@ public class NodeSerializer {
         return Optional.of(Version.fromString(object.asString()));
     }
 
-    private Optional<String> dockerImageRepoFromSlime(Inspector object) {
+    private Optional<DockerImage> dockerImageRepoFromSlime(Inspector object) {
         if ( ! object.valid() || object.asString().isEmpty()) return Optional.empty();
-        return Optional.of(object.asString());
+        return Optional.of(DockerImage.fromString(object.asString()));
     }
 
     private Optional<DockerImage> dockerImageFromSlime(Inspector object) {