Rules_docker: docker_run_flags not passed to docker run of lang_image

[JoostvDoorn]

Not sure if this is intended, but I want to pass certain env variables on bazel run of the py3_image below, the docker_run_flags can only be set to a container_image and not on the py3_image. Can the docker_run_flags be added to lang_image rules, or can it be inherited from the base image?

container_image(
    name = "py3_image_1",
    base = "@py3_image_base//image",
    legacy_run_behavior = False,
    docker_run_flags = "-i --rm --network=host -e ABC=ABC"
)

py3_image(
    name = "python_image",
    srcs = ["main.py"],
    base = ":py3_image_1",
    layers = [
        ":main",
    ],
    main = "main.py",
)

[xingao267]

@JoostvDoorn py3_image shouldn't take attributes other than those allowed in a py_binary rule. So it's intended that docker_run_flags cannot be specified in lang_image rules.

But we can probably make it inherit from the base image. Let me see what I can do there.

[xingao267]

Currently, the docker_run_flags attribute is consumed by the incremental load script but is not made available in the providers of container_image. I'll send a PR to support that soon.

[Globegitter]

@xingao267 I think it would still be nice if the <lang>_images would take docker_run_flags argument themselves. Just running into this also and having to create a custom base_image for all images seems a bit of an overkill. And the argument that xx_image should only allow what xx_binary supports does not hold true anymore anyway, as there are e.g. layers or base for most of the lang images.

Just to give a reason why, to get docker_run_flags set now properly I have to add about 13 lines per lang image where i want to add it, e.g.

DEFAULT_BASE = select({
    "@io_bazel_rules_docker//:fastbuild": "@nodejs_image_base//image",
    "@io_bazel_rules_docker//:debug": "@nodejs_debug_image_base//image",
    "@io_bazel_rules_docker//:optimized": "@nodejs_image_base//image",
    "//conditions:default": "@nodejs_debug_image_base//image",
})

container_image(
    name = "nodejs_image",
    base = DEFAULT_BASE,
    legacy_run_behavior = False,
    docker_run_flags = "-i --rm --network=host -e SOME_ENV=env"
)

plus overriding the base image. That does imo just clutter the build files for no other benefit.

But either way thank you for at least making it possible to somehow set docker_run_flags :)

[nlopezgi]

@Globegitter thanks for your comments. I do think it might be somewhat easier to have the docker_run_flags added to the xx_image rules, but this is not a path we are interested in pursuing. As you mention, we did allow this for layers. However, the case of layers is very particular, in that the implementation of layers is very specific per language in which it has been added (e.g., for py_image we recently commited https://github.com/bazelbuild/rules_docker/commit/c9065d170c076d540166f068aec0e04039a10e66 , which does lots of optimization that is very specific to how python dependencies work).

For concerns such as docker run flags, timestamps (https://github.com/bazelbuild/rules_docker/pull/560), entrypoints, commands, and any others that are agnostic to the language, and relate instead to the execution platform (i.e., to how the container is set up) we will continue to expect the path of "combining a container_image with a xx_image rule" to satisfy all use cases.

We are glad to hear suggestions wrt how to make that use case better w/o adding attrs to xx_image rules (e.g., if you have ideas as to how to combine better the base container_image with the xx_image, say, via support from macros or something like that, lets talk about that).

Also, imo, in the use case you illustrate above, it makes sense to have a single container_image rule set the platform properties (including the selection of the default base) and have the go_image rules (several of them, hopefully) depend on this single image. That seems to me cleaner than having the docker_run_flags and the DEFAULT_BASE spread out over every single go_image rule that I'm creating (but maybe its because I'm expecting reuse from these targets, which might not be your case).