From d122ea0aea0fc824c81e999d9fb8280d8eaa298a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 20 Oct 2024 11:14:47 +0200 Subject: [PATCH] api: GET /images/json: preserve original manifest order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `manifests` option, as used for the `--tree` option on `docker image ls` currently sorts manifests to put those that are present first. The intent was to present "available" images at the top of each tree, followed by images that were not pulled. However, there's some limitations to this. First of all, the current approach makes the output non-deterministic as the order in which variants are pulled determines the order in which they're presented, i.e., the last pulled variant is returned first (I omitted some variants in the example for brevity); Here's the result of pulling `linux/riscv64`, then pulling `linux/arm64`; docker pull --platform=linux/riscv64 alpine:latest docker image ls -a --tree IMAGE ID DISK USAGE CONTENT SIZE USED alpine:latest beefdbd8a1da 10.6MB 3.37MB ├─ linux/riscv64 80cde017a105 10.6MB 3.37MB ├─ linux/amd64 33735bd63cf8 0B 0B └─ linux/arm64/v8 9cee2b382fe2 0B 0B docker pull --platform=linux/arm64 alpine:latest docker image ls -a --tree IMAGE ID DISK USAGE CONTENT SIZE USED alpine:latest beefdbd8a1da 24.2MB 7.46MB ├─ linux/riscv64 80cde017a105 10.6MB 3.37MB ├─ linux/arm64/v8 9cee2b382fe2 13.6MB 4.09MB └─ linux/amd64 33735bd63cf8 0B 0B Repeating the steps but in reverse order results in the output to be reversed; docker image rm alpine:latest docker pull --platform=linux/arm64 alpine:latest docker image ls -a --tree IMAGE ID DISK USAGE CONTENT SIZE USED alpine:latest beefdbd8a1da 13.6MB 4.09MB ├─ linux/arm64/v8 9cee2b382fe2 13.6MB 4.09MB ├─ linux/amd64 33735bd63cf8 0B 0B └─ linux/riscv64 80cde017a105 0B 0B docker image ls -a --tree IMAGE ID DISK USAGE CONTENT SIZE USED alpine:latest beefdbd8a1da 24.2MB 7.46MB ├─ linux/riscv64 80cde017a105 10.6MB 3.37MB ├─ linux/arm64/v8 9cee2b382fe2 13.6MB 4.09MB └─ linux/amd64 33735bd63cf8 0B 0B The second limitation is that order sometimes matters; when matching a platform from a manifest-index, implementations may find multiple suitable candidates. In most cases the _most_ suitable candidate can be selected (e.g., prefer `linux/arm/v7` over `linux/arm/v6`), but manifest-indices do allow multiple entries for the same platform, in which case implementations match the first entry found. While these situations will be less common (and usually due to incorect use of tooling such as `docker manifest`), being able to observe the order in which manifests appeared in the index can help debugging or help the user understand why a specific variant was selected. We should therefore not re-order these manifests, and return them in the order in which they appeared. If we decide to present "present" variants before "non-present" variants, we can do this ordering on the client side. With this patch applied; docker pull --quiet --platform=linux/riscv64 alpine:latest docker pull --quiet --platform=linux/arm64 alpine:latest docker image ls --tree alpine IMAGE ID DISK USAGE CONTENT SIZE USED alpine:latest beefdbd8a1da 24.2MB 7.46MB ├─ linux/amd64 33735bd63cf8 0B 0B ├─ linux/arm/v6 50f635c8b04d 0B 0B ├─ linux/arm/v7 f2f82d424957 0B 0B ├─ linux/arm64/v8 9cee2b382fe2 13.6MB 4.09MB ├─ linux/386 b3e87f642f5c 0B 0B ├─ linux/ppc64le c7a6800e3dc5 0B 0B ├─ linux/riscv64 80cde017a105 10.6MB 3.37MB └─ linux/s390x 2b5b26e09ca2 0B 0B Which matches the order of the manifests in the index: docker buildx imagetools inspect --raw alpine:latest | jq -c .manifests[].platform {"architecture":"amd64","os":"linux"} {"architecture":"arm","os":"linux","variant":"v6"} {"architecture":"arm","os":"linux","variant":"v7"} {"architecture":"arm64","os":"linux","variant":"v8"} {"architecture":"386","os":"linux"} {"architecture":"ppc64le","os":"linux"} {"architecture":"riscv64","os":"linux"} {"architecture":"s390x","os":"linux"} Signed-off-by: Sebastiaan van Stijn --- daemon/containerd/image_list.go | 8 +------- daemon/containerd/image_list_test.go | 4 ++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/daemon/containerd/image_list.go b/daemon/containerd/image_list.go index 68b535bc5443d..2dd73c8d3aecd 100644 --- a/daemon/containerd/image_list.go +++ b/daemon/containerd/image_list.go @@ -248,13 +248,7 @@ func (i *ImageService) imageSummary(ctx context.Context, img images.Image, platf if opts.Manifests { defer func() { - // If the platform is available, prepend it to the list of platforms - // otherwise append it at the end. - if available { - manifestSummaries = append([]imagetypes.ManifestSummary{mfstSummary}, manifestSummaries...) - } else { - manifestSummaries = append(manifestSummaries, mfstSummary) - } + manifestSummaries = append(manifestSummaries, mfstSummary) }() } diff --git a/daemon/containerd/image_list_test.go b/daemon/containerd/image_list_test.go index 60269b9ef8b5c..6a8a420cdaa6c 100644 --- a/daemon/containerd/image_list_test.go +++ b/daemon/containerd/image_list_test.go @@ -247,11 +247,11 @@ func TestImageList(t *testing.T) { assert.Check(t, is.Equal(i.Manifests[0].Kind, imagetypes.ManifestKindImage)) if assert.Check(t, i.Manifests[0].ImageData != nil) { - assert.Check(t, is.Equal(i.Manifests[0].ImageData.Platform.Architecture, "arm64")) + assert.Check(t, is.Equal(i.Manifests[0].ImageData.Platform.Architecture, "amd64")) } assert.Check(t, is.Equal(i.Manifests[1].Kind, imagetypes.ManifestKindImage)) if assert.Check(t, i.Manifests[1].ImageData != nil) { - assert.Check(t, is.Equal(i.Manifests[1].ImageData.Platform.Architecture, "amd64")) + assert.Check(t, is.Equal(i.Manifests[1].ImageData.Platform.Architecture, "arm64")) } }, },