rules_docker icon indicating copy to clipboard operation
rules_docker copied to clipboard

Support multi-architecture base-images in go. Use the base image for …

Open valerian-roche opened this issue 3 years ago • 3 comments

Currently the go_image target always uses an amd64 base image even when goarch is provided by the user This can be partially worked-around by specifying a base image properly using the arm64 version of the distroless/base manifest, but the call to app_layer does not provide the architecture, resulting in the image being considered as amd64 by docker This PR makes sure to pass the architecture used to app_layer when actually provided. It is also sourcing the different architectures currently supported by the distroless/base image to be used automatically as base depending on the goarch provided (defaulting to amd64 - current behavior) if not set. If the user wants to still use an amd64 base with an arm64 binary (not sure if there is a use case, but current behavior), the user can specify the base image using the _amd64 target

We could enforce this goarch based on platform as done for go_binary here. This was not done to avoid a larger impact for users who would get an implicit change of base image if building on arm-based machines

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • [x] Bugfix
  • [x] Feature
  • [ ] Code style update (formatting, local variables)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] CI related changes
  • [ ] Documentation content changes
  • [ ] Other... Please describe:

What is the current behavior?

When building a go_image without providing the binary but providing the goarch to be used, the final image is based on the amd64 distroless/base image (even if a different arch is used to build the binary) and the new layer is defined as amd64 While the base image can be provided to properly provide an arm64 image, it cannot be configured for the newly added layer

Issue Number: N/A

What is the new behavior?

When building a go_image without providing a specific goarch, behavior should remain identical, with the base image based on amd64 and the added layer being amd64 When providing a goarch, go_image will now use the proper base architecture if supported by the distroless image or fail. In this case we expect the user to provide a base image (which can be one of the supported one). The newly added layer will also use goarch as the architecture provided to app_layer

Does this PR introduce a breaking change?

  • [x] Yes
  • [ ] No

There is a potential impact for people using goarch in go_image and not supported in distroless/base. User will have to provide a base image in this case, which can simply be @go_image_base//image

Other information

valerian-roche avatar Oct 07 '21 20:10 valerian-roche

@valerian-roche thanks this seems like a great issue to dig into! It's hard for me to understand whether this is a correct problem assessment and fix. Are you already using it internally? Knowing that would help to have more confidence.

@pcj are you available to do review here? Seems like it's in your area of expertise.

alexeagle avatar Nov 22 '21 19:11 alexeagle

@valerian-roche thanks this seems like a great issue to dig into! It's hard for me to understand whether this is a correct problem assessment and fix. Are you already using it internally? Knowing that would help to have more confidence.

Hey! The issue this is attempting to fix is the use of the go_image macro on architectures that are not amd64 In the setup at the time of this PR (might have changed since, will take a look), the macro does allow to use any architecture (it will default to the one of the laptop) while the image built will always be flagged as amd64 and built on top of distroless/amd64 This can be worked around by fully bypassing go_image and using app_layer directly but it makes it far more complex to maintain. Therefore this PR does two things:

  • Make sure the architecture is consistent between the go binary and the layer and base image
  • Add support for distroless images with other architectures to provide the base image to support. This can be done manually by the user but again somewhat breaks the ease of use of go_image if we push it back to the user

We have been using it internally on two repositories on our side. It does work well at least in our context of arm64 and amd64 coexisting based on local architecture and targets (local vs. remote). Sadly Bazel does not support today manifests so we still have to do some magic around to push multi-arch images, but it at least provides the necessary pieces

valerian-roche avatar Nov 22 '21 19:11 valerian-roche

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days. Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_docker!

github-actions[bot] avatar Aug 09 '22 03:08 github-actions[bot]

This PR was automatically closed because it went 30 days without a reply since it was labeled "Can Close?"

github-actions[bot] avatar Sep 08 '22 03:09 github-actions[bot]