-
Notifications
You must be signed in to change notification settings - Fork 693
rules_docker should use go-container registry #580
Comments
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. |
There already has been a PR that started getting pusher in #481 so maybe there is also room in the community to possibly help delivering this quicker. |
@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. |
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. |
I'd like to help make this happen. What would be the best way for me to help? |
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) |
The default python on my system is python3. This caused me to run into bazelbuild/rules_docker#454 when running `bazel build //...`. Update the documentation to indicate that `/usr/bin/env python` must resolve to python2 in order for all of the Bazel commands included in the documentation to work. We can remove this comment after Bazel better supports using python3 as the default system python. It appears from bazelbuild/rules_docker#580 that the `rules_docker` folks hope to merge the fix in mid to late Q1 2019.
@nlopezgi We are getting close to the end of Q1 anything new in regards to the migration plans? |
@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. |
@xingao267 is the migration underway? |
It has started with #830 |
@nlopezgi any chance for an update on this important effort? |
@xingao267 can you provide a quick update on how migration is going? |
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. |
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. |
@ittaiz by layer cache, do you mean the container_layer rule? |
I'm talking about the container_pull cache #706 |
@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. |
🤞 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) |
@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) |
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 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. |
Update on migration status: #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:
|
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 ***@***.***> wrote:
Update on migration status:
#1111 <#1111> which
implements join_layers in Go is currently being reviewed.
#1114 <#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
<#927> and #1075
<#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
<#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>
|
Update on migration:
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. |
Are the files in |
@aaliddell Yes, we plan to remove all references to @ContainerRegistry in core rules. So any references in |
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:
|
Step 1 is done. Step 2 is pending with PR #1140. |
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:
|
Step 4 is being attempted with #1142. |
See following issues for details * bazelbuild/rules_docker#842 * bazelbuild/rules_docker#580 * bazelbuild/bazel#7899 Signed-off-by: stuart.warren <stuart.warren@ocado.com>
#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. |
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. |
Step 6 was completed with #1186 & #1189. I have confirmed |
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 We have a couple of rules under |
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: google/containerregistry#114
What are the chances that we could move to go-container registry for pushing and pulling?
https://github.com/google/go-containerregistry
The text was updated successfully, but these errors were encountered: