Fallback to manifest list when no platform match by cpuguy83 · Pull Request #41955 · moby/moby

cpuguy83 · 2021-02-01T19:17:35Z

relates to / addresses docker/for-linux#1170 (comment)

In some cases, in fact many in the wild, an image may have the incorrect
platform on the image config.
This can lead to failures to run an image, particularly when a user
specifies a --platform.
Typically what we see in the wild is a manifest list with an an entry
for, as an example, linux/arm64 pointing to an image config that has
linux/amd64 on it.

This change falls back to looking up the manifest list for an image to
see if the manifest list shows the image as the correct one for that
platform.

In order to accomplish this we need to traverse the leases associated
with an image. Each image, if pulled with Docker 20.10, will have the
manifest list stored in the containerd content store with the resource
assigned to a lease keyed on the image ID.
So we look up the lease for the image, then look up the assocated
resources to find the manifest list, then check the manifest list for a
platform match, then ensure that manifest referes to our image config.

This is only used as a fallback when a user specified they want a
particular platform and the image config that we have does not match
that platform.

tianon · 2021-02-01T20:06:15Z

Just to note it somewhere - this is due to a quirk in containerd's platform matching where platforms.Normalize is invoked on both the argument to platforms.Only and the things it's being compared against, so linux/arm gets normalized to linux/arm/v7 in both cases (and thus fails if you specify --platform linux/arm/v6 on a single-arch, no-manifest-list image that only has os and architecture).

Ideally, we'd fix platforms.Only in containerd to handle this case better, but it's complicated. 😅

tianon

Conceptually this seems like a good workaround IMO - the logic in GetImage makes sense.

I did my best to review the flow in manifestMatchesPlatform and it seems sane to me, but I felt like I was stretching my understanding of all the APIs at play in reviewing it (so it should definitely get more eyes 🙈).

thaJeztah · 2021-02-04T20:09:57Z

Also check how it behaves on https://hub.docker.com/r/balenalib/rpi-raspbian, which is not multi-arch, but has wrong arch

In some cases, in fact many in the wild, an image may have the incorrect platform on the image config. This can lead to failures to run an image, particularly when a user specifies a `--platform`. Typically what we see in the wild is a manifest list with an an entry for, as an example, linux/arm64 pointing to an image config that has linux/amd64 on it. This change falls back to looking up the manifest list for an image to see if the manifest list shows the image as the correct one for that platform. In order to accomplish this we need to traverse the leases associated with an image. Each image, if pulled with Docker 20.10, will have the manifest list stored in the containerd content store with the resource assigned to a lease keyed on the image ID. So we look up the lease for the image, then look up the assocated resources to find the manifest list, then check the manifest list for a platform match, then ensure that manifest referes to our image config. This is only used as a fallback when a user specified they want a particular platform and the image config that we have does not match that platform. Signed-off-by: Brian Goff <[email protected]>

cpuguy83 · 2021-02-17T19:16:36Z

@thaJeztah That image doesn't have a manifest list so this PR doesn't improve anything there as far as I can tell.

cpuguy83 · 2021-02-17T20:44:15Z

I updated this to move the CPU variant fallback to a platform matcher that wraps platforms.Only.

re: #41955 (comment)

In the case where a user does specify a platform and there is no match we error out with the same details. I don't think it makes sense to warn and error.

thaJeztah · 2021-02-18T15:14:31Z

Looks like there's a linting failiure;

daemon/images/image.go:184:3: S1023: redundant `return` statement (gosimple)
		return
 		^

thaJeztah · 2021-02-18T15:15:11Z

Suggested change

retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but does not match the specified platform: wanted %s, actual: %s", refOrID, platforms.Format(p), platforms.Format(imgPlat)))

return

retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but does not match the specified platform: wanted %s, actual: %s", refOrID, platforms.Format(p), platforms.Format(imgPlat)))

(edited; noticed this was in the defer, so no error to return)

Wrap platforms.Only and fallback to our ignore mismatches due to empty CPU variants. This just cleans things up and makes the logic re-usable in other places. Signed-off-by: Brian Goff <[email protected]>

thaJeztah

SGTM

tonistiigi

SGTM

thaJeztah · 2021-02-18T19:59:45Z

Test failiure is unrelated;


