`--load` with moby containerd store should use the `oci` exporter #1813

jedevc · 2023-05-18T15:50:24Z

Moby with the containerd store supports importing multi-platform OCI images, which means we should be able to add functionality to have --load work with multi-platform images.

Before:

$ buildx build . --platform linux/amd64,linux/arm64 --builder container --load
[+] Building 0.1s (1/1) FINISHED                                                                                         docker-container:container
 => [internal] connecting to local controller                                                                                                  0.0s
ERROR: failed to build: docker exporter does not currently support exporting manifest lists

After:

$ buildx build . --platform linux/amd64,linux/arm64 --builder container --load
[+] Building 4.0s (24/24) FINISHED                                                                                       docker-container:container
 => [internal] connecting to local controller                                                                                                  0.0s
 => [internal] load build definition from Dockerfile                                                                                           0.0s
 => => transferring dockerfile: 2.20kB                                                                                                         0.0s
...
 => exporting to docker image format                                                                                                              0.6s
 => => exporting layers                                                                                                                        0.0s
 => => exporting manifest sha256:c0dfaa2978d8ea698f0af7ce2a36373789ed20deffd837a4504891f04c28e0f4                                              0.0s
 => => exporting config sha256:6c835b10e81d195b8795ae810667bff2c0c6565430f16f3649f5e4d4f2b6a650                                                0.0s
 => => exporting attestation manifest sha256:b11b99e5db8890a7e66713e4476d20d2d6fa622519140830d3097ef3112d7ed3                                  0.0s
 => => exporting manifest sha256:ecae44f665a1727aea6c7a2adff17ebc2f954ae5aad957443e5aa35b24277983                                              0.0s
 => => exporting config sha256:dbca1a6373dd85c3bb3ea08a6d980fc7674976126ddcfdbcc2bd61c2b57dfebe                                                0.0s
 => => exporting manifest list sha256:c2b6927cbd1a651e05e462294d11e546d83a4b1c4146d0c8195c234d8f98d490                                         0.0s
 => => sending tarball                                                                                                                         0.6s
 => importing to docker

This patch works by translating the docker exporter to an oci exporter (which supports multi-platform exports), just as we were previously translating the docker exporter to the moby exporter for the moby case.

I think this should just work? I've done some basic tests with some sample bake files, and build commands from my history, and get the expected results (and don't get any different behavior on non-containerd moby) 🎉

