From c4d2dd2945fa5c94b21f2a948cc7ea6329026477 Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Wed, 12 Jun 2019 23:03:07 +0200 Subject: Revert moving deleteUnusedDockerImages impl to DockerOperationsImpl --- .../com/yahoo/vespa/hosted/dockerapi/Docker.java | 8 +- .../dockerapi/DockerImageGarbageCollector.java | 187 +++++++++++++++++++++ .../yahoo/vespa/hosted/dockerapi/DockerImpl.java | 13 +- 3 files changed, 200 insertions(+), 8 deletions(-) create mode 100644 docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollector.java (limited to 'docker-api/src/main') diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java index d31fbd52d96..1729c4843ef 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. 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.Image; import com.yahoo.config.provision.DockerImage; import java.net.InetAddress; @@ -17,8 +16,6 @@ import java.util.OptionalLong; */ public interface Docker { - void deleteImage(DockerImage dockerImage); - interface CreateContainerCommand { CreateContainerCommand withHostName(String hostname); CreateContainerCommand withResources(ContainerResources containerResources); @@ -87,7 +84,10 @@ public interface Docker { /** List all containers, including those not running. */ List listAllContainers(); - List listAllImages(); + /** + * Deletes the local images that are currently not in use by any container and not recently used. + */ + boolean deleteUnusedDockerImages(List excludes, Duration minImageAgeToDelete); /** * @param containerName The name of the container 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 new file mode 100644 index 00000000000..242332ebd54 --- /dev/null +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollector.java @@ -0,0 +1,187 @@ +// Copyright 2017 Yahoo Holdings. 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; +import java.time.Instant; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; +import java.util.logging.Logger; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * This class keeps track of downloaded docker images and helps delete images that have not been recently used + * + *

Definitions: + *

    + *
  • Every image has exactly 1 id
    • + *
  • Every image has between 0..n tags, see + * docker tag for more
    • + *
  • Every image has 0..1 parent ids
    • + *
+ * + *

Limitations: + *

    + *
  1. Image that has more than 1 tag cannot be deleted by ID
    2. + *
  2. Deleting a tag of an image with multiple tags will only remove the tag, the image with the + * remaining tags will remain
    3. + *
  3. Deleting the last tag of an image will delete the entire image.
    4. + *
  4. Image cannot be deleted if:
    5. + *
      + *
    1. It has 1 or more children
      2. + *
    2. A container uses it
      3. + *
    + *
+ * + * @author freva + */ +class DockerImageGarbageCollector { + private static final Logger logger = Logger.getLogger(DockerImageGarbageCollector.class.getName()); + + private final Map lastTimeUsedByImageId = new ConcurrentHashMap<>(); + private final DockerImpl docker; + private final Clock clock; + + DockerImageGarbageCollector(DockerImpl docker) { + this(docker, Clock.systemUTC()); + } + + DockerImageGarbageCollector(DockerImpl docker, Clock clock) { + this.docker = docker; + this.clock = clock; + } + + /** + * 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 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 excludes, Duration minImageAgeToDelete) { + List images = docker.listAllImages(); + List containers = docker.listAllContainers(); + + Map imageByImageId = images.stream().collect(Collectors.toMap(Image::getId, Function.identity())); + + // Find all the ancestors for every local image id, this includes the image id itself + Map> ancestorsByImageId = images.stream() + .map(Image::getId) + .collect(Collectors.toMap( + Function.identity(), + imageId -> { + Set ancestors = new HashSet<>(); + while (!Strings.isNullOrEmpty(imageId)) { + ancestors.add(imageId); + imageId = Optional.of(imageId).map(imageByImageId::get).map(Image::getParentId).orElse(null); + } + return ancestors; + } + )); + + // The set of images that we want to keep is: + // 1. The images that were recently used + // 2. The images that were explicitly excluded + // 3. All of the ancestors of from images in 1 & 2 + Set imagesToKeep = Stream + .concat( + getRecentlyUsedImageIds(images, containers, minImageAgeToDelete).stream(), // 1 + dockerImageToImageIds(excludes, images).stream()) // 2 + .flatMap(imageId -> ancestorsByImageId.getOrDefault(imageId, Collections.emptySet()).stream()) // 3 + .collect(Collectors.toSet()); + + // Now take all the images we have locally + return imageByImageId.keySet().stream() + + // filter out images we want to keep + .filter(imageId -> !imagesToKeep.contains(imageId)) + + // Sort images in an order is safe to delete (children before parents) + .sorted((o1, o2) -> { + // If image2 is parent of image1, image1 comes before image2 + if (imageIsDescendantOf(imageByImageId, o1, o2)) return -1; + // If image1 is parent of image2, image2 comes before image1 + else if (imageIsDescendantOf(imageByImageId, o2, o1)) return 1; + // Otherwise, sort lexicographically by image name (For testing) + else return o1.compareTo(o2); + }) + + // Map back to image + .map(imageByImageId::get) + + // Delete image, if successful also remove last usage time to prevent re-download being instantly deleted + .peek(image -> { + // Deleting an image by image ID with multiple tags will fail -> delete by tags instead + Optional.ofNullable(image.getRepoTags()) + .map(Stream::of) + .orElse(Stream.of(image.getId())) + .forEach(imageReference -> { + logger.info("Deleting unused docker image " + imageReference); + docker.deleteImage(DockerImage.fromString(imageReference)); + }); + + lastTimeUsedByImageId.remove(image.getId()); + }) + .count() > 0; + } + + private Set getRecentlyUsedImageIds(List images, List containers, Duration minImageAgeToDelete) { + final Instant now = clock.instant(); + + // Add any already downloaded image to the list once + images.forEach(image -> lastTimeUsedByImageId.putIfAbsent(image.getId(), now)); + + // Update last used time for all current containers + containers.forEach(container -> lastTimeUsedByImageId.put(container.imageId(), now)); + + // Return list of images that have been used within minImageAgeToDelete + return lastTimeUsedByImageId.entrySet().stream() + .filter(entry -> Duration.between(entry.getValue(), now).minus(minImageAgeToDelete).isNegative()) + .map(Map.Entry::getKey) + .collect(Collectors.toSet()); + } + + /** + * 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 + * it, so no harm done. + */ + private Set dockerImageToImageIds(List dockerImages, List images) { + Map imageIdByImageTag = images.stream() + .flatMap(image -> Optional.ofNullable(image.getRepoTags()) + .map(Stream::of) + .orElseGet(Stream::empty) + .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 true if ancestor is a parent or grand-parent or grand-grand-parent, etc. of img + */ + private boolean imageIsDescendantOf(Map imageIdToImage, String img, String ancestor) { + while (imageIdToImage.containsKey(img)) { + img = imageIdToImage.get(img).getParentId(); + if (img == null) return false; + if (ancestor.equals(img)) return true; + } + return false; + } +} diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java index 42044d08c5c..683c8a98788 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java @@ -56,6 +56,7 @@ public class DockerImpl implements Docker { private final Set scheduledPulls = new HashSet<>(); private final DockerClient dockerClient; + private final DockerImageGarbageCollector dockerImageGC; private final CounterWrapper numberOfDockerDaemonFails; @Inject @@ -65,6 +66,7 @@ public class DockerImpl implements Docker { DockerImpl(DockerClient dockerClient, MetricReceiverWrapper metricReceiver) { this.dockerClient = dockerClient; + this.dockerImageGC = new DockerImageGarbageCollector(this); Dimensions dimensions = new Dimensions.Builder().add("role", "docker").build(); numberOfDockerDaemonFails = metricReceiver.declareCounter(MetricReceiverWrapper.APPLICATION_DOCKER, dimensions, "daemon.api_fails"); @@ -310,8 +312,7 @@ public class DockerImpl implements Docker { } } - @Override - public List listAllImages() { + List listAllImages() { try { return dockerClient.listImagesCmd().withShowAll(true).exec(); } catch (RuntimeException e) { @@ -320,8 +321,7 @@ public class DockerImpl implements Docker { } } - @Override - public void deleteImage(DockerImage dockerImage) { + void deleteImage(DockerImage dockerImage) { try { dockerClient.removeImageCmd(dockerImage.asString()).exec(); } catch (NotFoundException ignored) { @@ -332,6 +332,11 @@ public class DockerImpl implements Docker { } } + @Override + public boolean deleteUnusedDockerImages(List excludes, Duration minImageAgeToDelete) { + return dockerImageGC.deleteUnusedDockerImages(excludes, minImageAgeToDelete); + } + private class ImagePullCallback extends PullImageResultCallback { private final DockerImage dockerImage; -- cgit v1.2.3