Skip to content
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

c8d/list: Fix shared size calculation #47597

Merged
merged 3 commits into from Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion daemon/containerd/image_list.go
Expand Up @@ -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
Expand Down Expand Up @@ -650,6 +652,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
Expand Down
35 changes: 35 additions & 0 deletions integration/image/list_test.go
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
43 changes: 28 additions & 15 deletions internal/testutils/specialimage/multilayer.go
Expand Up @@ -16,20 +16,32 @@ import (
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
)

type SingleFileLayer struct {
Name string
Content []byte
}

func MultiLayer(dir string) (*ocispec.Index, error) {
const imageRef = "multilayer:latest"
return MultiLayerCustom(dir, "multilayer:latest", []SingleFileLayer{
{Name: "foo", Content: []byte("1")},
{Name: "bar", Content: []byte("2")},
{Name: "hello", Content: []byte("world")},
})
}

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 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{
Expand All @@ -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 {
Expand All @@ -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,
},
}

Expand Down Expand Up @@ -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)
}
Expand Down