rules_docker icon indicating copy to clipboard operation
rules_docker copied to clipboard

java_image args broken in 0.23.0+

Open joca-bt opened this issue 3 years ago • 23 comments

🐞 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 https://github.com/bazelbuild/rules_docker/pull/1957).

Previously, adding args to java_image would result in those args being propagated to the underlying java_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:

> bazel run :image
hello there!
[--stuff, --stuff]

0.23.0:

> bazel run :image
unknown flag: --stuff
See 'docker run --help'.

I am guessing this is happening with other language rules as well.

🔬 Minimal Reproduction

BUILD:

load("@io_bazel_rules_docker//java:image.bzl", "java_image")

java_library(name = "lib", srcs = [ "Main.java" ])

java_binary(name = "bin", main_class = "demo.Main", runtime_deps = [ ":lib" ])

java_image(name = "image", base = "@java//image", main_class = "demo.Main", args = [ "--stuff" ], runtime_deps = [ ":lib" ])

Main.java:

package demo;

import java.util.Arrays;

public class Main {
    public static void main(String[] args) {
        System.out.println("hello there!");
        System.out.println(Arrays.toString(args));
    }
}

WORKSPACE:

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "io_bazel_rules_docker",
    # sha256 = "59536e6ae64359b716ba9c46c39183403b01eabfbd57578e84398b4829ca499a",
    # strip_prefix = "rules_docker-0.22.0",
    # url = "https://github.com/bazelbuild/rules_docker/releases/download/v0.22.0/rules_docker-v0.22.0.tar.gz",
    sha256 = "85ffff62a4c22a74dbd98d05da6cf40f497344b3dbf1e1ab0a37ab2a1a6ca014",
    strip_prefix = "rules_docker-0.23.0",
    url = "https://github.com/bazelbuild/rules_docker/releases/download/v0.23.0/rules_docker-v0.23.0.tar.gz",
)

load("@io_bazel_rules_docker//repositories:repositories.bzl", "repositories")
repositories()

load("@io_bazel_rules_docker//repositories:deps.bzl", "deps")
deps()

load("@io_bazel_rules_docker//container:container.bzl", "container_pull")
container_pull(name = "java", registry = "gcr.io", repository = "distroless/java", tag = "latest")

🌍 Your Environment

Operating System:

Linux 5.18.9-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 02 Jul 2022 21:03:06 +0000 x86_64 GNU/Linux

Output of bazel version:

Bazelisk version: v1.12.0
Build label: 5.0.0
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Wed Jan 19 14:08:54 2022 (1642601334)
Build timestamp: 1642601334
Build timestamp as int: 1642601334

Rules_docker version:

0.23.0

joca-bt avatar Jul 07 '22 11:07 joca-bt

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?

kersson avatar Oct 03 '22 15:10 kersson

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 https://github.com/bazelbuild/rules_docker/issues/2124#issuecomment-1265651158, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVM4K6O4KISU3THLPDUT23WBL4JFANCNFSM525A2U7Q . You are receiving this because you are subscribed to this thread.Message ID: @.***>

psigen avatar Oct 03 '22 16:10 psigen

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 entrypoint, but then they are passed again in the command.

You can see the entrypoint correctly containing the args by directly running docker run --rm <image_name> on the image name printed in the build step (which will work correctly) and also verify using docker inspect on that image.

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 docker run as if they were specified at the CLI.

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.

psigen avatar Oct 10 '22 12:10 psigen

Any luck figuring it out?

joca-bt avatar Oct 30 '22 09:10 joca-bt

I created this detailed reproduction here: https://github.com/psigen/rules_docker-issue-2124

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.

psigen avatar Oct 31 '22 12:10 psigen

Oh my god I think it might be this INCREDIBLY HARD TO FIND detail: https://groups.google.com/g/bazel-discuss/c/RJy6IsqiJxY/m/5fkv5rKWDwAJ

Executable rules automatically get the args attribute, and the args attribute is used by the run command for starlark rules:

https://bazel.build/reference/be/common-definitions#binary.args

Command line arguments that Bazel will pass to the target when it is executed either by the run command or as a test. These arguments are passed before the ones that are specified on the bazel run or bazel test command line.

NOTE: The arguments are not passed when you run the target outside of Bazel (for example, by manually executing the binary in bazel-bin/).


I think this means that we are in a bit of a quandary. Currently, the way that images are built, the lang_image commands do the intuitive thing, which is to include the args in the image. This is because it's pretty unintuitive for those args to just disappear if you are using bazel build to generate an image which will do the thing you expected when running.

Simultaneously, bazel's documented behavior is to secretly save those args and pass them again if the target is triggered by bazel run, which adds a duplicate copy of the args which is ill-formed. The PR that I made just sensitizes this issue by making the error more visible (since it's passed to docker run which generally fails instead of secretly getting passed twice to the app args).

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 :sob:

