rules_docker should use go-container registry

[bajacondor]

this project needs to move to a supported method of pushing and pulling images. The python containerregistry is unsupported and no can no longer take merge requests. See: https://github.com/google/containerregistry/issues/114

What are the chances that we could move to go-container registry for pushing and pulling?
https://github.com/google/go-containerregistry

[xingao267]

Thanks for reporting. We have plan to look into migrating to use the go containerregistry in rules_docker, but will likely happen in 2 quarters.

[Globegitter]

There already has been a PR that started getting pusher in https://github.com/bazelbuild/rules_docker/pull/481 so maybe there is also room in the community to possibly help delivering this quicker.

[tmc]

@xingao267 in the shorter term could the different parts that make up this work have issues created? That may provide a path for community contributions.

[nlopezgi]

hi @tmc, yes we will try to break this up in such a way that it's feasible to get contributions. Please stay tuned, likely this plan will come mid to late Q1 2019.

[tmccombs]

I'd like to help make this happen. What would be the best way for me to help?

[nlopezgi]

Plan is still to have a plan mid to late Q1 2019, until then I'm not sure there is any way to help (unless you want to write up a design doc with a migration plan? I can help guide you but it would require heavy time investment from both you and me)

[Globegitter]

@nlopezgi We are getting close to the end of Q1 anything new in regards to the migration plans?

[xingao267]

@Globegitter we have drafted an internal plan to migrate everything (binaries, libraries) from python containerregistry to go containerregistry and the implementation will start mid-Q2.

[ittaiz]

@xingao267 is the migration underway?

[nlopezgi]

It has started with https://github.com/bazelbuild/rules_docker/pull/830
We expect to make lots of progress in the upcoming weeks

[ittaiz]

@nlopezgi any chance for an update on this important effort?
Thanks for all your work!

[nlopezgi]

@xingao267 can you provide a quick update on how migration is going?

[xingao267]

Hi, we are currently working on new_container_pull.bzl and new_container_push.bzl rules that wrap go binaries built based on go-containerregistry. The image will be directly pulled into an intermediate efficient format (instead of a tarball). Next step is to add new container image rules that can understand that format and use it. During meantime, we are working on extend the new_container_pull.bzl to optionally output a tarball of image so it can be used directly as well.

[ittaiz]

Wow awesome! all of these features sound super valuable. Thanks! We have a need indeed to have both intermediate efficient format for building images and tarballs for running them in tests.
Can't wait :)
Will you support layer cache like we have now?

[xingao267]

@ittaiz by layer cache, do you mean the container_layer rule?

[ittaiz]

I'm talking about the container_pull cache https://github.com/bazelbuild/rules_docker/pull/706

[xingao267]

@ittaiz we should. The cache option in new_container_pull is currently taken out https://github.com/bazelbuild/rules_docker/pull/880/files but we are looking into fixing and enabling that.

[ittaiz]

🤞 for us it had a huge effect on both CI (we have a cache which is closer than the registry) and on local dev machines (shared between projects)

[ittaiz]

@xingao267 any chance for a small update on the migration? Thanks for all your efforts (mainly I'm asking because we're really suffering from the fact that fetching images is hardcoded to 8 threads per image and we have many many timeouts)

[smukherj1]

Hi @ittaiz,

We currently have a couple of prototype rules new_container_pull and new_container_push implemented using go-containerregistry that you can try out. Please let us know if you encounter issues. #1075 is a known issue with new_container_pull that doesn't affect all cases but hasn't been triaged yet.

As for the overall status of migration, we still need to migrate the implementation of join_layers.py to completely sever the dependence of rules_docker on the python containerregistry. However, this work is currently paused because of other higher priority items. We might be able to spend some time here and there on the migration over the next two quarters but there is no ETA on when we expect to be done.

[ittaiz]

Interesting. I'll see who from our side can take a look at the new rules.
Any word on https://github.com/bazelbuild/rules_docker/issues/580#issuecomment-503908102 ?

[smukherj1]

Interesting. I'll see who from our side can take a look at the new rules.
Any word on #580 (comment) ?

Yes I believe the new_container_pull rule does implement a cache. We haven't benchmarked its performance though.

[shs96c]

Is there any way that people can contribute to this? It's not clear what the plan is to replace join_layers.py. Is there a design doc somewhere?

[smukherj1]

Is there any way that people can contribute to this? It's not clear what the plan is to replace join_layers.py. Is there a design doc somewhere?

