Merge pull request #14969 from vespa-engine/mpolden/container-registry

Let zone decide container registry

4 files changed, 109 insertions, 23 deletions

diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/DockerImage.java b/config-provisioning/src/main/java/com/yahoo/config/provision/DockerImage.java
index bbf65c1cd47..5ea2286c316 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/DockerImage.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/DockerImage.java
@@ -7,42 +7,63 @@ import java.util.Objects;
 import java.util.Optional;
 
 /**
- * A Docker image.
+ * A container image.
  *
  * @author mpolden
  */
+// TODO: Rename to ContainerImage. Compatibility with older config-models must be preserved.
 public class DockerImage {
 
-    public static final DockerImage EMPTY = new DockerImage("", Optional.empty());
+    public static final DockerImage EMPTY = new DockerImage("", "", Optional.empty());
 
+    private final String registry;
     private final String repository;
     private final Optional<String> tag;
 
-    private DockerImage(String repository, Optional<String> tag) {
+    DockerImage(String registry, String repository, Optional<String> tag) {
+        this.registry = Objects.requireNonNull(registry, "registry must be non-null");
         this.repository = Objects.requireNonNull(repository, "repository must be non-null");
         this.tag = Objects.requireNonNull(tag, "tag must be non-null");
     }
 
+    /** Returns the registry-part of this, i.e. the host/port of the registry. */
+    public String registry() {
+        return registry;
+    }
+
+    /** Returns the repository-part of this */
     public String repository() {
         return repository;
     }
 
+    /** Returns the registry and repository for this image, excluding its tag */
+    public String untagged() {
+        return new DockerImage(registry, repository, Optional.empty()).asString();
+    }
+
+    /** Returns this image's tag, if any */
     public Optional<String> tag() {
         return tag;
     }
 
-    /** Returns the tag as Version, {@link Version#emptyVersion} if tag is not set */
+    /** Returns the tag as a {@link Version}, {@link Version#emptyVersion} if tag is not set */
     public Version tagAsVersion() {
         return tag.map(Version::new).orElse(Version.emptyVersion);
     }
 
-    /** Returns the Docker image tagged with the given version */
+    /** Returns a copy of this tagged with the given version */
     public DockerImage withTag(Version version) {
-        return new DockerImage(repository, Optional.of(version.toFullString()));
+        return new DockerImage(registry, repository, Optional.of(version.toFullString()));
+    }
+
+    /** Returns a copy of this with registry set to given value */
+    public DockerImage withRegistry(String registry) {
+        return new DockerImage(registry, repository, tag);
     }
 
     public String asString() {
-        return repository + tag.map(t -> ':' + t).orElse("");
+        if (equals(EMPTY)) return "";
+        return registry + "/" + repository + tag.map(t -> ':' + t).orElse("");
     }
 
     @Override
@@ -55,25 +76,36 @@ public class DockerImage {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         DockerImage that = (DockerImage) o;
-        return repository.equals(that.repository) &&
-                tag.equals(that.tag);
+        return registry.equals(that.registry) &&
+               repository.equals(that.repository) &&
+               tag.equals(that.tag);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(repository, tag);
+        return Objects.hash(registry, repository, tag);
     }
 
-    public static DockerImage fromString(String name) {
-        if (name.isEmpty()) return EMPTY;
+    public static DockerImage from(String registry, String repository) {
+        return new DockerImage(registry, repository, Optional.empty());
+    }
+
+    public static DockerImage fromString(String s) {
+        if (s.isEmpty()) return EMPTY;
+
+        int firstPathSeparator = s.indexOf('/');
+        if (firstPathSeparator < 0) throw new IllegalArgumentException("Missing path separator in '" + s + "'");
 
-        int n = name.lastIndexOf(':');
-        if (n < 0) return new DockerImage(name, Optional.empty());
+        String registry = s.substring(0, firstPathSeparator);
+        String repository = s.substring(firstPathSeparator + 1);
+        if (repository.isEmpty()) throw new IllegalArgumentException("Repository must be non-empty in '" + s + "'");
 
-        String tag = name.substring(n + 1);
-        if (!tag.contains("/")) {
-            return new DockerImage(name.substring(0, n), Optional.of(tag));
-        }
-        return new DockerImage(name, Optional.empty());
+        int tagStart = repository.indexOf(':');
+        if (tagStart < 0) return new DockerImage(registry, repository, Optional.empty());
+
+        String tag = repository.substring(tagStart + 1);
+        repository = repository.substring(0, tagStart);
+        return new DockerImage(registry, repository, Optional.of(tag));
     }
+
 }
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 a071daa7427..137773ce8fe 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.repository());
+                object.setString(hostSpecDockerImageRepoKey, repo.untagged());
             });
         });
         toSlime(host.realResources(), object.setObject(realResourcesKey));
diff --git a/config-provisioning/src/main/resources/configdefinitions/config.provisioning.node-repository.def b/config-provisioning/src/main/resources/configdefinitions/config.provisioning.node-repository.def
index 864c226147d..6409bbd1966 100644
--- a/config-provisioning/src/main/resources/configdefinitions/config.provisioning.node-repository.def
+++ b/config-provisioning/src/main/resources/configdefinitions/config.provisioning.node-repository.def
@@ -1,9 +1,8 @@
 # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 namespace=config.provisioning
 
-# Docker image to use in REST API responses. This must be a fully qualified name, including registry, but excluding
-# version. Example: my-docker-registry.domain.tld:8080/dist/vespa
-dockerImage string default="dummyImage"
+# Default container image to use for nodes.
+containerImage string default="registry.example.com:9999/myorg/vespa"
 
 # Whether to cache data read from ZooKeeper in-memory.
 useCuratorClientCache bool default=false
diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/DockerImageTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/DockerImageTest.java
new file mode 100644
index 00000000000..ca41f4628cc
--- /dev/null
+++ b/config-provisioning/src/test/java/com/yahoo/config/provision/DockerImageTest.java
@@ -0,0 +1,55 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.config.provision;
+
+import org.junit.Test;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+/**
+ * @author mpolden
+ */
+public class DockerImageTest {
+
+    @Test
+    public void parse() {
+        Map<String, DockerImage> tests = Map.of(
+                "", DockerImage.EMPTY,
+                "registry.example.com:9999/vespa/vespa:7.42", new DockerImage("registry.example.com:9999", "vespa/vespa", Optional.of("7.42")),
+                "registry.example.com/vespa/vespa:7.42", new DockerImage("registry.example.com", "vespa/vespa", Optional.of("7.42")),
+                "registry.example.com:9999/vespa/vespa", new DockerImage("registry.example.com:9999", "vespa/vespa", Optional.empty()),
+                "registry.example.com/vespa/vespa", new DockerImage("registry.example.com", "vespa/vespa", Optional.empty())
+        );
+        tests.forEach((value, expected) -> {
+            DockerImage parsed = DockerImage.fromString(value);
+            assertEquals(value, parsed.asString());
+
+            String untagged = expected.equals(DockerImage.EMPTY)
+                    ? ""
+                    : expected.registry() + "/" + expected.repository();
+            assertEquals(untagged, parsed.untagged());
+        });
+    }
+
+    @Test
+    public void parse_invalid() {
+        List<String> tests = List.of(
+                "registry.example.com",
+                "registry.example.com/",
+                "foo",
+                "foo:1.2.3"
+        );
+        for (var value : tests) {
+            try {
+                DockerImage.fromString(value);
+                fail("Expected failure");
+            } catch (IllegalArgumentException ignored) {
+            }
+        }
+    }
+
+}