diff options
38 files changed, 322 insertions, 235 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) { + } + } + } + +} 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/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index a598c5d30e0..9337adb01de 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -79,7 +79,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer private final Map<DeploymentId, ServiceConvergence> serviceStatus = new HashMap<>(); private final Set<ApplicationId> disallowConvergenceCheckApplications = new HashSet<>(); private final Version initialVersion = new Version(6, 1, 0); - private final DockerImage initialDockerImage = DockerImage.fromString("dockerImage:6.1.0"); + private final DockerImage initialDockerImage = DockerImage.fromString("registry.example.com/vespa/vespa:6.1.0"); private final Set<DeploymentId> suspendedApplications = new HashSet<>(); private final Map<ZoneId, Set<LoadBalancer>> loadBalancers = new HashMap<>(); private final Set<Environment> deferLoadBalancerProvisioning = new HashSet<>(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json index e4786cd4aea..835bca7966e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json @@ -33,7 +33,7 @@ { "at": 0, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 0, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json index b1ebf43ec02..32bee6c7e7e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json @@ -23,7 +23,7 @@ { "at": 14503000, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 14503000, @@ -38,7 +38,7 @@ { "at": 14503000, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 14503000, @@ -53,7 +53,7 @@ { "at": 14503000, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 14503000, @@ -92,7 +92,7 @@ { "at": 14503000, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 14503000, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json index ccfaf3341a2..0e7c100ea4e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json @@ -28,7 +28,7 @@ { "at": "(ignore)", "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": "(ignore)", @@ -43,7 +43,7 @@ { "at": "(ignore)", "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": "(ignore)", @@ -58,7 +58,7 @@ { "at": "(ignore)", "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": "(ignore)", @@ -73,7 +73,7 @@ { "at": "(ignore)", "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": "(ignore)", @@ -112,7 +112,7 @@ { "at": "(ignore)", "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": "(ignore)", @@ -127,7 +127,7 @@ { "at": "(ignore)", "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": "(ignore)", @@ -142,7 +142,7 @@ { "at": "(ignore)", "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": "(ignore)", @@ -157,7 +157,7 @@ { "at": "(ignore)", "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": "(ignore)", @@ -172,7 +172,7 @@ { "at": "(ignore)", "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": "(ignore)", @@ -187,7 +187,7 @@ { "at": "(ignore)", "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": "(ignore)", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json index dea43549396..edaa01326ad 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json @@ -23,7 +23,7 @@ { "at": 0, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 0, @@ -38,7 +38,7 @@ { "at": 0, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 0, @@ -53,7 +53,7 @@ { "at": 0, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 0, @@ -68,7 +68,7 @@ { "at": 0, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 0, @@ -107,7 +107,7 @@ { "at": 0, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 0, @@ -122,7 +122,7 @@ { "at": 0, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 0, @@ -137,7 +137,7 @@ { "at": 0, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 0, @@ -152,7 +152,7 @@ { "at": 0, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 0, @@ -167,7 +167,7 @@ { "at": 0, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 0, @@ -182,7 +182,7 @@ { "at": 0, "type": "info", - "message": "--- platform dockerImage:6.1" + "message": "--- platform registry.example.com/vespa/vespa:6.1" }, { "at": 0, @@ -308,4 +308,4 @@ "failed": 0 } } -}
\ No newline at end of file +} diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java index 1f2a35a2a38..0322059745d 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.dockerapi; import com.github.dockerjava.api.DockerClient; @@ -41,6 +41,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; import java.util.stream.Stream; public class DockerEngine implements ContainerEngine { @@ -320,20 +321,21 @@ public class DockerEngine implements ContainerEngine { } } - void deleteImage(DockerImage dockerImage) { + void deleteImage(String imageReference) { try { - dockerClient.removeImageCmd(dockerImage.asString()).exec(); + dockerClient.removeImageCmd(imageReference).exec(); } catch (NotFoundException ignored) { // Image was already deleted, ignore } catch (RuntimeException e) { numberOfDockerApiFails.increment(); - throw new DockerException("Failed to delete docker image " + dockerImage.asString(), e); + throw new DockerException("Failed to delete image by reference '" + imageReference + "'", e); } } @Override public boolean deleteUnusedDockerImages(List<DockerImage> excludes, Duration minImageAgeToDelete) { - return dockerImageGC.deleteUnusedDockerImages(excludes, minImageAgeToDelete); + List<String> excludedRefs = excludes.stream().map(DockerImage::asString).collect(Collectors.toList()); + return dockerImageGC.deleteUnusedDockerImages(excludedRefs, minImageAgeToDelete); } private class ImagePullCallback extends PullImageResultCallback { diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollector.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollector.java index b0bd1e3c65a..0560d84577a 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollector.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollector.java @@ -1,11 +1,10 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.dockerapi; import com.github.dockerjava.api.model.Container; import com.github.dockerjava.api.model.Image; import com.google.common.base.Strings; import com.yahoo.collections.Pair; -import com.yahoo.config.provision.DockerImage; import java.time.Clock; import java.time.Duration; @@ -68,11 +67,11 @@ class DockerImageGarbageCollector { /** * This method must be called frequently enough to see all containers to know which images are being used * - * @param excludes List of images (by tag or id) that should not be deleted regardless of their used status + * @param excludes List of image references (tag or id) that should not be deleted regardless of their used status * @param minImageAgeToDelete Minimum duration after which an image can be removed if it has not been used * @return true iff at least 1 image was deleted */ - boolean deleteUnusedDockerImages(List<DockerImage> excludes, Duration minImageAgeToDelete) { + boolean deleteUnusedDockerImages(List<String> excludes, Duration minImageAgeToDelete) { List<Image> images = docker.listAllImages(); List<Container> containers = docker.listAllContainers(); @@ -100,7 +99,7 @@ class DockerImageGarbageCollector { Set<String> imagesToKeep = Stream .concat( getRecentlyUsedImageIds(images, containers, minImageAgeToDelete).stream(), // 1 - dockerImageToImageIds(excludes, images).stream()) // 2 + referencesToImages(excludes, images).stream()) // 2 .flatMap(imageId -> ancestorsByImageId.getOrDefault(imageId, Collections.emptySet()).stream()) // 3 .collect(Collectors.toSet()); @@ -127,8 +126,8 @@ class DockerImageGarbageCollector { .peek(image -> { // Deleting an image by image ID with multiple tags will fail -> delete by tags instead referencesOf(image).forEach(imageReference -> { - logger.info("Deleting unused docker image " + imageReference); - docker.deleteImage(DockerImage.fromString(imageReference)); + logger.info("Deleting unused image " + imageReference); + docker.deleteImage(imageReference); }); lastTimeUsedByImageId.remove(image.getId()); }) @@ -152,20 +151,20 @@ class DockerImageGarbageCollector { } /** - * Attemps to make dockerImages which may be image tags or image ids to image ids. This only works - * if the given tag is actually present locally. This is fine, because if it isn't - we can't delete + * Map given references (image tags or ids) to images. + * + * This only works if the given tag is actually present locally. This is fine, because if it isn't - we can't delete * it, so no harm done. */ - private Set<String> dockerImageToImageIds(List<DockerImage> dockerImages, List<Image> images) { + private Set<String> referencesToImages(List<String> references, List<Image> images) { Map<String, String> imageIdByImageTag = images.stream() .flatMap(image -> referencesOf(image).stream() .map(repoTag -> new Pair<>(repoTag, image.getId()))) .collect(Collectors.toMap(Pair::getFirst, Pair::getSecond)); - return dockerImages.stream() - .map(DockerImage::asString) - .map(tag -> imageIdByImageTag.getOrDefault(tag, tag)) - .collect(Collectors.toSet()); + return references.stream() + .map(ref -> imageIdByImageTag.getOrDefault(ref, ref)) + .collect(Collectors.toUnmodifiableSet()); } /** 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/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollectionTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollectionTest.java index a5dfd91051f..c725b0642c9 100644 --- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollectionTest.java +++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollectionTest.java @@ -1,11 +1,10 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.dockerapi; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.github.dockerjava.api.model.Image; -import com.yahoo.config.provision.DockerImage; import com.yahoo.test.ManualClock; import org.junit.Test; @@ -147,7 +146,7 @@ public class DockerImageGarbageCollectionTest { ImageBuilder.forId("image-2").withParentId("parent-image")) // Normally, image-1 and parent-image should also be deleted, but because we exclude image-1 // we cannot delete parent-image, so only image-2 is deleted - .expectDeletedImages(Collections.singletonList("image-1"), "image-2"); + .expectDeletedImages(List.of("image-1"), "image-2"); } /** Same as in {@link #doesNotDeleteExcludedByIdImages()} but with tags */ @@ -157,7 +156,7 @@ public class DockerImageGarbageCollectionTest { ImageBuilder.forId("parent-image").withTags("rhel-6"), ImageBuilder.forId("image-1").withParentId("parent-image").withTags("vespa:6.288.16"), ImageBuilder.forId("image-2").withParentId("parent-image").withTags("vespa:6.289.94")) - .expectDeletedImages(Collections.singletonList("vespa:6.288.16"), "vespa:6.289.94"); + .expectDeletedImages(List.of("vespa:6.288.16"), "vespa:6.289.94"); } @Test @@ -166,14 +165,14 @@ public class DockerImageGarbageCollectionTest { ImageBuilder.forId("parent-image").withTags("rhel-6"), ImageBuilder.forId("image-1").withParentId("parent-image").withTags("vespa:6.288.16"), ImageBuilder.forId("image-2").withParentId("parent-image").withTags("vespa:6.289.94")) - .expectDeletedImages(Collections.singletonList("vespa:6.300.1"), "vespa:6.288.16", "vespa:6.289.94", "rhel-6"); + .expectDeletedImages(List.of("vespa:6.300.1"), "vespa:6.288.16", "vespa:6.289.94", "rhel-6"); } private class ImageGcTester { private final DockerEngine docker = mock(DockerEngine.class); private final ManualClock clock = new ManualClock(); private final DockerImageGarbageCollector imageGC = new DockerImageGarbageCollector(docker, clock); - private final Map<DockerImage, Integer> numDeletes = new HashMap<>(); + private final Map<String, Integer> numDeletes = new HashMap<>(); private boolean initialized = false; private ImageGcTester withExistingImages(ImageBuilder... images) { @@ -210,17 +209,14 @@ public class DockerImageGarbageCollectionTest { clock.advance(Duration.ofMinutes(minutesAfter)); - imageGC.deleteUnusedDockerImages( - except.stream().map(DockerImage::fromString).collect(Collectors.toList()), - Duration.ofHours(1).minusSeconds(1)); + imageGC.deleteUnusedDockerImages(except, Duration.ofHours(1).minusSeconds(1)); Arrays.stream(imageIds) - .map(DockerImage::fromString) - .forEach(image -> { - int newValue = numDeletes.getOrDefault(image, 0) + 1; - numDeletes.put(image, newValue); - verify(docker, times(newValue)).deleteImage(eq(image)); - }); + .forEach(imageId -> { + int newValue = numDeletes.getOrDefault(imageId, 0) + 1; + numDeletes.put(imageId, newValue); + verify(docker, times(newValue)).deleteImage(eq(imageId)); + }); verify(docker, times(numDeletes.values().stream().mapToInt(i -> i).sum())).deleteImage(any()); return this; diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java index 2e40596bcf0..6d26e16f314 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java @@ -137,7 +137,7 @@ public class RealNodeRepositoryTest { hostname, new NodeAttributes() .withRestartGeneration(1) - .withDockerImage(DockerImage.fromString("image-1:6.2.3"))); + .withDockerImage(DockerImage.fromString("registry.example.com/image-1:6.2.3"))); } @Test diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImplTest.java index d01b42ba48c..4bbe7354ecb 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImplTest.java @@ -83,7 +83,7 @@ public class ContainerOperationsImplTest { } private Container makeContainer(String name, Container.State state, int pid) { - final Container container = new Container(name + ".fqdn", DockerImage.fromString("mock"), null, + final Container container = new Container(name + ".fqdn", DockerImage.fromString("registry.example.com/mock"), null, new ContainerName(name), state, pid); when(containerEngine.getContainer(eq(container.name))).thenReturn(Optional.of(container)); return container; diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java index 5688fcace6f..88932f221a6 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java @@ -19,7 +19,7 @@ public class DockerFailTest { @Test public void dockerFailTest() { try (DockerTester tester = new DockerTester()) { - final DockerImage dockerImage = DockerImage.fromString("dockerImage"); + final DockerImage dockerImage = DockerImage.fromString("registry.example.com/dockerImage"); final ContainerName containerName = new ContainerName("host1"); final String hostname = "host1.test.yahoo.com"; tester.addChildNodeRepositoryNode(NodeSpec.Builder diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java index 6f85703a4d5..84089d94d45 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java @@ -20,9 +20,10 @@ public class MultiDockerTest { @Test public void test() { try (DockerTester tester = new DockerTester()) { - addAndWaitForNode(tester, "host1.test.yahoo.com", DockerImage.fromString("image1")); + DockerImage image1 = DockerImage.fromString("registry.example.com/image1"); + addAndWaitForNode(tester, "host1.test.yahoo.com", image1); NodeSpec nodeSpec2 = addAndWaitForNode( - tester, "host2.test.yahoo.com", DockerImage.fromString("image2")); + tester, "host2.test.yahoo.com", DockerImage.fromString("registry.example.com/image2")); tester.addChildNodeRepositoryNode(NodeSpec.Builder.testSpec(nodeSpec2.hostname(), NodeState.dirty).build()); @@ -31,7 +32,7 @@ public class MultiDockerTest { argThat(context -> context.containerName().equals(new ContainerName("host2")))); tester.inOrder(tester.nodeRepository).setNodeState(eq(nodeSpec2.hostname()), eq(NodeState.ready)); - addAndWaitForNode(tester, "host3.test.yahoo.com", DockerImage.fromString("image1")); + addAndWaitForNode(tester, "host3.test.yahoo.com", image1); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RebootTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RebootTest.java index 6c69453ac71..d1bc6fc0daa 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RebootTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RebootTest.java @@ -23,7 +23,7 @@ import static org.mockito.ArgumentMatchers.eq; public class RebootTest { private final String hostname = "host1.test.yahoo.com"; - private final DockerImage dockerImage = DockerImage.fromString("dockerImage"); + private final DockerImage dockerImage = DockerImage.fromString("registry.example.com/dockerImage"); @Test public void test() { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RestartTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RestartTest.java index 6f887fb1179..645a89cc20e 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RestartTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RestartTest.java @@ -22,7 +22,7 @@ public class RestartTest { public void test() { try (DockerTester tester = new DockerTester()) { String hostname = "host1.test.yahoo.com"; - DockerImage dockerImage = DockerImage.fromString("dockerImage:1.2.3"); + DockerImage dockerImage = DockerImage.fromString("registry.example.com/dockerImage:1.2.3"); tester.addChildNodeRepositoryNode(NodeSpec.Builder.testSpec(hostname).wantedDockerImage(dockerImage).build()); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 7880209bbc8..ea5d7ac7529 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -55,7 +55,7 @@ public class NodeAgentImplTest { private static final String hostName = "host1.test.yahoo.com"; private final NodeAgentContextSupplier contextSupplier = mock(NodeAgentContextSupplier.class); - private final DockerImage dockerImage = DockerImage.fromString("dockerImage"); + private final DockerImage dockerImage = DockerImage.fromString("registry.example.com/dockerImage"); private final ContainerOperations containerOperations = mock(ContainerOperations.class); private final NodeRepository nodeRepository = mock(NodeRepository.class); private final Orchestrator orchestrator = mock(Orchestrator.class); @@ -175,7 +175,7 @@ public class NodeAgentImplTest { @Test public void containerIsNotStoppedIfNewImageMustBePulled() { - final DockerImage newDockerImage = DockerImage.fromString("new-image"); + final DockerImage newDockerImage = DockerImage.fromString("registry.example.com/new-image"); final NodeSpec node = nodeBuilder(NodeState.active) .wantedDockerImage(newDockerImage).currentDockerImage(dockerImage) .wantedVespaVersion(vespaVersion).currentVespaVersion(vespaVersion) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index f5c5db45f4a..0136c9bcd3b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -39,7 +39,7 @@ import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient; import com.yahoo.vespa.hosted.provision.persistence.DnsNameResolver; import com.yahoo.vespa.hosted.provision.persistence.JobControlFlags; import com.yahoo.vespa.hosted.provision.persistence.NameResolver; -import com.yahoo.vespa.hosted.provision.provisioning.DockerImages; +import com.yahoo.vespa.hosted.provision.provisioning.ContainerImages; import com.yahoo.vespa.hosted.provision.provisioning.FirmwareChecks; import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; import com.yahoo.vespa.hosted.provision.provisioning.ProvisionServiceProvider; @@ -107,7 +107,7 @@ public class NodeRepository extends AbstractComponent { private final OsVersions osVersions; private final InfrastructureVersions infrastructureVersions; private final FirmwareChecks firmwareChecks; - private final DockerImages dockerImages; + private final ContainerImages containerImages; private final JobControl jobControl; private final Applications applications; private final int spareCount; @@ -129,7 +129,7 @@ public class NodeRepository extends AbstractComponent { Clock.systemUTC(), zone, new DnsNameResolver(), - DockerImage.fromString(config.dockerImage()), + DockerImage.fromString(config.containerImage()), flagSource, config.useCuratorClientCache(), zone.environment().isProduction() && !zone.getCloud().dynamicProvisioning() ? 1 : 0, @@ -166,7 +166,7 @@ public class NodeRepository extends AbstractComponent { this.osVersions = new OsVersions(this); this.infrastructureVersions = new InfrastructureVersions(db); this.firmwareChecks = new FirmwareChecks(db, clock); - this.dockerImages = new DockerImages(db, dockerImage); + this.containerImages = new ContainerImages(db, dockerImage); this.jobControl = new JobControl(new JobControlFlags(db, flagSource)); this.applications = new Applications(db); this.spareCount = spareCount; @@ -190,9 +190,6 @@ public class NodeRepository extends AbstractComponent { /** Returns the curator database client used by this */ public CuratorDatabaseClient database() { return db; } - /** Returns the Docker image to use for given node */ - public DockerImage dockerImage(Node node) { return dockerImages.dockerImageFor(node.type()); } - /** @return The name resolver used to resolve hostname and ip addresses */ public NameResolver nameResolver() { return nameResolver; } @@ -206,7 +203,7 @@ public class NodeRepository extends AbstractComponent { public FirmwareChecks firmwareChecks() { return firmwareChecks; } /** Returns the docker images to use for nodes in this. */ - public DockerImages dockerImages() { return dockerImages; } + public ContainerImages containerImages() { return containerImages; } /** Returns the status of maintenance jobs managed by this. */ public JobControl jobControl() { return jobControl; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index 91c683d139e..e6564b52216 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -66,7 +66,7 @@ public class CuratorDatabaseClient { private static final Path inactiveJobsPath = root.append("inactiveJobs"); private static final Path infrastructureVersionsPath = root.append("infrastructureVersions"); private static final Path osVersionsPath = root.append("osVersions"); - private static final Path dockerImagesPath = root.append("dockerImages"); + private static final Path containerImagesPath = root.append("dockerImages"); private static final Path firmwareCheckPath = root.append("firmwareCheck"); private static final Duration defaultLockTimeout = Duration.ofMinutes(2); @@ -99,7 +99,7 @@ public class CuratorDatabaseClient { db.create(inactiveJobsPath); db.create(infrastructureVersionsPath); db.create(osVersionsPath); - db.create(dockerImagesPath); + db.create(containerImagesPath); db.create(firmwareCheckPath); db.create(loadBalancersPath); provisionIndexCounter.initialize(100); @@ -432,21 +432,21 @@ public class CuratorDatabaseClient { return db.lock(lockPath.append("osVersionsLock"), defaultLockTimeout); } - // Docker images ----------------------------------------------------------- + // Container images ----------------------------------------------------------- - public Map<NodeType, DockerImage> readDockerImages() { - return read(dockerImagesPath, NodeTypeDockerImagesSerializer::fromJson).orElseGet(TreeMap::new); + public Map<NodeType, DockerImage> readContainerImages() { + return read(containerImagesPath, NodeTypeContainerImagesSerializer::fromJson).orElseGet(TreeMap::new); } - public void writeDockerImages(Map<NodeType, DockerImage> dockerImages) { + public void writeContainerImages(Map<NodeType, DockerImage> images) { NestedTransaction transaction = new NestedTransaction(); CuratorTransaction curatorTransaction = db.newCuratorTransactionIn(transaction); - curatorTransaction.add(CuratorOperations.setData(dockerImagesPath.getAbsolute(), - NodeTypeDockerImagesSerializer.toJson(dockerImages))); + curatorTransaction.add(CuratorOperations.setData(containerImagesPath.getAbsolute(), + NodeTypeContainerImagesSerializer.toJson(images))); transaction.commit(); } - public Lock lockDockerImages() { + public Lock lockContainerImages() { return db.lock(lockPath.append("dockerImagesLock"), defaultLockTimeout); } 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/persistence/NodeTypeDockerImagesSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeContainerImagesSerializer.java index 6615dff24e5..058b5a45d8c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeDockerImagesSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeContainerImagesSerializer.java @@ -19,9 +19,9 @@ import java.util.TreeMap; * * @author freva */ -public class NodeTypeDockerImagesSerializer { +public class NodeTypeContainerImagesSerializer { - private NodeTypeDockerImagesSerializer() {} + private NodeTypeContainerImagesSerializer() {} public static byte[] toJson(Map<NodeType, DockerImage> dockerImages) { Slime slime = new Slime(); @@ -36,11 +36,11 @@ public class NodeTypeDockerImagesSerializer { } public static Map<NodeType, DockerImage> fromJson(byte[] data) { - Map<NodeType, DockerImage> dockerImages = new TreeMap<>(); // Use TreeMap to sort by node type + Map<NodeType, DockerImage> images = new TreeMap<>(); // Use TreeMap to sort by node type Inspector inspector = SlimeUtils.jsonToSlime(data).get(); inspector.traverse((ObjectTraverser) (key, value) -> - dockerImages.put(NodeSerializer.nodeTypeFromString(key), DockerImage.fromString(value.asString()))); - return dockerImages; + images.put(NodeSerializer.nodeTypeFromString(key), DockerImage.fromString(value.asString()))); + return images; } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImages.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImages.java new file mode 100644 index 00000000000..92518239258 --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImages.java @@ -0,0 +1,87 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.provisioning; + +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; +import com.yahoo.config.provision.DockerImage; +import com.yahoo.config.provision.NodeType; +import com.yahoo.vespa.curator.Lock; +import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient; + +import java.time.Duration; +import java.util.Collections; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.TimeUnit; +import java.util.logging.Logger; + +/** + * 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 + */ +public class ContainerImages { + + private static final Duration defaultCacheTtl = Duration.ofMinutes(1); + private static final Logger log = Logger.getLogger(ContainerImages.class.getName()); + + private final CuratorDatabaseClient db; + private final DockerImage defaultImage; + private final Duration cacheTtl; + + /** + * The container image is read on every request to /nodes/v2/node/[fqdn]. Cache current images to avoid + * unnecessary ZK reads. When images change, some nodes may need to wait for TTL until they see the new image, + * this is fine. + */ + private volatile Supplier<Map<NodeType, DockerImage>> images; + + public ContainerImages(CuratorDatabaseClient db, DockerImage defaultImage) { + this(db, defaultImage, defaultCacheTtl); + } + + ContainerImages(CuratorDatabaseClient db, DockerImage defaultImage, Duration cacheTtl) { + this.db = db; + this.defaultImage = defaultImage; + this.cacheTtl = cacheTtl; + createCache(); + } + + private void createCache() { + this.images = Suppliers.memoizeWithExpiration(() -> Collections.unmodifiableMap(db.readContainerImages()), + cacheTtl.toMillis(), TimeUnit.MILLISECONDS); + } + + /** Returns the current images for each node type */ + public Map<NodeType, DockerImage> getImages() { + return images.get(); + } + + /** Returns the current docker image for given node type, or the type for corresponding child nodes + * if it is a Docker host, or default */ + public DockerImage imageFor(NodeType type) { + NodeType typeToUseForLookup = type.isHost() ? type.childNodeType() : type; + DockerImage image = getImages().get(typeToUseForLookup); + if (image == null) { + return defaultImage; + } + return image.withRegistry(defaultImage.registry()); // Always use the registry configured for this zone. + } + + /** Set the docker image for nodes of given type */ + public void setImage(NodeType nodeType, Optional<DockerImage> image) { + if (nodeType.isHost()) { + throw new IllegalArgumentException("Setting container image for " + nodeType + " nodes is unsupported"); + } + try (Lock lock = db.lockContainerImages()) { + Map<NodeType, DockerImage> images = db.readContainerImages(); + image.ifPresentOrElse(img -> images.put(nodeType, img), + () -> images.remove(nodeType)); + db.writeContainerImages(images); + createCache(); // Throw away current cache + log.info("Set container image for " + nodeType + " nodes to " + image.map(DockerImage::asString).orElse(null)); + } + } + +} 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 deleted file mode 100644 index 3f55307d1f3..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.provisioning; - -import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import com.yahoo.config.provision.DockerImage; -import com.yahoo.config.provision.NodeType; -import com.yahoo.vespa.curator.Lock; -import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient; - -import java.time.Duration; -import java.util.Collections; -import java.util.Map; -import java.util.Optional; -import java.util.concurrent.TimeUnit; -import java.util.logging.Logger; - -/** - * Multithread safe class to get and set docker images for given node types. - * - * @author freva - */ -public class DockerImages { - - private static final Duration defaultCacheTtl = Duration.ofMinutes(1); - private static final Logger log = Logger.getLogger(DockerImages.class.getName()); - - private final CuratorDatabaseClient db; - private final DockerImage defaultImage; - private final Duration cacheTtl; - - /** - * Docker image is read on every request to /nodes/v2/node/[fqdn]. Cache current getDockerImages to avoid - * unnecessary ZK reads. When getDockerImages change, some nodes may need to wait for TTL until they see the new target, - * this is fine. - */ - private volatile Supplier<Map<NodeType, DockerImage>> dockerImages; - - public DockerImages(CuratorDatabaseClient db, DockerImage defaultImage) { - this(db, defaultImage, defaultCacheTtl); - } - - DockerImages(CuratorDatabaseClient db, DockerImage defaultImage, Duration cacheTtl) { - this.db = db; - this.defaultImage = defaultImage; - this.cacheTtl = cacheTtl; - createCache(); - } - - private void createCache() { - this.dockerImages = Suppliers.memoizeWithExpiration(() -> Collections.unmodifiableMap(db.readDockerImages()), - cacheTtl.toMillis(), TimeUnit.MILLISECONDS); - } - - /** Returns the current docker images for each node type */ - public Map<NodeType, DockerImage> getDockerImages() { - return dockerImages.get(); - } - - /** Returns the current docker image for given node type, or the type for corresponding child nodes - * if it is a Docker host, or default */ - public DockerImage dockerImageFor(NodeType type) { - NodeType typeToUseForLookup = type.isHost() ? type.childNodeType() : type; - return getDockerImages().getOrDefault(typeToUseForLookup, defaultImage); - } - - /** Set the docker image for nodes of given type */ - public void setDockerImage(NodeType nodeType, Optional<DockerImage> dockerImage) { - if (nodeType.isHost()) { - throw new IllegalArgumentException("Setting docker image for " + nodeType + " nodes is unsupported"); - } - try (Lock lock = db.lockDockerImages()) { - Map<NodeType, DockerImage> dockerImages = db.readDockerImages(); - dockerImage.ifPresentOrElse(image -> dockerImages.put(nodeType, image), - () -> dockerImages.remove(nodeType)); - db.writeDockerImages(dockerImages); - createCache(); // Throw away current cache - log.info("Set docker image for " + nodeType + " nodes to " + dockerImage.map(DockerImage::asString).orElse(null)); - } - } - -} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java index 0137bee5fbd..47ee1be7651 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java @@ -158,7 +158,7 @@ class NodesResponse extends HttpResponse { object.setLong("restartGeneration", allocation.restartGeneration().wanted()); object.setLong("currentRestartGeneration", allocation.restartGeneration().current()); object.setString("wantedDockerImage", allocation.membership().cluster().dockerImage() - .orElseGet(() -> nodeRepository.dockerImage(node).withTag(allocation.membership().cluster().vespaVersion()).asString())); + .orElseGet(() -> nodeRepository.containerImages().imageFor(node.type()).withTag(allocation.membership().cluster().vespaVersion()).asString())); object.setString("wantedVespaVersion", allocation.membership().cluster().vespaVersion().toFullString()); NodeResourcesSerializer.toSlime(allocation.requestedResources(), object.setObject("requestedResources")); allocation.networkPorts().ifPresent(ports -> NetworkPortsSerializer.toSlime(ports, object.setArray("networkPorts"))); @@ -222,7 +222,7 @@ class NodesResponse extends HttpResponse { .or(() -> Optional.of(node) .filter(n -> n.flavor().getType() != Flavor.Type.DOCKER_CONTAINER) .flatMap(n -> n.status().vespaVersion() - .map(version -> nodeRepository.dockerImage(n).withTag(version)))); + .map(version -> nodeRepository.containerImages().imageFor(n.type()).withTag(version)))); } private void ipAddressesToSlime(Set<String> ipAddresses, Cursor array) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java index a2d599eab6e..5d19271d2a7 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java @@ -118,7 +118,7 @@ public class NodesV2ApiHandler extends LoggingRequestHandler { if (pathS.equals( "/nodes/v2/command/")) return new ResourceResponse(request.getUri(), "restart", "reboot"); if (pathS.equals( "/nodes/v2/locks")) return new LocksResponse(); if (pathS.equals( "/nodes/v2/maintenance/")) return new JobsResponse(nodeRepository.jobControl()); - if (pathS.equals( "/nodes/v2/upgrade/")) return new UpgradeResponse(nodeRepository.infrastructureVersions(), nodeRepository.osVersions(), nodeRepository.dockerImages()); + if (pathS.equals( "/nodes/v2/upgrade/")) return new UpgradeResponse(nodeRepository.infrastructureVersions(), nodeRepository.osVersions(), nodeRepository.containerImages()); if (pathS.startsWith("/nodes/v2/capacity")) return new HostCapacityResponse(nodeRepository, request); if (path.matches("/nodes/v2/application")) return applicationList(request.getUri()); if (path.matches("/nodes/v2/application/{applicationId}")) return application(path.get("applicationId"), request.getUri()); @@ -363,7 +363,7 @@ public class NodesV2ApiHandler extends LoggingRequestHandler { boolean force = inspector.field("force").asBool(); Inspector versionField = inspector.field("version"); Inspector osVersionField = inspector.field("osVersion"); - Inspector dockerImageField = inspector.field("dockerImage"); + Inspector containerImageField = inspector.field("dockerImage"); Inspector upgradeBudgetField = inspector.field("upgradeBudget"); if (versionField.valid()) { @@ -395,12 +395,12 @@ public class NodesV2ApiHandler extends LoggingRequestHandler { } } - if (dockerImageField.valid()) { - Optional<DockerImage> dockerImage = Optional.of(dockerImageField.asString()) + if (containerImageField.valid()) { + Optional<DockerImage> dockerImage = Optional.of(containerImageField.asString()) .filter(s -> !s.isEmpty()) .map(DockerImage::fromString); - nodeRepository.dockerImages().setDockerImage(nodeType, dockerImage); - messageParts.add("docker image to " + dockerImage.map(DockerImage::asString).orElse(null)); + nodeRepository.containerImages().setImage(nodeType, dockerImage); + messageParts.add("container image to " + dockerImage.map(DockerImage::asString).orElse(null)); } if (messageParts.isEmpty()) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/UpgradeResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/UpgradeResponse.java index 1082e9cce60..b34f1b1a875 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/UpgradeResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/UpgradeResponse.java @@ -6,7 +6,7 @@ import com.yahoo.slime.Cursor; import com.yahoo.slime.JsonFormat; import com.yahoo.slime.Slime; import com.yahoo.vespa.hosted.provision.maintenance.InfrastructureVersions; -import com.yahoo.vespa.hosted.provision.provisioning.DockerImages; +import com.yahoo.vespa.hosted.provision.provisioning.ContainerImages; import com.yahoo.vespa.hosted.provision.os.OsVersions; import java.io.IOException; @@ -21,13 +21,13 @@ public class UpgradeResponse extends HttpResponse { private final InfrastructureVersions infrastructureVersions; private final OsVersions osVersions; - private final DockerImages dockerImages; + private final ContainerImages containerImages; - public UpgradeResponse(InfrastructureVersions infrastructureVersions, OsVersions osVersions, DockerImages dockerImages) { + public UpgradeResponse(InfrastructureVersions infrastructureVersions, OsVersions osVersions, ContainerImages containerImages) { super(200); this.infrastructureVersions = infrastructureVersions; this.osVersions = osVersions; - this.dockerImages = dockerImages; + this.containerImages = containerImages; } @Override @@ -43,7 +43,7 @@ public class UpgradeResponse extends HttpResponse { Cursor dockerImagesObject = root.setObject("dockerImages"); - dockerImages.getDockerImages().forEach((nodeType, image) -> dockerImagesObject.setString(nodeType.name(), image.asString())); + containerImages.getImages().forEach((nodeType, image) -> dockerImagesObject.setString(nodeType.name(), image.asString())); new JsonFormat(true).encode(stream, slime); } 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 e9f1f8442c7..0a78ead5ca9 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, 0, 1000); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeDockerImagesSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeContainerImagesSerializerTest.java index 9bfcf67324a..4d4669f0b42 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeDockerImagesSerializerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeContainerImagesSerializerTest.java @@ -13,7 +13,7 @@ import static org.junit.Assert.assertEquals; /** * @author freva */ -public class NodeTypeDockerImagesSerializerTest { +public class NodeTypeContainerImagesSerializerTest { @Test public void test_serialization() { @@ -21,7 +21,7 @@ public class NodeTypeDockerImagesSerializerTest { images.put(NodeType.host, DockerImage.fromString("docker.domain.tld/my/repo:1.2.3")); images.put(NodeType.confighost, DockerImage.fromString("docker.domain.tld/my/image:2.1")); - Map<NodeType, DockerImage> serialized = NodeTypeDockerImagesSerializer.fromJson(NodeTypeDockerImagesSerializer.toJson(images)); + Map<NodeType, DockerImage> serialized = NodeTypeContainerImagesSerializer.fromJson(NodeTypeContainerImagesSerializer.toJson(images)); assertEquals(images, serialized); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImagesTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImagesTest.java index d5437296620..320feed5d66 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImagesTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImagesTest.java @@ -14,7 +14,7 @@ import static org.junit.Assert.assertEquals; /** * @author mpolden */ -public class DockerImagesTest { +public class ContainerImagesTest { @Test public void image_selection() { @@ -22,14 +22,14 @@ public class DockerImagesTest { var tester = new ProvisioningTester.Builder().flagSource(flagSource).build(); var proxyImage = DockerImage.fromString("docker-registry.domain.tld:8080/dist/proxy"); - tester.nodeRepository().dockerImages().setDockerImage(NodeType.proxy, Optional.of(proxyImage)); + tester.nodeRepository().containerImages().setImage(NodeType.proxy, Optional.of(proxyImage)); // Host uses tenant default image (for preload purposes) var defaultImage = DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"); var hosts = tester.makeReadyNodes(2, "default", NodeType.host); tester.activateTenantHosts(); for (var host : hosts) { - assertEquals(defaultImage, tester.nodeRepository().dockerImages().dockerImageFor(host.type())); + assertEquals(defaultImage, tester.nodeRepository().containerImages().imageFor(host.type())); } // Tenant node uses tenant default image @@ -37,14 +37,14 @@ public class DockerImagesTest { for (var host : hosts) { var nodes = tester.makeReadyVirtualDockerNodes(2, resources, host.hostname()); for (var node : nodes) { - assertEquals(defaultImage, tester.nodeRepository().dockerImages().dockerImageFor(node.type())); + assertEquals(defaultImage, tester.nodeRepository().containerImages().imageFor(node.type())); } } // Proxy host uses image used by child nodes (proxy nodes), which is overridden in this case (for preload purposes) var proxyHosts = tester.makeReadyNodes(2, "default", NodeType.proxyhost); for (var host : proxyHosts) { - assertEquals(proxyImage, tester.nodeRepository().dockerImages().dockerImageFor(host.type())); + assertEquals(proxyImage, tester.nodeRepository().containerImages().imageFor(host.type())); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java index 9f6f5043ae8..3d2ac1329c4 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java @@ -229,7 +229,7 @@ public class NodesV2ApiTest { Utf8.toBytes("{\"modelName\": null}"), Request.Method.PATCH), "{\"message\":\"Updated dockerhost1.yahoo.com\"}"); tester.assertPartialResponse(new Request("http://localhost:8080/nodes/v2/node/dockerhost1.yahoo.com"), "modelName", false); - tester.container().handleRequest((new Request("http://localhost:8080/nodes/v2/upgrade/tenant", Utf8.toBytes("{\"dockerImage\": \"docker.domain.tld/my/image\"}"), Request.Method.PATCH))); + tester.container().handleRequest((new Request("http://localhost:8080/nodes/v2/upgrade/tenant", Utf8.toBytes("{\"dockerImage\": \"ignored-registry.example.com/my/image\"}"), Request.Method.PATCH))); ((OrchestratorMock) tester.container().components().getComponent(OrchestratorMock.class.getName())) .suspend(new HostName("host4.yahoo.com")); @@ -738,25 +738,25 @@ public class NodesV2ApiTest { 200, "{\"message\":\"Set osVersion to null for nodes of type confighost\"}"); - // Set docker image for config and tenant + // Set container image for config and tenant assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/tenant", Utf8.toBytes("{\"dockerImage\": \"my-repo.my-domain.example:1234/repo/tenant\"}"), Request.Method.PATCH), - "{\"message\":\"Set docker image to my-repo.my-domain.example:1234/repo/tenant for nodes of type tenant\"}"); + "{\"message\":\"Set container image to my-repo.my-domain.example:1234/repo/tenant for nodes of type tenant\"}"); assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/config", Utf8.toBytes("{\"dockerImage\": \"my-repo.my-domain.example:1234/repo/image\"}"), Request.Method.PATCH), - "{\"message\":\"Set docker image to my-repo.my-domain.example:1234/repo/image for nodes of type config\"}"); + "{\"message\":\"Set container image to my-repo.my-domain.example:1234/repo/image for nodes of type config\"}"); assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/"), "{\"versions\":{\"config\":\"6.123.456\",\"confighost\":\"6.124.42\",\"controller\":\"6.123.456\"},\"osVersions\":{\"host\":\"7.5.2\"},\"dockerImages\":{\"tenant\":\"my-repo.my-domain.example:1234/repo/tenant\",\"config\":\"my-repo.my-domain.example:1234/repo/image\"}}"); - // Cannot set docker image for non docker node type + // Cannot set container image for non docker node type tester.assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/confighost", Utf8.toBytes("{\"dockerImage\": \"my-repo.my-domain.example:1234/repo/image\"}"), Request.Method.PATCH), 400, - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Setting docker image for confighost nodes is unsupported\"}"); + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Setting container image for confighost nodes is unsupported\"}"); } @Test diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-after-changes.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-after-changes.json index 7ca5048ac3b..98b64e9eb25 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-after-changes.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node4-after-changes.json @@ -24,7 +24,7 @@ }, "restartGeneration": 1, "currentRestartGeneration": 1, - "wantedDockerImage": "docker.domain.tld/my/image:6.42.0", + "wantedDockerImage": "docker-registry.domain.tld:8080/my/image:6.42.0", "wantedVespaVersion": "6.42.0", "requestedResources": { "vcpu":1.0, "memoryGb":4.0, "diskGb":100.0, "bandwidthGbps":1.0, "diskSpeed":"fast", "storageType":"any" }, "allowedToBeDown": true, |