The plan to replace join_layers.py is as simple as looking at the current python code here and implement an equivalent binary in Go that uses the go-containerregistry. We haven't yet looked at the python code in detail to determine if there are any technical blockers and thus there's no document yet.

The other remaining steps in the migration are as follows:

1. Flip the default value of the legacy_create_image_config attribute on the container_image rule here to false. This will switch the image config creator binary from python to Go. This will break existing tests and they'll need to be updated accordingly.
2. Replace the image digester here with the Go digester binary here. The existing container_push rule must be deprecated at the same time and aliased to new_container_push. This is because container_push generates its own manifest which can result in the pushed image having a different digest than that indicated by the Go digester.
3. Trigage #927
4. Triage #1075

Among these, I believe community contributions would be most valuable for the following in decreasing order of impact:

1. Implement the Go equivalent of join_layers.py.
2. Point (1) above.
3. Point (2) above.
4. Point (3) & (4) above.

[smukherj1]

Note that I'm currently working on a Go implementation for join_layers.py.

[smukherj1]

Update on migration status:
#1111 which implements join_layers in Go is currently being reviewed.

1114 tries to finish the Go migration by flipping all implementations to the Go binaries & rules.

However, before that change can be merged the following items need to be completed:

1. Update the format attribute on new_container_push to behave exactly like container_push. new_container_push accepts legacy, docker or oci while container_push accepts Docker or OCI. This will fix #927 and #1075
2. The Go puller & digest currently interpret the --format=OCI flag as reading an OCI image. The python puller & digester interpret it as reading a docker image or an image in the rules_docker intermediate format and producing an OCI manifest. This will upload the image in OCI format to the registry. Go pusher/digester needs to be updated to behave like the python equivalents for backwards compatibility. This is needed for the test here to pass.
3. Discovered a new rule container_flatten that needs to be migrated to Go. The implementation looks fairly simply so this shouldn't be too bad.
4. The container tests here need to be migrated to a Go implementation or use the container_test rule as appropriate.

[ittaiz]

Sorry for sounding like a broken record but part of the migration is having
a layer level local cache, right?

On Wed, 28 Aug 2019 at 22:26 Suvanjan Mukherjee [e-mail]
wrote:

Update on migration status:

1111 https://github.com/bazelbuild/rules_docker/pull/1111 which

implements join_layers in Go is currently being reviewed.

1114 https://github.com/bazelbuild/rules_docker/pull/1114 tries to

finish the Go migration by flipping all implementations to the Go binaries
& rules.

However, before that change can be merged the following items need to be
completed:

1. Update the format attribute on new_container_push to behave exactly
   like container_push. new_container_push accepts legacy, docker or oci
   while container_push accepts Docker or OCI. This will fix #927
   https://github.com/bazelbuild/rules_docker/issues/927 and #1075
   https://github.com/bazelbuild/rules_docker/issues/1075
2. The Go puller & digest currently interpret the --format=OCI flag as
   reading an OCI image. The python puller & digester interpret it as
   reading a docker image or an image in the rules_docker intermediate format
   and producing an OCI manifest. This will upload the image in OCI format to
   the registry. Go pusher/digester needs to be updated to behave like the
   python equivalents for backwards compatibility. This is needed for the test
   here
   https://github.com/bazelbuild/rules_docker/blob/8650f276d214e84e43c1253bd839b838303d4197/testdata/BUILD#L425
   to pass.
3. Discovered a new rule container_flatten
   https://github.com/bazelbuild/rules_docker/blob/8650f276d214e84e43c1253bd839b838303d4197/container/flatten.bzl#L63
   that needs to be migrated to Go. The implementation looks fairly simply so
   this shouldn't be too bad.
4. The container tests here
   https://github.com/bazelbuild/rules_docker/blob/8650f276d214e84e43c1253bd839b838303d4197/container/image_test.py
   need to be migrated to a Go implementation or use the container_test
   rule as appropriate.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/bazelbuild/rules_docker/issues/580?email_source=notifications&email_token=AAKQQF256U27HBTWA7XUM4TQG3GMFA5CNFSM4GDLRK22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5MGR7Q#issuecomment-525887742,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKQQF2NG74HK5JYI5IEDKDQG3GMFANCNFSM4GDLRK2Q
.

>

Ittai Zeidman

Cell: 054-6735021

40 Hanamal street, Tel Aviv, Israel

http://www.wix.com

[smukherj1]

@ittaiz The puller Go implementation uses go-containerregistry's local cache here. I haven't looked at their implementation in detail to see how it works yet.

[smukherj1]

