Created DockerImageGarbageCollection

8 files changed, 386 insertions, 334 deletions

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 c7ef19ccb2a..479d0b2e3cd 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
@@ -6,7 +6,6 @@ import java.net.InetAddress;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 
 /**
@@ -73,9 +72,9 @@ public interface Docker {
     void buildImage(File dockerfile, DockerImage dockerImage);
 
     /**
-     * Deletes the local images that are currently not in use by any container and not in the except set.
+     * Deletes the local images that are currently not in use by any container and not recently used.
      */
-    void deleteUnusedDockerImages(Set<DockerImage> except);
+    void deleteUnusedDockerImages();
 
     /**
      * TODO: Make this function interruptible, see https://github.com/spotify/docker-client/issues/421
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..7a17abd7ea3
--- /dev/null
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollector.java
@@ -0,0 +1,115 @@
+// Copyright 2016 Yahoo Inc. 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.github.dockerjava.api.model.Container;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ * @author freva
+ */
+public class DockerImageGarbageCollector {
+    private final Duration MIN_AGE_IMAGE_GC;
+    private Map<String, Instant> lastTimeUsedByImageId = new HashMap<>();
+
+    public DockerImageGarbageCollector(Duration minAgeImageToDelete) {
+        MIN_AGE_IMAGE_GC = minAgeImageToDelete;
+    }
+
+    public void updateLastUsedTimeFor(String imageId) {
+        updateLastUsedTimeFor(imageId, Instant.now());
+    }
+
+    void updateLastUsedTimeFor(String imageId, Instant at) {
+        lastTimeUsedByImageId.put(imageId, at);
+    }
+
+    /**
+     * Generates lists of images that are safe to delete, in the order that is safe to delete them (children before
+     * parents). The function starts with the set of all local images and then filters out images that are used now
+     * and images that have been used recently (because they might be re-used again in near future).
+     *
+     * @param images List of all the local images
+     * @param containers List of all the containers, including the ones that are stopped
+     * @return List of image tags of unused images, if unused image has no tag, will return image ID instead.
+     */
+    public List<DockerImage> getUnusedDockerImages(List<Image> images, List<Container> containers) {
+        Map<String, Image> dockerImageByImageId = images.stream().collect(Collectors.toMap(Image::getId, img -> img));
+        Map<String, Image> unusedImagesByContainers = filterOutImagesUsedByContainers(dockerImageByImageId, containers);
+        Map<String, Image> unusedImagesByRecent = filterOutRecentImages(unusedImagesByContainers);
+
+        return unusedImagesByRecent.keySet().stream()
+                .sorted((o1, o2) -> {
+                    // If image2 is parent of image1, image1 comes before image2
+                    if (imageIsDescendantOf(unusedImagesByRecent, o1, o2)) return -1;
+                    // If image1 is parent of image2, image2 comes before image1
+                    else if (imageIsDescendantOf(unusedImagesByRecent, o2, o1)) return 1;
+                    // Otherwise, sort lexicographically by image name (For testing)
+                    else return o1.compareTo(o2);
+                })
+                .flatMap(imageId -> {
+                    // Deleting an image by image ID with multiple tags will fail -> map IDs to all the tags refering to the ID
+                    String[] repoTags = unusedImagesByRecent.get(imageId).getRepoTags();
+                    return (repoTags == null) ? Stream.of(imageId) : Stream.of(repoTags);
+                })
+                .map(DockerImage::new)
+                .collect(Collectors.toList());
+    }
+
+    private Map<String, Image> filterOutImagesUsedByContainers(
+            Map<String, Image> dockerImagesByImageId, List<com.github.dockerjava.api.model.Container> containerList) {
+        Map<String, Image> filteredDockerImagesByImageId = new HashMap<>(dockerImagesByImageId);
+
+        for (com.github.dockerjava.api.model.Container container : containerList) {
+            String imageToSpare = container.getImageId();
+            do {
+                // May be null if two images have have the same parent, the first image will remove the parent, the
+                // second will get null.
+                Image sparedImage = filteredDockerImagesByImageId.remove(imageToSpare);
+                imageToSpare = sparedImage == null ? "" : sparedImage.getParentId();
+            } while (!imageToSpare.isEmpty());
+        }
+
+        return filteredDockerImagesByImageId;
+    }
+
+    private Map<String, Image> filterOutRecentImages(Map<String, Image> dockerImageByImageId) {
+        Map<String, Image> filteredDockerImagesByImageId = new HashMap<>(dockerImageByImageId);
+
+        final Instant now = Instant.now();
+        filteredDockerImagesByImageId.keySet().forEach(imageId -> {
+            if (! lastTimeUsedByImageId.containsKey(imageId)) lastTimeUsedByImageId.put(imageId, now);
+        });
+
+        lastTimeUsedByImageId.entrySet().stream()
+                .filter(entry -> Duration.between(entry.getValue(), now).minus(MIN_AGE_IMAGE_GC).isNegative())
+                .map(Map.Entry::getKey)
+                .forEach(image -> {
+                    String imageToSpare = image;
+                    do {
+                        Image sparedImage = filteredDockerImagesByImageId.remove(imageToSpare);
+                        imageToSpare = sparedImage == null ? "" : sparedImage.getParentId();
+                    } while (!imageToSpare.isEmpty());
+                });
+        return filteredDockerImagesByImageId;
+    }
+
+    /**
+     * Returns true if ancestor is a parent or grand-parent or grand-grand-parent, etc. of img
+     */
+    private boolean imageIsDescendantOf(Map<String, Image> 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 80bcbeb6537..52de3bf91eb 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
@@ -6,8 +6,10 @@ import com.github.dockerjava.api.command.ExecCreateCmdResponse;
 import com.github.dockerjava.api.command.ExecStartCmd;
 import com.github.dockerjava.api.command.InspectContainerResponse;
 import com.github.dockerjava.api.command.InspectExecResponse;
+import com.github.dockerjava.api.command.InspectImageResponse;
 import com.github.dockerjava.api.exception.DockerClientException;
 import com.github.dockerjava.api.exception.DockerException;
+import com.github.dockerjava.api.exception.NotFoundException;
 import com.github.dockerjava.api.model.Image;
 import com.github.dockerjava.api.model.Network;
 import com.github.dockerjava.api.model.Statistics;
@@ -34,6 +36,7 @@ import java.io.IOException;
 import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.URI;
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -41,7 +44,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
@@ -71,6 +73,8 @@ public class DockerImpl implements Docker {
     private GaugeWrapper numberOfRunningContainersGauge;
     private CounterWrapper numberOfDockerDaemonFails;
 
+    private Optional<DockerImageGarbageCollector> dockerImageGC = Optional.empty();
+
     // For testing
     DockerImpl(final DockerClient dockerClient) {
         this.dockerClient = dockerClient;
@@ -82,6 +86,11 @@ public class DockerImpl implements Docker {
             boolean fallbackTo123OnErrors,
             boolean trySetupNetwork,
             MetricReceiverWrapper metricReceiverWrapper) {
+        if (config.imageGCEnabled()) {
+            Duration minAgeToDelete = Duration.ofMinutes(config.imageGCMinTimeToLiveMinutes());
+            dockerImageGC = Optional.of(new DockerImageGarbageCollector(minAgeToDelete));
+        }
+
         SECONDS_TO_WAIT_BEFORE_KILLING = config.secondsToWaitBeforeKillingContainer();
 
         initDockerConnection(config, fallbackTo123OnErrors);
@@ -154,7 +163,6 @@ public class DockerImpl implements Docker {
                 .withHostResource(sourcePath).withRemotePath(destinationPath).exec();
     }
 
-
     @Override
     public CompletableFuture<DockerImage> pullImageAsync(final DockerImage image) {
         final CompletableFuture<DockerImage> completionListener;
@@ -181,11 +189,14 @@ public class DockerImpl implements Docker {
      */
     @Override
     public boolean imageIsDownloaded(final DockerImage dockerImage) {
+        return inspectImage(dockerImage).isPresent();
+    }
+
+    private Optional<InspectImageResponse> inspectImage(DockerImage dockerImage) {
         try {
-            List<Image> images = dockerClient.listImagesCmd().withShowAll(true).exec();
-            return images.stream().
-                    flatMap(image -> Arrays.stream(image.getRepoTags())).
-                    anyMatch(tag -> tag.equals(dockerImage.asString()));
+            return Optional.of(dockerClient.inspectImageCmd(dockerImage.asString()).exec());
+        } catch (NotFoundException e) {
+            return Optional.empty();
         } catch (DockerException e) {
             numberOfDockerDaemonFails.add();
             throw new RuntimeException("Failed to list image name: '" + dockerImage + "'", e);
@@ -293,6 +304,8 @@ public class DockerImpl implements Docker {
     public void deleteContainer(ContainerName containerName) {
         Optional<com.github.dockerjava.api.model.Container> dockerContainer = getContainerFromName(containerName, true);
         if (dockerContainer.isPresent()) {
+            dockerImageGC.ifPresent(imageGC -> imageGC.updateLastUsedTimeFor(dockerContainer.get().getImageId()));
+
             try {
                 dockerClient.removeContainerCmd(dockerContainer.get().getId()).exec();
                 numberOfRunningContainersGauge.sample(getAllManagedContainers().size());
@@ -390,86 +403,14 @@ public class DockerImpl implements Docker {
         }
     }
 
-    private Map<String, Image> filterOutImagesUsedByContainers(
-            Map<String, Image> dockerImagesByImageId, List<com.github.dockerjava.api.model.Container> containerList) {
-        Map<String, Image> filteredDockerImagesByImageId = new HashMap<>(dockerImagesByImageId);
-
-        for (com.github.dockerjava.api.model.Container container : containerList) {
-            String imageToSpare = container.getImageId();
-            do {
-                // May be null if two images have have the same parent, the first image will remove the parent, the
-                // second will get null.
-                Image sparedImage = filteredDockerImagesByImageId.remove(imageToSpare);
-                imageToSpare = sparedImage == null ? "" : sparedImage.getParentId();
-            } while (!imageToSpare.isEmpty());
-        }
-
-        return filteredDockerImagesByImageId;
-    }
-
-    private Map<String, Image> filterOutExceptImages(Map<String, Image> dockerImagesByImageId, Set<DockerImage> except) {
-        Map<String, Image> dockerImageByImageTags = new HashMap<>();
-        // Transform map of image ID:image to map of image tag:image (for each tag the image has)
-        for (Map.Entry<String, Image> entry : dockerImagesByImageId.entrySet()) {
-            String[] repoTags = entry.getValue().getRepoTags();
-            // If no tags present, fall back to image ID
-            if (repoTags == null) {
-                dockerImageByImageTags.put(entry.getKey(), entry.getValue());
-            } else {
-                for (String tag : repoTags) {
-                    dockerImageByImageTags.put(tag, entry.getValue());
-                }
-            }
-        }
-
-        // Remove images we want to keep from the map of unused images, also recursively keep the parents of images we want to keep
-        except.forEach(image -> {
-            String imageToSpare = image.asString();
-            do {
-                Image sparedImage = dockerImageByImageTags.remove(imageToSpare);
-                imageToSpare = sparedImage == null ? "" : sparedImage.getParentId();
-            } while (!imageToSpare.isEmpty());
-        });
-        return dockerImageByImageTags;
-    }
+    @Override
+    public void deleteUnusedDockerImages() {
+        if (! dockerImageGC.isPresent()) return;
 
-    /**
-     * Generates lists of images that are safe to delete, in the order that is safe to delete them (children before
-     * parents). The function starts with a map of all images and the filters out images that are used now or will be
-     * used in near future (through the use of except set).
-     *
-     * @param except set of image tags to keep, regardless whether they are being used right now or not.
-     * @return List of image tags of unused images, if unused image has no tag, will return image ID instead.
-     */
-    List<DockerImage> getUnusedDockerImages(Set<DockerImage> except) {
         List<Image> images = dockerClient.listImagesCmd().withShowAll(true).exec();
-        Map<String, Image> dockerImageByImageId = images.stream().collect(Collectors.toMap(Image::getId, img -> img));
-
         List<com.github.dockerjava.api.model.Container> containers = dockerClient.listContainersCmd().withShowAll(true).exec();
-        Map<String, Image> unusedImagesByContainers = filterOutImagesUsedByContainers(dockerImageByImageId, containers);
-        Map<String, Image> unusedImagesByExcept = filterOutExceptImages(unusedImagesByContainers, except);
-
-        List<String> unusedImages = unusedImagesByExcept.keySet().stream().collect(Collectors.toList());
-        // unusedImages now contains all the unused images, all we need to do now is to order them in a way that is
-        // safe to delete with. The order is:
-        Collections.sort(unusedImages, (o1, o2) -> {
-            Image image1 = unusedImagesByExcept.get(o1);
-            Image image2 = unusedImagesByExcept.get(o2);
-
-            // If image2 is parent of image1, image1 comes before image2
-            if (Objects.equals(image1.getParentId(), image2.getId())) return -1;
-            // If image1 is parent of image2, image2 comes before image1
-            else if (Objects.equals(image2.getParentId(), image1.getId())) return 1;
-            // Otherwise, sort lexicographically by image name (For testing)
-            else return o1.compareTo(o2);
-        });
 
-        return unusedImages.stream().map(DockerImage::new).collect(Collectors.toList());
-    }
-
-    @Override
-    public void deleteUnusedDockerImages(Set<DockerImage> except) {
-        getUnusedDockerImages(except).forEach(this::deleteImage);
+        dockerImageGC.get().getUnusedDockerImages(images, containers).forEach(this::deleteImage);
     }
 
     private class ImagePullCallback extends PullImageResultCallback {
@@ -487,7 +428,9 @@ public class DockerImpl implements Docker {
 
         @Override
         public void onComplete() {
-            if (imageIsDownloaded(dockerImage)) {
+            Optional<InspectImageResponse> inspectImage = inspectImage(dockerImage);
+            if (inspectImage.isPresent()) { // Download successful, update image GC with the newly downloaded image
+                dockerImageGC.ifPresent(imageGC -> imageGC.updateLastUsedTimeFor(inspectImage.get().getId()));
                 removeScheduledPoll(dockerImage).complete(dockerImage);
             } else {
                 removeScheduledPoll(dockerImage).completeExceptionally(
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerTestUtils.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerTestUtils.java
index cca39aea316..30766392e01 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerTestUtils.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerTestUtils.java
@@ -21,7 +21,8 @@ public class DockerTestUtils {
             .clientKeyPath( operatingSystem == OS.Mac_OS_X ? prefix + "key.pem" : "")
             .uri(           operatingSystem == OS.Mac_OS_X ? "tcp://192.168.99.100:2376" : "tcp://localhost:2376")
             .connectTimeoutMillis(100)
-            .secondsToWaitBeforeKillingContainer(0));
+            .secondsToWaitBeforeKillingContainer(0)
+            .imageGCEnabled(false));
     private static DockerImpl docker;
 
     public static boolean dockerDaemonIsPresent() {
diff --git a/docker-api/src/main/resources/configdefinitions/docker.def b/docker-api/src/main/resources/configdefinitions/docker.def
index d7fd8f47d40..2323c31eb5e 100644
--- a/docker-api/src/main/resources/configdefinitions/docker.def
+++ b/docker-api/src/main/resources/configdefinitions/docker.def
@@ -10,4 +10,7 @@ secondsToWaitBeforeKillingContainer int default = 10
 maxPerRouteConnections int default = 10
 maxTotalConnections int default = 100
 connectTimeoutMillis int default = 100000 # 100 sec
-readTimeoutMillis int default = 1800000 # 30 min
\ No newline at end of file
+readTimeoutMillis int default = 1800000 # 30 min
+
+imageGCEnabled bool default = true
+imageGCMinTimeToLiveMinutes int default = 120
\ No newline at end of file
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
new file mode 100644
index 00000000000..fd2ab6f8b5f
--- /dev/null
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollectionTest.java
@@ -0,0 +1,222 @@
+// Copyright 2016 Yahoo Inc. 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 org.junit.Test;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * @author freva
+ */
+public class DockerImageGarbageCollectionTest {
+    @Test
+    public void noImagesMeansNoUnusedImages() throws Exception {
+        new ImageGcTester(0)
+                .withExistingImages()
+                .expectUnusedImages();
+    }
+
+    @Test
+    public void singleImageWithoutContainersIsUnused() throws Exception {
+        new ImageGcTester(0)
+                .withExistingImages(new ImageBuilder("image-1"))
+                .expectUnusedImages("image-1");
+    }
+
+    @Test
+    public void singleImageWithContainerIsUsed() throws Exception {
+        new ImageGcTester(0)
+                .withExistingImages(ImageBuilder.forId("image-1"))
+                .andExistingContainers(ContainerBuilder.forId("container-1").withImageId("image-1"))
+                .expectUnusedImages();
+    }
+
+    @Test
+    public void onlyLeafImageIsUnused() throws Exception {
+        new ImageGcTester(0)
+                .withExistingImages(
+                        ImageBuilder.forId("parent-image"),
+                        ImageBuilder.forId("leaf-image").withParentId("parent-image"))
+                .expectUnusedImages("leaf-image", "parent-image");
+    }
+
+    @Test
+    public void multipleUnusedImagesAreIdentified() throws Exception {
+        new ImageGcTester(0)
+                .withExistingImages(
+                        ImageBuilder.forId("image-1"),
+                        ImageBuilder.forId("image-2"))
+                .expectUnusedImages("image-1", "image-2");
+    }
+
+    @Test
+    public void multipleUnusedLeavesAreIdentified() throws Exception {
+        new ImageGcTester(0)
+                .withExistingImages(
+                        ImageBuilder.forId("parent-image"),
+                        ImageBuilder.forId("image-1").withParentId("parent-image"),
+                        ImageBuilder.forId("image-2").withParentId("parent-image"))
+                .expectUnusedImages("image-1", "image-2", "parent-image");
+    }
+
+    @Test
+    public void unusedLeafWithUsedSiblingIsIdentified() throws Exception {
+        new ImageGcTester(0)
+                .withExistingImages(
+                        ImageBuilder.forId("parent-image"),
+                        ImageBuilder.forId("image-1").withParentId("parent-image").withTags("latest"),
+                        ImageBuilder.forId("image-2").withParentId("parent-image").withTags("1.24"))
+                .andExistingContainers(ContainerBuilder.forId("vespa-node-1").withImageId("image-1"))
+                .expectUnusedImages("1.24");
+    }
+
+    @Test
+    public void unusedImagesWithMultipleTags() throws Exception {
+        new ImageGcTester(0)
+                .withExistingImages(
+                        ImageBuilder.forId("parent-image"),
+                        ImageBuilder.forId("image-1").withParentId("parent-image")
+                                .withTags("vespa-6", "vespa-6.28", "vespa:latest"))
+                .expectUnusedImages("vespa-6", "vespa-6.28", "vespa:latest", "parent-image");
+    }
+
+    @Test
+    public void taggedImageWithNoContainersIsUnused() throws Exception {
+        new ImageGcTester(0)
+                .withExistingImages(ImageBuilder.forId("image-1").withTags("vespa-6"))
+                .expectUnusedImages("vespa-6");
+    }
+
+    @Test
+    public void unusedImagesWithSimpleImageGc() throws Exception {
+        new ImageGcTester(20)
+                .withExistingImages(
+                        ImageBuilder.forId("parent-image").withLastUsedMinutesAgo(25),
+                        ImageBuilder.forId("image-1").withParentId("parent-image").withLastUsedMinutesAgo(5))
+                .expectUnusedImages();
+    }
+
+    @Test
+    public void unusedImagesWithImageGc() throws Exception {
+        new ImageGcTester(20)
+                .withExistingImages(
+                        ImageBuilder.forId("parent-1").withLastUsedMinutesAgo(40),
+                        ImageBuilder.forId("parent-2").withTags("p-tag:1").withLastUsedMinutesAgo(10),
+                        ImageBuilder.forId("image-1-1").withParentId("parent-1").withTags("i-tag:1", "i-tag:2", "i-tag-3").withLastUsedMinutesAgo(5),
+                        ImageBuilder.forId("image-1-2").withParentId("parent-1").withLastUsedMinutesAgo(25),
+                        ImageBuilder.forId("image-2-1").withParentId("parent-2").withTags("i-tag:4").withLastUsedMinutesAgo(30))
+                .andExistingContainers(
+                        ContainerBuilder.forId("cont-1").withImageId("image-1-1"))
+                .expectUnusedImages("image-1-2", "i-tag:4");
+    }
+
+    private static class ImageGcTester {
+        private static DockerImageGarbageCollector imageGC;
+
+        private List<Image> existingImages = Collections.emptyList();
+        private List<com.github.dockerjava.api.model.Container> existingContainers = Collections.emptyList();
+
+        private ImageGcTester(int imageGcMinTimeInMinutes) {
+            imageGC = new DockerImageGarbageCollector(Duration.ofMinutes(imageGcMinTimeInMinutes));
+        }
+
+        private ImageGcTester withExistingImages(final ImageBuilder... images) {
+            this.existingImages = Arrays.stream(images)
+                    .map(ImageBuilder::toImage)
+                    .collect(Collectors.toList());
+            return this;
+        }
+
+        private ImageGcTester andExistingContainers(final ContainerBuilder... containers) {
+            this.existingContainers = Arrays.stream(containers)
+                    .map(ContainerBuilder::toContainer)
+                    .collect(Collectors.toList());
+            return this;
+        }
+
+        private void expectUnusedImages(final String... imageIds) throws Exception {
+            final List<DockerImage> expectedUnusedImages = Arrays.stream(imageIds)
+                    .map(DockerImage::new)
+                    .collect(Collectors.toList());
+
+            assertThat(imageGC.getUnusedDockerImages(existingImages, existingContainers), is(expectedUnusedImages));
+        }
+    }
+
+    /**
+     * Serializes object to a JSON string using Jackson, then deserializes it to an instance of toClass
+     * (again using Jackson). This can be used to create Jackson classes with no public constructors.
+     * @throws IllegalArgumentException if Jackson fails to serialize or deserialize.
+     */
+    private static <T> T createFrom(Class<T> toClass, Object object) throws IllegalArgumentException {
+        final String serialized;
+        try {
+            serialized = new ObjectMapper().writeValueAsString(object);
+        } catch (JsonProcessingException e) {
+            throw new IllegalArgumentException("Failed to serialize object " + object + " to "
+                    + toClass + " with Jackson: " + e, e);
+        }
+        try {
+            return new ObjectMapper().readValue(serialized, toClass);
+        } catch (IOException e) {
+            throw new IllegalArgumentException("Failed to convert " + serialized + " to "
+                    + toClass + " with Jackson: " + e, e);
+        }
+    }
+
+    // Workaround for Image class that can't be instantiated directly in Java (instantiate via Jackson instead).
+    private static class ImageBuilder {
+        // Json property names must match exactly the property names in the Image class.
+        @JsonProperty("Id")
+        private final String id;
+
+        @JsonProperty("ParentId")
+        private String parentId = ""; // docker-java returns empty string and not null if the parent is not present
+
+        @JsonProperty("RepoTags")
+        private String[] repoTags = null;
+
+        private ImageBuilder(String id) { this.id = id; }
+
+        private static ImageBuilder forId(String id) { return new ImageBuilder(id); }
+        private ImageBuilder withParentId(String parentId) { this.parentId = parentId; return this; }
+        private ImageBuilder withTags(String... tags) { this.repoTags = tags; return this; }
+        private ImageBuilder withLastUsedMinutesAgo(int minutesAgo) {
+            ImageGcTester.imageGC.updateLastUsedTimeFor(id, Instant.now().minus(Duration.ofMinutes(minutesAgo)));
+            return this;
+        }
+
+        private Image toImage() { return createFrom(Image.class, this); }
+    }
+
+    // Workaround for Container class that can't be instantiated directly in Java (instantiate via Jackson instead).
+    private static class ContainerBuilder {
+        // Json property names must match exactly the property names in the Container class.
+        @JsonProperty("Id")
+        private final String id;
+
+        @JsonProperty("ImageID")
+        private String imageId;
+
+        private ContainerBuilder(String id) { this.id = id; }
+        private static ContainerBuilder forId(final String id) { return new ContainerBuilder(id); }
+        private ContainerBuilder withImageId(String imageId) { this.imageId = imageId; return this; }
+
+        private com.github.dockerjava.api.model.Container toContainer() {
+            return createFrom(com.github.dockerjava.api.model.Container.class, this);
+        }
+    }
+}
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java
index 78757cc00ec..e051addb0dd 100644
--- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java
@@ -1,18 +1,12 @@
 // Copyright 2016 Yahoo Inc. 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.DockerClient;
 import com.github.dockerjava.api.command.ExecCreateCmd;
 import com.github.dockerjava.api.command.ExecCreateCmdResponse;
 import com.github.dockerjava.api.command.ExecStartCmd;
 import com.github.dockerjava.api.command.InspectExecCmd;
 import com.github.dockerjava.api.command.InspectExecResponse;
-import com.github.dockerjava.api.command.ListContainersCmd;
-import com.github.dockerjava.api.command.ListImagesCmd;
-import com.github.dockerjava.api.model.Image;
 import com.github.dockerjava.core.DefaultDockerClientConfig;
 import com.github.dockerjava.core.command.ExecStartResultCallback;
 import org.junit.Test;
@@ -23,11 +17,6 @@ import java.security.KeyManagementException;
 import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
 import java.security.UnrecoverableKeyException;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
-import java.util.Set;
-import java.util.stream.Collectors;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
@@ -129,222 +118,4 @@ public class DockerImplTest {
         final ProcessResult result = docker.executeInContainer(new ContainerName(containerId), command);
         assertThat(result.getExitStatus(), is(exitCode));
     }
-
-    @Test
-    public void noImagesMeansNoUnusedImages() throws Exception {
-        ImageGcTester
-                .withExistingImages()
-                .expectUnusedImages();
-    }
-
-    @Test
-    public void singleImageWithoutContainersIsUnused() throws Exception {
-        ImageGcTester
-                .withExistingImages(new ImageBuilder("image-1"))
-                .expectUnusedImages("image-1");
-    }
-
-    @Test
-    public void singleImageWithContainerIsUsed() throws Exception {
-        ImageGcTester
-                .withExistingImages(ImageBuilder.forId("image-1"))
-                .andExistingContainers(ContainerBuilder.forId("container-1").withImageId("image-1"))
-                .expectUnusedImages();
-    }
-
-    @Test
-    public void onlyLeafImageIsUnused() throws Exception {
-        ImageGcTester
-                .withExistingImages(
-                        ImageBuilder.forId("parent-image"),
-                        ImageBuilder.forId("leaf-image").withParentId("parent-image"))
-                .expectUnusedImages("leaf-image", "parent-image");
-    }
-
-    @Test
-    public void multipleUnusedImagesAreIdentified() throws Exception {
-        ImageGcTester
-                .withExistingImages(
-                        ImageBuilder.forId("image-1"),
-                        ImageBuilder.forId("image-2"))
-                .expectUnusedImages("image-1", "image-2");
-    }
-
-    @Test
-    public void multipleUnusedLeavesAreIdentified() throws Exception {
-        ImageGcTester
-                .withExistingImages(
-                        ImageBuilder.forId("parent-image"),
-                        ImageBuilder.forId("image-1").withParentId("parent-image"),
-                        ImageBuilder.forId("image-2").withParentId("parent-image"))
-                .expectUnusedImages("image-1", "image-2", "parent-image");
-    }
-
-    @Test
-    public void unusedLeafWithUsedSiblingIsIdentified() throws Exception {
-        ImageGcTester
-                .withExistingImages(
-                        ImageBuilder.forId("parent-image"),
-                        ImageBuilder.forId("image-1").withParentId("parent-image").withTags("latest"),
-                        ImageBuilder.forId("image-2").withParentId("parent-image").withTags("1.24"))
-                .andExistingContainers(ContainerBuilder.forId("vespa-node-1").withImageId("image-1"))
-                .expectUnusedImages("1.24");
-    }
-
-    @Test
-    public void unusedImagesWithMultipleTags() throws Exception {
-        ImageGcTester
-                .withExistingImages(
-                        ImageBuilder.forId("parent-image"),
-                        ImageBuilder.forId("image-1").withParentId("parent-image")
-                                .withTags("vespa-6", "vespa-6.28", "vespa:latest"))
-                .expectUnusedImages("vespa-6", "vespa-6.28", "vespa:latest", "parent-image");
-    }
-
-    @Test
-    public void taggedImageWithNoContainersIsUnused() throws Exception {
-        ImageGcTester
-                .withExistingImages(ImageBuilder.forId("image-1").withTags("vespa-6"))
-                .expectUnusedImages("vespa-6");
-    }
-
-    @Test
-    public void unusedImagesWithMultipleTagsAndExcept() throws Exception {
-        ImageGcTester
-                .withExistingImages(
-                        ImageBuilder.forId("parent-image"),
-                        ImageBuilder.forId("image-1").withParentId("parent-image")
-                                .withTags("vespa-6", "vespa-6.28", "vespa:latest"))
-                .andExceptImages("vespa-6.28")
-                .expectUnusedImages("vespa-6", "vespa:latest");
-    }
-
-    @Test
-    public void unusedImagesWithExcept() throws Exception {
-        ImageGcTester
-                .withExistingImages(
-                        ImageBuilder.forId("parent-1"),
-                        ImageBuilder.forId("parent-2").withTags("p-tag:1"),
-                        ImageBuilder.forId("image-1-1").withParentId("parent-1").withTags("i-tag:1", "i-tag:2", "i-tag-3"),
-                        ImageBuilder.forId("image-1-2").withParentId("parent-1"),
-                        ImageBuilder.forId("image-2-1").withParentId("parent-2").withTags("i-tag:4"))
-                .andExceptImages("image-1-2")
-                .andExistingContainers(
-                        ContainerBuilder.forId("cont-1").withImageId("image-1-1"))
-                .expectUnusedImages("i-tag:4", "p-tag:1");
-    }
-
-    private static class ImageGcTester {
-        private final List<Image> existingImages;
-        private Set<DockerImage> exceptImages = Collections.emptySet();
-        private List<com.github.dockerjava.api.model.Container> existingContainers = Collections.emptyList();
-
-        private ImageGcTester(final List<Image> images) {
-            this.existingImages = images;
-        }
-
-        private static ImageGcTester withExistingImages(final ImageBuilder... images) {
-            final List<Image> existingImages = Arrays.stream(images)
-                    .map(ImageBuilder::toImage)
-                    .collect(Collectors.toList());
-            return new ImageGcTester(existingImages);
-        }
-
-        private ImageGcTester andExistingContainers(final ContainerBuilder... containers) {
-            this.existingContainers = Arrays.stream(containers)
-                    .map(ContainerBuilder::toContainer)
-                    .collect(Collectors.toList());
-            return this;
-        }
-
-        private ImageGcTester andExceptImages(final String... images) {
-            this.exceptImages = Arrays.stream(images)
-                    .map(DockerImage::new)
-                    .collect(Collectors.toSet());
-            return this;
-        }
-
-        private void expectUnusedImages(final String... imageIds) throws Exception {
-            final DockerClient dockerClient = mock(DockerClient.class);
-            final DockerImpl docker = new DockerImpl(dockerClient);
-            final ListImagesCmd listImagesCmd = mock(ListImagesCmd.class);
-            final ListContainersCmd listContainersCmd = mock(ListContainersCmd.class);
-
-            when(dockerClient.listImagesCmd()).thenReturn(listImagesCmd);
-            when(listImagesCmd.withShowAll(true)).thenReturn(listImagesCmd);
-            when(listImagesCmd.exec()).thenReturn(existingImages);
-
-            when(dockerClient.listContainersCmd()).thenReturn(listContainersCmd);
-            when(listContainersCmd.withShowAll(true)).thenReturn(listContainersCmd);
-            when(listContainersCmd.exec()).thenReturn(existingContainers);
-
-            final List<DockerImage> expectedUnusedImages = Arrays.stream(imageIds)
-                    .map(DockerImage::new)
-                    .collect(Collectors.toList());
-            assertThat(
-                    docker.getUnusedDockerImages(exceptImages),
-                    is(expectedUnusedImages));
-
-        }
-    }
-
-    /**
-     * Serializes object to a JSON string using Jackson, then deserializes it to an instance of toClass
-     * (again using Jackson). This can be used to create Jackson classes with no public constructors.
-     * @throws IllegalArgumentException if Jackson fails to serialize or deserialize.
-     */
-    private static <T> T createFrom(Class<T> toClass, Object object) throws IllegalArgumentException {
-        final String serialized;
-        try {
-            serialized = new ObjectMapper().writeValueAsString(object);
-        } catch (JsonProcessingException e) {
-            throw new IllegalArgumentException("Failed to serialize object " + object + " to "
-                    + toClass + " with Jackson: " + e, e);
-        }
-        try {
-            return new ObjectMapper().readValue(serialized, toClass);
-        } catch (IOException e) {
-            throw new IllegalArgumentException("Failed to convert " + serialized + " to "
-                    + toClass + " with Jackson: " + e, e);
-        }
-    }
-
-    // Workaround for Image class that can't be instantiated directly in Java (instantiate via Jackson instead).
-    private static class ImageBuilder {
-        // Json property names must match exactly the property names in the Image class.
-        @JsonProperty("Id")
-        private final String id;
-
-        @JsonProperty("ParentId")
-        private String parentId = ""; // docker-java returns empty string and not null if the parent is not present
-
-        @JsonProperty("RepoTags")
-        private String[] repoTags = null;
-
-        private ImageBuilder(String id) { this.id = id; }
-
-        private static ImageBuilder forId(String id) { return new ImageBuilder(id); }
-        private ImageBuilder withParentId(String parentId) { this.parentId = parentId; return this; }
-        private ImageBuilder withTags(String... tags) { this.repoTags = tags; return this; }
-
-        private Image toImage() { return createFrom(Image.class, this); }
-    }
-
-    // Workaround for Container class that can't be instantiated directly in Java (instantiate via Jackson instead).
-    private static class ContainerBuilder {
-        // Json property names must match exactly the property names in the Container class.
-        @JsonProperty("Id")
-        private final String id;
-
-        @JsonProperty("ImageID")
-        private String imageId;
-
-        private ContainerBuilder(String id) { this.id = id; }
-        private static ContainerBuilder forId(final String id) { return new ContainerBuilder(id); }
-        private ContainerBuilder withImageId(String imageId) { this.imageId = imageId; return this; }
-
-        private com.github.dockerjava.api.model.Container toContainer() {
-            return createFrom(com.github.dockerjava.api.model.Container.class, this);
-        }
-    }
 }
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java
index 7b76c11573c..f473f75edc9 100644
--- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java
@@ -9,14 +9,13 @@ import org.junit.Test;
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.URL;
-import java.util.HashSet;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 
-import static junit.framework.TestCase.fail;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeTrue;
 
 /**
@@ -33,26 +32,23 @@ import static org.junit.Assume.assumeTrue;
  *     docker stop $(docker ps -a -q)
  *     docker rm $(docker ps -a -q)
  *
- * @author valerijf
+ * @author freva
  * @author dybdahl
  */
 public class DockerTest {
     private DockerImpl docker;
     private static final DockerImage dockerImage = new DockerImage("simple-ipv6-server:Dockerfile");
+    private static final String CONTAINER_NAME_DOCKER_TEST_PREFIX = "docker-test-";
 
     // It is ignored since it is a bit slow and unstable, at least on Mac.
-    @Ignore
     @Test
-    public void testDockerImagePull() throws ExecutionException, InterruptedException {
+    public void testDockerImagePullDelete() throws ExecutionException, InterruptedException {
         DockerImage dockerImage = new DockerImage("busybox:1.24.0");
 
         // Pull the image and wait for the pull to complete
         docker.pullImageAsync(dockerImage).get();
+        assertTrue("Failed to download " + dockerImage.asString() + " image", docker.imageIsDownloaded(dockerImage));
 
-        List<DockerImage> unusedDockerImages = docker.getUnusedDockerImages(new HashSet<>());
-        if (! unusedDockerImages.contains(dockerImage)) {
-            fail("Did not find image as unused, here are all the unused images; " + unusedDockerImages);
-        }
         // Remove the image
         docker.deleteImage(dockerImage);
         assertFalse("Failed to delete " + dockerImage.asString() + " image", docker.imageIsDownloaded(dockerImage));
@@ -60,13 +56,13 @@ public class DockerTest {
 
     // Ignored because the test is very slow (several minutes) when swap is enabled, to disable: (Linux)
     // $ sudo swapoff -a
-    @Ignore
+
     @Test
     public void testOutOfMemoryDoesNotAffectOtherContainers() throws InterruptedException, ExecutionException, IOException {
         String hostName1 = "docker10.test.yahoo.com";
         String hostName2 = "docker11.test.yahoo.com";
-        ContainerName containerName1 = new ContainerName("test-container-1");
-        ContainerName containerName2 = new ContainerName("test-container-2");
+        ContainerName containerName1 = new ContainerName(CONTAINER_NAME_DOCKER_TEST_PREFIX + 1);
+        ContainerName containerName2 = new ContainerName(CONTAINER_NAME_DOCKER_TEST_PREFIX + 2);
         InetAddress inetAddress1 = InetAddress.getByName("172.18.0.10");
         InetAddress inetAddress2 = InetAddress.getByName("172.18.0.11");
 
@@ -98,7 +94,7 @@ public class DockerTest {
 
     @Test
     public void testContainerCycle() throws IOException, InterruptedException, ExecutionException {
-        ContainerName containerName = new ContainerName("foo");
+        ContainerName containerName = new ContainerName(CONTAINER_NAME_DOCKER_TEST_PREFIX + "foo");
         docker.createContainerCommand(dockerImage, containerName, "hostName1").create();
         List<Container> managedContainers = docker.getAllManagedContainers();
         assertThat(managedContainers.size(), is(1));
@@ -125,8 +121,8 @@ public class DockerTest {
     public void testDockerNetworking() throws InterruptedException, ExecutionException, IOException {
         String hostName1 = "docker10.test.yahoo.com";
         String hostName2 = "docker11.test.yahoo.com";
-        ContainerName containerName1 = new ContainerName("test-container-1");
-        ContainerName containerName2 = new ContainerName("test-container-2");
+        ContainerName containerName1 = new ContainerName(CONTAINER_NAME_DOCKER_TEST_PREFIX + 1);
+        ContainerName containerName2 = new ContainerName(CONTAINER_NAME_DOCKER_TEST_PREFIX + 2);
         InetAddress inetAddress1 = InetAddress.getByName("172.18.0.10");
         InetAddress inetAddress2 = InetAddress.getByName("172.18.0.11");
 
@@ -164,8 +160,10 @@ public class DockerTest {
 
         // Clean up any non deleted containers from previous tests
         docker.getAllManagedContainers().forEach(container -> {
-            if (container.isRunning) docker.stopContainer(container.name);
-            docker.deleteContainer(container.name);
+            if (container.name.asString().startsWith(CONTAINER_NAME_DOCKER_TEST_PREFIX)) {
+                if (container.isRunning) docker.stopContainer(container.name);
+                docker.deleteContainer(container.name);
+            }
         });
     }