(I can add some integration tests once #1770 merges)

cc @tonistiigi @vvoland @rumpl

Related issues

tonistiigi · 2023-05-18T16:37:43Z

Allow --output type=oci to act as in --output type=docker when no dest argument is provided, thereby loading the result into moby.
Change buildx build --load to default to using --output type=oci

I don't think this is necessarily what we want. I think we shouldn't change the behavior of -o type=oci or make --load dynamic. Instead, we should change -o type=docker to get the tarball in OCI format if the current Docker supports the OCI format. -o type=docker means "output to Docker". If Docker supports OCI then it should just output to Docker using OCI format internally, which doesn't have the quirks about not supporting multi-platform etc.

TBBle · 2023-05-19T01:47:52Z

I like the idea that when pushing to Docker, it automatically pushes in OCI format if supported. For --load (like the PR's approach) this "just works", which for me is the UX goal. It also means that if people are writing --load longhand (--output=type=docker) then it "just works" with manifest lists. (I'm not as tied to that as a UX goal personally.)

However, this latter approach means --output type=docker and --output type=docker,dest=- | docker load behave differently, in that the latter still fails for an image index/manifest list.

-o type=docker means "output to Docker".

I think this would be surprising to users, and isn't what the --output documentation emphasises, although the --load documentation implies this by being a fairly simple alias.

It's not inconsistent with the docs, but if taking this latter approach, we might also want to rework the --output docs a little to make it clear that type=docker is focussed on somehow getting it into a Docker instance, not on the Docker image tarball file format (which would be legacy under this construction). Right now, it is written as a direct contrast with the oci exporter, and is (apart from the note in dest) all about the tarballs.

tonistiigi · 2023-05-19T02:35:03Z

I think the confusion may come from that in buildx and buildctl -o type=docker do not behave the same. buildctl does export a "docker-style" tar (that actually is OCI compatible as well) while buildx behavior is to perform a docker load. With my suggestion, no current commands should break, and --load works as expected. While in current form this PR breaks some current users of type=oci. I agree that docs may need clarification.

tonistiigi · 2023-05-19T02:50:11Z

means --output type=docker and --output type=docker,dest=- | docker load behave differently, in that the latter still fails for an image index/manifest list.

First of all, there is no need for anyone to write --output type=docker,dest=- | docker load today. If anyone does, it would still work the same way as it works today. We could document that "dest" is an optional parameter, if provided, instead of loading tar, it will be exported in the path, with maximum compatibility with older Docker versions(1.10+). This would not upgrade automatically for a manifest list, but users would usually know if they export a manifest list and can easily change type=oci to strictly control the tar format if they need that.

Another thing is that you can imagine that there will be other ways to get build results into docker in the future, eg. moby/moby#44369 . The behavior of the exporter should not be defined by a specific implementation but the expected end goal.

For bake, note that load is not bake property, it is only a shortcut override flag. So if exporters are not defined precisely the intended behavior of "load into docker" can't be defined in a file.

jedevc · 2023-05-19T09:53:27Z

I don't think this is necessarily what we want. I think we shouldn't change the behavior of -o type=oci or make --load dynamic. Instead, we should change -o type=docker to get the tarball in OCI format if the current Docker supports the OCI format. -o type=docker means "output to Docker".

Should be good now 🎉

However, we actually need to pass the oci exporter to buildkit, since the docker exporter explicitly does not support multi-platform images: https://github.com/moby/buildkit/blob/4e4b7a8ff8a5700b0c170185dce4ca86e3a4e33d/exporter/oci/export.go#L115-L117

I wonder if we should maybe change this requirement in the next version of buildkit? We have safeguards to prevent this in buildx now (since v0.6 #582).

tonistiigi

cc @rumpl

build/build.go

tonistiigi · 2023-05-26T02:54:31Z

build/build.go

+				// rely on oci importer if available (which supports
+				// multi-platform images), otherwise fall back to docker
+				opt.Exports[i].Type = "oci"
+			} else if len(opt.Platforms) > 1 || len(attests) > 0 {
 				return nil, nil, errors.Errorf("docker exporter does not currently support exporting manifest lists")
 			}
 			if e.Output == nil {
 				if nodeDriver.IsMobyDriver() {


Not changing in this PR but shouldn't this also have a "context" check?

jedevc · 2023-05-26T08:48:14Z

util/dockerutil/client.go

@@ -63,6 +63,28 @@ func (c *Client) LoadImage(ctx context.Context, name string, status progress.Wri
 	}, nil
 }

+func (c *Client) Features(ctx context.Context, name string) (map[Feature]bool, error) {


In discussion with @crazy-max about #1846, we noticed that this probably needs a similar cache to ensure that we don't create a new docker client in each loop of the toSolveOpt call.

Can try and do this as a follow-up though, would be good to not block another RC on this.

jedevc · 2023-05-26T15:40:21Z

Aha, reworked the Features call, we shouldn't allow errors from it to propagate to the user - if no docker daemon is available, we should just assume the OCIImporter isn't either. This got detected in the new tests we have 🎉

build/build.go

rumpl · 2023-05-26T15:54:51Z

I have a couple of questions:

Does the OCI exporter unpack the image? We use the containerimage exporter and force it to unpack the image, we also sanitize the image name and set a dangling prefix for the cases when the user doesn't set a tag https://github.com/moby/moby/blob/master/builder/builder-next/exporter/overrides/wrapper.go#L33-L41
did you try and run the built images?

jedevc · 2023-05-26T16:11:02Z

Does the OCI exporter unpack the image?

Not directly, no. We take the output of the OCI exporter and pipe it into the ImageLoad docker API, so I think that should do the unpack operation?

did you try and run the built images?

An important point. This doesn't work when doing moby-buildkit for whatever reason for me... 🤔 (but that's using the type=image exporter, not the oci exporter).

Starting from an empty state (docker system prune -a) - for some reason, I can get into a state where I can't reproduce this, I wonder if there's something about intermediate layers not being loaded from multiplatform images or something?

$ docker buildx build . --target shell --builder desktop-linux --output type=docker -t buildx-shell --platform linux/amd64,linux/arm64
...
$ docker image ls
REPOSITORY                 TAG       IMAGE ID       CREATED         SIZE
buildx-shell               latest    6a1a88e8cb50   7 seconds ago   111MB
buildx-shell               latest    6a1a88e8cb50   7 seconds ago   104MB
$ docker image inspect buildx-shell | head -n 5
[
    {
        "Id": "sha256:6a1a88e8cb5056f4c9802ee8251a9f7eb7118a0a14a71d5b4c867909d045ff0d",
        "RepoTags": [
            "buildx-shell:latest"
$ docker run -it buildx-shell
Unable to find image 'buildx-shell:latest' locally

(note, single platform builds work fine)

This does work when using the container driver though (which with this patch uses oci exporter):

$ buildx build . --target shell --builder container -t buildx-shell-2 --platform linux/amd64,linux/arm64
...
$ docker image ls
REPOSITORY                 TAG       IMAGE ID       CREATED         SIZE
buildx-shell-2             latest    114e993fbed0   4 minutes ago   427MB
buildx-shell-2             latest    114e993fbed0   4 minutes ago   410MB
$ docker image inspect buildx-shell-2 | head -n 5
[
    {
        "Id": "sha256:114e993fbed0fba938a3c9f6c38b76fcb6b7ca7d6599f56dbbae9fc385a14f97",
        "RepoTags": [
            "buildx-shell-2:latest"
$ docker run -it buildx-shell-2
/work #

Slightly confused at the difference in sizes of images... is something maybe not getting unpacked?

jedevc · 2023-05-26T16:20:27Z

Could potentially be related to moby/buildkit#3891?

tonistiigi · 2023-05-26T16:38:50Z

This looks more like issue with the load command on dockerd with containerd enabled rather than build-side. We basically just issue docker load. All else is dockerd.

jedevc · 2023-05-30T08:08:40Z

The case with docker load (using a non-docker driver) works fine - it's when we use the type=image exporter with buildkit (using the docker driver) that we have the issue.

rumpl · 2023-05-30T08:31:18Z

The case with docker load (using a non-docker driver) works fine - it's when we use the type=image exporter with buildkit (using the docker driver) that we have the issue.

Are we still talking about the ERROR: failed to build: docker exporter does not currently support exporting manifest lists issue or are you talking about something else?

tonistiigi · 2023-05-31T05:02:48Z

it's when we use the type=image exporter with buildkit (using the docker driver) that we have the issue.

That case should be working since #1262

Signed-off-by: Justin Chadwell <me@jedevc.com>

jedevc · 2023-05-31T15:09:52Z

Done some more investigation - I think this out of scope for this PR, so we probably shouldn't block on it.

I can reproduce the same issue with the type=moby exporter (equivalent to type=image,unpack=true) when building a multi-platform image, so something seems incorrect here - which is covered by moby/buildkit#3891.

jedevc · 2023-06-06T14:56:38Z

@tonistiigi PTAL

tonistiigi · 2023-06-06T15:38:38Z

util/dockerutil/client.go

@@ -63,6 +63,23 @@ func (c *Client) LoadImage(ctx context.Context, name string, status progress.Wri
 	}, nil
 }

+func (c *Client) Features(ctx context.Context, name string) map[Feature]bool {
+	features := make(map[Feature]bool)
+	if dapi, err := c.API(name); err == nil {


In a follow-up this function could do caching these return values for bake and if we add any more Feature values we probably would need to ask for a specific one to make sure expensive call like info is not done needlessly.

rumpl

Just tested it, seems to work great, awesome work @jedevc, thanks!

Felixoid · 2023-08-07T11:16:01Z

Hello, is there a way to use OCI exported in case of --push? Our command looks like docker buildx build --push --output=type=image,push-by-digest=true, but it's very slow for low-power instances.

jedevc · 2023-08-07T11:18:05Z

@Felixoid this doesn't seem related to this PR, which is about --load - if you have other questions around it, can you open a separate issue?

In any case, if looks like you might want --output type=image,push-by-digest=true,oci-mediatypes=true.

Felixoid · 2023-08-07T11:19:34Z

Thank you, I'll try the parameter oci-mediatypes=true. It may solve our issue!

jedevc added this to the v0.11.0 milestone May 18, 2023

jedevc added area/moby kind/enhancement New feature or request labels May 18, 2023

jedevc force-pushed the oci-export-on-containerd branch from 55a5193 to 76f7bf4 Compare May 18, 2023 15:57

jedevc requested review from crazy-max and tonistiigi and removed request for crazy-max May 18, 2023 15:58

jedevc force-pushed the oci-export-on-containerd branch from 76f7bf4 to 931feca Compare May 19, 2023 09:50

tonistiigi reviewed May 26, 2023

View reviewed changes

jedevc commented May 26, 2023

View reviewed changes

jedevc force-pushed the oci-export-on-containerd branch 2 times, most recently from aa4afa7 to e89ed57 Compare May 26, 2023 15:37

tonistiigi reviewed May 26, 2023

View reviewed changes

build/build.go Outdated Show resolved Hide resolved

jedevc force-pushed the oci-export-on-containerd branch from e89ed57 to b3d573d Compare May 26, 2023 16:10

build: docker exporter should instead use oci with containerd

183a73a

Signed-off-by: Justin Chadwell <me@jedevc.com>

jedevc requested a review from tonistiigi May 31, 2023 15:09

jedevc force-pushed the oci-export-on-containerd branch from b3d573d to 183a73a Compare May 31, 2023 15:35

tonistiigi approved these changes Jun 6, 2023

View reviewed changes

jedevc requested a review from rumpl June 6, 2023 15:57

rumpl approved these changes Jun 7, 2023

View reviewed changes

jedevc merged commit 47e34f2 into docker:master Jun 7, 2023

jedevc deleted the oci-export-on-containerd branch June 7, 2023 13:20

jedevc mentioned this pull request Jun 13, 2023

dockerutil: cache feature collection #1877

Merged

This was referenced Jun 30, 2023

Image exporter with non-default platform fails moby/buildkit#3891

Closed

Fix containerimage multi-platform unpack moby/buildkit#3982

Merged

Felixoid mentioned this pull request Aug 3, 2023

"sending tarball" takes a long time even when the image already exists #107

Open

thaJeztah mentioned this pull request Aug 9, 2023

Potential regression: v0.12.x OCI exporter creates images which cannot be loaded in Docker moby/buildkit#4131

Closed

schmittfelix mentioned this pull request Feb 15, 2024

Local Build & Deploy (Step 6b) not working Fluent-networks/tailscale-mikrotik#24

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`--load` with moby containerd store should use the `oci` exporter #1813

`--load` with moby containerd store should use the `oci` exporter #1813

jedevc commented May 18, 2023 •

edited

Loading

tonistiigi commented May 18, 2023 •

edited

Loading

TBBle commented May 19, 2023

tonistiigi commented May 19, 2023

tonistiigi commented May 19, 2023

jedevc commented May 19, 2023

tonistiigi left a comment

tonistiigi May 26, 2023

jedevc May 26, 2023

jedevc May 26, 2023

jedevc commented May 26, 2023

rumpl commented May 26, 2023

jedevc commented May 26, 2023 •

edited

Loading

jedevc commented May 26, 2023

tonistiigi commented May 26, 2023

jedevc commented May 30, 2023

rumpl commented May 30, 2023

tonistiigi commented May 31, 2023

jedevc commented May 31, 2023

jedevc commented Jun 6, 2023

tonistiigi Jun 6, 2023

rumpl left a comment

Felixoid commented Aug 7, 2023

jedevc commented Aug 7, 2023

Felixoid commented Aug 7, 2023

--load with moby containerd store should use the oci exporter #1813

--load with moby containerd store should use the oci exporter #1813

Conversation

jedevc commented May 18, 2023 • edited Loading

Related issues

tonistiigi commented May 18, 2023 • edited Loading

TBBle commented May 19, 2023

tonistiigi commented May 19, 2023

tonistiigi commented May 19, 2023

jedevc commented May 19, 2023

tonistiigi left a comment

Choose a reason for hiding this comment

tonistiigi May 26, 2023

Choose a reason for hiding this comment

jedevc May 26, 2023

Choose a reason for hiding this comment

jedevc May 26, 2023

Choose a reason for hiding this comment

jedevc commented May 26, 2023

rumpl commented May 26, 2023

jedevc commented May 26, 2023 • edited Loading

jedevc commented May 26, 2023

tonistiigi commented May 26, 2023

jedevc commented May 30, 2023

rumpl commented May 30, 2023

tonistiigi commented May 31, 2023

jedevc commented May 31, 2023

jedevc commented Jun 6, 2023

tonistiigi Jun 6, 2023

Choose a reason for hiding this comment

rumpl left a comment

Choose a reason for hiding this comment

Felixoid commented Aug 7, 2023

jedevc commented Aug 7, 2023

Felixoid commented Aug 7, 2023

`--load` with moby containerd store should use the `oci` exporter #1813

`--load` with moby containerd store should use the `oci` exporter #1813

jedevc commented May 18, 2023 •

edited

Loading

tonistiigi commented May 18, 2023 •

edited

Loading

jedevc commented May 26, 2023 •

edited

Loading