Merge pull request #7073 from vespa-engine/freva/fix-image-gc

DockerApi: Fix immediate deleting of re-downloaded image

2 files changed, 20 insertions, 9 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 0ae6004be12..dd6ff436aae 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
@@ -118,18 +118,21 @@ class DockerImageGarbageCollector {
                     else return o1.compareTo(o2);
                 })
 
-                // Map image IDs to tags if there are any
-                .flatMap(imageId -> {
-                    // Deleting an image by image ID with multiple tags will fail -> map IDs to all the tags referring to the ID
-                    String[] repoTags = imageByImageId.get(imageId).getRepoTags();
-                    return repoTags == null ? Stream.of(imageId) : Stream.of(repoTags);
-                })
+                // 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 -> {
-                    logger.info("Deleting unused docker image " + image);
-                    docker.deleteImage(new DockerImage(image));
-                    lastTimeUsedByImageId.remove(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(new DockerImage(imageReference));
+                            });
+
+                    lastTimeUsedByImageId.remove(image.getId());
                 })
                 .count() > 0;
     }
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 5287d2cb45d..5a6efb68d14 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
@@ -118,6 +118,14 @@ public class DockerImageGarbageCollectionTest {
                 .expectDeletedImages("image"); // 1h after re-download it is deleted again
     }
 
+    @Test
+    public void reDownloadingImageIsNotImmediatelyDeletedWhenDeletingByTag() {
+        gcTester.withExistingImages(ImageBuilder.forId("image").withTags("image-1", "my-tag"))
+                .expectDeletedImages("image-1", "my-tag") // After 1h we delete image
+                .expectDeletedImagesAfterMinutes(0) // image is immediately re-downloaded, but is not deleted
+                .expectDeletedImagesAfterMinutes(10)
+                .expectDeletedImages("image-1", "my-tag"); // 1h after re-download it is deleted again
+    }
 
     /** Same scenario as in {@link #multipleUnusedImagesAreIdentified()} */
     @Test