exporter: containerimage: Add an option to unlazy image blobs #2800

ktock · 2022-04-12T09:04:04Z

Some blobs in the result image can be lazy (i.e. not existing in the local content store).
This doesn't allow non-buildkit containerd clients (e.g. ctr, nerdctl, ...) fetching the resulting image from containerd.
Unpack doesn't solve this because it only unlazies the default platform of the image.

# mkdir -p /tmp/ctx && cat <<EOF > /tmp/ctx/Dockerfile
FROM ubuntu:20.04
CMD echo hello
EOF
# buildctl build --progress=plain --frontend=dockerfile.v0 \
               --local context=/tmp/ctx --local dockerfile=/tmp/ctx \
               --opt platform=linux/amd64,linux/arm64 \
               --output=type=image,unpack=true,name=foo
# ctr image export --all-platforms /tmp/out3.tar foo
ctr: failed to get reader: content digest sha256:185e8a4c100571f111d924b5d4399d89f163bf95d71ce2c6a33f656a66c52f0a: not found

This commit fixes this by adding a new option ensure-blobs to containerimage exporter.
This option unlazies blobs of the build result.

AkihiroSuda · 2022-04-12T09:32:16Z

README.md

@@ -232,6 +232,7 @@ Keys supported by image output:
 * `registry.insecure=true`: push to insecure HTTP registry
 * `oci-mediatypes=true`: use OCI mediatypes in configuration JSON instead of Docker's
 * `unpack=true`: unpack image after creation (for use with containerd)
+* `ensure-blobs=true`: ensure blobs of the result image are locally available


"locally" needs to be explained

Thank you for the comment. Fixed the explanation to "exist in the content store" instead of "locally available".

tonistiigi · 2022-04-13T00:42:51Z

README.md

@@ -232,6 +232,7 @@ Keys supported by image output:
 * `registry.insecure=true`: push to insecure HTTP registry
 * `oci-mediatypes=true`: use OCI mediatypes in configuration JSON instead of Docker's
 * `unpack=true`: unpack image after creation (for use with containerd)
+* `ensure-blobs=true`: ensure blobs of the result image exist in the content store


Why do we need the extra key? Don't we always need to run this when exporting images to containerd?

@tonistiigi Thank you for the comment. This is because tests require blobs being lazy even after exporting it with type=image,push=true https://github.com/moby/buildkit/runs/6000088966?check_suite_focus=true

If it creates a broken image in containerd then it looks wrong. Maybe we need an option that would allow to push the image without adding it to the containerd image store. In that case blobs could remain lazy.

an option that would allow to push the image without adding it

It won't pass some tests like testLazyImagePush which performs the check using lazy images in the image store.
How about making ensure-blobs=true the default behaviour when a client creates an image in the image store?

Maybe better would be store=false that would skip saving image to the image store.

Added store option. I think this should be store=true by default for keeping the backward-compatibility of type=image behaviour.

https://github.com/moby/buildkit#containerd-image-store

The containerd worker needs to be used
buildctl build ... --output type=image,name=docker.io/username/image
ctr --namespace=buildkit images ls

tonistiigi · 2022-04-21T00:55:35Z

README.md

@@ -239,6 +239,7 @@ Keys supported by image output:
 * `force-compression=true`: forcefully apply `compression` option to all layers (including already existing layers).
 * `buildinfo=true`: inline build info in [image config](docs/build-repro.md#image-config) (default `true`).
 * `buildinfo-attrs=true`: inline build info attributes in [image config](docs/build-repro.md#image-config) (default `false`).
+* `store=true`: stores the result images to the worker(e.g. containerd)'s image store as well as ensures that the image has all blobs in the content store (default `true`). Ignored if the worker doesn't have image store (e.g. OCI worker).


the worker's (e.g. containerd)

Fixed this.

tonistiigi · 2022-04-21T00:56:47Z

client/client_test.go

+					"name":                   target,
+					"push":                   "true",
+					"store":                  "true",
+					"store-allow-incomplete": "true",


I don't think we need "store-allow-incomplete" as it creates broken images that can't be pushed or extracted. The user has the option to store with blobs or not store at all.

If it is only for tests (and there is no other way), then prefix with unsafe-internal-

Added unsafe-internal- prefix.

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

AkihiroSuda reviewed Apr 12, 2022

View reviewed changes

ktock force-pushed the unlazy-opt branch from 09b76e3 to 9d43242 Compare April 12, 2022 10:43

tonistiigi reviewed Apr 13, 2022

View reviewed changes

ktock force-pushed the unlazy-opt branch from 9d43242 to d155bed Compare April 13, 2022 01:14

ktock marked this pull request as draft April 13, 2022 02:06

ktock force-pushed the unlazy-opt branch from d155bed to 13d483f Compare April 13, 2022 02:28

ktock marked this pull request as ready for review April 13, 2022 02:32

ktock force-pushed the unlazy-opt branch 3 times, most recently from 8235fd9 to b9ebe45 Compare April 21, 2022 00:32

tonistiigi reviewed Apr 21, 2022

View reviewed changes

exporter: containerimage: Add an option to unlazy image blobs

59835d3

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

ktock force-pushed the unlazy-opt branch from b9ebe45 to 59835d3 Compare April 21, 2022 01:31

tonistiigi approved these changes Apr 22, 2022

View reviewed changes

tonistiigi requested a review from AkihiroSuda April 22, 2022 01:39

AkihiroSuda approved these changes Apr 22, 2022

View reviewed changes

AkihiroSuda merged commit cd5af97 into moby:master Apr 22, 2022

ktock deleted the unlazy-opt branch May 1, 2022 05:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

exporter: containerimage: Add an option to unlazy image blobs #2800

exporter: containerimage: Add an option to unlazy image blobs #2800

ktock commented Apr 12, 2022

AkihiroSuda Apr 12, 2022

ktock Apr 12, 2022

tonistiigi Apr 13, 2022

ktock Apr 13, 2022 •

edited

tonistiigi Apr 13, 2022

ktock Apr 14, 2022

tonistiigi Apr 14, 2022

ktock Apr 20, 2022

tonistiigi Apr 21, 2022

ktock Apr 21, 2022

tonistiigi Apr 21, 2022

ktock Apr 21, 2022

exporter: containerimage: Add an option to unlazy image blobs #2800

exporter: containerimage: Add an option to unlazy image blobs #2800

Conversation

ktock commented Apr 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktock Apr 13, 2022 • edited

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

ktock Apr 13, 2022 •

edited