Repository: ansible/ansible-builder

Issue:
feature request: multiple tags on build
Currently specifying multiple tags like so: `ansible-builder build -t tag1 -t tag2`, results in `docker build -f context/Dockerfile -t tag2 context`

The underlying tools support multiple tags so it would be nice if specifying multiple tags with `ansible-builder` was possible as well. 

Discussion:

Patch:
diff --git a/ansible_builder/cli.py b/ansible_builder/cli.py
index 349fe7b..4ea6dfb 100644
--- a/ansible_builder/cli.py
+++ b/ansible_builder/cli.py
@@ -104,10 +104,15 @@ def add_container_options(parser):
         )
     )
 
+    # Because of the way argparse works, if we specify the default here, it would
+    # always be included in the value list if a tag value was supplied. We don't want
+    # that, so we must, instead, set the default AFTER the argparse.parse_args() call.
+    # See https://bugs.python.org/issue16399 for more info.
     build_command_parser.add_argument(
         '-t', '--tag',
-        default=constants.default_tag,
-        help='The name for the container image being built (default: %(default)s)')
+        action='extend',
+        nargs='+',
+        help=f'The name(s) for the container image being built (default: {constants.default_tag})')
 
     build_command_parser.add_argument(
         '--container-runtime',
@@ -229,6 +234,11 @@ def parse_args(args=sys.argv[1:]):
     add_container_options(container_parser)
 
     args = parser.parse_args(args)
+
+    # Tag default must be handled differently. See comment for --tag option.
+    if 'tag' not in vars(args):
+        args.tag = [constants.default_tag]
+
     return args
 
 
diff --git a/ansible_builder/main.py b/ansible_builder/main.py
index 0b4dd6b..72b71e1 100644
--- a/ansible_builder/main.py
+++ b/ansible_builder/main.py
@@ -36,7 +36,7 @@ class AnsibleBuilder:
                  filename=constants.default_file,
                  build_args=None,
                  build_context=constants.default_build_context,
-                 tag=constants.default_tag,
+                 tag=None,
                  container_runtime=constants.default_container_runtime,
                  output_filename=None,
                  no_cache=False,
@@ -44,7 +44,7 @@ class AnsibleBuilder:
         self.action = action
         self.definition = UserDefinition(filename=filename)
 
-        self.tag = tag
+        self.tags = tag or []
         self.build_context = build_context
         self.build_outputs_dir = os.path.join(
             build_context, constants.user_content_subfolder)
@@ -55,8 +55,7 @@ class AnsibleBuilder:
             definition=self.definition,
             build_context=self.build_context,
             container_runtime=self.container_runtime,
-            output_filename=output_filename,
-            tag=self.tag)
+            output_filename=output_filename)
         self.verbosity = verbosity
 
     @property
@@ -99,10 +98,12 @@ class AnsibleBuilder:
     def build_command(self):
         command = [
             self.container_runtime, "build",
-            "-f", self.containerfile.path,
-            "-t", self.tag,
+            "-f", self.containerfile.path
         ]
 
+        for tag in self.tags:
+            command.extend(["-t", tag])
+
         for key, value in self.build_args.items():
             if value:
                 build_arg = f"--build-arg={key}={value}"
@@ -119,7 +120,7 @@ class AnsibleBuilder:
         return command
 
     def build(self):
-        logger.debug(f'Ansible Builder is building your execution environment image, "{self.tag}".')
+        logger.debug(f'Ansible Builder is building your execution environment image. Tags: {", ".join(self.tags)}')
         self.write_containerfile()
         run_command(self.build_command)
         return True
