c8d/prune: Keep the last tagged image instead of creating dangling image#48076
Merged
thaJeztah merged 3 commits intomoby:masterfrom Sep 12, 2024
Merged
c8d/prune: Keep the last tagged image instead of creating dangling image#48076thaJeztah merged 3 commits intomoby:masterfrom
thaJeztah merged 3 commits intomoby:masterfrom
Conversation
1ac742d to
783325f
Compare
This comment was marked as resolved.
This comment was marked as resolved.
de6286b to
78fb7ab
Compare
fce4d86 to
f0a608e
Compare
Member
|
I was testing this on graphdrivers vs containerd, and noticed on graphdrivers, we seem to always remove the last tag alphanumerically, but with the containerd implementation it seems a bit random. I wrote a repro here: #!/usr/bin/env bash
set -ex
while true; do
docker image prune -af
docker pull busybox:latest
repoDigest=$(docker inspect busybox:latest | jq .[0].RepoDigests[0] | tr -d '"')
docker tag busybox:latest busybox:a
docker tag busybox:latest busybox:b
docker tag busybox:latest busybox:c
docker rmi busybox:latest
ctrId=$(docker run -d $repoDigest sleep 100)
yes | docker image prune -a
tags=$(docker images --filter=reference='busy*' --format json | jq .Tag | tr -d '"')
tagsLength=${#tags[@]}
if [[ $tagsLength -ne 1 ]]; then
echo "this shouldn't happen! tags length != 1"
exit 1
fi
if [[ $tags != c ]]; then
exit 1
fi
docker kill $ctrId
docker rm $ctrId
done |
Member
|
I don't know know whether we must keep this behavior, but if it doesn't impact performance to sort alphanumerically we should 😅. |
Don't turn images into dangling when they are used by containers created with an image specified by an ID only (e.g. `docker run 82d1e9d`). Keep the last image reference with the same target when all other references would be pruned. If the container was created with a digested and tagged reference (e.g. `docker run alpine:latest@sha256:82d1e9d7ed48a7523bdebc18cf6290bdb97b82302a8a9c27d4fe885949ea94d1`), the `alpine:latest` image won't get untagged. This change makes the behavior consistent with the graphdriver implementation. Signed-off-by: Paweł Gronowski <[email protected]>
Signed-off-by: Paweł Gronowski <[email protected]>
b331db1 to
ec50f0d
Compare
laurazard
reviewed
Sep 11, 2024
ec50f0d to
da49d28
Compare
When untagging multiple images targetting the same digest, delete the images in lexographic order to be consistent with graphdrivers. Signed-off-by: Paweł Gronowski <[email protected]>
da49d28 to
fefa98a
Compare
thaJeztah
approved these changes
Sep 12, 2024
Comment on lines
+113
to
+115
| for name := range imagesToPrune { | ||
| sorted = append(sorted, name) | ||
| } |
Member
There was a problem hiding this comment.
We can probably start looking at the maps package, and for these kind of things, use maps.Keys()
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Don't turn images into dangling when they are used by containers created with an image specified by an ID only (e.g.
docker run 82d1e9d).Keep the last image reference with the same target when all other references would be pruned.
If the container was created with a digested and tagged reference (e.g.
docker run alpine:latest@sha256:82d1e9d7ed48a7523bdebc18cf6290bdb97b82302a8a9c27d4fe885949ea94d1), thealpine:latestimage won't get untagged.This change makes the behavior consistent with the graphdriver implementation.
- What I did
- How I did it
- How to verify it
TestPruneDontDeleteUsedImage
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)