Refactor CI container deploy wrapper: add modes, rebuild/no-cache flags, and transparent inventory/deploy arg forwarding; update tests accordingly (see ChatGPT conversation: https://chatgpt.com/share/6931e58c-2ddc-800f-88d2-f7887ec13e25)

cli/deploy/container.py

@@ -5,13 +5,30 @@ import subprocess
 import sys
 import time
 import uuid
-from typing import List
+from typing import List, Tuple
 
 
-def ensure_image(image: str) -> None:
+WORKDIR_DEFAULT = "/opt/infinito-src"
+
+
+def ensure_image(image: str, rebuild: bool = False, no_cache: bool = False) -> None:
     """
-    Ensure the Docker image exists locally. If not, build it with docker build.
+    Handle Docker image creation rules:
+      - rebuild=True  => always rebuild
+      - rebuild=False & image missing => build once
+      - no_cache=True => add '--no-cache' to docker build
     """
+    build_args = ["docker", "build", "--network=host", "--pull"]
+    if no_cache:
+        build_args.append("--no-cache")
+    build_args += ["-t", image, "."]
+
+    if rebuild:
+        print(f">>> Forcing rebuild of Docker image '{image}'...")
+        subprocess.run(build_args, check=True)
+        print(f">>> Docker image '{image}' rebuilt (forced).")
+        return
+
     print(f">>> Checking if Docker image '{image}' exists...")
     result = subprocess.run(
         ["docker", "image", "inspect", image],
@@ -23,14 +40,16 @@ def ensure_image(image: str) -> None:
         return
 
     print(f">>> Docker image '{image}' not found. Building it...")
-    subprocess.run(
-        ["docker", "build", "--network=host", "--pull", "-t", image, "."],
-        check=True,
-    )
+    subprocess.run(build_args, check=True)
     print(f">>> Docker image '{image}' successfully built.")
 
 
-def docker_exec(container: str, args: List[str], workdir: str | None = None, check: bool = True) -> subprocess.CompletedProcess:
+def docker_exec(
+    container: str,
+    args: List[str],
+    workdir: str | None = None,
+    check: bool = True,
+) -> subprocess.CompletedProcess:
     """
     Helper to run `docker exec` with optional working directory.
     """
@@ -48,7 +67,7 @@ def wait_for_inner_docker(container: str, timeout: int = 60) -> None:
     Poll `docker exec <container> docker info` until inner dockerd is ready.
     """
     print(">>> Waiting for inner Docker daemon inside CI container...")
-    for i in range(timeout):
+    for _ in range(timeout):
         result = subprocess.run(
             ["docker", "exec", container, "docker", "info"],
             stdout=subprocess.DEVNULL,
@@ -62,72 +81,97 @@ def wait_for_inner_docker(container: str, timeout: int = 60) -> None:
     raise RuntimeError("Inner Docker daemon did not become ready in time")
 
 
+def start_ci_container(
+    image: str,
+    build: bool,
+    rebuild: bool,
+    no_cache: bool,
+    name: str | None = None,
+) -> str:
+    """
+    Start a CI container running dockerd inside.
+
+    Returns the container name.
+    """
+    if build or rebuild:
+        ensure_image(image, rebuild=rebuild, no_cache=no_cache)
+
+    if not name:
+        name = f"infinito-ci-{uuid.uuid4().hex[:8]}"
+
+    print(f">>> Starting CI container '{name}' with inner dockerd...")
+    subprocess.run(
+        [
+            "docker",
+            "run",
+            "-d",
+            "--name",
+            name,
+            "--network=host",
+            "--privileged",
+            "--cgroupns=host",
+            image,
+            "dockerd",
+            "--debug",
+            "--host=unix:///var/run/docker.sock",
+            "--storage-driver=vfs",
+        ],
+        check=True,
+    )
+
+    wait_for_inner_docker(name)
+    print(f">>> CI container '{name}' started and inner dockerd is ready.")
+    return name
+
+
 def run_in_container(
     image: str,
-    exclude: str,
-    forwarded_args: List[str],
     build: bool,
+    rebuild: bool,
+    no_cache: bool,
+    inventory_args: List[str],
+    deploy_args: List[str],
+    name: str | None = None,
 ) -> None:
     """
-    Orchestrate everything from the *host*:
-      - start CI container with inner dockerd
-      - wait for inner docker
-      - create inventory (cli.create.inventory)
-      - ensure vault password file
-      - run cli.deploy.dedicated
-    All heavy lifting inside the container happens via direct `docker exec` calls.
+    Full CI "run" mode:
+      - start CI container with dockerd
+      - run cli.create.inventory (with forwarded inventory_args)
+      - ensure CI vault password file
+      - run cli.deploy.dedicated (with forwarded deploy_args)
+      - always remove container at the end
     """
-    if build:
-        ensure_image(image)
-
-    container_name = f"infinito-ci-{uuid.uuid4().hex[:8]}"
-    workdir = "/opt/infinito-src"
-
+    container_name = None
     try:
-        # 1) Start CI container with dockerd as PID 1
-        print(f">>> Starting CI container '{container_name}' with inner dockerd...")
-        subprocess.run(
-            [
-                "docker",
-                "run",
-                "-d",
-                "--name",
-                container_name,
-                "--network=host",
-                "--privileged",
-                "--cgroupns=host",
-                image,
-                "dockerd",
-                "--debug",
-                "--host=unix:///var/run/docker.sock",
-                "--storage-driver=vfs",
-            ],
-            check=True,
+        container_name = start_ci_container(
+            image=image,
+            build=build,
+            rebuild=rebuild,
+            no_cache=no_cache,
+            name=name,
         )
 
-        # 2) Wait until inner docker responds
-        wait_for_inner_docker(container_name)
-
-        # 3) Create CI inventory via Python module
+        # 1) Create CI inventory
         print(">>> Creating CI inventory inside container (cli.create.inventory)...")
+        inventory_cmd: List[str] = [
+            "python3",
+            "-m",
+            "cli.create.inventory",
+            "inventories/github-ci",
+            "--host",
+            "localhost",
+            "--ssl-disabled",
+        ]
+        inventory_cmd.extend(inventory_args)
+
         docker_exec(
             container_name,
-            [
-                "python3",
-                "-m",
-                "cli.create.inventory",
-                "inventories/github-ci",
-                "--host",
-                "localhost",
-                "--exclude",
-                exclude,
-                "--ssl-disabled",
-            ],
-            workdir=workdir,
+            inventory_cmd,
+            workdir=WORKDIR_DEFAULT,
             check=True,
         )
 
-        # 4) Ensure vault password file exists
+        # 2) Ensure vault password file exists
         print(">>> Ensuring CI vault password file exists...")
         docker_exec(
             container_name,
@@ -138,11 +182,11 @@ def run_in_container(
                 "[ -f inventories/github-ci/.password ] || "
                 "printf '%s\n' 'ci-vault-password' > inventories/github-ci/.password",
             ],
-            workdir=workdir,
+            workdir=WORKDIR_DEFAULT,
             check=True,
         )
 
-        # 5) Run dedicated deploy
+        # 3) Run dedicated deploy
         print(">>> Running cli.deploy.dedicated inside CI container...")
         cmd = [
             "python3",
@@ -151,96 +195,179 @@ def run_in_container(
             "inventories/github-ci/servers.yml",
             "-p",
             "inventories/github-ci/.password",
-            *forwarded_args,
+            *deploy_args,
         ]
-        result = docker_exec(container_name, cmd, workdir=workdir, check=False)
+        result = docker_exec(container_name, cmd, workdir=WORKDIR_DEFAULT, check=False)
 
         if result.returncode != 0:
-            raise subprocess.CalledProcessError(
-                result.returncode, cmd
-            )
+            raise subprocess.CalledProcessError(result.returncode, cmd)
 
         print(">>> Deployment finished successfully inside CI container.")
 
     finally:
-        print(f">>> Cleaning up CI container '{container_name}'...")
-        subprocess.run(
-            ["docker", "rm", "-f", container_name],
-            stdout=subprocess.DEVNULL,
-            stderr=subprocess.DEVNULL,
-        )
+        if container_name:
+            print(f">>> Cleaning up CI container '{container_name}'...")
+            subprocess.run(
+                ["docker", "rm", "-f", container_name],
+                stdout=subprocess.DEVNULL,
+                stderr=subprocess.DEVNULL,
+            )
+
+
+def stop_container(name: str) -> None:
+    print(f">>> Stopping container '{name}'...")
+    subprocess.run(["docker", "stop", name], check=True)
+    print(f">>> Container '{name}' stopped.")
+
+
+def remove_container(name: str) -> None:
+    print(f">>> Removing container '{name}'...")
+    subprocess.run(["docker", "rm", "-f", name], check=True)
+    print(f">>> Container '{name}' removed.")
+
+
+def exec_in_container(name: str, cmd_args: List[str], workdir: str | None = WORKDIR_DEFAULT) -> int:
+    if not cmd_args:
+        print("Error: exec mode requires a command to run inside the container.", file=sys.stderr)
+        return 1
+
+    print(f">>> Executing command in container '{name}': {' '.join(cmd_args)}")
+    result = docker_exec(name, cmd_args, workdir=workdir, check=False)
+    return result.returncode
+
+
+def split_inventory_and_deploy_args(rest: List[str]) -> Tuple[List[str], List[str]]:
+    """
+    Split remaining arguments into:
+      - inventory_args: passed to cli.create.inventory
+      - deploy_args:    passed to cli.deploy.dedicated
+
+    Convention:
+      - [inventory-args ...] -- [deploy-args ...]
+      - If no '--' is present: inventory_args = [], deploy_args = all rest.
+    """
+    if not rest:
+        return [], []
+
+    if "--" in rest:
+        idx = rest.index("--")
+        inventory_args = rest[:idx]
+        deploy_args = rest[idx + 1 :]
+    else:
+        inventory_args = []
+        deploy_args = rest
+
+    return inventory_args, deploy_args
 
 
 def main() -> int:
+    # Capture raw arguments without program name
+    raw_argv = sys.argv[1:]
+
+    # Split container-args vs forwarded args using first "--"
+    if "--" in raw_argv:
+        sep_index = raw_argv.index("--")
+        container_argv = raw_argv[:sep_index]
+        rest = raw_argv[sep_index + 1:]
+    else:
+        container_argv = raw_argv
+        rest = []
+
     parser = argparse.ArgumentParser(
         prog="infinito-deploy-container",
         description=(
-            "Run cli.deploy.dedicated inside an infinito Docker image with an inner "
-            "Docker daemon (dockerd + vfs) and auto-generated CI inventory."
-        ),
-    )
-
-    parser.add_argument(
-        "--image",
-        default=os.environ.get("INFINITO_IMAGE", "infinito:latest"),
-        help="Docker image to use (default: %(default)s, overridable via INFINITO_IMAGE).",
-    )
-
-    parser.add_argument(
-        "--exclude",
-        default=os.environ.get("EXCLUDED_ROLES", ""),
-        help=(
-            "Comma-separated list of roles to exclude when creating the CI inventory "
-            "(default taken from EXCLUDED_ROLES env var)."
-        ),
-    )
-
-    parser.add_argument(
-        "--build",
-        action="store_true",
-        help="If set, ensure the Docker image exists by building it when missing.",
-    )
-
-    parser.add_argument(
-        "forwarded",
-        nargs=argparse.REMAINDER,
-        help=(
-            "Arguments to forward to cli.deploy.dedicated. "
-            "Use '--' to separate wrapper options from dedicated options."
-        ),
-    )
-
-    args = parser.parse_args()
-
-    forwarded_args = list(args.forwarded)
-    if forwarded_args and forwarded_args[0] == "--":
-        forwarded_args = forwarded_args[1:]
-
-    if not forwarded_args:
-        print(
-            "Error: no arguments forwarded to dedicated deploy script.\n"
-            "Hint: use '--' to separate wrapper options from dedicated options, e.g.\n"
-            "  python -m cli.deploy.container --build -- -T server --debug --skip-tests",
-            file=sys.stderr,
+            "Run Ansible deploy inside an infinito Docker image with an inner "
+            "Docker daemon (dockerd + vfs) and auto-generated CI inventory.\n\n"
+            "Usage (run mode):\n"
+            "  python -m cli.deploy.container run [container-opts] -- \\\n"
+            "    [inventory-args ...] -- [deploy-args ...]\n\n"
+            "Example:\n"
+            "  python -m cli.deploy.container run --build -- \\\n"
+            "    --include svc-db-mariadb -- \\\n"
+            "    -T server --debug\n"
         )
+    )
+
+    parser.add_argument(
+        "mode",
+        choices=["run", "start", "stop", "exec", "remove"],
+        help="Container mode: run, start, stop, exec, remove."
+    )
+
+    parser.add_argument("--image", default=os.environ.get("INFINITO_IMAGE", "infinito:latest"))
+    parser.add_argument("--build", action="store_true")
+    parser.add_argument("--rebuild", action="store_true")
+    parser.add_argument("--no-cache", action="store_true")
+    parser.add_argument("--name")
+
+    # Parse only container-level arguments
+    args = parser.parse_args(container_argv)
+
+    mode = args.mode
+
+    # --- RUN MODE ---
+    if mode == "run":
+        inventory_args, deploy_args = split_inventory_and_deploy_args(rest)
+
+        if not deploy_args:
+            print(
+                "Error: missing deploy arguments in run mode.\n"
+                "Use:  container run [opts] -- [inventory args] -- [deploy args]",
+                file=sys.stderr
+            )
+            return 1
+
+        try:
+            run_in_container(
+                image=args.image,
+                build=args.build,
+                rebuild=args.rebuild,
+                no_cache=args.no_cache,
+                inventory_args=inventory_args,
+                deploy_args=deploy_args,
+                name=args.name,
+            )
+        except subprocess.CalledProcessError as exc:
+            print(f"[ERROR] Deploy failed with exit code {exc.returncode}", file=sys.stderr)
+            return exc.returncode
+
+        return 0
+
+    # --- START MODE ---
+    if mode == "start":
+        try:
+            name = start_ci_container(
+                image=args.image,
+                build=args.build,
+                rebuild=args.rebuild,
+                no_cache=args.no_cache,
+                name=args.name,
+            )
+        except Exception as exc:
+            print(f"[ERROR] {exc}", file=sys.stderr)
+            return 1
+
+        print(f">>> Started CI container: {name}")
+        return 0
+
+    # For stop/remove/exec, a container name is mandatory
+    if not args.name:
+        print(f"Error: '{mode}' requires --name", file=sys.stderr)
         return 1
 
-    try:
-        run_in_container(
-            image=args.image,
-            exclude=args.exclude,
-            forwarded_args=forwarded_args,
-            build=args.build,
-        )
-    except subprocess.CalledProcessError as exc:
-        print(f"[ERROR] Container run failed with exit code {exc.returncode}", file=sys.stderr)
-        return exc.returncode
-    except RuntimeError as exc:
-        print(f"[ERROR] {exc}", file=sys.stderr)
-        return 1
+    if mode == "stop":
+        stop_container(args.name)
+        return 0
 
-    return 0
+    if mode == "remove":
+        remove_container(args.name)
+        return 0
 
+    if mode == "exec":
+        return exec_in_container(args.name, rest)
+
+    print(f"Unknown mode: {mode}", file=sys.stderr)
+    return 1
 
 if __name__ == "__main__":
     raise SystemExit(main())

tests/unit/cli/deploy/test_container.py

@@ -11,22 +11,26 @@ from cli.deploy import container as deploy_container
 class TestEnsureImage(unittest.TestCase):
     @mock.patch("subprocess.run")
     def test_ensure_image_skips_build_when_image_exists(self, mock_run):
-        # docker image inspect → returncode 0 means "image exists"
+        """
+        If the image already exists, ensure_image should only call
+        `docker image inspect` and NOT run `docker build`.
+        """
+        # docker image inspect → rc=0 (image exists)
         mock_run.return_value = subprocess.CompletedProcess(
-            ["docker", "image", "inspect", "infinito:latest"],
-            0,
-            stdout=b"",
-            stderr=b"",
+            args=["docker", "image", "inspect", "infinito:latest"],
+            returncode=0,
+            stdout="",
+            stderr="",
         )
 
         deploy_container.ensure_image("infinito:latest")
 
-        # Only one call: docker image inspect
+        # Exactly one call: docker image inspect
         self.assertEqual(mock_run.call_count, 1)
         cmd = mock_run.call_args_list[0].args[0]
         self.assertEqual(cmd[:3], ["docker", "image", "inspect"])
 
-        # No docker build command
+        # Ensure docker build was never called
         self.assertFalse(
             any(
                 call.args[0][:2] == ["docker", "build"]
@@ -37,17 +41,40 @@ class TestEnsureImage(unittest.TestCase):
 
     @mock.patch("subprocess.run")
     def test_ensure_image_builds_when_missing(self, mock_run):
+        """
+        If the image does not exist, ensure_image should call
+        `docker image inspect` first and then `docker build`.
+        """
         calls: List[List[str]] = []
 
         def _side_effect(cmd, *args, **kwargs):
             calls.append(cmd)
-            # First inspect fails → triggers build
+
+            # First: docker image inspect → rc=1 (missing)
             if cmd[:3] == ["docker", "image", "inspect"]:
-                return subprocess.CompletedProcess(cmd, 1, b"", b"missing")
-            # Build succeeds
+                return subprocess.CompletedProcess(
+                    args=cmd,
+                    returncode=1,
+                    stdout="",
+                    stderr="missing",
+                )
+
+            # Then: docker build → rc=0 (success)
             if cmd[:2] == ["docker", "build"]:
-                return subprocess.CompletedProcess(cmd, 0, b"", b"")
-            return subprocess.CompletedProcess(cmd, 0, b"", b"")
+                return subprocess.CompletedProcess(
+                    args=cmd,
+                    returncode=0,
+                    stdout="",
+                    stderr="",
+                )
+
+            # Any other commands (should not happen here)
+            return subprocess.CompletedProcess(
+                args=cmd,
+                returncode=0,
+                stdout="",
+                stderr="",
+            )
 
         mock_run.side_effect = _side_effect
 
@@ -55,44 +82,53 @@ class TestEnsureImage(unittest.TestCase):
 
         self.assertTrue(
             any(c[:3] == ["docker", "image", "inspect"] for c in calls),
-            "Expected docker image inspect to be called",
+            "Expected 'docker image inspect' to be called",
         )
         self.assertTrue(
             any(c[:2] == ["docker", "build"] for c in calls),
-            "Expected docker build to run when image is missing",
+            "Expected 'docker build' to run when image is missing",
         )
 
+
 class TestRunInContainer(unittest.TestCase):
     @mock.patch("subprocess.run")
     def test_run_in_container_calls_single_docker_run(self, mock_run):
         """
-        Current container.py starts exactly one Docker container via
-        'docker run' to launch the inner dockerd daemon. All further work
-        (inventory creation + dedicated deploy) happens via docker exec.
+        run_in_container should start exactly one 'docker run' CI container,
+        then use docker exec for everything else.
         """
         calls: List[List[str]] = []
 
         def _side_effect(cmd, *args, **kwargs):
             calls.append(cmd)
-            return subprocess.CompletedProcess(cmd, 0, b"", b"")
+            # Always succeed → wait_for_inner_docker stops on first call
+            return subprocess.CompletedProcess(
+                args=cmd,
+                returncode=0,
+                stdout="",
+                stderr="",
+            )
 
         mock_run.side_effect = _side_effect
 
         image = "infinito:latest"
-        exclude = "foo,bar"
-        forwarded = ["-T", "server", "--debug", "--skip-tests"]
+        inventory_args = ["--include", "svc-db-mariadb"]
+        deploy_args = ["-T", "server", "--debug", "--skip-tests"]
 
         deploy_container.run_in_container(
             image=image,
-            exclude=exclude,
-            forwarded_args=forwarded,
             build=False,
+            rebuild=False,
+            no_cache=False,
+            inventory_args=inventory_args,
+            deploy_args=deploy_args,
+            name=None,
         )
 
-        # We expect at least one subprocess.run call
+        # At least one command must have been executed
         self.assertGreaterEqual(len(calls), 1)
 
-        # Only count *docker run* invocations, not docker exec / image / build
+        # Filter all docker run invocations
         docker_run_calls = [
             c
             for c in calls
@@ -105,16 +141,12 @@ class TestRunInContainer(unittest.TestCase):
         self.assertEqual(
             len(docker_run_calls),
             1,
-            "Expected exactly one 'docker run' call",
+            "Expected exactly one 'docker run' call (for the CI container)",
         )
 
         cmd = docker_run_calls[0]
-
-        # Basic structure of the docker run call
         self.assertEqual(cmd[0], "docker")
         self.assertEqual(cmd[1], "run")
-
-        # New behavior: container runs dockerd in detached mode
         self.assertIn("-d", cmd)
         self.assertIn("--name", cmd)
         self.assertIn("--network=host", cmd)
@@ -123,17 +155,23 @@ class TestRunInContainer(unittest.TestCase):
         self.assertIn(image, cmd)
         self.assertIn("dockerd", cmd)
 
-        # We *no longer* expect '--rm', '/bin/sh', or '-lc' here,
-        # because all higher-level logic is executed by docker exec.
-
 
 class TestMain(unittest.TestCase):
     @mock.patch("cli.deploy.container.run_in_container")
     def test_main_requires_forwarded_args(self, mock_run_in_container):
         """
-        If no arguments follow '--', main() must return exit code 1 and not call run_in_container().
+        In 'run' mode, main() must return 1 and not call run_in_container()
+        if no deploy arguments are provided after the first '--'.
         """
-        argv = ["cli.deploy.container"]  # realistic argv from `python -m ...`
+        argv = [
+            "cli.deploy.container",
+            "run",
+            "--image",
+            "myimage:latest",
+            "--build",
+            "--",
+            # no inventory/deploy args here
+        ]
         with mock.patch.object(sys, "argv", argv):
             rc = deploy_container.main()
 
@@ -143,15 +181,21 @@ class TestMain(unittest.TestCase):
     @mock.patch("cli.deploy.container.run_in_container")
     def test_main_passes_arguments_to_run_in_container(self, mock_run_in_container):
         """
-        Ensure CLI args are parsed and forwarded correctly.
+        Ensure that main() correctly splits container args vs inventory/deploy
+        args and passes them to run_in_container().
         """
         argv = [
             "cli.deploy.container",
-            "--image", "myimage:latest",
-            "--exclude", "foo,bar",
+            "run",
+            "--image",
+            "myimage:latest",
             "--build",
             "--",
-            "-T", "server",
+            "--exclude",
+            "foo,bar",
+            "--",
+            "-T",
+            "server",
             "--debug",
         ]
 
@@ -161,14 +205,24 @@ class TestMain(unittest.TestCase):
         self.assertEqual(rc, 0)
         mock_run_in_container.assert_called_once()
 
-        # access kwargs passed to run_in_container
         kwargs = mock_run_in_container.call_args.kwargs
 
+        # Container-level options
         self.assertEqual(kwargs["image"], "myimage:latest")
-        self.assertEqual(kwargs["exclude"], "foo,bar")  # <-- fixed key
         self.assertTrue(kwargs["build"])
+        self.assertFalse(kwargs["rebuild"])
+        self.assertFalse(kwargs["no_cache"])
+        self.assertIsNone(kwargs["name"])
+
+        # Inventory args: first segment after first '--' until second '--'
         self.assertEqual(
-            kwargs["forwarded_args"],
+            kwargs["inventory_args"],
+            ["--exclude", "foo,bar"],
+        )
+
+        # Deploy args: everything after the second '--'
+        self.assertEqual(
+            kwargs["deploy_args"],
             ["-T", "server", "--debug"],
         )