-
Notifications
You must be signed in to change notification settings - Fork 690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
java_image args broken in 0.23.0+ #2124
Comments
Yes, I'm having the same issue with |
I will try to create a PR to fix this, I think it's just a typo that was
not caught when I made the other PR.
…On Mon, Oct 3, 2022, 11:34 Krishna Ersson ***@***.***> wrote:
I am guessing this is happening with other language rules as well.
Yes, I'm having the same issue with py_image. Is there something we're
missing? How should we be passing args to a binary within the container?
—
Reply to this email directly, view it on GitHub
<#2124 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVM4K6O4KISU3THLPDUT23WBL4JFANCNFSM525A2U7Q>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Ok, so something very weird is going on here. When I run the repro, the image that is built is already created with the args embedded in the You can see the entrypoint correctly containing the args by directly running I can't yet figure out where the second set of arguments is being passed (or stored) at all, since all the arguments are actually in the entrypoint already. They are passed as positional args to the wrapper script that is calling Right now, I think what happened is that at some point in the past, the args started getting baked into entrypoint, but the run command was not updated to reflect this and still passes them again as CLI args. We need to find some way for them not to be passed as positional args, which will solve the issue. |
Any luck figuring it out? |
I created this detailed reproduction here: I am still completely stuck trying to figure out where the args from the image instruction are being stored somewhere in bazel in the run info for the target. I think it's happening implicitly somewhere in the declaration of the target, but I'm struggling a bit to find it. |
Oh my god I think it might be this INCREDIBLY HARD TO FIND detail:
https://bazel.build/reference/be/common-definitions#binary.args
I think this means that we are in a bit of a quandary. Currently, the way that images are built, the Simultaneously, bazel's documented behavior is to secretly save those args and pass them again if the target is triggered by I guess one hack-on-top-of-hack we could do is to also pass args a third time to the incremental load template, and cancel out the second copy of the args that are passed. But this just seems like an egregious hack 😭 |
I believe this also break the following
from https://github.com/bazelbuild/rules_docker/#using-with-docker-locally. |
What about reverting #1957 until this is fixed? |
Reverting would go back to not being able to pass any args to I made this PR, can you see if it helps? |
For the example I pasted above it is working with the added bonus of removing the duplication of arguments:
However, I tried this on a more complex project and got the following:
vs in 0.22:
It is now injecting
I've added |
Thanks for testing!
Is there any other info you can provide on the complex target that is not
working? I would love to get that into my repro and try to fix it.
Maybe a snippet of the args to that target?
…On Wed, Nov 9, 2022, 10:29 João Guerra ***@***.***> wrote:
I've added print(ctx.attr.args) just before your change and the arguments
are correct. No /bin/bash in there.
—
Reply to this email directly, view it on GitHub
<#2124 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVM4K244QTAKMERCVTBUTDWHO7N3ANCNFSM525A2U7Q>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
I shared the args above (updated earlier via edit and also other info). I can try reproducing it in a mock setup but it will take me some time. |
Hmm, I tried a few different ways to replicate your behavior in my repro and I'm having trouble breaking things. Here's what I tried to use to replicate:
With the above, I get the expected output of: $ bazel run //:image2
[...]
Loaded image ID: sha256:5a1a09302dd51caee5aa82c4edfff984e1a059d46038becbe4332d54816b7dbd
Tagging 5a1a09302dd51caee5aa82c4edfff984e1a059d46038becbe4332d54816b7dbd as bazel:image2
hello there!
[--spring.config.location=/app/config/]
I'm slightly suspicious of something conflicting in the |
Neither images (base and java_image) specify an entrypoint. I will look again for |
Ok, if there's anything else you can suggest in my target above that you
think might replicate the issue, please let me know.
Maybe changing number of arguments or something interesting about the base
image?
…On Sun, Nov 13, 2022, 05:29 João Guerra ***@***.***> wrote:
Neither images (base and java_image) specify an entrypoint. I will look
again for CMD.
—
Reply to this email directly, view it on GitHub
<#2124 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVM4K6UWZ6FJMANCRSXI3TWIC7HHANCNFSM525A2U7Q>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
No CMDs or ENTRYPOINTs at all. The base image just install a bunch of company-specific stuff. |
Darn: can you share what the base image for that image is? Or post some (sanitized) output from docker-inspect to see what Entrypoint and Cmd is being used? |
I was able to reproduce this situation. The parent image (
I have modified my original example, see
|
Awesome, I will try this out right away!
…On Sun, Nov 27, 2022, 08:23 João Guerra ***@***.***> wrote:
Seems like one of the parent images is setting cmd:
[
{
"ContainerConfig": {
"Cmd": [
"/bin/bash"
],
"Entrypoint": [],
I have modified my original example to include this (docker.zip).
Run > bazel run :image.
docker.zip
<https://github.com/bazelbuild/rules_docker/files/10098409/docker.zip>
—
Reply to this email directly, view it on GitHub
<#2124 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVM4K2UJEPWJH6P2LCDX3TWKNOELANCNFSM525A2U7Q>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Ping on this - my instructions for having folks test out locally built containers won't work with a |
@psigen we are almost there, will you be able to finish, or is there a way we can help? |
I will give this another shot this weekend. Sorry, I changed jobs in the meantime so I got a bit distracted! |
This issue 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. |
🐞 bug report
Affected Rule
java_image
(And possibly all other language rules.)
Is this a regression?
Yes.
0.22.0: worked fine
0.23.0: broken
Description
java_image
args
parameter broken in 0.23.0 (probably by #1957).Previously, adding
args
tojava_image
would result in those args being propagated to the underlyingjava_binary
rule and hence to the java application when starting it.This behaviour was broken in 0.23.0 and these args are now being passed to docker.
0.22.0:
0.23.0:
I am guessing this is happening with other language rules as well.
🔬 Minimal Reproduction
BUILD
:Main.java
:WORKSPACE
:🌍 Your Environment
Operating System:
Output of
bazel version
:Rules_docker version:
The text was updated successfully, but these errors were encountered: