search

GoogleCloudPlatform / ruby-docker

Ruby runtime for Google Cloud Platform
Apache License 2.0
134 stars 56 forks source link

feat: add `--entrypoint` to support buildpack-based images #200

Closed glasnt closed 3 years ago

glasnt commented 3 years ago

exec-wrapper can be used for data migrations for Cloud Run applications, as seen in the Django on Cloud Run tutorial.

However, for Cloud Run applications whose images are built with Cloud Buildpacks, the Docker command in execute.sh needs to specify an entrypoint so that a custom command can be run (as Buildpacks define the entrypoint as the web process defined in Procfile, and this needs to be overridden)

This PR adds this functionality, as well as some additions to the exec-wrapper readme to show it's usage.

glasnt commented 3 years ago

@dazuma for review

frankieeedeee commented 3 years ago

Hey @dazuma and @glasnt - Apologies if I'm missing something here, but I believe Docker entrypoints only specify an executable, thereby excluding additional commands/arguments in entrypoints.

The implication of having included additional quotes (ENTRYPOINT="--entrypoint \"${ENTRYPOINT}\"") around the $ENTRYPOINT variable means the underlying docker run command is broken when an entrypoint is provided to this image.

Example: (apologies, I'm coming from PHP land 🙂) $ docker run gcr.io/google-appengine/exec-wrapper -i php -r php -- my_example_migration.php

...produces: "\"php\"": executable file not found in $PATH: unknown.

^^ note the inclusion of the backslashes here. Ignoring the fact that overwriting the entrypoint in this particular PHP image is redundant, this demonstrates that line 104 should simply be reverted to:

ENTRYPOINT="--entrypoint ${ENTRYPOINT}"

Happy to learn if I've missed something here? But currently can't run this wrapper image when the application image defines an entrypoint which needs to be overridden in order to run the migration :)

glasnt commented 3 years ago

Hi @frankieeedeee, thanks for the reply!

Have you had a look at this comment on the thread: https://github.com/GoogleCloudPlatform/ruby-docker/pull/200#discussion_r670839966. I'm yet to do more experiments here, but this was the design discussion around this change.

frankieeedeee commented 3 years ago

Thanks @glasnt! Yes I did catch that review comment, which is what caught my eye whilst debugging a cloud build process I'm working on. With respect to that comment specifically, I believe the command should be:

docker run gcr.io/google-appengine/exec-wrapper -i my-application-image -r bundle -e SOME_ENV=foo -- exec ruby

Note how -r simply includes a single, executable. Not the subsequent commands exec ruby. This is because by design Docker entrypoints only support a single executable/executable path, (docker actually runs a check to make sure it exists before running the container).