Update on migration:

1. #1118 makes the Go pusher & digester backwards compatible in terms of capabilities. This makes the new_container_push a drop-in replacement for container_push. Note that we expect the Go pusher/digester will produce a different digest for the same image vs the python pusher/digester. This is because the Go binaries format the image manifest slightly differently.
2. #1122 implements a Go flattener to be used by the container_flatten rule.
3. #1123 which depends on #1122 fixes a performance issue with the Go pusher/digester and a couple of other fixes related to foreign layers (applicable to windows images only) & images derived from tarball images.

Once all the above changes are in, I'll make another attempt to flip all the binaries used in our container rules to the Go implementations and run tests in our internal repo to see what issues remain.

[aaliddell]

Are the files in /contrib also intending to get updates for the new implementations when the flip goes ahead? https://github.com/bazelbuild/rules_docker/blob/master/contrib/push-all.bzl might actually be the only one that depends on the underlying @containerregistry.

[smukherj1]

@aaliddell Yes, we plan to remove all references to @containerregistry in core rules. So any references in //contrib will be migrated as well. The migration of image_test.py might happen after the big flip of the rule implementations to Go. However, the use of the python containerregistry in this test file shouldn't affect users of rules_docker.

[smukherj1]

Update: With #1131, the migration of rules_docker rules to Go is feature complete. This means, as far as we know, we have equivalent Go code for all the python code that depend on the python containerregistry library and are not aware of any pending issues. The next step is to actually start aliasing existing rules and flipping boolean flags so that the rules in rules_docker start using the Go code instead of the python code. Our plan going forward is as follows in the given order:

1. Cut release 0.10.0 as the last rules_docker release where all the implementation is python.
2. Rename new_container_pull to container_pull and new_container_load to container_load. This will be done in a way such that current users of container_pull and container_load will be automatically switched to the Go implementation. We'll also rename the old container_pull and container_load to legacy_container_pull and legacy_container_load.
3. Wait for a week to see if there are any major issues. All watchers of this repo/issue are strongly encouraged to try out the new rules and report issues. Ideally if you want to help us find issues right now, please try new_container_pull and new_container_load right now at commit 8f0b744 or later and report any issues. Please file Github issues on rules_docker for this.
4. Update all remaining container_* rules in //container and //contrib to use the Go implementation instead of python by default. With this change, we'll add a section to our readme on how to switch between the Go & python implementations should they discover issues.
5. Cut release 0.11.0.
6. Remove all python containerregistry code from the core rules. The idea will be to get the repository in a state where users can import rules_docker and its dependencies without having to set the host_force_python flag. This will also remove the legacy_container_pull and legacy_container_load rules and remove the ability to switch the other rules to use python code from containerregistry.
7. Cut release 0.12.0.

[smukherj1]

Update: With #1131, the migration of rules_docker rules to Go is feature complete. This means, as far as we know, we have equivalent Go code for all the python code that depend on the python containerregistry library and are not aware of any pending issues. The next step is to actually start aliasing existing rules and flipping boolean flags so that the rules in rules_docker start using the Go code instead of the python code. Our plan going forward is as follows in the given order:

1. Cut release 0.10.0 as the last rules_docker release where all the implementation is python.
2. Rename new_container_pull to container_pull and new_container_load to container_load. This will be done in a way such that current users of container_pull and container_load will be automatically switched to the Go implementation. We'll also rename the old container_pull and container_load to legacy_container_pull and legacy_container_load.
3. Wait for a week to see if there are any major issues. All watchers of this repo/issue are strongly encouraged to try out the new rules and report issues. Ideally if you want to help us find issues right now, please try new_container_pull and new_container_load right now at commit 8f0b744 or later and report any issues. Please file Github issues on rules_docker for this.
4. Update all remaining container_* rules in //container and //contrib to use the Go implementation instead of python by default. With this change, we'll add a section to our readme on how to switch between the Go & python implementations should they discover issues.
5. Cut release 0.11.0.
6. Remove all python containerregistry code from the core rules. The idea will be to get the repository in a state where users can import rules_docker and it's dependencies without having to set the host_force_python flag. This will also remove the legacy_container_pull and legacy_container_load rules and remote the ability to switch the other rules to use python code from containerregistry.
7. Cut release 0.12.0.

Step 1 is done. Step 2 is pending with PR #1140.

[smukherj1]

