diff options
11 files changed, 109 insertions, 30 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..8350badc1fe 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,57 @@ import java.util.Objects; import java.util.Optional; /** - * A Docker image. + * A container image. * * @author mpolden */ 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 */ public DockerImage withTag(Version version) { - return new DockerImage(repository, Optional.of(version.toFullString())); + return new DockerImage(registry, repository, Optional.of(version.toFullString())); } 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 +70,32 @@ 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 fromString(String s) { + if (s.isEmpty()) return EMPTY; - int n = name.lastIndexOf(':'); - if (n < 0) return new DockerImage(name, Optional.empty()); + int firstPathSeparator = s.indexOf('/'); + if (firstPathSeparator < 0) throw new IllegalArgumentException("Missing path separator 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()); + 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 + "'"); + + 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/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) { + } + } + } + +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java index 2b3d3c3649a..74d123341f6 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java @@ -205,7 +205,7 @@ public class SessionZooKeeperClient { } public void writeDockerImageRepository(Optional<DockerImage> dockerImageRepository) { - dockerImageRepository.ifPresent(repo -> configCurator.putData(dockerImageRepositoryPath(), repo.repository())); + dockerImageRepository.ifPresent(repo -> configCurator.putData(dockerImageRepositoryPath(), repo.untagged())); } public Instant readCreateTime() { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java index 8860da1dca4..3af408b90f6 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java @@ -293,7 +293,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { @Test public void test_docker_image_repository() { long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); - String dockerImageRepository = "https://foo.bar.com:4443/baz"; + String dockerImageRepository = "foo.bar.com:4443/baz"; request(HttpRequest.Method.PUT, sessionId, Map.of("dockerImageRepository", dockerImageRepository, "applicationName", applicationId().application().value())); applicationRepository.activate(tenantRepository.getTenant(tenant), sessionId, timeoutBudget, false); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java index 2ac3785682c..d8f818f19d9 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java @@ -51,7 +51,7 @@ public class ZKApplicationPackageTest { ClusterMembership.from("container/test/0/0", Version.fromString("6.73.1"), Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar"))), Optional.of(Version.fromString("6.0.1")), Optional.empty(), - Optional.of(DockerImage.fromString("docker repo"))))); + Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar"))))); private ConfigCurator configCurator; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 9597068b606..6458c8e93b7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -544,12 +544,12 @@ public class InternalStepRunner implements StepRunner { private String wantedPlatform(Node node) { - return node.wantedDockerImage().repository() + ":" + node.wantedVersion(); + return node.wantedDockerImage().untagged() + ":" + node.wantedVersion(); } private String currentPlatform(Node node) { - String currentRepo = node.currentDockerImage().repository(); - String wantedRepo = node.wantedDockerImage().repository(); + String currentRepo = node.currentDockerImage().untagged(); + String wantedRepo = node.wantedDockerImage().untagged(); return (currentRepo.equals(wantedRepo) ? "" : currentRepo + ":") + node.currentVersion(); } diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerEngineTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerEngineTest.java index 34f6c99a00e..792955ed130 100644 --- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerEngineTest.java +++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerEngineTest.java @@ -75,7 +75,7 @@ public class DockerEngineTest { @Test @SuppressWarnings({"unchecked", "rawtypes"}) public void pullImageAsyncIfNeededSuccessfully() { - final DockerImage image = DockerImage.fromString("test:1.2.3"); + final DockerImage image = DockerImage.fromString("registry.example.com/test:1.2.3"); InspectImageResponse inspectImageResponse = mock(InspectImageResponse.class); when(inspectImageResponse.getId()).thenReturn(image.asString()); @@ -104,7 +104,7 @@ public class DockerEngineTest { @Test @SuppressWarnings({"unchecked", "rawtypes"}) public void pullImageAsyncIfNeededWithError() { - final DockerImage image = DockerImage.fromString("test:1.2.3"); + final DockerImage image = DockerImage.fromString("registry.example.com/test:1.2.3"); InspectImageCmd imageInspectCmd = mock(InspectImageCmd.class); when(imageInspectCmd.exec()).thenThrow(new NotFoundException("Image not found")); @@ -127,4 +127,5 @@ public class DockerEngineTest { assertFalse(docker.imageIsDownloaded(image)); assertTrue("Should return true, new pull scheduled", docker.pullImageAsyncIfNeeded(image)); } + } 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 f3c5158bc18..6a247365c52 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 @@ -191,7 +191,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.repository())); + allocation.membership().cluster().dockerImageRepo().ifPresent(repo -> object.setString(wantedDockerImageRepoKey, repo.untagged())); allocation.networkPorts().ifPresent(ports -> NetworkPortsSerializer.toSlime(ports, object.setArray(networkPortsKey))); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java index 3f55307d1f3..e89b0f9669d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java @@ -16,7 +16,8 @@ import java.util.concurrent.TimeUnit; import java.util.logging.Logger; /** - * Multithread safe class to get and set docker images for given node types. + * Multi-thread safe class to get and set container images for given node types. Images are stored in ZooKeeper so that + * nodes receive the same image from all config servers. * * @author freva */ diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java index 258e2c7cd42..01bf2f8a50e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java @@ -257,7 +257,7 @@ public class FailedExpirerTest { clock, zone, new MockNameResolver().mockAnyLookup(), - DockerImage.fromString("docker-image"), + DockerImage.fromString("registry.example.com/docker-image"), new InMemoryFlagSource(), true, false, |