containerd: ensure loaded images are dangling with containerd backend#49669
containerd: ensure loaded images are dangling with containerd backend#49669jsternberg wants to merge 1 commit intomoby:masterfrom
Conversation
d04ea74 to
196a401
Compare
Force images loaded using `LoadImage` to create a dangling reference for any created images. This ensures that the image will persist if it is overridden by another pull, load, import, or build. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
196a401 to
774c26b
Compare
|
I've made a change to the image list so it returns the repo digests correctly. The problem was that when there was a dangling image and a non-dangling form the dangling form overwrote the non-dangling one during deduplication. I've now made it so that won't happen anymore. |
vvoland
left a comment
There was a problem hiding this comment.
Ideally, IMO it would be better to go into an opposite direction - completely get rid of these "dangling" images.
However this might be a viable strategy too, I need to give it a little bit more thought.
Since this would also probably require adjustments of other commands (and possibly even use the new containerd v2 feature to GC these dangling images), let me leave a blocking review for now.
For now, let's fix the immediate docker load issue with #49650 first.
|
@vvoland I'd agree getting rid of the dangling images entirely and using GC features in containerd would be ideal. At least for the interim, I think this method is better than #49650. Both of those would be different solutions to the current interim problem of the dangling images. I think this one is more consistent. I also have #49672 that overlaps well with this and the change we made into buildkit moby/buildkit#5858. It's certainly easier to implement until we have a viable way to avoid the dangling image name entirely. |
It is not consistent with the assumption present in the whole codebase that a dangling image should only exist if there isn't a named image with the same target. You can find places where we log a warning if we encounter such case: moby/daemon/containerd/image_inspect.go Lines 138 to 146 in 330857a We can't guarantee that whatever interacts with the containerd image store will also be creating these dangling images too. And they really shouldn't, because that's just a concept we brought from the graphdrivers for compatibility. |
|
Going to convert this one to draft temporarily and possibly close it. |
|
I've had a bit more time to think about this and I do think this is still a better way to handle the situation even if there are currently some areas in the code that assume something otherwise. My original explanation for pursuing this method is here: #48907 (comment) I still think it is likely easier for us to ensure the behavior of dangling images by performing the tagging on creation rather than trying to find all of the ways we can potentially overwrite an image. The current method also doesn't allow us to atomically overwrite images. We have to tag the overwritten image as dangling, then call into 3rd-party code (usually containerd client code or a buildkit exporter), and then hope that it succeeds and also overwrites the correct image. If a failure happens between tagging something as dangling and the actual creation of the image, we end up in this same inconsistent state. I'm not sure it matters much that someone could interact with the containerd image store outside of docker and keeping this behavior. I think it would be expected that if you manually interact with the containerd image store, you may not get the same behavior regarding dangling images. |
|
After we attempted to convert other portions of the code base to use dangling by default, we ran into some difficulty and ultimately decided that trying to convert the code base to do that when we want to remove dangling images entirely were contradictory goals. We've now been able to fix the areas that weren't leaving dangling images in #48907 so I'm going to close this PR. |
Force images loaded using
LoadImageto create a dangling referencefor any created images. This ensures that the image will persist if it
is overridden by another pull, load, import, or build.