exporter: export multiple refs with local/image exporter #499

tonistiigi · 2018-07-11T06:00:00Z

~~based on #496~~

Add support for:

exporting map of references with local/image/oci exporter
specifying multiple target platforms for Dockerfile frontend
Implicit platform build args in Dockerfile. Updated version of builder: Proposal: Add TARGETPLATFORM/BUILDPLATFORM default ARG to Dockerfile moby#32487

Example:

from --platform=$BUILDPLATFORM busybox AS bb
arg TARGETPLATFORM
run echo "i am $TARGETPLATFORM" > /whoami
from scratch
copy --from=bb whoami .

buildctl build --frontend-opt platform=linux/arm,linux/amd64,windows/arm,windows/amd64 --frontend=dockerfile.v0 --local context=. --local dockerfile=. --exporter=local --exporter-opt output=/tmp/out

> ls -l /tmp/out
drwxr-xr-x 2 root root 4096 Jul 10 22:49 linux_amd64
drwxr-xr-x 2 root root 4096 Jul 10 22:49 linux_arm_v7
drwxr-xr-x 2 root root 4096 Jul 10 22:49 windows_amd64
drwxr-xr-x 2 root root 4096 Jul 10 22:49 windows_arm_v7
> cat /tmp/out/linux_arm_v7/whoami
i am linux/arm/v7

AkihiroSuda · 2018-07-11T06:32:40Z

frontend/dockerfile/dockerfile2llb/convert.go

