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

FR: oci_tarball loading is not incremental #454

Open
alexeagle opened this issue Jan 3, 2024 · 9 comments
Open

FR: oci_tarball loading is not incremental #454

alexeagle opened this issue Jan 3, 2024 · 9 comments

Comments

@alexeagle
Copy link
Collaborator

rules_docker has a fancy program https://github.com/bazelbuild/rules_docker/blob/master/container/incremental_load.sh.tpl#L4

@thesayyn
Copy link
Collaborator

thesayyn commented Jan 3, 2024

my 2 cents on this issue;

This is not really a bazel-shaped problem that i am keen on solving anytime soon. oci_tarball never meant to be a node deep in the dependency graph but a leaf.

a quick look at this issue will show us how messy this is and lead to more issues being reported google/go-containerregistry#895

@thesayyn
Copy link
Collaborator

thesayyn commented Jan 3, 2024

one way I solved this in the past was by using a neat trick to start a registry daemon on the host and have docker runtime pull images from there.

@juanzolotoochin
Copy link

juanzolotoochin commented Jan 3, 2024

Here's my prototype implementation of a loader that works incrementally: https://github.com/juanique/bazel-examples/blob/load_image_prototype_7ff37/bazel6/rules_oci/2-go-image/app/BUILD.bazel#L15 if anyone wants to use it.

@thesayyn I understand that oci_tarball may not be meant for this, but the use case of "I need to load this docker image on multiple remote executors for integration tests" is a valid one that worked very well with rules_docker and it's just not well supported on rules_oci (large artifacts need to be passed via remote caching). So at least there should be some guidance on what to do for that use case.

@thesayyn
Copy link
Collaborator

thesayyn commented Jan 3, 2024

I understand the need for such a thing. It's a tradeoff between maintenance and performance for us. Using tarballs for this purpose is not the right solution.

Some things can be done to improve DX but maintaining a loader monstrosity is not one of them.

I am trying to find the right solution rather than a workaround.

FWIW rules_docker's tarball loader shifts the responsibility away from bazel action to runtime where blobs are closest to the executor so it is mostly fast.

@thesayyn
Copy link
Collaborator

thesayyn commented Jan 3, 2024

@jonjohnsonjr tried to implement this in google/go-containerregistry#559

@alexeagle
Copy link
Collaborator Author

BTW to be clear, if anyone was willing to step up and maintain rules_docker, it could be unarchived. Our goal was never to have feature parity with it.

@gzm0
Copy link
Contributor

gzm0 commented Jan 5, 2024

FWIW: I have built a clone of the rules_docker tarball loader using dockerode and tar-stream. It uses the same hack of sending the config only first and based on the returned error, identifying the layers that are actually required. Then it sends only these.

It will eventually appear on https://github.com/datahouse/bazel_buildlib. If there is interest, please LMK and I can make that happen sooner.

In any case, I have a feeling that rules for container runtimes should probably live somewhere else.

@thesayyn
Copy link
Collaborator

@gzm0
Copy link
Contributor

gzm0 commented Jan 24, 2024

Here is the version we use: https://github.com/datahouse/bazel_buildlib/blob/oss/buildlib/private/docker/src/loadImageToDocker.ts (someone requested it privately so I published it).

@thesayyn thesayyn changed the title oci_tarball loading is not incremental FR: oci_tarball loading is not incremental May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants