Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support tar files for multi-arch images #325

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

illicitonion
Copy link
Contributor

Fixes #112

@illicitonion illicitonion force-pushed the multi-arch-tar branch 3 times, most recently from 21ac36c to 40d3e19 Compare August 8, 2023 16:54
oci/private/tarball.sh.tpl Outdated Show resolved Hide resolved
@thesayyn
Copy link
Collaborator

thesayyn commented Aug 8, 2023

I am confused about how this fixes oci_image_index support for oci_tarball.

@illicitonion
Copy link
Contributor Author

I am confused about how this fixes oci_image_index support for oci_tarball.

When the image dir contains an image index, this now bundles things up properly so that docker load will work with the tar (assuming you've enabled containerd) - it's the same format as skopeo copy docker://some:image "oci-archive:oci-archive.tar" --multi-arch all -f v2s2 would output.

The layout of the tar file is an index.json which is an index of images (which may have annotations indicating their repo tags), which point at manifests by digest (in the blobs directory), and then a blobs directory containing all of the layers (and manifests) referenced.

I've been using it to take an oci_image_index and produce a .tar file which can be docker loaded, just like oci_tarball already supports for oci_image.

(My goal is to be able to docker load an multi-arch image, as I have systems which then consume these docker load-able tars)

@thesayyn
Copy link
Collaborator

thesayyn commented Aug 8, 2023

I am more confused now. oci_image_index doesn't produce more than one image. it just makes a multi-arch image out of multiple images and then produces a single image. and docker style tarballs don't support multi-platform images inside the tarballs.

Could you please add an example to the examples folder to demonstrate how this works?

bundles things up properly

Could you elaborate a bit more on this?

@illicitonion
Copy link
Contributor Author

Sorry for not being clear! Hopefully the example will help. I've just pushed an example of a multi-platform image with some Go code built for Linux in both amd64 and arm64.

Assuming you've enabled containerd for a local docker install, the tarball it produces can be docker loaded and then docker.com/example:latest will be runnable. This will work whether you're on an amd64 or arm64 machine.

If you have an Apple Silicon machine, you can do this:

% bazel build //examples/multi_arch_go:image-multiarch-tar                       
INFO: Invocation ID: 0713b09c-525d-42e3-a825-b732f6dd581e
INFO: Analyzed target //examples/multi_arch_go:image-multiarch-tar (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //examples/multi_arch_go:image-multiarch-tar up-to-date:
  bazel-bin/examples/multi_arch_go/image-multiarch-tar/tarball.tar
INFO: Elapsed time: 0.339s, Critical Path: 0.13s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
% docker load <bazel-bin/examples/multi_arch_go/image-multiarch-tar/tarball.tar
Loaded image: docker.com/example
% docker run -it --rm --platform linux/amd64 docker.com/example 
yo
% docker run -it --rm --platform linux/arm64 docker.com/example 
yo

From this you can see this .tar was docker load-ed and could be run on both architectures. As far as I can tell, this wasn't possbile using rules_oci before this PR.

@illicitonion
Copy link
Contributor Author

(I think one of the key pieces of information here is that while docker style tarballs don't support multi-platform images, oci style ones do, and docker can be configured to work with oci style tarballs)

@thesayyn
Copy link
Collaborator

thesayyn commented Aug 8, 2023

Assuming you've enabled containerd for a local docker install

is this the case for standard setup?

@illicitonion
Copy link
Contributor Author

Assuming you've enabled containerd for a local docker install

is this the case for standard setup?

With docker desktop, there's a "features in development" flag you can enable: https://docs.docker.com/desktop/containerd/

(I'm not actually using docker desktop, I'm using a different system which understands OCI format images, but it's easy to test in docker desktop :))

I'm assuming this feature is reasonably in scope for rules_oci, given it's about oci-spec formatting rather than specifically docker interoperation.

@thesayyn
Copy link
Collaborator

thesayyn commented Aug 8, 2023

(I think one of the key pieces of information here is that while docker style tarballs don't support multi-platform images, oci style ones do, and docker can be configured to work with oci style tarballs)

ideally, we should not implement hacks. it's the reason why we don't produce both oci and docker style tarballs in just one tarball by putting both manifest.json and index.json into the tarball. I would assume that containerd just supports oci style tarballs and ignores manifest.json. I don't want our API docs to say oci_image_index is supported but isn't in standard setups.

@illicitonion
Copy link
Contributor Author

ideally, we should not implement hacks

I wouldn't classify this as a hack... It is unfortunate that we're having to generate the index.json file ourselves, but I view it as the same level as unfortunate that we're generating a manifest.json file ourselves in this file before this PR...

Would it feel more clean if there were an attribute on oci_tarball which let you select whether you were expecting an oci or docker format tar?

I agree that it's confusing that there are two possible output formats (oci and docker), and that oci_tarball assumes you're outputting the non-oci format and only works if the inputs are non-oci format... Maybe a good solution here would be to rename oci_tarball to docker_tarball, and make a new rule named oci_tarball which only deals with the oci format?

@illicitonion
Copy link
Contributor Author

The more I think about it, the more I think that having an output_format attr which can take values "docker" or "oci" makes sense, and that we should just error if we can't translate between the formats correctly.

So the current state of the repo would only support "docker", and would error if you gave it an oci image to tar up (e.g. an oci_image_index. With this PR, we could support oci -> oci, but we'd still error on oci -> docker or docker -> oci. In the future we may be able to do translation between the formats, but we wouldn't necessarily ever actually implement these.

@thesayyn
Copy link
Collaborator

thesayyn commented Aug 17, 2023

So the current state of the repo would only support "docker"

I am not sure what you mean by this but docker-style tarballs are supported by all container runtimes.

I am warm to the idea of having two output formats specified by a format = "oci" | "docker" attribute defaulting to docker.

while format = "oci" accepts both oci_image and oci_image_index, format = "docker" only supports oci_image and errors out if anything else is given.

Also, if you could keep the diff minimal, that'd be great.

@kriscfoster
Copy link

Hi @illicitonion,

Thank you very much for working on this, it is also a feature we would really like. Just curious, when we were trying it out we saw the following error:

bazel run examples/multi_arch_go:image-multiarch-tar
...
open /var/lib/docker/tmp/docker-import-3371548048/blobs/json: no such file or directory

Just curious if you know why that would be. Thank you.

@illicitonion illicitonion force-pushed the multi-arch-tar branch 7 times, most recently from 290c48a to 3a94776 Compare October 6, 2023 11:30
@illicitonion
Copy link
Contributor Author

@thesayyn Thanks for the review, and sorry for the ridiculous delay in getting back to you.

I've added the format attr as described - for now I've only implemented oci support for image_index targets, but we could add image support as/when needed (I imagine I'll probably add it soon, but wanted to keep the PR manageable).

I've also reworked the PR slightly to minimise the diff - the old codepath now hopefully is more clearly mostly unchanged, and the new codepath is mostly just additions not modifications.

Please let me know if you have more ideas or comments! :)

@illicitonion
Copy link
Contributor Author

Hi @illicitonion,

Thank you very much for working on this, it is also a feature we would really like. Just curious, when we were trying it out we saw the following error:

bazel run examples/multi_arch_go:image-multiarch-tar
...
open /var/lib/docker/tmp/docker-import-3371548048/blobs/json: no such file or directory

Just curious if you know why that would be. Thank you.

@kriscfoster It sounds like you're running docker without containerd enabled - if you enable it as per https://docs.docker.com/desktop/containerd/ your docker run should work.

@kriscfoster
Copy link

Thank you @illicitonion, this worked!

@kriscfoster
Copy link

Do we know what is needed for this to make it to main? It's working well for us but we would prefer to use it directly from a main release instead of from a fork.

@alexeagle
Copy link
Collaborator

Note, this will have big merge conflict against #385, we'll see who gets in first :)

@illicitonion
Copy link
Contributor Author

Hopefully this is ~good to go!

@alexeagle What's your timeline for #385? I'm happy to do either rebase (it should be trivial, just every cp_f_with_mkdir becaomes an add_to_tar call (and removing the ${STAGING_DIR} prefix from each second arg), and would love to get both in as soon as we can! :)

oci/private/tarball.sh.tpl Outdated Show resolved Hide resolved
oci/private/tarball.sh.tpl Outdated Show resolved Hide resolved
oci/private/tarball.sh.tpl Outdated Show resolved Hide resolved
@thesayyn thesayyn merged commit 1fce184 into bazel-contrib:main Oct 30, 2023
16 checks passed
@illicitonion illicitonion deleted the multi-arch-tar branch October 31, 2023 10:54
@ar3s3ru
Copy link

ar3s3ru commented Dec 12, 2023

Sorry to dig up an old issue, @illicitonion since you authored these changes and you seem to have experience in the topic, I've hit the same error as @kriscfoster but through Skaffold (which is using Docker Engine API).

More context: GoogleContainerTools/skaffold#9217

Any idea why this wouldn't work? I would assume it should, since I'm able to podman load the image produced by oci_image_index -> oci_tarball locally. (I also assume you're using Podman, from your earlier comments)

Is there any way we could add support for multi-arch oci_tarball in Skaffold using Docker Engine API's client? Or is there a different client that supports both Docker and OCI format?

@ar3s3ru
Copy link

ar3s3ru commented Dec 20, 2023

Friendly ping @illicitonion, if you have any pointers about the above message ☝🏻 🙏🏻

@illicitonion
Copy link
Contributor Author

Sorry, I don't have any useful context here - it sounds like skaffold needs OCI format support.

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.

oci_tarball: handle OCI-format images
5 participants