Handle untagged images during Docker image GC

2 files changed, 36 insertions, 12 deletions

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 4eeb00c7685..e2116f7037e 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
@@ -10,6 +10,7 @@ import com.yahoo.config.provision.DockerImage;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
@@ -125,14 +126,10 @@ class DockerImageGarbageCollector {
                 // 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));
-                            });
-
+                    referencesOf(image).forEach(imageReference -> {
+                        logger.info("Deleting unused docker image " + imageReference);
+                        docker.deleteImage(DockerImage.fromString(imageReference));
+                    });
                     lastTimeUsedByImageId.remove(image.getId());
                 })
                 .count() > 0;
@@ -161,10 +158,8 @@ class DockerImageGarbageCollector {
      */
     private Set<String> dockerImageToImageIds(List<DockerImage> dockerImages, List<Image> images) {
         Map<String, String> imageIdByImageTag = images.stream()
-                .flatMap(image -> Optional.ofNullable(image.getRepoTags())
-                        .map(Stream::of)
-                        .orElseGet(Stream::empty)
-                        .map(repoTag -> new Pair<>(repoTag, image.getId())))
+                .flatMap(image -> referencesOf(image).stream()
+                                                     .map(repoTag -> new Pair<>(repoTag, image.getId())))
                 .collect(Collectors.toMap(Pair::getFirst, Pair::getSecond));
 
         return dockerImages.stream()
@@ -184,4 +179,23 @@ class DockerImageGarbageCollector {
         }
         return false;
     }
+
+    /**
+     * Returns list of references to given image, preferring image tag(s), if any exist.
+     *
+     * If image is untagged, its ID is returned instead.
+     */
+    private static List<String> referencesOf(Image image) {
+        if (image.getRepoTags() == null) {
+            return List.of(image.getId());
+        }
+        return Arrays.stream(image.getRepoTags())
+                     // Docker API returns untagged images as having the tag "<none>:<none>".
+                     .map(tag -> {
+                         if ("<none>:<none>".equals(tag)) return image.getId();
+                         return tag;
+                     })
+                     .collect(Collectors.toUnmodifiableList());
+    }
+
 }
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 04377f0b73c..40c4cb167a2 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
@@ -90,6 +90,16 @@ public class DockerImageGarbageCollectionTest {
                 .expectDeletedImages("vespa-6", "vespa-6.28", "vespa:latest", "parent-image");
     }
 
+
+    @Test
+    public void unusedImagesWithMultipleUntagged() {
+        gcTester.withExistingImages(ImageBuilder.forId("image1")
+                                                .withTags("<none>:<none>"),
+                                    ImageBuilder.forId("image2")
+                                                .withTags("<none>:<none>"))
+                .expectDeletedImages("image1", "image2");
+    }
+
     @Test
     public void taggedImageWithNoContainersIsUnused() {
         gcTester.withExistingImages(ImageBuilder.forId("image-1").withTags("vespa-6"))