From d0aac64c676ce3649cca98a8181a27ab1b860d96 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Thu, 4 Dec 2025 20:49:04 +0100 Subject: [PATCH] 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 | 395 ++++++++++++++++-------- tests/unit/cli/deploy/test_container.py | 136 +++++--- 2 files changed, 356 insertions(+), 175 deletions(-) diff --git a/cli/deploy/container.py b/cli/deploy/container.py index 73302ac9..d7b7f35d 100644 --- a/cli/deploy/container.py +++ b/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 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()) diff --git a/tests/unit/cli/deploy/test_container.py b/tests/unit/cli/deploy/test_container.py index da312660..3f2febde 100644 --- a/tests/unit/cli/deploy/test_container.py +++ b/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"], )