From 0c2d83b5fb1d284897dbb2339ee74c7c56eedd0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Tue, 19 Mar 2024 23:10:34 +0100 Subject: [PATCH 1/3] c8d/list: Handle unpacked layers when calculating shared size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a535a65c4bbd056760128835a63fa172a935d321 the size reported by the image list was changed to include all platforms of that image. This made the "shared size" calculation consider all diff ids of all the platforms available in the image which caused "snapshot not found" errors when multiple images were sharing the same layer which wasn't unpacked. Signed-off-by: Paweł Gronowski --- daemon/containerd/image_list.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/daemon/containerd/image_list.go b/daemon/containerd/image_list.go index 600260a3fd..b7fb926725 100644 --- a/daemon/containerd/image_list.go +++ b/daemon/containerd/image_list.go @@ -650,6 +650,11 @@ func computeSharedSize(chainIDs []digest.Digest, layers map[digest.Digest]int, s } size, err := sizeFn(chainID) if err != nil { + // Several images might share the same layer and neither of them + // might be unpacked (for example if it's a non-host platform). + if cerrdefs.IsNotFound(err) { + continue + } return 0, err } sharedSize += size From ad8a5a5732ee3e66100d92f70de8298bd7dfdb1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Wed, 20 Mar 2024 11:14:15 +0100 Subject: [PATCH 2/3] c8d/list: Fix diffIDs being outputted instead of chainIDs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `identity.ChainIDs` call was accidentally removed in b37ced255103c2dfd36c42685bdd9390f74eb10f. This broke the shared size calculation for images with more than one layer that were sharing the same compressed layer. This was could be reproduced with: ``` $ docker pull docker.io/docker/desktop-kubernetes-coredns:v1.11.1 $ docker pull docker.io/docker/desktop-kubernetes-etcd:3.5.10-0 $ docker system df ``` Signed-off-by: Paweł Gronowski --- daemon/containerd/image_list.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/daemon/containerd/image_list.go b/daemon/containerd/image_list.go index b7fb926725..80b2409015 100644 --- a/daemon/containerd/image_list.go +++ b/daemon/containerd/image_list.go @@ -255,11 +255,13 @@ func (i *ImageService) imageSummary(ctx context.Context, img images.Image, platf target := img.Target() - chainIDs, err := img.RootFS(ctx) + diffIDs, err := img.RootFS(ctx) if err != nil { return err } + chainIDs := identity.ChainIDs(diffIDs) + ts, _, err := i.singlePlatformSize(ctx, img) if err != nil { return err From 3312b8251545f1179bdc2d481019216f32875119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Wed, 20 Mar 2024 12:32:18 +0100 Subject: [PATCH 3/3] c8d/list: Add a test case for images sharing a top layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Gronowski --- integration/image/list_test.go | 35 +++++++++++++++ internal/testutils/specialimage/multilayer.go | 45 ++++++++++++------- 2 files changed, 64 insertions(+), 16 deletions(-) diff --git a/integration/image/list_test.go b/integration/image/list_test.go index b653802491..54d725315f 100644 --- a/integration/image/list_test.go +++ b/integration/image/list_test.go @@ -10,10 +10,14 @@ import ( "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/image" "github.com/docker/docker/integration/internal/container" + "github.com/docker/docker/internal/testutils/specialimage" "github.com/docker/docker/testutil" + "github.com/docker/docker/testutil/daemon" "github.com/google/go-cmp/cmp/cmpopts" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" + "gotest.tools/v3/skip" ) // Regression : #38171 @@ -192,3 +196,34 @@ func TestAPIImagesFilters(t *testing.T) { }) } } + +// Verify that the size calculation operates on ChainIDs and not DiffIDs. +// This test calls an image list with two images that share one, top layer. +func TestAPIImagesListSizeShared(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux") + + ctx := setupTest(t) + + daemon := daemon.New(t) + daemon.Start(t) + defer daemon.Stop(t) + + client := daemon.NewClientT(t) + + specialimage.Load(ctx, t, client, func(dir string) (*ocispec.Index, error) { + return specialimage.MultiLayerCustom(dir, "multilayer:latest", []specialimage.SingleFileLayer{ + {Name: "bar", Content: []byte("2")}, + {Name: "foo", Content: []byte("1")}, + }) + }) + + specialimage.Load(ctx, t, client, func(dir string) (*ocispec.Index, error) { + return specialimage.MultiLayerCustom(dir, "multilayer2:latest", []specialimage.SingleFileLayer{ + {Name: "asdf", Content: []byte("3")}, + {Name: "foo", Content: []byte("1")}, + }) + }) + + _, err := client.ImageList(ctx, image.ListOptions{SharedSize: true}) + assert.NilError(t, err) +} diff --git a/internal/testutils/specialimage/multilayer.go b/internal/testutils/specialimage/multilayer.go index ee20380084..b7352d019d 100644 --- a/internal/testutils/specialimage/multilayer.go +++ b/internal/testutils/specialimage/multilayer.go @@ -16,20 +16,32 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) -func MultiLayer(dir string) (*ocispec.Index, error) { - const imageRef = "multilayer:latest" +type SingleFileLayer struct { + Name string + Content []byte +} - layer1Desc, err := writeLayerWithOneFile(dir, "foo", []byte("1")) - if err != nil { - return nil, err - } - layer2Desc, err := writeLayerWithOneFile(dir, "bar", []byte("2")) - if err != nil { - return nil, err - } - layer3Desc, err := writeLayerWithOneFile(dir, "hello", []byte("world")) - if err != nil { - return nil, err +func MultiLayer(dir string) (*ocispec.Index, error) { + return MultiLayerCustom(dir, "multilayer:latest", []SingleFileLayer{ + {Name: "foo", Content: []byte("1")}, + {Name: "bar", Content: []byte("2")}, + {Name: "hello", Content: []byte("world")}, + }) +} + +func MultiLayerCustom(dir string, imageRef string, layers []SingleFileLayer) (*ocispec.Index, error) { + var layerDescs []ocispec.Descriptor + var layerDgsts []digest.Digest + var layerBlobs []string + for _, layer := range layers { + layerDesc, err := writeLayerWithOneFile(dir, layer.Name, layer.Content) + if err != nil { + return nil, err + } + + layerDescs = append(layerDescs, layerDesc) + layerDgsts = append(layerDgsts, layerDesc.Digest) + layerBlobs = append(layerBlobs, blobPath(layerDesc)) } configDesc, err := writeJsonBlob(dir, ocispec.MediaTypeImageConfig, ocispec.Image{ @@ -39,7 +51,7 @@ func MultiLayer(dir string) (*ocispec.Index, error) { }, RootFS: ocispec.RootFS{ Type: "layers", - DiffIDs: []digest.Digest{layer1Desc.Digest, layer2Desc.Digest, layer3Desc.Digest}, + DiffIDs: layerDgsts, }, }) if err != nil { @@ -49,14 +61,14 @@ func MultiLayer(dir string) (*ocispec.Index, error) { manifest := ocispec.Manifest{ MediaType: ocispec.MediaTypeImageManifest, Config: configDesc, - Layers: []ocispec.Descriptor{layer1Desc, layer2Desc, layer3Desc}, + Layers: layerDescs, } legacyManifests := []manifestItem{ { Config: blobPath(configDesc), RepoTags: []string{imageRef}, - Layers: []string{blobPath(layer1Desc), blobPath(layer2Desc), blobPath(layer3Desc)}, + Layers: layerBlobs, }, } @@ -128,6 +140,7 @@ func writeLayerWithOneFile(dir string, filename string, content []byte) (ocispec if err != nil { return ocispec.Descriptor{}, err } + defer rd.Close() return writeBlob(dir, ocispec.MediaTypeImageLayer, rd) }