psigen avatar Oct 31 '22 12:10 psigen

I believe this also break the following

You can suppress this behavior by passing the single flag: bazel run :foo -- --norun

from https://github.com/bazelbuild/rules_docker/#using-with-docker-locally.

joca-bt avatar Nov 06 '22 11:11 joca-bt

What about reverting https://github.com/bazelbuild/rules_docker/pull/1957 until this is fixed?

joca-bt avatar Nov 06 '22 11:11 joca-bt

Reverting would go back to not being able to pass any args to bazel run on the command line, which would break anyone using that functionality.

I made this PR, can you see if it helps? https://github.com/bazelbuild/rules_docker/pull/2184

psigen avatar Nov 06 '22 14:11 psigen

For the example I pasted above it is working with the added bonus of removing the duplication of arguments:

> bazel run :image
hello there!
[--stuff]

--norun also seems to be restored. It is loading the image into Docker instead of failing:

> bazel run --stamp :image -- --norun

However, I tried this on a more complex project and got the following:

> bazel run <target>
[--spring.config.location=/app/config/, /bin/bash] # printing app args as on the simple example above

vs in 0.22:

> bazel run <target>
[--spring.config.location=/app/config/, --spring.config.location=/app/config/]

It is now injecting /bin/bash in the image arguments. Trying to figure out where this is coming from. This is my java_image call:

java_image(
    name = name,
    tags = tags,
    base = base_image,
    main_class = main_class,
    jvm_flags = JVM_FLAGS,
    args = [
        "--spring.config.location=/app/config/",
    ],
    runtime_deps = runtime_deps,
)

I've added print(ctx.attr.args) just before your change and the arguments are correct. No /bin/bash in there. I've also printed $@ before shift "%{run_num_args}" and there is also no /bin/bash in there.

joca-bt avatar Nov 09 '22 15:11 joca-bt

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 https://github.com/bazelbuild/rules_docker/issues/2124#issuecomment-1308936708, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVM4K244QTAKMERCVTBUTDWHO7N3ANCNFSM525A2U7Q . You are receiving this because you are subscribed to this thread.Message ID: @.***>

psigen avatar Nov 09 '22 16:11 psigen

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.

joca-bt avatar Nov 09 '22 17:11 joca-bt

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:

java_image(
    name = "image2",
    args = ["--spring.config.location=/app/config/"],
    base = "@java//image",
    jvm_flags = [
        "-Dparam1=value1",
        "-Dparam2=value2",
    ],
    main_class = "demo.Main",
    tags = [
        "apple",
        "bees",
    ],
    runtime_deps = [":lib"],
)

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 base image you are using. Do you know if it has /bin/bash in its ENTRYPOINT or CMD directive?

psigen avatar Nov 12 '22 23:11 psigen

Neither images (base and java_image) specify an entrypoint. I will look again for CMD.

joca-bt avatar Nov 13 '22 10:11 joca-bt

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 https://github.com/bazelbuild/rules_docker/issues/2124#issuecomment-1312696600, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVM4K6UWZ6FJMANCRSXI3TWIC7HHANCNFSM525A2U7Q . You are receiving this because you are subscribed to this thread.Message ID: @.***>

psigen avatar Nov 13 '22 14:11 psigen

No CMDs or ENTRYPOINTs at all. The base image just install a bunch of company-specific stuff.

joca-bt avatar Nov 15 '22 08:11 joca-bt

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?

psigen avatar Nov 15 '22 13:11 psigen

I was able to reproduce this situation. The parent image (quay.io/centos/centos:stream8) is setting cmd:

[
    {
        "ContainerConfig": {
            "Cmd": [
                "/bin/bash"
            ],
            "Entrypoint": [],

I have modified my original example, see docker.zip.

> bazel run :image
hello there!
[--stuff, /bin/bash]

docker.zip

joca-bt avatar Nov 27 '22 13:11 joca-bt

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 https://github.com/bazelbuild/rules_docker/issues/2124#issuecomment-1328246040, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVM4K2UJEPWJH6P2LCDX3TWKNOELANCNFSM525A2U7Q . You are receiving this because you are subscribed to this thread.Message ID: @.***>

psigen avatar Nov 27 '22 13:11 psigen

Ping on this - my instructions for having folks test out locally built containers won't work with a java_image(args=["config.file"]) as a result of this issue, is there any news to be had?

werkt avatar May 04 '23 00:05 werkt

@psigen we are almost there, will you be able to finish, or is there a way we can help?

joca-bt avatar Jun 12 '23 11:06 joca-bt

I will give this another shot this weekend. Sorry, I changed jobs in the meantime so I got a bit distracted!

psigen avatar Jun 15 '23 14:06 psigen