Fixes output_image_name in install_pkgs to be optional

Previously `output_image_name` needed to be unique across the entire build tree. When building many containers in parallel this could result in clashes between the containers without warning or error. This patches changes `output_image_name` to be optional, with the default value being set to the current target full label making it unique across large builds.
bazelbuild · Jan 18, 2023 · 6975f63 · 6975f63
1 parent aada190
commit 6975f63
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 6 deletions.
diff --git a/docker/package_managers/README.md b/docker/package_managers/README.md
@@ -453,9 +453,9 @@ within a container.
     <tr id="install_pkgs-output_image_name">
       <td><code>output_image_name</code></td>
       <td>
-        String; required
+        String; optional
         <p>
-          Name of container_image produced with the packages installed.
+          Name of container_image produced with the packages installed. By default the label of the target will be used. *Must be unique* across the entire build tree.
         </p>
       </td>
     </tr>

diff --git a/docker/package_managers/install_pkgs.bzl b/docker/package_managers/install_pkgs.bzl
@@ -66,13 +66,17 @@ def _impl(ctx, image_tar = None, installables_tar = None, installation_cleanup_c
       image_tar: File, overrides ctx.file.image_tar
       installables_tar: File, overrides ctx.file.installables_tar
       installation_cleanup_commands: str, overrides ctx.attr.installation_cleanup_commands
-      output_image_name: str, overrides ctx.attr.output_image_name
+      output_image_name: str, overrides the image name
       output_tar: File, overrides ctx.outputs.out
     """
+
+    # Construct the name of the image if not given based on the full label path with :output tag.
+    default_output_image_name = (ctx.label.package + "_" + ctx.label.name).replace(":", "_").replace("@", "_").replace("/", "_") + ":output"
+
     image_tar = image_tar or ctx.file.image_tar
     installables_tar = installables_tar or ctx.file.installables_tar
     installation_cleanup_commands = installation_cleanup_commands or ctx.attr.installation_cleanup_commands
-    output_image_name = output_image_name or ctx.attr.output_image_name
+    output_image_name = output_image_name or ctx.attr.output_image_name or default_output_image_name
     output_tar = output_tar or ctx.outputs.out
 
     installables_tar_path = installables_tar.path
@@ -167,8 +171,9 @@ _attrs = {
         default = "",
     ),
     "output_image_name": attr.string(
-        doc = ("Name of container_image produced with the packages installed."),
-        mandatory = True,
+        doc = ("Name of container_image produced with the packages installed. " +
+               "If not specified, resolves to the current target target label. " +
+               "*Must be unique* across the entire build tree."),
     ),
     "_config_stripper": attr.label(
         default = "//docker/util:config_stripper",

diff --git a/tests/docker/package_managers/BUILD b/tests/docker/package_managers/BUILD
@@ -101,6 +101,28 @@ rule_test(
     rule = "test_install_pkgs",
 )
 
+# Install without specifying output_image_name.
+[
+    install_pkgs(
+        name = "test_install_pkgs{}".format(n),
+        image_tar = "@ubuntu1604//:ubuntu1604_vanilla.tar",
+        installables_tar = ":test_download_pkgs.tar",
+    )
+    for n in range(2)
+]
+
+[
+    rule_test(
+        name = "test_install_pkgs{}_rule".format(n),
+        generates = [
+            "test_install_pkgs{}.build".format(n),
+            "test_install_pkgs{}.tar".format(n),
+        ],
+        rule = "test_install_pkgs{}".format(n),
+    )
+    for n in range(2)
+]
+
 add_apt_key(
     name = "gpg_image",
     image = "@debian9//:builder.tar",