diff --git a/container/image.bzl b/container/image.bzl index 99b6c5069..37ba26a4b 100644 --- a/container/image.bzl +++ b/container/image.bzl @@ -312,7 +312,6 @@ def _impl( workdir = None, null_cmd = None, null_entrypoint = None, - docker_run_flags = "", commit = False, commit_output = None, extract = False, @@ -351,7 +350,6 @@ def _impl( workdir: str, overrides ctx.attr.workdir null_cmd: bool, overrides ctx.attr.null_cmd null_entrypoint: bool, overrides ctx.attr.null_entrypoint - docker_run_flags: str List, flags to pass to docker when running the container commit: bool, whether to run the container and commit the result commit_output: File to use as output for the committed container extract: bool, whether to run the container and extract a file from it @@ -382,16 +380,17 @@ def _impl( # We do not use the default argument of attrs.string() in order to distinguish between # an image using the default and an image intentionally overriding the base's run flags. # Since this is a string attribute, the default value is the empty string. - if docker_run_flags == "": - if ctx.attr.docker_run_flags != "": - docker_run_flags = ctx.attr.docker_run_flags - elif ctx.attr.base and ImageInfo in ctx.attr.base: - docker_run_flags = ctx.attr.base[ImageInfo].docker_run_flags - else: - # Run the container using host networking, so that the service is - # available to the developer without having to poke around with - # docker inspect. - docker_run_flags = "-i --rm --network=host" + docker_run_flags_are_default = False + if ctx.attr.docker_run_flags != "": + docker_run_flags = ctx.attr.docker_run_flags + elif ctx.attr.base and ImageInfo in ctx.attr.base: + docker_run_flags = ctx.attr.base[ImageInfo].docker_run_flags + elif not commit and not extract: + docker_run_flags_are_default = True + # Run the container using host networking, so that the service is + # available to the developer without having to poke around with + # docker inspect. + docker_run_flags = "-i --rm --network=host" if ctx.attr.launcher: if not file_map: @@ -546,7 +545,7 @@ def _impl( ImageInfo( container_parts = container_parts, legacy_run_behavior = ctx.attr.legacy_run_behavior, - docker_run_flags = docker_run_flags, + docker_run_flags = "" if docker_run_flags_are_default else docker_run_flags, ), DefaultInfo( executable = build_executable, diff --git a/container/layer_tools.bzl b/container/layer_tools.bzl index 64024d00a..893146a5e 100644 --- a/container/layer_tools.bzl +++ b/container/layer_tools.bzl @@ -226,7 +226,11 @@ def incremental_load( # Default to interactively launching the container, # and cleaning up when it exits. - run_flags = run_flags or "-i --rm" + if not run_flags: + if commit or extract: + run_flags = "" + else: + run_flags = "-i --rm" if len(images) > 1 and run: fail("Bazel run does not currently support execution of " + @@ -289,6 +293,9 @@ def incremental_load( image["config_digest"].path if action_run else _get_runfile_path(ctx, image["config_digest"]), ), ] + if commit or extract: + if not "-d" in run_flags and not "--detach" in run_flags: + run_flags += " -d" if run or commit or extract: # Args are embedded into the image, so omitted here. run_statements += [ diff --git a/docker/util/run.bzl b/docker/util/run.bzl index ce9797f8a..ef147e4bc 100644 --- a/docker/util/run.bzl +++ b/docker/util/run.bzl @@ -24,9 +24,8 @@ load("//container:image.bzl", _image = "image") def _extract_impl( ctx, name = "", - image = None, - commands = None, - docker_run_flags = None, + base = None, + cmd = None, extract_file = "", output_file = "", script_file = "", @@ -40,9 +39,8 @@ def _extract_impl( Args: ctx: The bazel rule context name: String, overrides ctx.label.name - image: File, overrides ctx.file.image_tar - commands: String list, overrides ctx.attr.commands - docker_run_flags: String list, overrides ctx.attr.docker_run_flags + base: File, overrides ctx.attr.base + cmd: str List, overrides ctx.attr.cmd extract_file: File, overrides ctx.outputs.out output_file: File, overrides ctx.outputs.output_file script_file: File, overrides ctx.output.script_file @@ -52,9 +50,6 @@ def _extract_impl( by another action. """ name = name or ctx.label.name - image = image or ctx.file.image - commands = commands or ctx.attr.commands - docker_run_flags = docker_run_flags or ctx.attr.docker_run_flags extract_file = extract_file or ctx.attr.extract_file output_file = output_file or ctx.outputs.out script_file = script_file or ctx.outputs.script @@ -63,20 +58,16 @@ def _extract_impl( output_config = ctx.actions.declare_file(name + ".config") output_layer = ctx.actions.declare_file(name + ".layer") - if not "-d" in docker_run_flags: - docker_run_flags = docker_run_flags + ["-d"] - image_result = _image.implementation( ctx, name, - image, - cmd = _process_commands(commands), + base = base, + cmd = cmd, output_executable = script_file, output_tarball = output_tarball, output_digest = output_digest, output_config = output_config, output_layer = output_layer, - docker_run_flags = " ".join(docker_run_flags), extract = True, extract_path = extract_file, extract_output = output_file, @@ -86,7 +77,7 @@ def _extract_impl( ctx.actions.run( executable = script_file, tools = image_result[1].default_runfiles.files, - inputs = [image], + inputs = [base] if base else [], outputs = [output_file], use_default_shell_env = True, ) @@ -98,26 +89,10 @@ def _extract_impl( ] _extract_attrs = dicts.add(_image.attrs, { - "commands": attr.string_list( - doc = "A list of commands to run (sequentially) in the container.", - mandatory = True, - allow_empty = False, - ), - "docker_run_flags": attr.string_list( - doc = "Extra flags to pass to the docker run command.", - mandatory = False, - ), "extract_file": attr.string( doc = "Path to file to extract from container.", mandatory = True, ), - "image": attr.label( - executable = True, - doc = "The image to run the commands in.", - mandatory = True, - allow_single_file = True, - cfg = "target", - ), }) _extract_outputs = { @@ -145,9 +120,8 @@ container_run_and_extract = rule( def _commit_impl( ctx, name = None, - image = None, - commands = None, - docker_run_flags = None, + base = None, + cmd = None, output_image_tar = None): """Implementation for the container_run_and_commit rule. @@ -157,23 +131,16 @@ def _commit_impl( Args: ctx: The bazel rule context name: A unique name for this rule. - image: The input image tarball - commands: The commands to run in the input imnage container - docker_run_flags: String list, overrides ctx.attr.docker_run_flags + base: The input image + cmd: str List, overrides ctx.attr.cmd output_image_tar: The output image obtained as a result of running the commands on the input image """ name = name or ctx.attr.name - image = image or ctx.file.image - commands = commands or ctx.attr.commands - docker_run_flags = docker_run_flags or ctx.attr.docker_run_flags script = ctx.outputs.build output_image_tar = output_image_tar or ctx.outputs.out - if not "-d" in docker_run_flags: - docker_run_flags = docker_run_flags + ["-d"] - run_image = "%s.run" % name run_image_output_tarball = ctx.actions.declare_file("%s.tar" % run_image) run_image_output_layer = ctx.actions.declare_file("%s-layer.tar" % run_image) @@ -183,14 +150,13 @@ def _commit_impl( image_result = _image.implementation( ctx, name, - image, - cmd = _process_commands(commands), + base = base, + cmd = cmd, output_executable = script, output_tarball = run_image_output_tarball, output_layer = run_image_output_layer, output_digest = run_image_output_digest, output_config = run_image_output_config, - docker_run_flags = " ".join(docker_run_flags), commit = True, commit_output = output_image_tar, action_run = True, @@ -199,7 +165,7 @@ def _commit_impl( ctx.actions.run( executable = script, tools = image_result[1].default_runfiles.files, - inputs = [image], + inputs = [base] if base else [], outputs = [output_image_tar], use_default_shell_env = True, ) @@ -210,23 +176,7 @@ def _commit_impl( ), ] -_commit_attrs = dicts.add(_image.attrs, { - "commands": attr.string_list( - doc = "A list of commands to run (sequentially) in the container.", - mandatory = True, - allow_empty = False, - ), - "docker_run_flags": attr.string_list( - doc = "Extra flags to pass to the docker run command.", - mandatory = False, - ), - "image": attr.label( - doc = "The image to run the commands in.", - mandatory = True, - allow_single_file = True, - cfg = "target", - ), -}) +_commit_attrs = _image.attrs _commit_outputs = { "out": "%{name}_commit.tar", "build": "%{name}.build", @@ -249,7 +199,3 @@ commit = struct( outputs = _commit_outputs, implementation = _commit_impl, ) - -def _process_commands(command_list): - # Use the $ to allow escape characters in string - return ['sh', '-c', " && ".join(command_list)] diff --git a/tests/docker/util/BUILD b/tests/docker/util/BUILD index 61395f23a..84534bd72 100644 --- a/tests/docker/util/BUILD +++ b/tests/docker/util/BUILD @@ -33,16 +33,14 @@ load( container_run_and_extract( name = "test_container_extract", - commands = [ - "touch /foo.txt", - "echo 'test' > /foo.txt", - ], - docker_run_flags = [ - "-u", - "root", + cmd = [ + "bash", + "-c", + "touch /foo.txt && echo 'test' > /foo.txt", ], + docker_run_flags = "-u root", extract_file = "/foo.txt", - image = "@debian_base//image", + base = "@debian_base//image", ) rule_test( @@ -68,12 +66,9 @@ file_test( container_run_and_commit( name = "test_container_commit", - commands = ["touch /foo.txt"], - docker_run_flags = [ - "-u", - "root", - ], - image = "@debian_base//image", + cmd = ["touch", "/foo.txt"], + docker_run_flags = "-u root", + base = "@debian_base//image", ) rule_test(