Update: With #1131, the migration of rules_docker rules to Go is feature complete. This means, as far as we know, we have equivalent Go code for all the python code that depend on the python containerregistry library and are not aware of any pending issues. The next step is to actually start aliasing existing rules and flipping boolean flags so that the rules in rules_docker start using the Go code instead of the python code. Our plan going forward is as follows in the given order:

1. Cut release 0.10.0 as the last rules_docker release where all the implementation is python.
2. Rename new_container_pull to container_pull and new_container_load to container_load. This will be done in a way such that current users of container_pull and container_load will be automatically switched to the Go implementation. We'll also rename the old container_pull and container_load to legacy_container_pull and legacy_container_load.
3. Wait for a week to see if there are any major issues. All watchers of this repo/issue are strongly encouraged to try out the new rules and report issues. Ideally if you want to help us find issues right now, please try new_container_pull and new_container_load right now at commit 8f0b744 or later and report any issues. Please file Github issues on rules_docker for this.
4. Update all remaining container_* rules in //container and //contrib to use the Go implementation instead of python by default. With this change, we'll add a section to our readme on how to switch between the Go & python implementations should they discover issues.
5. Cut release 0.11.0.
6. Remove all python containerregistry code from the core rules. The idea will be to get the repository in a state where users can import rules_docker and it's dependencies without having to set the host_force_python flag. This will also remove the legacy_container_pull and legacy_container_load rules and remote the ability to switch the other rules to use python code from containerregistry.
7. Cut release 0.12.0.

Update: Step 4 was being attempted with #1142 which revealed some issues in the image config creator with further testing. These fixes have been split out to #1148. Testing #1142 in our internal repo showed a couple of other backward compatibility issues that will need to be fixed upstream in go-containerregistry which I'll create issues for next. So the next steps before I can proceed with Step 4 are:

1. Wait for #1148 to be merged.
2. Wait for the backwards compatibility go-containerregistry issues to be fixed (I need to actually create these issues first).

[smukherj1]

Update: With #1131, the migration of rules_docker rules to Go is feature complete. This means, as far as we know, we have equivalent Go code for all the python code that depend on the python containerregistry library and are not aware of any pending issues. The next step is to actually start aliasing existing rules and flipping boolean flags so that the rules in rules_docker start using the Go code instead of the python code. Our plan going forward is as follows in the given order:

1. Cut release 0.10.0 as the last rules_docker release where all the implementation is python.
2. Rename new_container_pull to container_pull and new_container_load to container_load. This will be done in a way such that current users of container_pull and container_load will be automatically switched to the Go implementation. We'll also rename the old container_pull and container_load to legacy_container_pull and legacy_container_load.
3. Wait for a week to see if there are any major issues. All watchers of this repo/issue are strongly encouraged to try out the new rules and report issues. Ideally if you want to help us find issues right now, please try new_container_pull and new_container_load right now at commit 8f0b744 or later and report any issues. Please file Github issues on rules_docker for this.
4. Update all remaining container_* rules in //container and //contrib to use the Go implementation instead of python by default. With this change, we'll add a section to our readme on how to switch between the Go & python implementations should they discover issues.
5. Cut release 0.11.0.
6. Remove all python containerregistry code from the core rules. The idea will be to get the repository in a state where users can import rules_docker and it's dependencies without having to set the host_force_python flag. This will also remove the legacy_container_pull and legacy_container_load rules and remote the ability to switch the other rules to use python code from containerregistry.
7. Cut release 0.12.0.

Step 4 is being attempted with #1142.

[smukherj1]

Update: With #1131, the migration of rules_docker rules to Go is feature complete. This means, as far as we know, we have equivalent Go code for all the python code that depend on the python containerregistry library and are not aware of any pending issues. The next step is to actually start aliasing existing rules and flipping boolean flags so that the rules in rules_docker start using the Go code instead of the python code. Our plan going forward is as follows in the given order:

1. Cut release 0.10.0 as the last rules_docker release where all the implementation is python.
2. Rename new_container_pull to container_pull and new_container_load to container_load. This will be done in a way such that current users of container_pull and container_load will be automatically switched to the Go implementation. We'll also rename the old container_pull and container_load to legacy_container_pull and legacy_container_load.
3. Wait for a week to see if there are any major issues. All watchers of this repo/issue are strongly encouraged to try out the new rules and report issues. Ideally if you want to help us find issues right now, please try new_container_pull and new_container_load right now at commit 8f0b744 or later and report any issues. Please file Github issues on rules_docker for this.
4. Update all remaining container_* rules in //container and //contrib to use the Go implementation instead of python by default. With this change, we'll add a section to our readme on how to switch between the Go & python implementations should they discover issues.
5. Cut release 0.11.0.
6. Remove all python containerregistry code from the core rules. The idea will be to get the repository in a state where users can import rules_docker and it's dependencies without having to set the host_force_python flag. This will also remove the legacy_container_pull and legacy_container_load rules and remote the ability to switch the other rules to use python code from containerregistry.
7. Cut release 0.12.0.

