rules_docker icon indicating copy to clipboard operation
rules_docker copied to clipboard

Add configurable entrypoint to py3_image

Open dbruggner opened this issue 1 year ago • 3 comments

This makes entrypoint for py3_image configurable so that a user can provide path to a hermetic python environment.

PR Checklist

Please check if your PR fulfills the following requirements:

  • [ ] 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?

  • [ ] 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?

Issue Number: N/A

Currently the Python binary is fixed as /usr/bin/python.

What is the new behavior?

The Python binary can be choses by overrideing the entrypoint for example to point to a hermetic Python.

Does this PR introduce a breaking change?

  • [ ] Yes
  • [x] No

Other information

dbruggner avatar Apr 03 '23 09:04 dbruggner

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Apr 03 '23 09:04 google-cla[bot]

I wrote an essentially identical PR before discovering this one (https://github.com/bazelbuild/rules_docker/pull/2247).

It seems like this has been discussed a few times:

  • https://github.com/bazelbuild/rules_docker/issues/498
  • https://github.com/bazelbuild/rules_docker/pull/1264

It seems like the current direction is to override the entrypoint with an additional layer: https://github.com/bazelbuild/rules_docker/issues/309#issuecomment-477593393

Personally, I think this highlights a usability problem. Although users can extend the image using a separate target, this isn't the most intuitive, nor perhaps the best data model (the entrypoint is a function of the base image, not the Python build ontop of it).

Maybe we should add support for something like container_options that get expanded into the **kwargs of the underlying container_image? This addresses the scalability concerns (since there is no logic inside the language-specific rules), while allowing users to do the intuitive thing.

DerekTBrown avatar Apr 13 '23 19:04 DerekTBrown

It seems like the current direction is to override the entrypoint with an additional layer: #309 (comment)

In my opinion, overridden entrypoint is not the best approach, because it displaces the python binary target to be executed from the final entrypoint command. This way you should also add /app/path/to/target/image.binary to entrypoint.

I see symlinking /bin/usr/python to actual python you want to use (as mentioned here and here) as a preferable solution. It could be done once to generate a base python image to prevent all further images from adding extra container_image layer. After current PR gets merged, it could be done in more straightforward way, declaring base image with entrypoint.

Your suggestion to add container_options to be passed to underlying container_image seems good, but we should consider more use cases, imho

x-mass avatar Jun 03 '23 06:06 x-mass