From bb9d0fdd518cde03c347f012be54779f5de7f4e8 Mon Sep 17 00:00:00 2001 From: Ricardo Boni Date: Tue, 25 Aug 2026 23:14:55 -0400 Subject: [PATCH] fix(installer): accept a TestGen image that is already local MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tg install --image` refused any image built locally: The Docker engine could not access TestGen's image. The requirement ran `docker manifest inspect`, which asks the registry. That answers "is this image published?", but what the install needs to know is "can Docker obtain this image?" — and one already in the local engine needs no pull at all. So a locally built image could never satisfy the check, which makes testing a dev build of TestGen impossible. Requirement gains an optional alt_cmd, tried when cmd fails, and the image check now asks the local engine first and falls back to the registry. Local first because it is instant, and because it lets an install proceed from cache when the registry is unreachable. This keeps the diagnostic the check was added for: a name that is neither local nor pullable still fails with the same message. An opt-out flag would not have — it disables the check wholesale, so a user hitting a genuine registry problem and reaching for it would lose exactly the signal they need. Co-Authored-By: Claude Opus 5 (1M context) --- dk-installer.py | 41 ++++++++++++-------- tests/test_requirement.py | 78 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 15 deletions(-) create mode 100644 tests/test_requirement.py diff --git a/dk-installer.py b/dk-installer.py index 02f2ba5..cadb9ca 100755 --- a/dk-installer.py +++ b/dk-installer.py @@ -480,22 +480,27 @@ class Requirement: cmd: tuple[typing.Union[str, pathlib.Path], ...] fail_msg: tuple[str, ...] label: typing.Optional[str] = None + #: Second way of satisfying the same requirement, tried when ``cmd`` fails. + alt_cmd: typing.Optional[tuple[typing.Union[str, pathlib.Path], ...]] = None def check_availability(self, action, args, quiet=False): - try: - action.run_cmd_retries( - *(seg.format(**args.__dict__) for seg in self.cmd), - timeout=REQ_CHECK_TIMEOUT, - retries=1, - ) - except CommandFailed: - if not quiet: - CONSOLE.space() - for line in self.fail_msg: - CONSOLE.msg(line.format(**args.__dict__)) - return False - else: - return True + for cmd in (c for c in (self.cmd, self.alt_cmd) if c is not None): + try: + action.run_cmd_retries( + *(seg.format(**args.__dict__) for seg in cmd), + timeout=REQ_CHECK_TIMEOUT, + retries=1, + ) + except CommandFailed: + continue + else: + return True + + if not quiet: + CONSOLE.space() + for line in self.fail_msg: + CONSOLE.msg(line.format(**args.__dict__)) + return False class CommandFailed(Exception): @@ -1195,12 +1200,18 @@ def run(self, parent=None): ) REQ_TESTGEN_IMAGE = Requirement( "TESTGEN_IMAGE", - ("docker", "manifest", "inspect", "{image}"), + # An image already in the local engine needs no pull, so it satisfies this outright -- + # `docker manifest inspect` alone would reject a locally built one. Checked first because + # it is instant, and because it lets an install proceed from cache when the registry is + # unreachable. A name that is neither local nor in a registry still fails, which is the + # signal this requirement exists to give. + ("docker", "image", "inspect", "{image}"), ( "The Docker engine could not access TestGen's image.", "Make sure your networking policy allows Docker to pull the {image} image.", ), label="TestGen image reachable", + alt_cmd=("docker", "manifest", "inspect", "{image}"), ) diff --git a/tests/test_requirement.py b/tests/test_requirement.py new file mode 100644 index 0000000..3bb3ed4 --- /dev/null +++ b/tests/test_requirement.py @@ -0,0 +1,78 @@ +from argparse import Namespace +from unittest.mock import MagicMock + +import pytest + +from tests.installer import REQ_TESTGEN_IMAGE, CommandFailed, Requirement + + +def make_action(failing_cmds): + """An action whose run_cmd_retries fails for any command whose joined form + contains one of *failing_cmds*.""" + action = MagicMock() + + def run(*cmd, **kwargs): + joined = " ".join(str(c) for c in cmd) + if any(bad in joined for bad in failing_cmds): + raise CommandFailed() + + action.run_cmd_retries.side_effect = run + return action + + +@pytest.mark.unit +def test_requirement_passes_on_primary_without_running_alt(): + req = Requirement("K", ("true", "primary"), ("nope",), alt_cmd=("true", "fallback")) + action = make_action([]) + + assert req.check_availability(action, Namespace()) is True + # The fallback costs a round trip, so it must not run when the primary already passed. + assert "fallback" not in str(action.run_cmd_retries.call_args_list) + + +@pytest.mark.unit +def test_requirement_falls_through_to_alt_cmd(): + req = Requirement("K", ("true", "primary"), ("nope",), alt_cmd=("true", "fallback")) + + assert req.check_availability(make_action(["primary"]), Namespace()) is True + + +@pytest.mark.unit +def test_requirement_fails_when_neither_command_works(console_msg_mock): + req = Requirement("K", ("true", "primary"), ("it broke",), alt_cmd=("true", "fallback")) + + assert req.check_availability(make_action(["primary", "fallback"]), Namespace()) is False + console_msg_mock.assert_any_msg_contains("it broke") + + +@pytest.mark.unit +def test_requirement_without_alt_cmd_still_fails_cleanly(console_msg_mock): + """The alt_cmd default must not change how single-command requirements behave.""" + req = Requirement("K", ("true", "primary"), ("it broke",)) + + assert req.check_availability(make_action(["primary"]), Namespace()) is False + console_msg_mock.assert_any_msg_contains("it broke") + + +@pytest.mark.unit +@pytest.mark.parametrize( + "available, expected", + ( + ("local", True), # built locally, absent from any registry + ("registry", True), # normal install: pullable, not yet local + ("neither", False), # a typo'd name or an unreachable registry + ), +) +def test_testgen_image_accepts_local_or_registry(available, expected): + """A locally built image needs no pull, so it satisfies the check. A name that is + neither local nor pullable still fails -- that is the signal the check exists for.""" + failing = { + "local": ["manifest inspect"], + "registry": ["image inspect"], + "neither": ["manifest inspect", "image inspect"], + }[available] + + action = make_action(failing) + args = Namespace(image="dataops-testgen-local:dev") + + assert REQ_TESTGEN_IMAGE.check_availability(action, args, quiet=True) is expected