c8d: Fix re-pull of an image when the snapshotter is changed#45647
c8d: Fix re-pull of an image when the snapshotter is changed#45647neersighted merged 1 commit intomoby:masterfrom
Conversation
f793648 to
19e2c9e
Compare
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM, but left two questions 😅
daemon/containerd/image_snapshot.go
Outdated
| } | ||
| } | ||
|
|
||
| desc, err = containerdimages.Config(ctx, cs, desc, matcher) |
There was a problem hiding this comment.
Silly question; does this step need the unpacked image to do its job? (Mostly wondering if the unpack can be done later)
There was a problem hiding this comment.
Config doesn't need the image to be unpacked, I can move the code but the descriptor returned by Config is immediately used to get the RootFS so I thought it might be nicer to bundle related things closer
There was a problem hiding this comment.
Gotcha; my take was "try to do lightweight checks first, before we do heavy-lifting".
I must admit that I haven't checked in-depth on that, but perhaps you know from the top of your head.
There was a problem hiding this comment.
Config is in fact a heavy-lifter :)
I did however removed the call to resolveDescriptor and are calling resolveImage directly, that way we don't have to get the image twice
| return err | ||
| } | ||
| platformImg := containerd.NewImageWithPlatform(i.client, img, matcher) | ||
| unpacked, err := platformImg.IsUnpacked(ctx, i.snapshotter) |
There was a problem hiding this comment.
At a quick glance, PrepareShapshot() is only called on container create, correct? So in this case we must always (at least currently) use the snapshotter that's used as a default, and never take existing containers (which were potentially created with "previous default") into account?
There was a problem hiding this comment.
Yeah, this is only used when a container is created. The containers from other snapshotters are filtered out and not seen IIRC
|
Failure is unrelated, and tracked in #44128 (still quite flaky that one) |
Switching snapshotter implementations would result in an error when preparing a snapshot, check that the image is indeed unpacked for the current snapshot before trying to prepare a snapshot. Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
19e2c9e to
ed32f5e
Compare
- What I did
Changing the snapshotter would result in docker re-pulling the image because the
PrepareSnapshotmethod would return an error. We make sure that the image we want to prepare a snapshot for is indeed unpacked for the current snapshotter before preparing it.Relates to #45273
- How I did it
- How to verify it
overlayfssnapshotterdocker run --rm hello-worldnativefor example)docker images, you should see thehello-worldimage in the listdocker run --rm hello-world, docker should just run the image without re-pulling it- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)