[2021-02-18T17:11:53.367Z] === FAIL: amd64.integration.image TestRemoveImageGarbageCollector (1.08s)
[2021-02-18T17:11:53.367Z]     remove_unix_test.go:82: assertion failed: error is not nil: Error: No such image: test-garbage-collector

that test has the same problem as I fixed in #41936

cpuguy83 requested review from tianon and tonistiigi February 1, 2021 19:17

cpuguy83 force-pushed the fallback_manifest_on_bad_plat branch from 0d33ce8 to 217f0db Compare February 1, 2021 19:41

tianon reviewed Feb 1, 2021

View reviewed changes

Comment thread daemon/images/image.go Outdated

cpuguy83 force-pushed the fallback_manifest_on_bad_plat branch 2 times, most recently from c9476b8 to 7f83ec5 Compare February 1, 2021 19:48

tianon reviewed Feb 1, 2021

View reviewed changes

tianon approved these changes Feb 1, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

cpuguy83 force-pushed the fallback_manifest_on_bad_plat branch from 7f83ec5 to 504e418 Compare February 17, 2021 20:42

cpuguy83 force-pushed the fallback_manifest_on_bad_plat branch from 504e418 to 78fb419 Compare February 17, 2021 21:21

thaJeztah added area/distribution Image Distribution area/images Image Service status/2-code-review labels Feb 17, 2021

cpuguy83 force-pushed the fallback_manifest_on_bad_plat branch from 78fb419 to ca81b2a Compare February 18, 2021 00:44

thaJeztah added this to the 21.xx milestone Feb 18, 2021

thaJeztah added the process/cherry-pick label Feb 18, 2021

thaJeztah reviewed Feb 18, 2021

View reviewed changes

Move cpu variant checks into platform matcher

50f39e7

Wrap platforms.Only and fallback to our ignore mismatches due to empty CPU variants. This just cleans things up and makes the logic re-usable in other places. Signed-off-by: Brian Goff <[email protected]>

cpuguy83 force-pushed the fallback_manifest_on_bad_plat branch from ca81b2a to 50f39e7 Compare February 18, 2021 16:59

thaJeztah approved these changes Feb 18, 2021

View reviewed changes

tonistiigi approved these changes Feb 18, 2021

View reviewed changes

thaJeztah merged commit 56ffa61 into moby:master Feb 18, 2021

cpuguy83 mentioned this pull request Feb 18, 2021

[20.10] Fallback to manifest list when no platform match #42045

Merged

cpuguy83 deleted the fallback_manifest_on_bad_plat branch February 18, 2021 20:18

thaJeztah added process/cherry-picked impact/changelog and removed process/cherry-pick labels Feb 18, 2021

thaJeztah mentioned this pull request Feb 23, 2021

[v20.10] Platform architecture detection is broken docker/for-linux#1170

Closed

3 tasks

thaJeztah mentioned this pull request Apr 26, 2021

docker pull --platform=linux/arm64 silently falls back to linux/amd64 image docker/for-mac#5625

Closed

3 tasks

thaJeztah mentioned this pull request Aug 1, 2024

c8d/image: Simplify presentImages and better "platform not found" error #48276

Merged

coderabbitai Bot mentioned this pull request Jul 8, 2026

CI: probe Windows runner for a Linux Docker engine (throwaway) Oxen-AI/Oxen#710

Closed

	retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but does not match the specified platform: wanted %s, actual: %s", refOrID, platforms.Format(p), platforms.Format(imgPlat)))
	return
	retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but does not match the specified platform: wanted %s, actual: %s", refOrID, platforms.Format(p), platforms.Format(imgPlat)))

Uh oh!

Conversation

cpuguy83 commented Feb 1, 2021 • edited by thaJeztah Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

tianon Feb 1, 2021

Choose a reason for hiding this comment

Uh oh!

tianon left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

thaJeztah commented Feb 4, 2021

Uh oh!

cpuguy83 commented Feb 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cpuguy83 commented Feb 17, 2021

Uh oh!

thaJeztah commented Feb 18, 2021

Uh oh!

thaJeztah Feb 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah Feb 18, 2021

Choose a reason for hiding this comment

Uh oh!

cpuguy83 Feb 18, 2021

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

tonistiigi left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Feb 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cpuguy83 commented Feb 1, 2021 •

edited by thaJeztah

Loading

cpuguy83 commented Feb 17, 2021 •

edited

Loading

thaJeztah Feb 18, 2021 •

edited

Loading