@@ -294,8 +295,7 @@ class Containerfile:
     def __init__(self, definition,
                  build_context=None,
                  container_runtime=None,
-                 output_filename=None,
-                 tag=None):
+                 output_filename=None):
 
         self.build_context = build_context
         self.build_outputs_dir = os.path.join(
@@ -307,7 +307,6 @@ class Containerfile:
             filename = output_filename
         self.path = os.path.join(self.build_context, filename)
         self.container_runtime = container_runtime
-        self.tag = tag
         # Build args all need to go at top of file to avoid errors
         self.steps = [
             "ARG EE_BASE_IMAGE={}".format(
diff --git a/docs/usage.rst b/docs/usage.rst
index 8d0b7ce..16d6664 100644
--- a/docs/usage.rst
+++ b/docs/usage.rst
@@ -59,6 +59,11 @@ To customize the tagged name applied to the built image:
 
    $ ansible-builder container build --tag=my-custom-ee
 
+More recent versions of ``ansible-builder`` support multiple tags:
+
+.. code::
+
+   $ ansible-builder container build --tag=tag1 --tag=tag2
 
 ``--file``
 **********


Test patch:
diff --git a/test/integration/test_build.py b/test/integration/test_build.py
index 704139b..6202137 100644
--- a/test/integration/test_build.py
+++ b/test/integration/test_build.py
@@ -1,6 +1,9 @@
 import pytest
 import os
 
+# Need to call this directly for multiple tag testing
+from test.integration.conftest import delete_image
+
 
 @pytest.mark.test_all_runtimes
 def test_build_fail_exitcode(cli, runtime, ee_tag, tmp_path, data_dir):
@@ -21,7 +24,7 @@ def test_build_fail_exitcode(cli, runtime, ee_tag, tmp_path, data_dir):
 
 @pytest.mark.test_all_runtimes
 def test_blank_execution_environment(cli, runtime, ee_tag, tmp_path, data_dir):
-    """Just makes sure that the buld process does not require any particular input"""
+    """Just makes sure that the build process does not require any particular input"""
     bc = tmp_path
     ee_def = data_dir / 'blank' / 'execution-environment.yml'
     cli(
@@ -31,6 +34,22 @@ def test_blank_execution_environment(cli, runtime, ee_tag, tmp_path, data_dir):
     assert 'This is a simple test' in result.stdout, result.stdout
 
 
+@pytest.mark.test_all_runtimes
+def test_multiple_tags(cli, runtime, ee_tag, tmp_path, data_dir):
+    """Make sure multiple tagging works"""
+    bc = tmp_path
+    ee_def = data_dir / 'blank' / 'execution-environment.yml'
+    cli(
+        f'ansible-builder build -c {bc} -f {ee_def} -t {ee_tag} -t testmultitags --container-runtime {runtime}'
+    )
+    result = cli(f'{runtime} run --rm {ee_tag} echo "test: test_multiple_tags 1"')
+    assert 'test: test_multiple_tags 1' in result.stdout, result.stdout
+
+    result = cli(f'{runtime} run --rm testmultitags echo "test: test_multiple_tags 2"')
+    assert 'test: test_multiple_tags 2' in result.stdout, result.stdout
+    delete_image(runtime, 'testmultitags')
+
+
 @pytest.mark.test_all_runtimes
 def test_user_system_requirement(cli, runtime, ee_tag, tmp_path, data_dir):
     bc = tmp_path
diff --git a/test/integration/test_help.py b/test/integration/test_help.py
index 59b1467..6f40cb9 100644
--- a/test/integration/test_help.py
+++ b/test/integration/test_help.py
@@ -33,14 +33,14 @@ def test_container_help(cli):
 def test_container_build_help(cli):
     result = cli('ansible-builder container build --help', check=False)
     help_text = result.stdout
-    assert 'usage: ansible-builder container build [-h] [-t TAG]' in help_text
+    assert 'usage: ansible-builder container build [-h]' in help_text
     assert re.search(r'Creates a build context', help_text)
 
 
 def test_backward_compat_build_help(cli):
     result = cli('ansible-builder build --help', check=False)
     help_text = result.stdout
-    assert 'usage: ansible-builder container build [-h] [-t TAG]' in help_text
+    assert 'usage: ansible-builder container build [-h]' in help_text
     assert re.search(r'Creates a build context', help_text)
 
 
diff --git a/test/unit/test_cli.py b/test/unit/test_cli.py
index d252526..52cfb1e 100644
--- a/test/unit/test_cli.py
+++ b/test/unit/test_cli.py
@@ -55,3 +55,12 @@ def test_build_no_cache(good_exec_env_definition_path, tmp_path):
 
     assert '--no-cache' not in aee.build_command
     assert '--no-cache' in aee_no_cache.build_command
+
+
+def test_build_multiple_tags(exec_env_definition_file, tmp_path):
+    content = {'version': 1}
+    path = str(exec_env_definition_file(content=content))
+
+    # test with 'container' sub-command
+    aee = prepare(['container', 'build', '--tag', 'TAG1', '--tag', 'TAG2', '-f', path, '-c', str(tmp_path)])
+    assert aee.tags == ['TAG1', 'TAG2']