From 3d943b57dcb41238549c8b16a0cf27bb0919bd70 Mon Sep 17 00:00:00 2001 From: Emil Simeonov Date: Fri, 19 Jun 2026 03:09:19 +0200 Subject: [PATCH] GRM-19: refactor: resolve_runner returns gitea_url, add --url CLI option, force remove improvements, code quality fixes - _resolve_runner now returns gitea_url from registry so disable/remove can reuse the URL stored at install time without requiring env vars. - Added --url option to install, disable, and remove CLI commands. - remove(force=True) no longer requires gitea_url or token. - Moved _parse_status outside the for loop in list_runners. - Updated all translations and tests to match. --- src/gitea_runner_manager/cli.py | 24 ++++++- src/gitea_runner_manager/i18n.py | 7 ++ src/gitea_runner_manager/runner_manager.py | 69 +++++++++++------- tests/unit/test_cli.py | 70 ++++++++++++++++++- tests/unit/test_runner_manager.py | 81 +++++++++++++++++++--- 5 files changed, 214 insertions(+), 37 deletions(-) diff --git a/src/gitea_runner_manager/cli.py b/src/gitea_runner_manager/cli.py index dfc6f37..227bf9f 100644 --- a/src/gitea_runner_manager/cli.py +++ b/src/gitea_runner_manager/cli.py @@ -76,6 +76,11 @@ def cli() -> None: default="docker", help=_("Gitea Runner deployment mode (default: docker)"), ) +@click.option( + "--url", + default=lambda: os.getenv("GITEA_URL", ""), + help=_("Gitea URL (env: GITEA_URL)"), +) @click.option( "--admin-token", "-a", @@ -101,6 +106,7 @@ def install( name: str | None, token: str | None, mode: str, + url: str, admin_token: str | None, integration_retries: int, ask_become_pass: bool, @@ -113,7 +119,7 @@ def install( key=key, name=name, token=token, - gitea_url=os.getenv("GITEA_URL", ""), + gitea_url=url, mode=mode, admin_token=admin_token, integration_retries=integration_retries, @@ -251,6 +257,11 @@ def enable( type=click.Choice(["docker", "binary"]), help=_("Override deployment mode from registry"), ) +@click.option( + "--url", + default=lambda: os.getenv("GITEA_URL", ""), + help=_("Gitea URL (env: GITEA_URL)"), +) @_handle_errors("Disable failed: {error}") def disable( runner_name: str, @@ -259,6 +270,7 @@ def disable( key: str | None, token: str | None, mode: str | None, + url: str, ask_become_pass: bool, ) -> None: manager = RunnerManager() @@ -268,7 +280,7 @@ def disable( user=user, key=key, token=token, - gitea_url=os.getenv("GITEA_URL", ""), + gitea_url=url, mode=mode, ask_become_pass=ask_become_pass, ) @@ -318,6 +330,11 @@ def status( type=click.Choice(["docker", "binary"]), help=_("Override deployment mode from registry"), ) +@click.option( + "--url", + default=lambda: os.getenv("GITEA_URL", ""), + help=_("Gitea URL (env: GITEA_URL)"), +) @click.option( "--force", "-f", @@ -332,6 +349,7 @@ def remove( key: str | None, token: str | None, mode: str | None, + url: str, force: bool, ask_become_pass: bool, ) -> None: @@ -342,7 +360,7 @@ def remove( user=user, key=key, token=token, - gitea_url=os.getenv("GITEA_URL", ""), + gitea_url=url, mode=mode, ask_become_pass=ask_become_pass, force=force, diff --git a/src/gitea_runner_manager/i18n.py b/src/gitea_runner_manager/i18n.py index e511481..83a8290 100644 --- a/src/gitea_runner_manager/i18n.py +++ b/src/gitea_runner_manager/i18n.py @@ -114,6 +114,13 @@ TRANSLATIONS: dict[str, dict[str, str]] = { "ru": "Токен регистрации (env: GITEA_REGISTRATION_TOKEN)", "zh": "注册令牌（环境变量: GITEA_REGISTRATION_TOKEN）", }, + "Gitea URL (env: GITEA_URL)": { + "en": "Gitea URL (env: GITEA_URL)", + "bg": "Gitea URL (env: GITEA_URL)", + "de": "Gitea-URL (env: GITEA_URL)", + "ru": "URL Gitea (env: GITEA_URL)", + "zh": "Gitea URL（环境变量: GITEA_URL）", + }, "Gitea Runner deployment mode (default: docker)": { "en": "Gitea Runner deployment mode (default: docker)", "bg": "Режим на разполагане на Gitea Runner (по подразбиране: docker)", diff --git a/src/gitea_runner_manager/runner_manager.py b/src/gitea_runner_manager/runner_manager.py index 4602667..aea5791 100644 --- a/src/gitea_runner_manager/runner_manager.py +++ b/src/gitea_runner_manager/runner_manager.py @@ -96,9 +96,15 @@ class RunnerManager: user: str | None = None, key: str | None = None, mode: str | None = None, - ) -> tuple[str, str, str | None, str]: - """Look up runner metadata from registry, applying CLI overrides.""" + ) -> tuple[str, str, str | None, str, str]: + """Look up runner metadata from registry, applying CLI overrides. + + Returns ``(host, user, key, mode, gitea_url)`` where *gitea_url* is + taken from the registry when available, allowing ``disable`` and + ``remove`` to reuse the value stored at install time. + """ info = self._registry.get(name) + actual_gitea_url = info.get("gitea_url", "") if info else "" if host and user: # Explicit connection details — bypass registry actual_host = host @@ -117,7 +123,7 @@ class RunnerManager: name=name, ) ) - return actual_host, actual_user, actual_key, actual_mode + return actual_host, actual_user, actual_key, actual_mode, actual_gitea_url def start( self, @@ -129,7 +135,9 @@ class RunnerManager: ask_become_pass: bool = False, ) -> None: """Start a runner instance on a remote host.""" - actual_host, actual_user, actual_key, actual_mode = self._resolve_runner(name, host, user, key, mode) + actual_host, actual_user, actual_key, actual_mode, _gitea_url = self._resolve_runner( + name, host, user, key, mode + ) with track_steps() as tracker: tracker.begin(_("Starting Gitea Runner {name} on {host}", name=name, host=actual_host)) extra_vars = f"runner_name={name} runner_mode={actual_mode}" @@ -148,7 +156,7 @@ class RunnerManager: ask_become_pass: bool = False, ) -> None: """Stop a runner instance on a remote host.""" - actual_host, actual_user, actual_key, _mode = self._resolve_runner(name, host, user, key) + actual_host, actual_user, actual_key, _mode, _gitea_url = self._resolve_runner(name, host, user, key) with track_steps() as tracker: tracker.begin(_("Stopping Gitea Runner {name} on {host}", name=name, host=actual_host)) extra_vars = f"runner_name={name}" @@ -167,7 +175,7 @@ class RunnerManager: ask_become_pass: bool = False, ) -> None: """Enable a runner instance to start on boot.""" - actual_host, actual_user, actual_key, _mode = self._resolve_runner(name, host, user, key) + actual_host, actual_user, actual_key, _mode, _gitea_url = self._resolve_runner(name, host, user, key) with track_steps() as tracker: tracker.begin(_("Enabling Gitea Runner {name} on {host}", name=name, host=actual_host)) extra_vars = f"runner_name={name}" @@ -191,15 +199,19 @@ class RunnerManager: ask_become_pass: bool = False, ) -> None: """Disable and deregister a runner instance.""" - if not gitea_url: + actual_host, actual_user, actual_key, actual_mode, registry_gitea_url = self._resolve_runner( + name, host, user, key, mode + ) + resolved_gitea_url = gitea_url or registry_gitea_url + if not resolved_gitea_url: raise AnsibleError(_("GITEA_URL must be set (or pass --url)")) - actual_host, actual_user, actual_key, actual_mode = self._resolve_runner(name, host, user, key, mode) if not token: raise AnsibleError(_("GITEA_REGISTRATION_TOKEN must be set (or pass --token)")) with track_steps() as tracker: tracker.begin(_("Disabling Gitea Runner {name} on {host}", name=name, host=actual_host)) extra_vars = ( - f"runner_name={name} registration_token={token} gitea_url={gitea_url} runner_mode={actual_mode}" + f"runner_name={name} registration_token={token}" + f" gitea_url={resolved_gitea_url} runner_mode={actual_mode}" ) cmd = self._build_cmd( "disable-runner.yml", actual_host, actual_user, extra_vars, actual_key, ask_become_pass @@ -219,7 +231,9 @@ class RunnerManager: ask_become_pass: bool = False, ) -> None: """Check the status of a runner instance.""" - actual_host, actual_user, actual_key, actual_mode = self._resolve_runner(name, host, user, key, mode) + actual_host, actual_user, actual_key, actual_mode, _gitea_url = self._resolve_runner( + name, host, user, key, mode + ) with track_steps() as tracker: tracker.begin(_("Checking status of Gitea Runner {name} on {host}", name=name, host=actual_host)) extra_vars = f"runner_name={name} runner_mode={actual_mode}" @@ -249,16 +263,22 @@ class RunnerManager: only remove the local registry entry. Use this when the remote host is already gone or unreachable. """ - if not gitea_url: - raise AnsibleError(_("GITEA_URL must be set (or pass --url)")) - actual_host, actual_user, actual_key, actual_mode = self._resolve_runner(name, host, user, key, mode) - if not token: - raise AnsibleError(_("GITEA_REGISTRATION_TOKEN must be set (or pass --token)")) + actual_host, actual_user, actual_key, actual_mode, registry_gitea_url = self._resolve_runner( + name, host, user, key, mode + ) + resolved_gitea_url = gitea_url or registry_gitea_url + resolved_token = token + if not force: + if not resolved_gitea_url: + raise AnsibleError(_("GITEA_URL must be set (or pass --url)")) + if not resolved_token: + raise AnsibleError(_("GITEA_REGISTRATION_TOKEN must be set (or pass --token)")) with track_steps() as tracker: tracker.begin(_("Removing Gitea Runner {name} from {host}", name=name, host=actual_host)) if not force: extra_vars = ( - f"runner_name={name} registration_token={token} gitea_url={gitea_url} runner_mode={actual_mode}" + f"runner_name={name} registration_token={resolved_token}" + f" gitea_url={resolved_gitea_url} runner_mode={actual_mode}" ) cmd = self._build_cmd( "remove-runner.yml", actual_host, actual_user, extra_vars, actual_key, ask_become_pass @@ -290,11 +310,6 @@ class RunnerManager: ) ) - def _parse_status(stdout: str) -> str: - ansible_noise = (" | CHANGED | ", " | FAILED | ", " | UNREACHABLE | ", "[WARNING]", "ssh:", ">>") - lines = [ln for ln in stdout.splitlines() if ln.strip() and not any(p in ln for p in ansible_noise)] - return lines[-1].strip() if lines else "unknown" - service_status = "unknown" if mode == "docker": # Try expected container name first, then host-based fallback for legacy installs. @@ -317,7 +332,7 @@ class RunnerManager: ) except Exception: continue - status = _parse_status(stdout) + status = self._parse_status(stdout) if status != "unknown": docker_map = {"running": "active", "exited": "inactive", "dead": "failed"} service_status = docker_map.get(status, "unknown") @@ -335,7 +350,7 @@ class RunnerManager: ask_become_pass=True, check=False, ) - service_status = _parse_status(stdout) + service_status = self._parse_status(stdout) except Exception: service_status = "unknown" else: @@ -350,7 +365,7 @@ class RunnerManager: ask_become_pass=True, check=False, ) - service_status = _parse_status(stdout) + service_status = self._parse_status(stdout) except Exception: service_status = "unknown" # Translate known status values @@ -368,6 +383,12 @@ class RunnerManager: ) return result + @staticmethod + def _parse_status(stdout: str) -> str: + ansible_noise = (" | CHANGED | ", " | FAILED | ", " | UNREACHABLE | ", "[WARNING]", "ssh:", ">>") + lines = [ln for ln in stdout.splitlines() if ln.strip() and not any(p in ln for p in ansible_noise)] + return lines[-1].strip() if lines else "unknown" + def _build_cmd( self, playbook_name: str, diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index b595904..7bc9ed0 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -70,6 +70,31 @@ class TestCLI: assert result.exit_code != 0 assert "GITEA_URL must be set" in result.output + @patch("gitea_runner_manager.cli.RunnerManager") + def test_install_with_url_flag(self, mock_manager_class: MagicMock) -> None: + mock_manager = MagicMock() + mock_manager_class.return_value = mock_manager + + with patch.dict("os.environ", {"GITEA_REGISTRATION_TOKEN": "tok"}, clear=True): + runner = CliRunner() + result = runner.invoke( + cli, + ["install", "host1", "--user", "ubuntu", "--token", "tok", "--url", "https://git.example.com"], + ) + assert result.exit_code == 0 + mock_manager.install.assert_called_once_with( + host="host1", + user="ubuntu", + key=None, + name=None, + token="tok", + gitea_url="https://git.example.com", + mode="docker", + admin_token=None, + integration_retries=3, + ask_become_pass=True, + ) + @patch("gitea_runner_manager.cli.RunnerManager") def test_install_missing_token(self, mock_manager_class: MagicMock) -> None: mock_manager = MagicMock() @@ -349,6 +374,26 @@ class TestCLI: assert result.exit_code != 0 assert "GITEA_URL must be set" in result.output + @patch("gitea_runner_manager.cli.RunnerManager") + def test_disable_with_url_flag(self, mock_manager_class: MagicMock) -> None: + mock_manager = MagicMock() + mock_manager_class.return_value = mock_manager + + with patch.dict("os.environ", {"GITEA_REGISTRATION_TOKEN": "tok"}, clear=True): + runner = CliRunner() + result = runner.invoke(cli, ["disable", "r1", "--token", "tok", "--url", "https://git.example.com"]) + assert result.exit_code == 0 + mock_manager.disable.assert_called_once_with( + name="r1", + host=None, + user=None, + key=None, + token="tok", + gitea_url="https://git.example.com", + mode=None, + ask_become_pass=True, + ) + @patch("gitea_runner_manager.cli.RunnerManager") def test_disable_error(self, mock_manager_class: MagicMock) -> None: mock_manager = MagicMock() @@ -395,8 +440,8 @@ class TestCLI: token="tok", gitea_url="https://git.example.com", mode=None, - ask_become_pass=True, force=False, + ask_become_pass=True, ) @patch("gitea_runner_manager.cli.RunnerManager") @@ -415,8 +460,8 @@ class TestCLI: token="tok", gitea_url="https://git.example.com", mode=None, - ask_become_pass=True, force=True, + ask_become_pass=True, ) @patch("gitea_runner_manager.cli.RunnerManager") @@ -485,6 +530,27 @@ class TestCLI: assert result.exit_code != 0 assert "GITEA_URL must be set" in result.output + @patch("gitea_runner_manager.cli.RunnerManager") + def test_remove_with_url_flag(self, mock_manager_class: MagicMock) -> None: + mock_manager = MagicMock() + mock_manager_class.return_value = mock_manager + + with patch.dict("os.environ", {"GITEA_REGISTRATION_TOKEN": "tok"}, clear=True): + runner = CliRunner() + result = runner.invoke(cli, ["remove", "r1", "--token", "tok", "--url", "https://git.example.com"]) + assert result.exit_code == 0 + mock_manager.remove.assert_called_once_with( + name="r1", + host=None, + user=None, + key=None, + token="tok", + gitea_url="https://git.example.com", + mode=None, + force=False, + ask_become_pass=True, + ) + @patch("gitea_runner_manager.cli.RunnerManager") def test_remove_error(self, mock_manager_class: MagicMock) -> None: mock_manager = MagicMock() diff --git a/tests/unit/test_runner_manager.py b/tests/unit/test_runner_manager.py index 968640d..ebbc1b7 100644 --- a/tests/unit/test_runner_manager.py +++ b/tests/unit/test_runner_manager.py @@ -186,34 +186,51 @@ class TestRunnerManager: def test_resolve_runner_from_registry(self) -> None: mock_registry = MagicMock() - mock_registry.get.return_value = {"host": "10.0.0.1", "user": "ubuntu", "key": "/key", "mode": "docker"} + mock_registry.get.return_value = { + "host": "10.0.0.1", + "user": "ubuntu", + "key": "/key", + "mode": "docker", + "gitea_url": "https://git.example.com", + } manager = RunnerManager(registry=mock_registry) - host, user, key, mode = manager._resolve_runner("r1") + host, user, key, mode, gitea_url = manager._resolve_runner("r1") assert host == "10.0.0.1" assert user == "ubuntu" assert key == "/key" assert mode == "docker" + assert gitea_url == "https://git.example.com" mock_registry.get.assert_called_once_with("r1") def test_resolve_runner_explicit_host_user(self) -> None: mock_registry = MagicMock() mock_registry.get.return_value = None manager = RunnerManager(registry=mock_registry) - host, user, key, mode = manager._resolve_runner("r1", host="10.0.0.2", user="root") + host, user, key, mode, gitea_url = manager._resolve_runner("r1", host="10.0.0.2", user="root") assert host == "10.0.0.2" assert user == "root" assert key is None assert mode == "docker" + assert gitea_url == "" def test_resolve_runner_override(self) -> None: mock_registry = MagicMock() - mock_registry.get.return_value = {"host": "10.0.0.1", "user": "ubuntu", "key": "/key", "mode": "docker"} + mock_registry.get.return_value = { + "host": "10.0.0.1", + "user": "ubuntu", + "key": "/key", + "mode": "docker", + "gitea_url": "https://git.example.com", + } manager = RunnerManager(registry=mock_registry) - host, user, key, mode = manager._resolve_runner("r1", host="10.0.0.2", user="root", key="/new", mode="binary") + host, user, key, mode, gitea_url = manager._resolve_runner( + "r1", host="10.0.0.2", user="root", key="/new", mode="binary" + ) assert host == "10.0.0.2" assert user == "root" assert key == "/new" assert mode == "binary" + assert gitea_url == "https://git.example.com" def test_resolve_runner_not_found(self) -> None: mock_registry = MagicMock() @@ -293,7 +310,13 @@ class TestRunnerManager: def test_disable(self) -> None: mock_registry = MagicMock() - mock_registry.get.return_value = {"host": "host", "user": "user", "key": None, "mode": "docker"} + mock_registry.get.return_value = { + "host": "host", + "user": "user", + "key": None, + "mode": "docker", + "gitea_url": "", + } manager = RunnerManager(registry=mock_registry) mock_executor = MagicMock() manager._executor = mock_executor @@ -308,6 +331,24 @@ class TestRunnerManager: assert "runner_mode=docker" in cmd_str assert "Disabling Gitea Runner r1 on host" in mock_executor.run.call_args.kwargs["description"] + def test_disable_uses_registry_gitea_url(self) -> None: + mock_registry = MagicMock() + mock_registry.get.return_value = { + "host": "host", + "user": "user", + "key": None, + "mode": "docker", + "gitea_url": "https://registry.example.com", + } + manager = RunnerManager(registry=mock_registry) + mock_executor = MagicMock() + manager._executor = mock_executor + + manager.disable("r1", token="tok") + cmd = mock_executor.run.call_args.args[0] + cmd_str = " ".join(cmd) + assert "gitea_url=https://registry.example.com" in cmd_str + def test_disable_missing_token(self) -> None: mock_registry = MagicMock() mock_registry.get.return_value = {"host": "host", "user": "user"} @@ -339,7 +380,13 @@ class TestRunnerManager: def test_remove(self) -> None: mock_registry = MagicMock() - mock_registry.get.return_value = {"host": "host", "user": "user", "key": None, "mode": "docker"} + mock_registry.get.return_value = { + "host": "host", + "user": "user", + "key": None, + "mode": "docker", + "gitea_url": "", + } manager = RunnerManager(registry=mock_registry) mock_executor = MagicMock() manager._executor = mock_executor @@ -383,7 +430,13 @@ class TestRunnerManager: def test_remove_force_skips_playbook(self) -> None: mock_registry = MagicMock() - mock_registry.get.return_value = {"host": "host", "user": "user", "key": None, "mode": "docker"} + mock_registry.get.return_value = { + "host": "host", + "user": "user", + "key": None, + "mode": "docker", + "gitea_url": "", + } manager = RunnerManager(registry=mock_registry) mock_executor = MagicMock() manager._executor = mock_executor @@ -392,6 +445,18 @@ class TestRunnerManager: mock_executor.run.assert_not_called() mock_registry.remove.assert_called_once_with("r1") + def test_remove_force_without_token_or_url(self) -> None: + """Force removal should not require token or gitea_url.""" + mock_registry = MagicMock() + mock_registry.get.return_value = {"host": "host", "user": "user"} + manager = RunnerManager(registry=mock_registry) + mock_executor = MagicMock() + manager._executor = mock_executor + + manager.remove("r1", force=True) + mock_executor.run.assert_not_called() + mock_registry.remove.assert_called_once_with("r1") + def test_list_runners_docker(self) -> None: mock_registry = MagicMock() mock_registry.list.return_value = {