-
Notifications
You must be signed in to change notification settings - Fork 18.9k
c8d/inspect: Fix image inspect for incomplete images #51629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| // }, | ||
| // ``` | ||
| func TestImageInspectWithoutSomeBlobs(t *testing.T) { | ||
| t.Skip("TODO(vvoland): Come up with minimal images for this test") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
moby/daemon/containerd/image_inspect_test.go
Lines 64 to 88 in 2e3a23c
| 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))) | |
| }) |
196632b to
3b3c49b
Compare
1c9798b to
53bdde1
Compare
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]>
53bdde1 to
2e3a23c
Compare
| resp.Author = img.Author | ||
| resp.Architecture = img.Architecture | ||
| resp.Variant = img.Variant | ||
| resp.Os = img.OS | ||
| resp.OsVersion = img.OSVersion |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
thaJeztah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
docker image inspectbecomes corrupted #51566When 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 inspectreturns as much information as possible.- What I did
- How I did it
- How to verify it
TestImageInspect/inspect image with one layer missingunit test- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)