+
+func getPlatformArgs(bp, tp specs.Platform) []instructions.KeyValuePairOptional {
+	m := map[string]string{
+		"BUILDPLATFORM":  platforms.Format(bp),


Maybe we should add underscore prefixes to these reserved args?

One thing to note is that these args are in no way "reserved". I think this can be introduced with no disruption to current users. These args do not leak the RUN processes unless explicitly exposed with ARG, like the current global args. If the user happens to use or wants to use an ARG with the same name they can do that and it overrides the automatic values.

That being said we should, of course, get the names right so we don't need to change them.

cc @thaJeztah

If the user happens to use or wants to use an ARG with the same name they can do that and it overrides the automatic values.

They can but they should not?
Maybe we should print warning, because people may use the same name accidentally without being aware of this PR

Actually someone is already using ENV TARGETOS=.. TARGETARCH=.. in his/her own Dockerfile

https://raw.githubusercontent.com/ARwMq9b6/dnsproxy/master/cmd/dnsproxy/Dockerfile

Users should always get the behavior they expect. If these vars are used in the Dockerfile and user doesn't know about the automatic values and uses them as regular ones they will behave as regular ones. Only way the automatic behavior applies is if the Dockerfile author knows about the automatic behavior and uses these vars without passing in the values.

For the link, there is no behavior change. This Dockerfile will continue to work exactly like before.

To fill these vars automatically they would need to expose it as arg in a stage and not provide the value(eg. explicitly break the current behavior)

Namespacing these will be tricky yes; not sure it underscores would work (esthetically, it feels ugly, but of course that's personal).

we're a bit limited because arg double as env vars, so (officially) dots aren't allowed, which could've been used for namespacing (then again, we don't want these names to become too long)

Separate from the namespacing discussion; wondering if they should be named TARGET_OS and TARGET_ARCH

For TARGET_OS, the current is similar to GOOS/GOARCH

hm yeah 🤔

tonistiigi · 2018-07-16T18:26:14Z

I added support to image/oci exporter as well so we can now export manifest lists. Still needs more tests and a containerd vendor update. I'll try to move some commits to separate PRs to ease the review.

demo:

tonistiigi · 2018-07-17T22:38:14Z

Added tests for the image and local exporters. Ready for review.

tonistiigi · 2018-07-19T06:14:13Z

@tiborvass Do you want to review this as well?

tonistiigi · 2018-07-23T18:01:24Z

ping @tiborvass

tiborvass · 2018-07-25T18:41:07Z

exporter/containerimage/writer.go

+
+	var p exptypes.Platforms
+	if err := json.Unmarshal(platformsBytes, &p); err != nil {
+		return nil, err


errors.Wrapf

tiborvass · 2018-07-25T18:45:18Z

exporter/containerimage/writer.go

+	}
+
+	refs := make([]cache.ImmutableRef, 0, len(inp.Refs))
+	layersMap := make(map[string]int, len(refs))


len(refs) == 0 all the time

I believe you meant len(inp.Refs)

tiborvass · 2018-07-25T19:17:34Z

frontend/dockerfile/dockerfile2llb/convert.go

@@ -242,6 +247,7 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
 		}
 		if d.base != nil {
 			d.state = d.base.state
+			d.platform = d.base.platform


Sorry I'm not very familiar with this part, but just making sure we're not overwriting d.platform. Shouldn't this be under if d.platform == nil ?

If you are inheriting from a stage you should also inherit the platform of that stage. I don't think any other case makes sense.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

tiborvass

LGTM

ijc · 2018-08-03T16:15:24Z

exporter/containerimage/export.go

+			if i.meta == nil {
+				i.meta = make(map[string][]byte)
+				i.meta[k] = []byte(v)
+			}


Should this not be:

if i.meta == nil { i.meta = make(map[string][]byte) } i.meta[k] = []byte(v)

? Same in the OCI one.

I decided yes, or at least worthy of a PR: #558

westurner · 2020-04-27T19:23:03Z

Is there a list of the values of e.g. TARGETARCH somewhere?

https://github.com/moby/buildkit/pull/499/files#diff-d2c6cca61811c3741627c28c7e7b8ab6R183-R185
https://docs.docker.com/engine/reference/builder/#automatic-platform-args-in-the-global-scope lists variable names but not values
https://gist.github.com/sujaypillai/74d15e0d690a265a1b705cf3b26c6082#file-predefined_platform_arg This Dockerfile echos each variable's value for that build; but not all of the possible values

The use case here is templating TARGETARCH into a URL that already uses x86_64 | aarch64 | ppe64le. E.g. arm is not specific enough to distinguish between the various arm architectures.

tonistiigi · 2020-04-27T19:28:03Z

@westurner I'm not sure what you mean by all possible values. There is no strict list. Eg. https://github.com/tonistiigi/wasm-cli-plugin uses --platform=wasi/wasm and didn't require any changes. https://github.com/containerd/containerd/blob/master/platforms/platforms.go#L17 defines the format.

westurner · 2020-04-27T19:38:43Z

When I specify --platform=linux/arm64, is there a platform variable with the value aarch64? Such as maybe TARGETARCH? Or do I get to sound it out every time?

A table with the most common outputs given the inputs in the docs would eliminate the need to run https://gist.github.com/sujaypillai/74d15e0d690a265a1b705cf3b26c6082#file-predefined_platform_arg with each platform value listed in the docs:
https://github.com/docker/buildx/#---platformvaluevalue:

docker buildx build --platform=linux/arm64 .
docker buildx build --platform=linux/amd64,linux/arm64,linux/arm/v7 .
docker buildx build --platform=darwin .

(Another related documentation issue: It's not clear (from the buildx docs) whether it's still necessary to specify docker buildx build --platform. docker build --platform works now, correct?)

tonistiigi · 2020-04-27T19:47:25Z

is there a platform variable with the value aarch64?

No, based on https://github.com/containerd/containerd/blob/master/platforms/platforms.go#L86-L94 aarch64 is normalized to arm64. --platform=linux/arm64 means TARGETARCH=arm64 . --platform=linux/aarch64 means TARGETARCH=arm64. --platform=foo/bar means TARGETARCH=bar.

By running https://gist.github.com/sujaypillai/74d15e0d690a265a1b705cf3b26c6082#file-predefined_platform_arg you just see the values for one specific node and a specific --platform value. As I said, there is no definitive list. If you add different nodes and pass different --platform values you get different sets of output.

docker build --platform works now, correct?

The difference is that buildx allows you to specify multiple values with --platform with container driver while docker build currently does not.

westurner · 2020-04-27T21:11:49Z

Got it. Thanks!

…

On Mon, Apr 27, 2020, 3:47 PM Tõnis Tiigi ***@***.***> wrote: is there a platform variable with the value aarch64? No, based on https://github.com/containerd/containerd/blob/master/platforms/platforms.go#L86-L94 aarch64 is normalized to arm64. --platform=linux/arm64 means TARGETARCH=arm64 . --platform=linux/aarch64 means TARGETARCH=arm64. --platform=foo/bar means TARGETARCH=bar. By running https://gist.github.com/sujaypillai/74d15e0d690a265a1b705cf3b26c6082#file-predefined_platform_arg you just see the values for one specific node and a specific --platform value. As I said, there is no definitive list. If you add different nodes and pass different --platform values you get different sets of output. docker build --platform works now, correct? The difference is that buildx allows you to specify multiple values with --platform with container driver while docker build currently does not. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#499 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAMNS7MDFN7FZJ73Z52D7LROXONXANCNFSM4FJKEVCA> .

tonistiigi added the status/do-not-merge label Jul 11, 2018

AkihiroSuda reviewed Jul 11, 2018

View reviewed changes

tonistiigi force-pushed the caps3 branch from 74fa0c6 to 79e2ea9 Compare July 11, 2018 17:44

tonistiigi mentioned this pull request Jul 12, 2018

gateway: update client and allow map return #496

Merged

tonistiigi force-pushed the caps3 branch 2 times, most recently from f1ea11d to fb31d5a Compare July 13, 2018 00:55

tonistiigi mentioned this pull request Jul 16, 2018

builder: Proposal: Add TARGETPLATFORM/BUILDPLATFORM default ARG to Dockerfile moby/moby#32487

Closed

tonistiigi changed the title ~~exporter: export multiple refs with local exporter~~ exporter: export multiple refs with local/image exporter Jul 16, 2018

tonistiigi force-pushed the caps3 branch 4 times, most recently from 14c3bcd to 3323f6f Compare July 16, 2018 20:37

This was referenced Jul 16, 2018

vendor: update fsutil to cb2ad947b #506

Merged

buildkitd: allow custom worker platforms for containerd #508

Merged

diff: implement windows layer support for linux #510

Merged

tonistiigi force-pushed the caps3 branch 4 times, most recently from ec6acd3 to 910a2f4 Compare July 17, 2018 18:58

tonistiigi removed the status/do-not-merge label Jul 17, 2018

tonistiigi requested review from tiborvass and AkihiroSuda July 19, 2018 05:42

AkihiroSuda approved these changes Jul 19, 2018

View reviewed changes

tonistiigi force-pushed the caps3 branch from 8debb0d to ebf0fd2 Compare July 20, 2018 23:12

tiborvass reviewed Jul 25, 2018

View reviewed changes

tonistiigi added 2 commits July 25, 2018 14:37

exporter: update export signature

d70d816

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

dockerfile: support exporting multiple platforms

33f4382

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

tonistiigi force-pushed the caps3 branch from ebf0fd2 to 6cb6207 Compare July 25, 2018 21:43

tonistiigi added 5 commits July 25, 2018 16:01

dockerfile: allow automatic platform variables

69bb8e8

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

exporter: allow export map with local exporter

0e122d4

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

exporter: reuse exptypes constants

8c06540

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

export: manifest list support

6be1257

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

dockerfile: add tests for multi-platform exports

f719aad

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

tonistiigi force-pushed the caps3 branch from 6cb6207 to f719aad Compare July 25, 2018 23:01

tiborvass approved these changes Jul 25, 2018

View reviewed changes

tiborvass merged commit d87d9d4 into moby:master Jul 25, 2018

ijc reviewed Aug 3, 2018

View reviewed changes

westurner mentioned this pull request Apr 27, 2020

Multi-Arch image conda-forge/docker-images#102

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

exporter: export multiple refs with local/image exporter #499

exporter: export multiple refs with local/image exporter #499

tonistiigi commented Jul 11, 2018 •

edited

Loading

AkihiroSuda Jul 11, 2018

tonistiigi Jul 11, 2018

tonistiigi Jul 11, 2018

AkihiroSuda Jul 12, 2018

AkihiroSuda Jul 12, 2018

tonistiigi Jul 12, 2018

tonistiigi Jul 12, 2018

thaJeztah Jul 16, 2018

tonistiigi Jul 16, 2018

thaJeztah Jul 17, 2018

tonistiigi commented Jul 16, 2018 •

edited

Loading

tonistiigi commented Jul 17, 2018

tonistiigi commented Jul 19, 2018

tonistiigi commented Jul 23, 2018

tiborvass Jul 25, 2018

tiborvass Jul 25, 2018

tiborvass Jul 25, 2018

tiborvass Jul 25, 2018

tonistiigi Jul 25, 2018

tiborvass left a comment

ijc Aug 3, 2018

ijc Aug 6, 2018

westurner commented Apr 27, 2020

tonistiigi commented Apr 27, 2020

westurner commented Apr 27, 2020

tonistiigi commented Apr 27, 2020

westurner commented Apr 27, 2020 via email

exporter: export multiple refs with local/image exporter #499

exporter: export multiple refs with local/image exporter #499

Conversation

tonistiigi commented Jul 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tonistiigi commented Jul 16, 2018 • edited Loading

tonistiigi commented Jul 17, 2018

tonistiigi commented Jul 19, 2018

tonistiigi commented Jul 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiborvass left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westurner commented Apr 27, 2020

tonistiigi commented Apr 27, 2020

westurner commented Apr 27, 2020

tonistiigi commented Apr 27, 2020

westurner commented Apr 27, 2020 via email

tonistiigi commented Jul 11, 2018 •

edited

Loading

tonistiigi commented Jul 16, 2018 •

edited

Loading