Skip to content

check local image matches the required platform#10546

Merged
ndeloof merged 1 commit intodocker:v2from
ndeloof:check_image_platform
May 10, 2023
Merged

check local image matches the required platform#10546
ndeloof merged 1 commit intodocker:v2from
ndeloof:check_image_platform

Conversation

@ndeloof
Copy link
Copy Markdown
Contributor

@ndeloof ndeloof commented May 10, 2023

What I did
Added a verification that a local image found for service matches the configured platform. Otherwise report error so user can pull the desired image. Please note we can't just force pull as docker engine image store (until configured to run in "containerd image store" mode with Docker Desktop) only stores a single image for a tag.

Related issue
fixes #9740

(not mandatory) A picture of a cute animal, if possible in relation to what you did

@ndeloof ndeloof requested review from a team, StefanScherer, glours, laurazard, milas, nicksieger and ulyssessouza and removed request for a team May 10, 2023 08:48
@codecov
Copy link
Copy Markdown

codecov Bot commented May 10, 2023

Codecov Report

Patch coverage: 10.00% and project coverage change: -0.14 ⚠️

Comparison is base (0c1a691) 59.81% compared to head (f25e7a8) 59.67%.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2   #10546      +/-   ##
==========================================
- Coverage   59.81%   59.67%   -0.14%     
==========================================
  Files         107      107              
  Lines        9294     9312      +18     
==========================================
- Hits         5559     5557       -2     
- Misses       3166     3185      +19     
- Partials      569      570       +1     
Impacted Files Coverage Δ
pkg/compose/build.go 73.05% <10.00%> (-3.58%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Copy Markdown
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good To Me #sorryPrivateJoke

@laurazard
Copy link
Copy Markdown
Member

laurazard commented May 10, 2023

@ndeloof right, this is definitely better than what we had before. I wonder though if we should align with engine behaviour:

$ docker pull --platform=linux/arm64 alpine:3.17
3.17: Pulling from library/alpine
c41833b44d91: Already exists
Digest: sha256:124c7d2707904eea7431fffe91522a01e5a861a624ee31d03372cc1d138a3126
Status: Downloaded newer image for alpine:3.17
docker.io/library/alpine:3.1

$ docker run --platform=linux/amd64 --rm -it alpine:3.17 sh
Unable to find image 'alpine:3.17' locally
3.17: Pulling from library/alpine
f56be85fc22e: Pull complete
Digest: sha256:124c7d2707904eea7431fffe91522a01e5a861a624ee31d03372cc1d138a3126
Status: Downloaded newer image for alpine:3.17
/ # exit

$ docker images
REPOSITORY       TAG       IMAGE ID       CREATED        SIZE
alpine           3.17      9ed4aefc74f6   5 weeks ago    7.05MB
alpine           <none>    51e60588ff2c   5 weeks ago    7.46M

$ docker inspect alpine:3.17
[
    {
        "Id": "sha256:9ed4aefc74f6792b5a804d1d146fe4b4a2299147b0f50eaf2b08435d7b38c27e",
        [...],
        "RepoTags": [
            "alpine:3.17"
        ],
        "RepoDigests": [],
        "Architecture": "amd64",
        [...],
    }
]

$ docker inspect 51e60588ff2c
[
    {
        "Id": "sha256:51e60588ff2cd9f45792b23de89bfface0a7fbd711d17c5f5ce900a4f6b16260",
        [...],
        "RepoTags": [],
        "RepoDigests": [
            "alpine@sha256:124c7d2707904eea7431fffe91522a01e5a861a624ee31d03372cc1d138a3126
        ],
        "Architecture": "arm64",
        "Variant": "v8",
        [...],
    }
]

@ndeloof
Copy link
Copy Markdown
Contributor Author

ndeloof commented May 10, 2023

I don't strictly have the same behavior, I get a WARNING:

$ docker pull --platform=linux/arm64 alpine:3.17
3.17: Pulling from library/alpine
Digest: sha256:124c7d2707904eea7431fffe91522a01e5a861a624ee31d03372cc1d138a3126
Status: Downloaded newer image for alpine:3.17
docker.io/library/alpine:3.17

$ docker run --platform=linux/amd64 --rm -it alpine:3.17 sh
Unable to find image 'alpine:3.17' locally
3.17: Pulling from library/alpine
f56be85fc22e: Pull complete 
Digest: sha256:124c7d2707904eea7431fffe91522a01e5a861a624ee31d03372cc1d138a3126
Status: Downloaded newer image for alpine:3.17
WARNING: image with reference alpine was found but does not match the specified platform: wanted linux/amd64, actual: linux/arm64/v8
/ #

Anyway we probably can do something comparable, just considering image is missing and get it pulled. Let me give this a try

@ndeloof
Copy link
Copy Markdown
Contributor Author

ndeloof commented May 10, 2023

I ran some tests, we can support a comparable workflow, while this requires some additional changes so that we don't track required images by tag, but need to add platforms so we can handle some funky compose file like this one:

services:
  hello:
    image: alpine:3.17
    platform: linux/arm64
  toto:
    image: alpine:3.17
    platform: linux/amd64

if you don't mind, I'll keep this for future improvement, so that we - at least - get a correct error message reported to the user when we don't get the expected platform for requested image.

@laurazard
Copy link
Copy Markdown
Member

Sounds good! I made that suggestion but more for discussion, it's possible that we might not even want to do it (and I didn't consider Compose files like that). This is still better than the previous state.

Copy link
Copy Markdown
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ndeloof ndeloof merged commit b776826 into docker:v2 May 10, 2023
@ndeloof ndeloof deleted the check_image_platform branch May 10, 2023 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

On apple M1, docker-compose does not honor the platform: tag for amd64 if only the arm64 version of the same image is present

3 participants