Support OS version in platform string #5614

cpuguy83 · 2024-12-27T21:35:45Z

This allows platforms following the new platforms.FormatAll function, which allows for setting the OSVersion field of the platform with <os>(<ver>)/<arch>.

Closes #5115

Note: In order to make FROM <img> match the correct OS version with the new platform format some changes need to be made to containerd/platforms (see vendor/ changes).

frontend/dockerfile/dockerfile_test.go

tonistiigi · 2025-01-07T22:29:27Z

@cpuguy83 Added this to v0.19 , RC planned for 01/10

cpuguy83 · 2025-01-08T01:44:18Z

Upstreaming those vendor changes at containerd/platforms#22

cpuguy83 · 2025-01-13T20:30:24Z

I have updated this to use the new containerd/platforms module version which made the platform matchers for Windows available on non-Windows platforms so that the test can do the version comparisons.

This should be ready for a full review.

cpuguy83 · 2025-01-13T20:32:01Z

exporter/verifier/platforms.go

-				Short: []byte(fmt.Sprintf("Requested platform %q does not match result platform %q", req.Platforms[0], platforms.Format(pp))),
-			}}, nil
+		if _, ok := reqMap[platforms.FormatAll(pp)]; !ok {
+			// The requested platform will often not have an OSVersion on it, but the


This was a little janky.
When the platform string is not exactly what was requested (i.e. no OS version) then the returned result which has the OS Version will trigger a warning.

cpuguy83 · 2025-01-13T22:27:26Z

Looks like linter test is failing now with updated platforms package.

tonistiigi · 2025-01-13T22:30:32Z

Error message change

        	Error:      	Error message not equal:
        	            	expected: "failed to solve: failed to parse platform linux/${MYARCH}: \"\" is an invalid component of \"linux/\": platform specifier component must match \"^[A-Za-z0-9_-]+$\": invalid argument (did you mean MY_ARCH?)"
        	            	actual  : "failed to solve: failed to parse platform linux/${MYARCH}: \"\" is an invalid component of \"linux/\": platform specifier component must match \"^[A-Za-z0-9_.-]+$\": invalid argument (did you mean MY_ARCH?)"

tonistiigi · 2025-01-13T23:00:22Z

frontend/dockerfile/dockerfile_test.go

+
+	dockerfile := []byte(`
+ARG TARGETOS TARGETOSVERSION TARGETARCH
+FROM --platform=${TARGETOS}(${TARGETOSVERSION})/${TARGETARCH} ` + target + ` AS reg


This platform flag in here shouldn't be needed for functionality as the default should work the same. FROM --platform=$TARGETPLATFORM should also work the same.

If you want to check that the parser works in these cases then maybe add other stages for different variants (reg1, reg2, reg3) and build all in a loop.

Your right no need for setting anything here.

Since you brought it up, I did not change TARGETPLATFORM or BUILDPLATFORM to use FormatAll for fear of breaking things.
I guess TARGETPLATFORM shouldn't be a big deal since it'd just be what the user specified.
BUILDPLATFORM would be different... at least on Windows... which maybe isn't a big deal since its experimental?

Yeah, I think TARGETPLATFORM is fine. BUILDPLATFORM maybe indeed shouldn't set OSVersion. This would be different if there would be "default" osversion and different DefaultSpec() and MaximumSpec(). Eg. in linux/amd64, DefaultSpec() variant is always v1, doesn't matter if the machine actually supports higher versions. In OSVersion this concept doesn't seem to exist and it goes to syscall https://github.com/containerd/platforms/blob/main/defaults_windows.go#L29C33-L29C55 . It's possible we need to define our own DefaultSpec in the follow-up.

tonistiigi · 2025-01-13T23:04:10Z

frontend/dockerfile/dockerfile_test.go

+
+	dt, err = os.ReadFile(filepath.Join(destDir, "osversion"))
+	require.NoError(t, err)
+	require.Equal(t, p2.OSVersion+"\n", string(dt))


Should we check that the correct OSVersion is set in the image config, or is this already covered somewhere?

As a note: We discussed this on Slack and will follow-up with this after merge.

tonistiigi · 2025-01-14T05:23:08Z

@cpuguy83 Containerd v2 was merged so this needs rebase for the platforms pkg vendor.

This allows platforms following the new `platforms.FormatAll` function, which allows for setting the `OSVersion` field of the platform with `<os>(<ver>)/<arch>`. Signed-off-by: Brian Goff <cpuguy83@gmail.com>

github-actions bot added area/dockerfile area/llb area/testing area/dependencies Pull requests that update a dependency file area/client area/frontend area/exporter area/solver area/source labels Dec 27, 2024

cpuguy83 force-pushed the platforms_format_all branch 6 times, most recently from 688785a to 171ad5b Compare January 1, 2025 00:19

tonistiigi requested a review from profnandaa January 2, 2025 17:28

profnandaa reviewed Jan 7, 2025

View reviewed changes

frontend/dockerfile/dockerfile_test.go Show resolved Hide resolved

cpuguy83 force-pushed the platforms_format_all branch 5 times, most recently from a881f71 to 33ecd88 Compare January 7, 2025 18:15

tonistiigi added this to the v0.19.0 milestone Jan 7, 2025

cpuguy83 force-pushed the platforms_format_all branch 2 times, most recently from 1646474 to 212a947 Compare January 7, 2025 23:56

cpuguy83 mentioned this pull request Jan 8, 2025

Move windows matcher logic so all platforms can use containerd/platforms#22

Merged

profnandaa mentioned this pull request Jan 8, 2025

tests: wcow: revisist testPlatformWithOSVersion after mount support #5633

Open

profnandaa approved these changes Jan 8, 2025

View reviewed changes

cpuguy83 force-pushed the platforms_format_all branch from 212a947 to 3713196 Compare January 13, 2025 19:55

cpuguy83 changed the title ~~WIP: Support OS version in platform string~~ Support OS version in platform string Jan 13, 2025

github-actions bot added the area/util label Jan 13, 2025

cpuguy83 force-pushed the platforms_format_all branch 2 times, most recently from 71274ac to eadb841 Compare January 13, 2025 20:28

github-actions bot removed the area/util label Jan 13, 2025

cpuguy83 commented Jan 13, 2025

View reviewed changes

cpuguy83 mentioned this pull request Jan 13, 2025

Add support for specifying multiple base images to use in the spec Azure/dalec#492

Open

cpuguy83 force-pushed the platforms_format_all branch from eadb841 to 357fa67 Compare January 13, 2025 22:17

cpuguy83 force-pushed the platforms_format_all branch from 357fa67 to db512d6 Compare January 13, 2025 22:38

tonistiigi reviewed Jan 13, 2025

View reviewed changes

pmengelbert mentioned this pull request Jan 14, 2025

[REQ] Remove forked buildkit code Azure/dalec#497

Open

1 task

Support OS version in platform string

94ddeb7

This allows platforms following the new `platforms.FormatAll` function, which allows for setting the `OSVersion` field of the platform with `<os>(<ver>)/<arch>`. Signed-off-by: Brian Goff <cpuguy83@gmail.com>

cpuguy83 force-pushed the platforms_format_all branch from db512d6 to 94ddeb7 Compare January 14, 2025 17:27

github-actions bot removed the area/dependencies Pull requests that update a dependency file label Jan 14, 2025

tonistiigi approved these changes Jan 14, 2025

View reviewed changes

tonistiigi added the needs/follow-up label Jan 14, 2025

tonistiigi merged commit e8c26b8 into moby:master Jan 14, 2025
96 checks passed

crazy-max mentioned this pull request Jan 15, 2025

test: missing direct push check feature compat #5661

Merged

cpuguy83 deleted the platforms_format_all branch January 17, 2025 03:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support OS version in platform string #5614

Support OS version in platform string #5614

cpuguy83 commented Dec 27, 2024 •

edited

Loading

tonistiigi commented Jan 7, 2025

cpuguy83 commented Jan 8, 2025

cpuguy83 commented Jan 13, 2025

cpuguy83 Jan 13, 2025

cpuguy83 commented Jan 13, 2025

tonistiigi commented Jan 13, 2025

tonistiigi Jan 13, 2025

cpuguy83 Jan 14, 2025

tonistiigi Jan 14, 2025

tonistiigi Jan 13, 2025

cpuguy83 Jan 14, 2025

tonistiigi commented Jan 14, 2025

Support OS version in platform string #5614

Support OS version in platform string #5614

Conversation

cpuguy83 commented Dec 27, 2024 • edited Loading

tonistiigi commented Jan 7, 2025

cpuguy83 commented Jan 8, 2025

cpuguy83 commented Jan 13, 2025

cpuguy83 Jan 13, 2025

Choose a reason for hiding this comment

cpuguy83 commented Jan 13, 2025

tonistiigi commented Jan 13, 2025

tonistiigi Jan 13, 2025

Choose a reason for hiding this comment

cpuguy83 Jan 14, 2025

Choose a reason for hiding this comment

tonistiigi Jan 14, 2025

Choose a reason for hiding this comment

tonistiigi Jan 13, 2025

Choose a reason for hiding this comment

cpuguy83 Jan 14, 2025

Choose a reason for hiding this comment

tonistiigi commented Jan 14, 2025

cpuguy83 commented Dec 27, 2024 •

edited

Loading