Skip to content

Conversation

@vvoland
Copy link
Contributor

@vvoland vvoland commented Dec 1, 2025

When inspecting multi-platform images where some layer blobs were missing from the content store, the image inspect operation would return too early causing some data (like config details or unpacked size) to be omitted even though are available.

This ensures that docker image inspect returns as much information as possible.

- What I did

- How I did it

- How to verify it

TestImageInspect/inspect image with one layer missing unit test

- Human readable description for the release notes

containerd image store: Fix `docker image inspect` failing to return available image data in case where not all distributable blobs are available locally.

- A picture of a cute animal (not mandatory but encouraged)

@vvoland vvoland added this to the 29.1.2 milestone Dec 1, 2025
@vvoland vvoland self-assigned this Dec 1, 2025
@vvoland vvoland added impact/changelog area/images Image Distribution kind/bugfix PR's that fix bugs containerd-integration Issues and PRs related to containerd integration labels Dec 1, 2025
@github-actions github-actions bot added the area/daemon Core Engine label Dec 1, 2025
// },
// ```
func TestImageInspectWithoutSomeBlobs(t *testing.T) {
t.Skip("TODO(vvoland): Come up with minimal images for this test")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO, but perhaps still fine to merge if we were to do a patch release.

Copy link
Contributor Author

@vvoland vvoland Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A unit test is here:

t.Run("inspect image with one layer missing", func(t *testing.T) {
ctx := logtest.WithT(ctx, t)
service := fakeImageService(t, ctx, cs)
img := toContainerdImage(t, specialimage.MultiLayer)
_, err := service.images.Create(ctx, img)
assert.NilError(t, err)
// Get the manifest to access the layers
mfst, err := c8dimages.Manifest(ctx, cs, img.Target, nil)
assert.NilError(t, err)
assert.Check(t, len(mfst.Layers) > 0, "image should have at least one layer")
// Delete the last layer from the content store
lastLayer := mfst.Layers[len(mfst.Layers)-1]
err = cs.Delete(ctx, lastLayer.Digest)
assert.NilError(t, err)
inspect, err := service.ImageInspect(ctx, img.Name, imagebackend.ImageInspectOpts{})
assert.NilError(t, err)
assert.Check(t, inspect.Config != nil)
assert.Check(t, is.Len(inspect.RootFS.Layers, len(mfst.Layers)))
})

@vvoland vvoland force-pushed the c8d-fix-images branch 3 times, most recently from 1c9798b to 53bdde1 Compare December 1, 2025 16:38
When inspecting multi-platform images where some layer blobs were
missing from the content store, the image inspect operation would return
too early causing some data (like config details or unpacked size) to be
omitted even though are available.

This ensures that `docker image inspect` returns as much information as
possible.

Signed-off-by: Paweł Gronowski <[email protected]>
@vvoland vvoland marked this pull request as ready for review December 1, 2025 16:39
Comment on lines +112 to +116
resp.Author = img.Author
resp.Architecture = img.Architecture
resp.Variant = img.Variant
resp.Os = img.OS
resp.OsVersion = img.OSVersion
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could possibly move some of these together with where we construct the initial response. Did a quick try to see what it would look like; https://github.com/vvoland/moby/compare/c8d-fix-images...thaJeztah:51629_suggestions?expand=1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, not sure if that's better though - currently there's a nicer separation around which fields come from config and it's also in the same place as the special handling for Comment and DiffIDs.

@vvoland vvoland requested a review from thaJeztah December 2, 2025 10:42
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/daemon Core Engine area/images Image Distribution containerd-integration Issues and PRs related to containerd integration impact/changelog kind/bugfix PR's that fix bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Under certain conditions the API output of docker image inspect becomes corrupted

2 participants