1142 is merged which means Step 4 is done (except the README part). The change has also been merged in our internal repo. We're going to wait a week to see if things break internally before cutting a new release.

All the container_* rules in rules docker now use the Go implementation by default.

[smukherj1]

Update: With #1131, the migration of rules_docker rules to Go is feature complete. This means, as far as we know, we have equivalent Go code for all the python code that depend on the python containerregistry library and are not aware of any pending issues. The next step is to actually start aliasing existing rules and flipping boolean flags so that the rules in rules_docker start using the Go code instead of the python code. Our plan going forward is as follows in the given order:

1. Cut release 0.10.0 as the last rules_docker release where all the implementation is python.
2. Rename new_container_pull to container_pull and new_container_load to container_load. This will be done in a way such that current users of container_pull and container_load will be automatically switched to the Go implementation. We'll also rename the old container_pull and container_load to legacy_container_pull and legacy_container_load.
3. Wait for a week to see if there are any major issues. All watchers of this repo/issue are strongly encouraged to try out the new rules and report issues. Ideally if you want to help us find issues right now, please try new_container_pull and new_container_load right now at commit 8f0b744 or later and report any issues. Please file Github issues on rules_docker for this.
4. Update all remaining container_* rules in //container and //contrib to use the Go implementation instead of python by default. With this change, we'll add a section to our readme on how to switch between the Go & python implementations should they discover issues.
5. Cut release 0.11.0.
6. Remove all python containerregistry code from the core rules. The idea will be to get the repository in a state where users can import rules_docker and its dependencies without having to set the host_force_python flag. This will also remove the legacy_container_pull and legacy_container_load rules and remote the ability to switch the other rules to use python code from containerregistry.
7. Cut release 0.12.0.

Step 5 is done. Release v0.11.0 has been created and the README is being updated with #1181.

Will start working on Step 6 next.

[smukherj1]

Update: With #1131, the migration of rules_docker rules to Go is feature complete. This means, as far as we know, we have equivalent Go code for all the python code that depend on the python containerregistry library and are not aware of any pending issues. The next step is to actually start aliasing existing rules and flipping boolean flags so that the rules in rules_docker start using the Go code instead of the python code. Our plan going forward is as follows in the given order:

1. Cut release 0.10.0 as the last rules_docker release where all the implementation is python.
2. Rename new_container_pull to container_pull and new_container_load to container_load. This will be done in a way such that current users of container_pull and container_load will be automatically switched to the Go implementation. We'll also rename the old container_pull and container_load to legacy_container_pull and legacy_container_load.
3. Wait for a week to see if there are any major issues. All watchers of this repo/issue are strongly encouraged to try out the new rules and report issues. Ideally if you want to help us find issues right now, please try new_container_pull and new_container_load right now at commit 8f0b744 or later and report any issues. Please file Github issues on rules_docker for this.
4. Update all remaining container_* rules in //container and //contrib to use the Go implementation instead of python by default. With this change, we'll add a section to our readme on how to switch between the Go & python implementations should they discover issues.
5. Cut release 0.11.0.
6. Remove all python containerregistry code from the core rules. The idea will be to get the repository in a state where users can import rules_docker and its dependencies without having to set the host_force_python flag. This will also remove the legacy_container_pull and legacy_container_load rules and remove the ability to switch the other rules to use python code from containerregistry.
7. Cut release 0.12.0.

Step 6 was completed with #1186 & #1189. I have confirmed container_pull, container_image, container_flatten and container_push no longer require setting the --host_force_python=PY2 flag at rules_docker HEAD.

[smukherj1]

rules_docker release v0.12.0 has been created with completes the migration of the core image rules to go-containerregistry from python containerregistry.

The README has been updated with instructions to download the new rules. Note that we no longer need to specify --host_force_python=PY2 in the .bazelrc file. This only applies to the rules in //container, //contrib & the <lang>_image rules.

We have a couple of rules under //docker that depend on some python scripts that not python3 compatible yet. These are fully self contained scripts (i.e., they don't depend on the python containerregistry library) so they should be easy to fix. Regardless, the migration of rules_docker to go-containerregistry is now complete.