diff options
Diffstat (limited to 'docker-api/src')
3 files changed, 31 insertions, 34 deletions
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/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; |