From f64680aa22abf17f461495aaf7328255f2c9347e Mon Sep 17 00:00:00 2001 From: Emil Simeonov Date: Sat, 8 Aug 2026 21:22:53 +0200 Subject: [PATCH] fix(healthcheck): use Gitea API for runner registration detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The healthcheck's `journalctl --user` command fails with "No journal files were opened due to insufficient permissions" for runner users that lack journal access. This caused the healthcheck to always report "OK: runner healthy" even when all runners were unregistered — the auto-recovery never triggered. Replace journal-based detection with a Gitea API query: read the runner's ID from the .runner file and verify it exists in GET /api/v1/admin/actions/runners. This works regardless of journal permissions. Also add scripts/cleanup_stale_runners.py for bulk cleanup of stale runner registrations (runners that haven't been online for a configurable threshold). Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../templates/runner-healthcheck.sh.j2 | 86 ++++++- scripts/cleanup_stale_runners.py | 136 +++++++++++ scripts/tests/test_cleanup_stale_runners.py | 217 ++++++++++++++++++ 3 files changed, 429 insertions(+), 10 deletions(-) create mode 100644 scripts/cleanup_stale_runners.py create mode 100644 scripts/tests/test_cleanup_stale_runners.py diff --git a/ansible/roles/gitea_runner/templates/runner-healthcheck.sh.j2 b/ansible/roles/gitea_runner/templates/runner-healthcheck.sh.j2 index 1b47bfa..2e4ba95 100644 --- a/ansible/roles/gitea_runner/templates/runner-healthcheck.sh.j2 +++ b/ansible/roles/gitea_runner/templates/runner-healthcheck.sh.j2 @@ -48,6 +48,14 @@ fi # runner (e.g., server restore, runner record deleted, Gitea restart with # token salt change), the runner logs "unregistered runner" every few seconds. # A service restart will not fix this; re-registration is required. +# +# Detection method: query the Gitea API to verify the runner's UUID still +# exists. This is more reliable than parsing journal logs (which requires +# journal access permissions that runner users may not have — see the +# 2026-08-08 incident where journalctl --user returned "No journal files +# were opened due to insufficient permissions" for all runner users, +# causing the healthcheck to always report "OK: runner healthy" even +# though all runners were unregistered). {% if gitea_runner_auto_recover_api_token %} # Auto-recovery is enabled: fetch a new registration token from the Gitea API # and re-register the runner automatically. A cooldown prevents infinite loops. @@ -58,10 +66,50 @@ GITEA_URL="{{ gitea_url }}" RUNNER_NAME="{{ gitea_runner_name }}" RUNNER_LABELS="{{ gitea_runner_labels }}" BINARY="{{ gitea_runner_binary_path }}" +RUNNER_FILE="{{ gitea_runner_data_dir }}/.runner" {% endif %} -recent_errors=$(journalctl --user -u gitea-runner.service --since "5 minutes ago" --no-pager -q 2>/dev/null | grep -c "unregistered runner" || true) -if [[ "$recent_errors" -ge 3 ]]; then - echo "CRITICAL: runner is unregistered in Gitea (re-login failed $recent_errors times in 5 minutes)." +runner_unregistered=0 + +# Primary detection: query the Gitea API to check if the runner's ID +# still exists in Gitea's runner list. This works regardless of journal +# permissions. +{% if gitea_runner_auto_recover_api_token %} +if [[ -f "$GITEA_API_TOKEN_FILE" && -f "$RUNNER_FILE" ]]; then + API_TOKEN=$(cat "$GITEA_API_TOKEN_FILE" 2>/dev/null || true) + RUNNER_ID=$(python3 -c "import json; print(json.load(open('$RUNNER_FILE')).get('id',''))" 2>/dev/null || true) + if [[ -n "$API_TOKEN" && -n "$RUNNER_ID" ]]; then + # List all runners and check if our ID is present + runner_found=$(curl -sf --connect-timeout 5 --max-time 10 \ + -H "Authorization: token $API_TOKEN" \ + "${GITEA_URL}/api/v1/admin/actions/runners" 2>/dev/null \ + | python3 -c " +import sys, json +try: + data = json.load(sys.stdin) + runners = data if isinstance(data, list) else data.get('runners', []) + ids = [str(r.get('id', '')) for r in runners] + print('1' if '$RUNNER_ID' in ids else '0') +except Exception: + print('0') +" 2>/dev/null || echo "0") + if [[ "$runner_found" != "1" ]]; then + runner_unregistered=1 + echo "CRITICAL: runner ID $RUNNER_ID not found in Gitea (unregistered)." + fi + fi +fi +{% endif %} + +# Fallback detection: check journal logs (if accessible) +if [[ "$runner_unregistered" -eq 0 ]]; then + recent_errors=$(journalctl --user -u gitea-runner.service --since "5 minutes ago" --no-pager -q 2>/dev/null | grep -c "unregistered runner" || true) + if [[ "$recent_errors" -ge 3 ]]; then + runner_unregistered=1 + echo "CRITICAL: runner is unregistered in Gitea (re-login failed $recent_errors times in 5 minutes)." + fi +fi + +if [[ "$runner_unregistered" -ge 1 ]]; then {% if gitea_runner_auto_recover_api_token %} # Check cooldown — skip if we recently attempted recovery if [[ -f "$COOLDOWN_FILE" ]]; then @@ -135,14 +183,32 @@ if [[ "$recent_errors" -ge 3 ]]; then systemctl --user start gitea-runner.service sleep 3 - # Verify recovery — check if unregistered errors stopped - new_errors=$(journalctl --user -u gitea-runner.service --since "10 seconds ago" --no-pager -q 2>/dev/null | grep -c "unregistered runner" || true) - if [[ "$new_errors" -eq 0 ]]; then - echo "OK: runner recovered and no longer reporting unregistered errors" - # Clear cooldown on success - rm -f "$COOLDOWN_FILE" 2>/dev/null || true + # Verify recovery — query the Gitea API to confirm the new ID is registered + NEW_ID=$(python3 -c "import json; print(json.load(open('$RUNNER_FILE')).get('id',''))" 2>/dev/null || true) + if [[ -n "$NEW_ID" ]]; then + new_found=$(curl -sf --connect-timeout 5 --max-time 10 \ + -H "Authorization: token $API_TOKEN" \ + "${GITEA_URL}/api/v1/admin/actions/runners" 2>/dev/null \ + | python3 -c " +import sys, json +try: + data = json.load(sys.stdin) + runners = data if isinstance(data, list) else data.get('runners', []) + ids = [str(r.get('id', '')) for r in runners] + print('1' if '$NEW_ID' in ids else '0') +except Exception: + print('0') +" 2>/dev/null || echo "0") + if [[ "$new_found" == "1" ]]; then + echo "OK: runner recovered and registered with new ID $NEW_ID" + # Clear cooldown on success + rm -f "$COOLDOWN_FILE" 2>/dev/null || true + else + echo "WARN: runner re-registered but ID not found in Gitea API. Will retry after cooldown." + exit 3 + fi else - echo "WARN: runner still showing unregistered errors after re-registration. Will retry after cooldown." + echo "WARN: could not read new .runner file after re-registration. Will retry after cooldown." exit 3 fi {% else %} diff --git a/scripts/cleanup_stale_runners.py b/scripts/cleanup_stale_runners.py new file mode 100644 index 0000000..c172fe3 --- /dev/null +++ b/scripts/cleanup_stale_runners.py @@ -0,0 +1,136 @@ +#!/usr/bin/env python3 +"""Clean up stale runner registrations from Gitea. + +A runner is considered stale if it hasn't been online for more than a +configurable threshold (default: 1 hour). Stale runners accumulate when: +- A runner host is rebuilt or re-provisioned (old registration remains) +- A runner is re-registered (old entry remains alongside the new one) +- A runner process dies and the healthcheck can't auto-recover + +This script queries the Gitea API for all runners, identifies stale ones, +and deletes them via ``DELETE /api/v1/admin/actions/runners/{id}``. + +Usage:: + + python3 scripts/cleanup_stale_runners.py --gitea-url https://git.example.com --token + python3 scripts/cleanup_stale_runners.py --gitea-url https://git.example.com --token --dry-run + python3 scripts/cleanup_stale_runners.py --gitea-url https://git.example.com --token \\ + --stale-threshold 3600 +""" + +from __future__ import annotations + +import argparse +import json +import sys +import time +import urllib.error +import urllib.request # noqa: PTH123 # nosec B404 +from typing import Any + + +def _api_request(base_url: str, token: str, method: str, path: str) -> Any: + url = f"{base_url.rstrip('/')}/api/v1{path}" + req = urllib.request.Request(url, method=method) # nosec B310 + req.add_header("Authorization", f"token {token}") + req.add_header("Accept", "application/json") + try: + with urllib.request.urlopen(req) as resp: # noqa: PTH123 # nosec B310 + if resp.status == 204: + return None + raw = resp.read() + return json.loads(raw) if raw else None + except urllib.error.HTTPError as e: + detail = e.read().decode("utf-8", errors="replace") + raise RuntimeError(f"Gitea API error {e.code}: {detail}") from e + + +def list_runners(base_url: str, token: str) -> list[dict[str, Any]]: + data = _api_request(base_url, token, "GET", "/admin/actions/runners") + if data is None: + return [] + if isinstance(data, list): + return data + if isinstance(data, dict): + return data.get("runners", []) + return [] + + +def delete_runner(base_url: str, token: str, runner_id: int) -> bool: + try: + _api_request(base_url, token, "DELETE", f"/admin/actions/runners/{runner_id}") + return True + except RuntimeError as e: + print(f" ERROR deleting runner {runner_id}: {e}", file=sys.stderr) + return False + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="Clean up stale Gitea runner registrations") + parser.add_argument("--gitea-url", required=True, help="Gitea base URL") + parser.add_argument("--token", required=True, help="Gitea admin API token") + parser.add_argument( + "--stale-threshold", + type=int, + default=3600, + help="Seconds since last_online before a runner is considered stale (default: 3600 = 1h)", + ) + parser.add_argument("--dry-run", action="store_true", help="List stale runners without deleting") + args = parser.parse_args(argv) + + runners = list_runners(args.gitea_url, args.token) + if not runners: + print("No runners found.") + return 0 + + now = int(time.time()) + stale: list[dict[str, Any]] = [] + online: list[dict[str, Any]] = [] + + for runner in runners: + last_online = runner.get("last_online", 0) or 0 + seconds_since = now - last_online + runner["seconds_since_online"] = seconds_since + if seconds_since > args.stale_threshold: + stale.append(runner) + else: + online.append(runner) + + print(f"Total runners: {len(runners)}") + print(f"Online (within {args.stale_threshold}s): {len(online)}") + print(f"Stale (>{args.stale_threshold}s): {len(stale)}") + print() + + if not stale: + print("No stale runners to clean up.") + return 0 + + print("Stale runners:") + for r in stale: + rid = r.get("id", "?") + name = r.get("name", "?") + uuid = r.get("uuid", "?")[:8] + secs = r.get("seconds_since_online", 0) + hours = secs / 3600 + print(f" id={rid} name={name} uuid={uuid}... offline={hours:.1f}h ago") + + if args.dry_run: + print("\n--dry-run: not deleting. Remove --dry-run to clean up.") + return 0 + + print(f"\nDeleting {len(stale)} stale runners...") + deleted = 0 + for r in stale: + rid = r.get("id") + if rid is None: + continue + if delete_runner(args.gitea_url, args.token, rid): + deleted += 1 + print(f" Deleted runner id={rid} ({r.get('name', '?')})") + + print(f"\nDone: {deleted}/{len(stale)} stale runners deleted.") + return 0 if deleted == len(stale) else 1 + + +if __name__ == "__main__": # pragma: no cover + sys.exit(main()) diff --git a/scripts/tests/test_cleanup_stale_runners.py b/scripts/tests/test_cleanup_stale_runners.py new file mode 100644 index 0000000..8548535 --- /dev/null +++ b/scripts/tests/test_cleanup_stale_runners.py @@ -0,0 +1,217 @@ +"""Tests for cleanup_stale_runners.py.""" + +from __future__ import annotations + +import time +from unittest.mock import MagicMock, patch + +from scripts.cleanup_stale_runners import ( + _api_request, + delete_runner, + list_runners, + main, +) + + +class TestListRunners: + """Tests for list_runners().""" + + @patch("scripts.cleanup_stale_runners._api_request") + def test_returns_list_of_runners(self, mock_req: MagicMock) -> None: + mock_req.return_value = [{"id": 1, "name": "runner-1"}, {"id": 2, "name": "runner-2"}] + result = list_runners("https://git.example.com", "token") + assert len(result) == 2 + assert result[0]["id"] == 1 + + @patch("scripts.cleanup_stale_runners._api_request") + def test_returns_empty_on_none(self, mock_req: MagicMock) -> None: + mock_req.return_value = None + result = list_runners("https://git.example.com", "token") + assert result == [] + + @patch("scripts.cleanup_stale_runners._api_request") + def test_extracts_runners_from_dict(self, mock_req: MagicMock) -> None: + mock_req.return_value = {"runners": [{"id": 1}]} + result = list_runners("https://git.example.com", "token") + assert len(result) == 1 + assert result[0]["id"] == 1 + + @patch("scripts.cleanup_stale_runners._api_request") + def test_returns_empty_on_non_list_non_dict(self, mock_req: MagicMock) -> None: + mock_req.return_value = "not a list" + result = list_runners("https://git.example.com", "token") + assert result == [] + + +class TestDeleteRunner: + """Tests for delete_runner().""" + + @patch("scripts.cleanup_stale_runners._api_request") + def test_returns_true_on_success(self, mock_req: MagicMock) -> None: + mock_req.return_value = None + assert delete_runner("https://git.example.com", "token", 42) is True + + @patch("scripts.cleanup_stale_runners._api_request") + def test_returns_false_on_error(self, mock_req: MagicMock) -> None: + mock_req.side_effect = RuntimeError("API error 404: not found") + assert delete_runner("https://git.example.com", "token", 42) is False + + +class TestApiRequest: + """Tests for _api_request().""" + + @patch("scripts.cleanup_stale_runners.urllib.request.urlopen") + def test_returns_json_on_success(self, mock_urlopen: MagicMock) -> None: + mock_resp = MagicMock() + mock_resp.status = 200 + mock_resp.read.return_value = b'{"key": "value"}' + mock_urlopen.return_value.__enter__.return_value = mock_resp + result = _api_request("https://git.example.com", "token", "GET", "/test") + assert result == {"key": "value"} + + @patch("scripts.cleanup_stale_runners.urllib.request.urlopen") + def test_returns_none_on_204(self, mock_urlopen: MagicMock) -> None: + mock_resp = MagicMock() + mock_resp.status = 204 + mock_urlopen.return_value.__enter__.return_value = mock_resp + result = _api_request("https://git.example.com", "token", "DELETE", "/test/1") + assert result is None + + @patch("scripts.cleanup_stale_runners.urllib.request.urlopen") + def test_returns_none_on_empty_body(self, mock_urlopen: MagicMock) -> None: + mock_resp = MagicMock() + mock_resp.status = 200 + mock_resp.read.return_value = b"" + mock_urlopen.return_value.__enter__.return_value = mock_resp + result = _api_request("https://git.example.com", "token", "GET", "/test") + assert result is None + + @patch("scripts.cleanup_stale_runners.urllib.request.urlopen") + def test_raises_on_http_error(self, mock_urlopen: MagicMock) -> None: + import urllib.error + + mock_error = urllib.error.HTTPError( + "url", + 404, + "Not Found", + {}, + None, + ) + mock_error.read = MagicMock(return_value=b'{"message": "not found"}') + mock_urlopen.side_effect = mock_error + import pytest + + with pytest.raises(RuntimeError, match="404"): + _api_request("https://git.example.com", "token", "GET", "/test") + + +class TestMain: + """Tests for main().""" + + @patch("scripts.cleanup_stale_runners.list_runners") + def test_no_runners(self, mock_list: MagicMock) -> None: + mock_list.return_value = [] + rc = main(["--gitea-url", "https://git.example.com", "--token", "t"]) + assert rc == 0 + + @patch("scripts.cleanup_stale_runners.list_runners") + def test_no_stale_runners(self, mock_list: MagicMock) -> None: + now = int(time.time()) + mock_list.return_value = [ + {"id": 1, "name": "runner-1", "last_online": now - 60}, + ] + rc = main(["--gitea-url", "https://git.example.com", "--token", "t"]) + assert rc == 0 + + @patch("scripts.cleanup_stale_runners.list_runners") + def test_dry_run_does_not_delete(self, mock_list: MagicMock) -> None: + now = int(time.time()) + mock_list.return_value = [ + {"id": 1, "name": "runner-1", "last_online": now - 7200}, + ] + with patch("scripts.cleanup_stale_runners.delete_runner") as mock_del: + rc = main( + [ + "--gitea-url", + "https://git.example.com", + "--token", + "t", + "--dry-run", + ] + ) + assert rc == 0 + mock_del.assert_not_called() + + @patch("scripts.cleanup_stale_runners.list_runners") + @patch("scripts.cleanup_stale_runners.delete_runner") + def test_deletes_stale_runners(self, mock_del: MagicMock, mock_list: MagicMock) -> None: + now = int(time.time()) + mock_list.return_value = [ + {"id": 1, "name": "runner-1", "last_online": now - 60}, + {"id": 2, "name": "runner-2", "last_online": now - 7200}, + {"id": 3, "name": "runner-3", "last_online": now - 9999}, + ] + mock_del.return_value = True + rc = main( + [ + "--gitea-url", + "https://git.example.com", + "--token", + "t", + "--stale-threshold", + "3600", + ] + ) + assert rc == 0 + assert mock_del.call_count == 2 + + @patch("scripts.cleanup_stale_runners.list_runners") + @patch("scripts.cleanup_stale_runners.delete_runner") + def test_returns_1_on_partial_failure(self, mock_del: MagicMock, mock_list: MagicMock) -> None: + now = int(time.time()) + mock_list.return_value = [ + {"id": 1, "name": "runner-1", "last_online": now - 7200}, + {"id": 2, "name": "runner-2", "last_online": now - 7200}, + ] + mock_del.side_effect = [True, False] + rc = main(["--gitea-url", "https://git.example.com", "--token", "t"]) + assert rc == 1 + + @patch("scripts.cleanup_stale_runners.list_runners") + def test_runner_with_zero_last_online(self, mock_list: MagicMock) -> None: + """Runners with last_online=0 should be considered stale.""" + mock_list.return_value = [ + {"id": 1, "name": "runner-1", "last_online": 0}, + ] + with patch("scripts.cleanup_stale_runners.delete_runner") as mock_del: + mock_del.return_value = True + rc = main(["--gitea-url", "https://git.example.com", "--token", "t"]) + assert rc == 0 + mock_del.assert_called_once() + + @patch("scripts.cleanup_stale_runners.list_runners") + def test_runner_with_missing_last_online(self, mock_list: MagicMock) -> None: + """Runners with missing last_online should be considered stale.""" + mock_list.return_value = [ + {"id": 1, "name": "runner-1"}, + ] + with patch("scripts.cleanup_stale_runners.delete_runner") as mock_del: + mock_del.return_value = True + rc = main(["--gitea-url", "https://git.example.com", "--token", "t"]) + assert rc == 0 + mock_del.assert_called_once() + + @patch("scripts.cleanup_stale_runners.list_runners") + @patch("scripts.cleanup_stale_runners.delete_runner") + def test_skips_runner_with_none_id(self, mock_del: MagicMock, mock_list: MagicMock) -> None: + """Runners with id=None should be skipped during deletion.""" + now = int(time.time()) + mock_list.return_value = [ + {"id": None, "name": "bad-runner", "last_online": now - 7200}, + {"id": 2, "name": "runner-2", "last_online": now - 7200}, + ] + mock_del.return_value = True + rc = main(["--gitea-url", "https://git.example.com", "--token", "t"]) + # 1/2 deleted (None id skipped), so rc=1 (partial) + assert rc == 1 + mock_del.assert_called_once_with("https://git.example.com", "t", 2)