Support for docker run args for docker image targets #1957
Conversation
This adds parsing of command-line arguments being passed to 'bazel run' when executing an image target. The "--" delimiter is used to separate arguments for "docker run" from arguments to the container. This makes the following style of calls possible: ``` bazel run //:my_cool_image -- -p 8080:8080 -v /tmp:/tmp -- --my-cool-flag ``` which is translated into ``` docker run -i --rm -p 8080:8080 -v /tmp:/tmp bazel/my_cool_image:image --my-cool-flag ```
psigen
requested review from
alexeagle,
gravypod,
pcj and
smukherj1
as code owners
psigen
force-pushed
the
docker_run_args
branch
from
074ddd0
to
dd0b7e9
Compare
psigen
changed the title
There was a problem hiding this comment.
I don't see any initial reason to not merge this. @alexeagle any thoughts? It looks like it should be backwards compatible.
|
@gravypod Since previously all the args were just being ignored and we preserved |
|
Where could I add a unit test for the behavior? I think I got the docs updated. (Also, that build error in CI looks totally unrelated to the PR) |
|
I'd love to move this change forward! Please just let me know what I need to do next to get it approved 😄 |
|
For starters. It seems there's a build failure in CI. Fixing that may move this forward |
|
The build failure has nothing to do with this PR though (I changed the readme in that commit and the failure seems to be ~an image fetch~ a package installation reproducibility failure).
Is there a way to re-kick the build? I guess I can just fiddle with whitespace to make it rebuild.
|
|
I have successfully reworded the |
There was a problem hiding this comment.
Thanks for updating the docs. Looks good to me now. Going to check in with @alexeagle but I think this should be backwards compatible.
|
Just remembered, we've previously discussed this. I think we're good to go here. |
|
Thanks @gravypod! |
|
I think there may have been a mistake in the merge earlier in this PR, line |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The
bazel runcommand applied to docker images currently does not have functionality to pass any arguments to the underlying docker run command. This means that it is impossible to use docker image targets to perform things like CLI operations on files, or set their network settings.Issue Number: N/A
What is the new behavior?
This PR changes
bazel run's behavior on image targets to mirrordocker run's behavior. Docker images can now be run with arbitrary docker flags like:which mirrors a call to
Since
--norunis not a valid docker run flag, we can strip and parse it with no changes, preserving existing functionality.Does this PR introduce a breaking change?
Other information
Can someone please help me with defining appropriate testing and documentation for this change? 🙏