From 69e53e2cb250a5f8f51f87c5f26d166dc0913f6e Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 16 Jul 2026 13:38:50 +0000 Subject: [PATCH 01/20] ci: schedule OpenStack CI resource cleanup Add a script that deletes dangling servers, keypairs, images and test security groups left by integration tests, plus a 6-hourly workflow on the private-endpoint runner. --- .../cleanup_openstack_ci_resources.yaml | 69 +++ scripts/cleanup_openstack_ci_resources.py | 583 ++++++++++++++++++ 2 files changed, 652 insertions(+) create mode 100644 .github/workflows/cleanup_openstack_ci_resources.yaml create mode 100755 scripts/cleanup_openstack_ci_resources.py diff --git a/.github/workflows/cleanup_openstack_ci_resources.yaml b/.github/workflows/cleanup_openstack_ci_resources.yaml new file mode 100644 index 000000000..5f9221677 --- /dev/null +++ b/.github/workflows/cleanup_openstack_ci_resources.yaml @@ -0,0 +1,69 @@ +name: Cleanup OpenStack CI resources + +on: + schedule: + # Every 6 hours UTC + - cron: "0 */6 * * *" + workflow_dispatch: + inputs: + min-age-hours: + description: Minimum resource age in hours before deletion + required: false + default: "6" + apply: + description: Actually delete (true) or dry-run (false) + required: false + default: "true" + +concurrency: + group: cleanup-openstack-ci-resources + cancel-in-progress: false + +permissions: + contents: read + +jobs: + cleanup: + name: Cleanup dangling OpenStack CI resources + runs-on: self-hosted-linux-amd64-noble-private-endpoint-small + timeout-minutes: 60 + steps: + - name: Checkout + uses: actions/checkout@v6.0.2 + + - name: Install openstacksdk + run: | + python3 -m pip install --user --upgrade pip + python3 -m pip install --user 'openstacksdk>=3.0.0' + + - name: Run cleanup + env: + # Sensitive OpenStack / integration env vars are injected via the same + # name→value slot mapping used by integration tests (see CONTRIBUTING.md). + # Expect OS_AUTH_URL, OS_PASSWORD, OS_PROJECT_NAME, OS_USERNAME, + # OS_USER_DOMAIN_NAME, OS_PROJECT_DOMAIN_NAME, OS_REGION_NAME (etc.) + # to arrive via those slots. + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_1 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_1 }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_2 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_2 }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_3 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_3 }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_4 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_4 }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_5 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_5 }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_6 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_6 }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_7 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_7 }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_8 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_8 }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_9 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_9 }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_10 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_10 }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_11 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_11 }} + ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_12 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_12 }} + OS_IDENTITY_API_VERSION: "3" + run: | + set -euo pipefail + export PATH="${HOME}/.local/bin:${PATH}" + MIN_AGE="${{ github.event.inputs.min-age-hours || '6' }}" + APPLY="${{ github.event.inputs.apply || 'true' }}" + extra_flags=(--min-age-hours "${MIN_AGE}" --aggressive-keypairs --json-summary) + if [ "${APPLY}" = "true" ]; then + extra_flags+=(--apply) + fi + python3 scripts/cleanup_openstack_ci_resources.py -v "${extra_flags[@]}" diff --git a/scripts/cleanup_openstack_ci_resources.py b/scripts/cleanup_openstack_ci_resources.py new file mode 100755 index 000000000..a0ba1b465 --- /dev/null +++ b/scripts/cleanup_openstack_ci_resources.py @@ -0,0 +1,583 @@ +#!/usr/bin/env python3 +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Delete old CI-created OpenStack resources in a single tenant/project. + +Designed for the scheduled GitHub Actions workflow (and ad-hoc local runs). +Default mode is dry-run; pass --apply to delete. + +Resources covered (name-pattern based): + * servers (Nova instances / builder VMs / runner VMs) + * keypairs + * glance images (built test images / base seeds with CI prefixes) + * security groups (test-only; shared permanent groups are never touched) + +Authentication (any one of): + * OS_* environment variables (OS_AUTH_URL, OS_PROJECT_NAME, ...) + * openstacksdk clouds.yaml via --os-cloud / OS_CLOUD + * explicit --clouds-file + --os-cloud + +Examples: + # List what would be deleted (older than 6h): + ./cleanup_openstack_ci_resources.py --min-age-hours 6 + + # Actually delete: + ./cleanup_openstack_ci_resources.py --min-age-hours 6 --apply + + # Use a named cloud from clouds.yaml: + OS_CLOUD=ci-tenant ./cleanup_openstack_ci_resources.py --apply --min-age-hours 12 +""" + +from __future__ import annotations + +import argparse +import logging +import os +import re +import sys +from dataclasses import dataclass, field +from datetime import datetime, timedelta, timezone +from typing import Any, Callable, Iterable + +try: + import openstack + from openstack.connection import Connection +except ImportError: # pragma: no cover - runtime dependency check + print( + "error: openstacksdk is required. Install with:\n" + " pip install openstacksdk\n" + " # or: apt install python3-openstacksdk", + file=sys.stderr, + ) + sys.exit(2) + +LOG = logging.getLogger("cleanup_openstack_ci") + +# --------------------------------------------------------------------------- +# Name matchers +# +# Derived from integration fixtures / product naming in: +# github-runner-operator (charm + github-runner-manager) +# github-runner-image-builder-operator (charm + app) +# +# Permanent / shared names that production and refits reuse must stay excluded. +# --------------------------------------------------------------------------- + +# Never delete these even if they match something else. +PROTECTED_EXACT_NAMES: frozenset[str] = frozenset( + { + # github-runner-manager shared SG (openstack_cloud._SECURITY_GROUP_NAME) + "github-runner-v1", + # image-builder shared SG (openstack_builder.SHARED_SECURITY_GROUP_NAME) + "github-runner-image-builder-v1", + # default OpenStack groups + "default", + } +) + +# Server / keypair / image name patterns treated as CI leftovers. +# Each pattern is applied to the full resource name. +NAME_PATTERNS: tuple[re.Pattern[str], ...] = tuple( + re.compile(p) + for p in ( + # github-runner-manager integration: TestConfig.vm_prefix = test-runner-{8} + r"^test-runner-[a-z0-9]{6,12}($|-)", + # github-runner charm integration app: test-{8} (+ unit/VM suffix) + r"^test-[a-z0-9]{6,12}($|-)", + # image-builder operator charm app names + r"^image-builder-operator-[0-9a-f]{6,16}($|-)", + r"^image-builder-charmhub-[0-9a-f]{6,16}($|-)", + # GRO deploys image-builder as github-runner-image-builder-{suffix} + r"^github-runner-image-builder-[a-z0-9]{6,12}($|-)", + # image-builder app integration fixtures + r"^test-image-builder-", + # keypair / builder naming: {prefix}-image-builder-... + # 2-char test_id prefixes only accepted when the rest is image-builder-related + r"^[a-z0-9]{2,16}-image-builder-(base-|ssh-key$|test$|[a-z0-9]+-[a-z0-9]+$)", + r"^[a-z0-9]{2,16}-image-builder-[a-z0-9]+-[a-z0-9]+$", + ) +) + +# Extra security-group patterns (not covered cleanly by server-name family). +SG_PATTERNS: tuple[re.Pattern[str], ...] = tuple( + re.compile(p) + for p in ( + r"^github-runner-image-builder-test-security-group-", + r"^github-runner-image-builder-operator-test-security-group", + # manager-test SGs named after vm_prefix if any + r"^test-runner-[a-z0-9]{6,12}-", + r"^test-[a-z0-9]{6,12}-", + ) +) + + +@dataclass +class ResourceHit: + kind: str + name: str + resource_id: str + created_at: datetime | None + age: timedelta | None + detail: str = "" + + +@dataclass +class Stats: + considered: int = 0 + matched: int = 0 + too_young: int = 0 + protected: int = 0 + deleted: int = 0 + failed: int = 0 + details: list[str] = field(default_factory=list) + + +def _parse_openstack_timestamp(value: Any) -> datetime | None: + """Parse OpenStack created_at / updated_at into aware UTC datetime.""" + if value is None: + return None + if isinstance(value, datetime): + if value.tzinfo is None: + return value.replace(tzinfo=timezone.utc) + return value.astimezone(timezone.utc) + text = str(value).strip() + if not text: + return None + # Common forms: 2026-07-16T12:34:56Z / 2026-07-16T12:34:56.123456 + text = text.replace("Z", "+00:00") + # Some APIs omit timezone; treat as UTC. + try: + dt = datetime.fromisoformat(text) + except ValueError: + for fmt in ("%Y-%m-%dT%H:%M:%S", "%Y-%m-%dT%H:%M:%S.%f"): + try: + dt = datetime.strptime(text.split("+")[0], fmt) + break + except ValueError: + continue + else: + LOG.warning("unparseable timestamp %r", value) + return None + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) + return dt.astimezone(timezone.utc) + + +def _name_matches(name: str | None, patterns: Iterable[re.Pattern[str]]) -> bool: + if not name: + return False + if name in PROTECTED_EXACT_NAMES: + return False + return any(p.search(name) for p in patterns) + + +def _connect(args: argparse.Namespace) -> Connection: + kwargs: dict[str, Any] = {} + if args.clouds_file: + kwargs["config_files"] = [args.clouds_file] + if args.os_cloud: + kwargs["cloud"] = args.os_cloud + # openstack.connect() also reads OS_* env vars automatically. + conn = openstack.connect(**kwargs) + # Force auth early so failures are loud. + conn.authorize() + return conn + + +def _age_ok(created: datetime | None, min_age: timedelta, now: datetime) -> tuple[bool, timedelta | None]: + """Return (eligible, age). Missing created_at: treat as eligible (stale leftover).""" + if created is None: + return True, None + age = now - created + return age >= min_age, age + + +def _scan_servers(conn: Connection, min_age: timedelta, now: datetime) -> list[ResourceHit]: + hits: list[ResourceHit] = [] + for server in conn.list_servers(bare=True) or []: + name = getattr(server, "name", None) + if not _name_matches(name, NAME_PATTERNS): + continue + created = _parse_openstack_timestamp(getattr(server, "created_at", None) or getattr(server, "created", None)) + eligible, age = _age_ok(created, min_age, now) + hits.append( + ResourceHit( + kind="server", + name=name or "", + resource_id=str(server.id), + created_at=created, + age=age, + detail=f"status={getattr(server, 'status', '?')}" + ("" if eligible else " [TOO_YOUNG]"), + ) + ) + if not eligible: + hits[-1].detail = hits[-1].detail # marker used below + return hits + + +def _scan_keypairs(conn: Connection, min_age: timedelta, now: datetime) -> list[ResourceHit]: + # Keypairs often lack created_at. Unknown-age keypairs are filtered later + # (skipped unless --aggressive-keypairs or --min-age-hours 0). + hits: list[ResourceHit] = [] + for key in conn.list_keypairs() or []: + name = getattr(key, "name", None) + if not _name_matches(name, NAME_PATTERNS): + continue + created = _parse_openstack_timestamp(getattr(key, "created_at", None)) + eligible, age = _age_ok(created, min_age, now) + hits.append( + ResourceHit( + kind="keypair", + name=name or "", + resource_id=name or "", + created_at=created, + age=age, + detail="" if eligible else "[TOO_YOUNG]", + ) + ) + return hits + + +def _scan_images(conn: Connection, min_age: timedelta, now: datetime) -> list[ResourceHit]: + hits: list[ResourceHit] = [] + # Prefer project-owned images; list_images may include public base images. + for image in conn.list_images() or []: + name = getattr(image, "name", None) + # Do not touch public/shared marketplace images even if name collides. + visibility = str(getattr(image, "visibility", "") or "").lower() + if visibility in {"public", "community"}: + continue + if not _name_matches(name, NAME_PATTERNS): + continue + created = _parse_openstack_timestamp(getattr(image, "created_at", None)) + eligible, age = _age_ok(created, min_age, now) + hits.append( + ResourceHit( + kind="image", + name=name or "", + resource_id=str(image.id), + created_at=created, + age=age, + detail=f"status={getattr(image, 'status', '?')} visibility={visibility or '?'}" + + ("" if eligible else " [TOO_YOUNG]"), + ) + ) + return hits + + +def _scan_security_groups(conn: Connection, min_age: timedelta, now: datetime) -> list[ResourceHit]: + hits: list[ResourceHit] = [] + for sg in conn.list_security_groups() or []: + name = getattr(sg, "name", None) + if not name or name in PROTECTED_EXACT_NAMES: + continue + if not _name_matches(name, SG_PATTERNS) and not _name_matches(name, NAME_PATTERNS): + continue + created = _parse_openstack_timestamp(getattr(sg, "created_at", None)) + eligible, age = _age_ok(created, min_age, now) + hits.append( + ResourceHit( + kind="security_group", + name=name, + resource_id=str(sg.id), + created_at=created, + age=age, + detail="" if eligible else "[TOO_YOUNG]", + ) + ) + return hits + + +def _format_age(age: timedelta | None) -> str: + if age is None: + return "age=unknown" + hours = age.total_seconds() / 3600.0 + if hours < 48: + return f"age={hours:.1f}h" + return f"age={hours / 24.0:.1f}d" + + +def _delete_hit(conn: Connection, hit: ResourceHit, wait_timeout: int) -> None: + if hit.kind == "server": + LOG.info("deleting server %s (%s)", hit.name, hit.resource_id) + ok = conn.delete_server(hit.resource_id, wait=True, timeout=wait_timeout) + if not ok: + # Some SDK versions return False if already gone. + if conn.get_server(hit.resource_id): + raise RuntimeError(f"delete_server returned False for {hit.resource_id}") + elif hit.kind == "keypair": + LOG.info("deleting keypair %s", hit.name) + conn.delete_keypair(hit.name) + elif hit.kind == "image": + LOG.info("deleting image %s (%s)", hit.name, hit.resource_id) + conn.delete_image(hit.resource_id, wait=True) + elif hit.kind == "security_group": + LOG.info("deleting security_group %s (%s)", hit.name, hit.resource_id) + conn.delete_security_group(hit.resource_id) + else: + raise ValueError(f"unknown kind {hit.kind}") + + +def _filter_eligible( + hits: list[ResourceHit], + min_age: timedelta, + now: datetime, + aggressive_keypairs: bool, +) -> tuple[list[ResourceHit], list[ResourceHit]]: + """Split into (to_delete, skipped_young_or_policy).""" + to_delete: list[ResourceHit] = [] + skipped: list[ResourceHit] = [] + for hit in hits: + # Unknown age keypairs: only with aggressive flag, or when min_age is 0. + if hit.kind == "keypair" and hit.created_at is None: + if aggressive_keypairs or min_age.total_seconds() <= 0: + to_delete.append(hit) + else: + hit.detail = (hit.detail + " [SKIP_UNKNOWN_AGE_KEYPAIR]").strip() + skipped.append(hit) + continue + eligible, age = _age_ok(hit.created_at, min_age, now) + hit.age = age + if eligible: + to_delete.append(hit) + else: + hit.detail = (hit.detail + " [TOO_YOUNG]").strip() + skipped.append(hit) + return to_delete, skipped + + +def build_parser() -> argparse.ArgumentParser: + p = argparse.ArgumentParser( + description=( + "Cleanup dangling OpenStack CI resources from github-runner and " + "github-runner-image-builder integration tests." + ), + formatter_class=argparse.RawDescriptionHelpFormatter, + epilog=__doc__, + ) + p.add_argument( + "--apply", + action="store_true", + help="Actually delete matched resources. Without this flag, only list them.", + ) + p.add_argument( + "--min-age-hours", + type=float, + default=6.0, + help=( + "Only touch resources older than this many hours " + "(default: 6). Avoids racing active CI. Keypairs without timestamps " + "are skipped unless --aggressive-keypairs or min-age is 0." + ), + ) + p.add_argument( + "--aggressive-keypairs", + action="store_true", + help="Delete matching keypairs even when created_at is unavailable.", + ) + p.add_argument( + "--kinds", + default="server,keypair,image,security_group", + help="Comma-separated resource kinds to consider (default: all).", + ) + p.add_argument( + "--os-cloud", + default=os.environ.get("OS_CLOUD"), + help="Cloud name from clouds.yaml (or OS_CLOUD).", + ) + p.add_argument( + "--clouds-file", + default=None, + help="Path to clouds.yaml if not in the default search path.", + ) + p.add_argument( + "--wait-timeout", + type=int, + default=300, + help="Seconds to wait for server/image delete (default: 300).", + ) + p.add_argument( + "--max-deletes", + type=int, + default=0, + help="Safety cap on deletes this run (0 = unlimited).", + ) + p.add_argument( + "-v", + "--verbose", + action="count", + default=0, + help="Increase log verbosity (-v, -vv).", + ) + p.add_argument( + "--json-summary", + action="store_true", + help="Print a machine-readable summary line on stdout at the end.", + ) + return p + + +def main(argv: list[str] | None = None) -> int: + args = build_parser().parse_args(argv) + logging.basicConfig( + level=logging.DEBUG if args.verbose >= 2 else logging.INFO if args.verbose else logging.WARNING, + format="%(asctime)s %(levelname)s %(message)s", + datefmt="%Y-%m-%dT%H:%M:%S", + ) + # Always show cleanup decisions at INFO when not quiet — promote default to INFO. + if args.verbose == 0: + logging.getLogger().setLevel(logging.INFO) + + kinds = {k.strip() for k in args.kinds.split(",") if k.strip()} + valid = {"server", "keypair", "image", "security_group"} + unknown = kinds - valid + if unknown: + LOG.error("unknown --kinds values: %s (valid: %s)", unknown, sorted(valid)) + return 2 + + min_age = timedelta(hours=args.min_age_hours) + now = datetime.now(tz=timezone.utc) + + try: + conn = _connect(args) + except Exception as exc: # noqa: BLE001 - top-level auth failure + LOG.error("failed to connect to OpenStack: %s", exc) + return 2 + + project = None + try: + project = conn.current_project_id + except Exception: # noqa: BLE001 + pass + LOG.info( + "connected project_id=%s cloud=%s apply=%s min_age_hours=%s kinds=%s", + project, + args.os_cloud or "(env/default)", + args.apply, + args.min_age_hours, + ",".join(sorted(kinds)), + ) + + scanners: dict[str, Callable[[Connection, timedelta, datetime], list[ResourceHit]]] = { + "server": _scan_servers, + "keypair": _scan_keypairs, + "image": _scan_images, + "security_group": _scan_security_groups, + } + + all_hits: list[ResourceHit] = [] + for kind in ("server", "keypair", "image", "security_group"): + if kind not in kinds: + continue + try: + hits = scanners[kind](conn, min_age, now) + except Exception as exc: # noqa: BLE001 + LOG.error("failed listing %s: %s", kind, exc) + return 1 + all_hits.extend(hits) + + # Delete order: servers first (may hold SG/keypair refs), then images, keypairs, SGs. + order = {"server": 0, "image": 1, "keypair": 2, "security_group": 3} + all_hits.sort(key=lambda h: (order.get(h.kind, 9), h.name)) + + to_delete, skipped = _filter_eligible( + all_hits, + min_age=min_age, + now=now, + aggressive_keypairs=args.aggressive_keypairs, + ) + + stats = Stats(considered=len(all_hits), matched=len(all_hits)) + + for hit in skipped: + stats.too_young += 1 + LOG.info( + "SKIP %s name=%s id=%s %s %s", + hit.kind, + hit.name, + hit.resource_id, + _format_age(hit.age), + hit.detail, + ) + + if args.max_deletes and len(to_delete) > args.max_deletes: + LOG.warning( + "truncating candidate list from %s to --max-deletes=%s", + len(to_delete), + args.max_deletes, + ) + to_delete = to_delete[: args.max_deletes] + + mode = "DELETE" if args.apply else "DRY-RUN" + for hit in to_delete: + LOG.info( + "%s %s name=%s id=%s %s %s", + mode, + hit.kind, + hit.name, + hit.resource_id, + _format_age(hit.age), + hit.detail, + ) + if not args.apply: + continue + try: + _delete_hit(conn, hit, wait_timeout=args.wait_timeout) + stats.deleted += 1 + except Exception as exc: # noqa: BLE001 + stats.failed += 1 + LOG.error( + "FAILED %s name=%s id=%s: %s", + hit.kind, + hit.name, + hit.resource_id, + exc, + ) + + LOG.info( + "summary considered=%s candidates=%s skipped_young_or_policy=%s deleted=%s failed=%s apply=%s", + stats.considered, + len(to_delete), + stats.too_young, + stats.deleted, + stats.failed, + args.apply, + ) + if args.json_summary: + import json + + print( + json.dumps( + { + "project_id": project, + "apply": args.apply, + "min_age_hours": args.min_age_hours, + "considered": stats.considered, + "candidates": len(to_delete), + "skipped": stats.too_young, + "deleted": stats.deleted, + "failed": stats.failed, + "candidates_detail": [ + { + "kind": h.kind, + "name": h.name, + "id": h.resource_id, + "age_seconds": None if h.age is None else h.age.total_seconds(), + } + for h in to_delete + ], + } + ) + ) + + try: + conn.close() + except Exception: # noqa: BLE001 + pass + + return 1 if stats.failed else 0 + + +if __name__ == "__main__": + sys.exit(main()) From 7fa7b9475548c9134461eacdc49eaa6704f8f929 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 16 Jul 2026 13:48:11 +0000 Subject: [PATCH 02/20] ci: reclaim OpenStack orphans at integration test start Drop the scheduled cleanup workflow. Force-cancelled runs leave resources behind; the next integration suite now deletes CI-named OpenStack leftovers older than 6h before creating new ones. --- .../cleanup_openstack_ci_resources.yaml | 69 --- .../tests/integration/conftest.py | 4 + .../tests/integration/orphan_cleanup.py | 116 ++++ scripts/cleanup_openstack_ci_resources.py | 583 ------------------ tests/integration/conftest.py | 4 + tests/integration/helpers/orphan_cleanup.py | 162 +++++ 6 files changed, 286 insertions(+), 652 deletions(-) delete mode 100644 .github/workflows/cleanup_openstack_ci_resources.yaml create mode 100644 github-runner-manager/tests/integration/orphan_cleanup.py delete mode 100755 scripts/cleanup_openstack_ci_resources.py create mode 100644 tests/integration/helpers/orphan_cleanup.py diff --git a/.github/workflows/cleanup_openstack_ci_resources.yaml b/.github/workflows/cleanup_openstack_ci_resources.yaml deleted file mode 100644 index 5f9221677..000000000 --- a/.github/workflows/cleanup_openstack_ci_resources.yaml +++ /dev/null @@ -1,69 +0,0 @@ -name: Cleanup OpenStack CI resources - -on: - schedule: - # Every 6 hours UTC - - cron: "0 */6 * * *" - workflow_dispatch: - inputs: - min-age-hours: - description: Minimum resource age in hours before deletion - required: false - default: "6" - apply: - description: Actually delete (true) or dry-run (false) - required: false - default: "true" - -concurrency: - group: cleanup-openstack-ci-resources - cancel-in-progress: false - -permissions: - contents: read - -jobs: - cleanup: - name: Cleanup dangling OpenStack CI resources - runs-on: self-hosted-linux-amd64-noble-private-endpoint-small - timeout-minutes: 60 - steps: - - name: Checkout - uses: actions/checkout@v6.0.2 - - - name: Install openstacksdk - run: | - python3 -m pip install --user --upgrade pip - python3 -m pip install --user 'openstacksdk>=3.0.0' - - - name: Run cleanup - env: - # Sensitive OpenStack / integration env vars are injected via the same - # name→value slot mapping used by integration tests (see CONTRIBUTING.md). - # Expect OS_AUTH_URL, OS_PASSWORD, OS_PROJECT_NAME, OS_USERNAME, - # OS_USER_DOMAIN_NAME, OS_PROJECT_DOMAIN_NAME, OS_REGION_NAME (etc.) - # to arrive via those slots. - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_1 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_1 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_2 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_2 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_3 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_3 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_4 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_4 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_5 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_5 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_6 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_6 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_7 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_7 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_8 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_8 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_9 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_9 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_10 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_10 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_11 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_11 }} - ${{ vars.INTEGRATION_TEST_SECRET_ENV_NAME_12 }}: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_12 }} - OS_IDENTITY_API_VERSION: "3" - run: | - set -euo pipefail - export PATH="${HOME}/.local/bin:${PATH}" - MIN_AGE="${{ github.event.inputs.min-age-hours || '6' }}" - APPLY="${{ github.event.inputs.apply || 'true' }}" - extra_flags=(--min-age-hours "${MIN_AGE}" --aggressive-keypairs --json-summary) - if [ "${APPLY}" = "true" ]; then - extra_flags+=(--apply) - fi - python3 scripts/cleanup_openstack_ci_resources.py -v "${extra_flags[@]}" diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index 81257d7b2..c665aee5c 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -182,6 +182,10 @@ def openstack_connection( project_domain_name=openstack_config.project_domain_name, region_name=openstack_config.region_name, ) + # Reclaim leftovers from force-cancelled previous CI runs before creating new ones. + from .orphan_cleanup import cleanup_stale_openstack_resources + + cleanup_stale_openstack_resources(conn) yield conn conn.close() diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py new file mode 100644 index 000000000..7bb901f61 --- /dev/null +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -0,0 +1,116 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Delete stale OpenStack resources left by interrupted manager integration runs.""" + +from __future__ import annotations + +import logging +import re +from datetime import datetime, timedelta, timezone +from typing import Iterable + +from openstack.connection import Connection + +logger = logging.getLogger(__name__) + +_PROTECTED_NAMES = frozenset({"github-runner-v1", "default"}) + +# Matches TestConfig.vm_prefix = test-runner-{8 alnum} +_NAME_PATTERNS: tuple[re.Pattern[str], ...] = ( + re.compile(r"^test-runner-[a-z0-9]{6,12}($|-)"), +) + +_SG_PATTERNS: tuple[re.Pattern[str], ...] = ( + re.compile(r"^test-runner-[a-z0-9]{6,12}-"), +) + + +def _parse_created_at(value: object) -> datetime | None: + if value is None: + return None + if isinstance(value, datetime): + return value if value.tzinfo else value.replace(tzinfo=timezone.utc) + text = str(value).strip().replace("Z", "+00:00") + if not text: + return None + try: + dt = datetime.fromisoformat(text) + except ValueError: + return None + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) + return dt.astimezone(timezone.utc) + + +def _matches(name: str | None, patterns: Iterable[re.Pattern[str]]) -> bool: + if not name or name in _PROTECTED_NAMES: + return False + return any(p.search(name) for p in patterns) + + +def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: + created = _parse_created_at(created_at) + if created is None: + return True + return now - created >= min_age + + +def _safe_delete(label: str, name: str, delete_fn) -> None: + try: + delete_fn() + logger.info("Orphan cleanup deleted %s %s", label, name) + except Exception as exc: # noqa: BLE001 + logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc) + + +def cleanup_stale_openstack_resources( + connection: Connection, + min_age: timedelta = timedelta(hours=6), +) -> None: + """Remove manager-IT OpenStack leftovers older than ``min_age``.""" + now = datetime.now(tz=timezone.utc) + logger.info( + "OpenStack orphan cleanup starting (min_age=%sh)", + min_age.total_seconds() / 3600.0, + ) + + for server in connection.list_servers(bare=True) or []: + name = getattr(server, "name", None) + if not _matches(name, _NAME_PATTERNS): + continue + if not _is_stale( + getattr(server, "created_at", None) or getattr(server, "created", None), + min_age, + now, + ): + continue + _safe_delete( + "server", + name or server.id, + lambda s=server: connection.delete_server(s.id, wait=False), + ) + + for keypair in connection.list_keypairs() or []: + name = getattr(keypair, "name", None) + if not _matches(name, _NAME_PATTERNS): + continue + if not _is_stale(getattr(keypair, "created_at", None), min_age, now): + continue + _safe_delete("keypair", name or "", lambda n=name: connection.delete_keypair(n)) + + for sg in connection.list_security_groups() or []: + name = getattr(sg, "name", None) + if name in _PROTECTED_NAMES: + continue + if not (_matches(name, _SG_PATTERNS) or _matches(name, _NAME_PATTERNS)): + continue + if not _is_stale(getattr(sg, "created_at", None), min_age, now): + continue + _safe_delete( + "security_group", + name or sg.id, + lambda g=sg: connection.delete_security_group(g.id), + ) + + logger.info("OpenStack orphan cleanup finished") diff --git a/scripts/cleanup_openstack_ci_resources.py b/scripts/cleanup_openstack_ci_resources.py deleted file mode 100755 index a0ba1b465..000000000 --- a/scripts/cleanup_openstack_ci_resources.py +++ /dev/null @@ -1,583 +0,0 @@ -#!/usr/bin/env python3 -# Copyright 2026 Canonical Ltd. -# See LICENSE file for licensing details. - -"""Delete old CI-created OpenStack resources in a single tenant/project. - -Designed for the scheduled GitHub Actions workflow (and ad-hoc local runs). -Default mode is dry-run; pass --apply to delete. - -Resources covered (name-pattern based): - * servers (Nova instances / builder VMs / runner VMs) - * keypairs - * glance images (built test images / base seeds with CI prefixes) - * security groups (test-only; shared permanent groups are never touched) - -Authentication (any one of): - * OS_* environment variables (OS_AUTH_URL, OS_PROJECT_NAME, ...) - * openstacksdk clouds.yaml via --os-cloud / OS_CLOUD - * explicit --clouds-file + --os-cloud - -Examples: - # List what would be deleted (older than 6h): - ./cleanup_openstack_ci_resources.py --min-age-hours 6 - - # Actually delete: - ./cleanup_openstack_ci_resources.py --min-age-hours 6 --apply - - # Use a named cloud from clouds.yaml: - OS_CLOUD=ci-tenant ./cleanup_openstack_ci_resources.py --apply --min-age-hours 12 -""" - -from __future__ import annotations - -import argparse -import logging -import os -import re -import sys -from dataclasses import dataclass, field -from datetime import datetime, timedelta, timezone -from typing import Any, Callable, Iterable - -try: - import openstack - from openstack.connection import Connection -except ImportError: # pragma: no cover - runtime dependency check - print( - "error: openstacksdk is required. Install with:\n" - " pip install openstacksdk\n" - " # or: apt install python3-openstacksdk", - file=sys.stderr, - ) - sys.exit(2) - -LOG = logging.getLogger("cleanup_openstack_ci") - -# --------------------------------------------------------------------------- -# Name matchers -# -# Derived from integration fixtures / product naming in: -# github-runner-operator (charm + github-runner-manager) -# github-runner-image-builder-operator (charm + app) -# -# Permanent / shared names that production and refits reuse must stay excluded. -# --------------------------------------------------------------------------- - -# Never delete these even if they match something else. -PROTECTED_EXACT_NAMES: frozenset[str] = frozenset( - { - # github-runner-manager shared SG (openstack_cloud._SECURITY_GROUP_NAME) - "github-runner-v1", - # image-builder shared SG (openstack_builder.SHARED_SECURITY_GROUP_NAME) - "github-runner-image-builder-v1", - # default OpenStack groups - "default", - } -) - -# Server / keypair / image name patterns treated as CI leftovers. -# Each pattern is applied to the full resource name. -NAME_PATTERNS: tuple[re.Pattern[str], ...] = tuple( - re.compile(p) - for p in ( - # github-runner-manager integration: TestConfig.vm_prefix = test-runner-{8} - r"^test-runner-[a-z0-9]{6,12}($|-)", - # github-runner charm integration app: test-{8} (+ unit/VM suffix) - r"^test-[a-z0-9]{6,12}($|-)", - # image-builder operator charm app names - r"^image-builder-operator-[0-9a-f]{6,16}($|-)", - r"^image-builder-charmhub-[0-9a-f]{6,16}($|-)", - # GRO deploys image-builder as github-runner-image-builder-{suffix} - r"^github-runner-image-builder-[a-z0-9]{6,12}($|-)", - # image-builder app integration fixtures - r"^test-image-builder-", - # keypair / builder naming: {prefix}-image-builder-... - # 2-char test_id prefixes only accepted when the rest is image-builder-related - r"^[a-z0-9]{2,16}-image-builder-(base-|ssh-key$|test$|[a-z0-9]+-[a-z0-9]+$)", - r"^[a-z0-9]{2,16}-image-builder-[a-z0-9]+-[a-z0-9]+$", - ) -) - -# Extra security-group patterns (not covered cleanly by server-name family). -SG_PATTERNS: tuple[re.Pattern[str], ...] = tuple( - re.compile(p) - for p in ( - r"^github-runner-image-builder-test-security-group-", - r"^github-runner-image-builder-operator-test-security-group", - # manager-test SGs named after vm_prefix if any - r"^test-runner-[a-z0-9]{6,12}-", - r"^test-[a-z0-9]{6,12}-", - ) -) - - -@dataclass -class ResourceHit: - kind: str - name: str - resource_id: str - created_at: datetime | None - age: timedelta | None - detail: str = "" - - -@dataclass -class Stats: - considered: int = 0 - matched: int = 0 - too_young: int = 0 - protected: int = 0 - deleted: int = 0 - failed: int = 0 - details: list[str] = field(default_factory=list) - - -def _parse_openstack_timestamp(value: Any) -> datetime | None: - """Parse OpenStack created_at / updated_at into aware UTC datetime.""" - if value is None: - return None - if isinstance(value, datetime): - if value.tzinfo is None: - return value.replace(tzinfo=timezone.utc) - return value.astimezone(timezone.utc) - text = str(value).strip() - if not text: - return None - # Common forms: 2026-07-16T12:34:56Z / 2026-07-16T12:34:56.123456 - text = text.replace("Z", "+00:00") - # Some APIs omit timezone; treat as UTC. - try: - dt = datetime.fromisoformat(text) - except ValueError: - for fmt in ("%Y-%m-%dT%H:%M:%S", "%Y-%m-%dT%H:%M:%S.%f"): - try: - dt = datetime.strptime(text.split("+")[0], fmt) - break - except ValueError: - continue - else: - LOG.warning("unparseable timestamp %r", value) - return None - if dt.tzinfo is None: - dt = dt.replace(tzinfo=timezone.utc) - return dt.astimezone(timezone.utc) - - -def _name_matches(name: str | None, patterns: Iterable[re.Pattern[str]]) -> bool: - if not name: - return False - if name in PROTECTED_EXACT_NAMES: - return False - return any(p.search(name) for p in patterns) - - -def _connect(args: argparse.Namespace) -> Connection: - kwargs: dict[str, Any] = {} - if args.clouds_file: - kwargs["config_files"] = [args.clouds_file] - if args.os_cloud: - kwargs["cloud"] = args.os_cloud - # openstack.connect() also reads OS_* env vars automatically. - conn = openstack.connect(**kwargs) - # Force auth early so failures are loud. - conn.authorize() - return conn - - -def _age_ok(created: datetime | None, min_age: timedelta, now: datetime) -> tuple[bool, timedelta | None]: - """Return (eligible, age). Missing created_at: treat as eligible (stale leftover).""" - if created is None: - return True, None - age = now - created - return age >= min_age, age - - -def _scan_servers(conn: Connection, min_age: timedelta, now: datetime) -> list[ResourceHit]: - hits: list[ResourceHit] = [] - for server in conn.list_servers(bare=True) or []: - name = getattr(server, "name", None) - if not _name_matches(name, NAME_PATTERNS): - continue - created = _parse_openstack_timestamp(getattr(server, "created_at", None) or getattr(server, "created", None)) - eligible, age = _age_ok(created, min_age, now) - hits.append( - ResourceHit( - kind="server", - name=name or "", - resource_id=str(server.id), - created_at=created, - age=age, - detail=f"status={getattr(server, 'status', '?')}" + ("" if eligible else " [TOO_YOUNG]"), - ) - ) - if not eligible: - hits[-1].detail = hits[-1].detail # marker used below - return hits - - -def _scan_keypairs(conn: Connection, min_age: timedelta, now: datetime) -> list[ResourceHit]: - # Keypairs often lack created_at. Unknown-age keypairs are filtered later - # (skipped unless --aggressive-keypairs or --min-age-hours 0). - hits: list[ResourceHit] = [] - for key in conn.list_keypairs() or []: - name = getattr(key, "name", None) - if not _name_matches(name, NAME_PATTERNS): - continue - created = _parse_openstack_timestamp(getattr(key, "created_at", None)) - eligible, age = _age_ok(created, min_age, now) - hits.append( - ResourceHit( - kind="keypair", - name=name or "", - resource_id=name or "", - created_at=created, - age=age, - detail="" if eligible else "[TOO_YOUNG]", - ) - ) - return hits - - -def _scan_images(conn: Connection, min_age: timedelta, now: datetime) -> list[ResourceHit]: - hits: list[ResourceHit] = [] - # Prefer project-owned images; list_images may include public base images. - for image in conn.list_images() or []: - name = getattr(image, "name", None) - # Do not touch public/shared marketplace images even if name collides. - visibility = str(getattr(image, "visibility", "") or "").lower() - if visibility in {"public", "community"}: - continue - if not _name_matches(name, NAME_PATTERNS): - continue - created = _parse_openstack_timestamp(getattr(image, "created_at", None)) - eligible, age = _age_ok(created, min_age, now) - hits.append( - ResourceHit( - kind="image", - name=name or "", - resource_id=str(image.id), - created_at=created, - age=age, - detail=f"status={getattr(image, 'status', '?')} visibility={visibility or '?'}" - + ("" if eligible else " [TOO_YOUNG]"), - ) - ) - return hits - - -def _scan_security_groups(conn: Connection, min_age: timedelta, now: datetime) -> list[ResourceHit]: - hits: list[ResourceHit] = [] - for sg in conn.list_security_groups() or []: - name = getattr(sg, "name", None) - if not name or name in PROTECTED_EXACT_NAMES: - continue - if not _name_matches(name, SG_PATTERNS) and not _name_matches(name, NAME_PATTERNS): - continue - created = _parse_openstack_timestamp(getattr(sg, "created_at", None)) - eligible, age = _age_ok(created, min_age, now) - hits.append( - ResourceHit( - kind="security_group", - name=name, - resource_id=str(sg.id), - created_at=created, - age=age, - detail="" if eligible else "[TOO_YOUNG]", - ) - ) - return hits - - -def _format_age(age: timedelta | None) -> str: - if age is None: - return "age=unknown" - hours = age.total_seconds() / 3600.0 - if hours < 48: - return f"age={hours:.1f}h" - return f"age={hours / 24.0:.1f}d" - - -def _delete_hit(conn: Connection, hit: ResourceHit, wait_timeout: int) -> None: - if hit.kind == "server": - LOG.info("deleting server %s (%s)", hit.name, hit.resource_id) - ok = conn.delete_server(hit.resource_id, wait=True, timeout=wait_timeout) - if not ok: - # Some SDK versions return False if already gone. - if conn.get_server(hit.resource_id): - raise RuntimeError(f"delete_server returned False for {hit.resource_id}") - elif hit.kind == "keypair": - LOG.info("deleting keypair %s", hit.name) - conn.delete_keypair(hit.name) - elif hit.kind == "image": - LOG.info("deleting image %s (%s)", hit.name, hit.resource_id) - conn.delete_image(hit.resource_id, wait=True) - elif hit.kind == "security_group": - LOG.info("deleting security_group %s (%s)", hit.name, hit.resource_id) - conn.delete_security_group(hit.resource_id) - else: - raise ValueError(f"unknown kind {hit.kind}") - - -def _filter_eligible( - hits: list[ResourceHit], - min_age: timedelta, - now: datetime, - aggressive_keypairs: bool, -) -> tuple[list[ResourceHit], list[ResourceHit]]: - """Split into (to_delete, skipped_young_or_policy).""" - to_delete: list[ResourceHit] = [] - skipped: list[ResourceHit] = [] - for hit in hits: - # Unknown age keypairs: only with aggressive flag, or when min_age is 0. - if hit.kind == "keypair" and hit.created_at is None: - if aggressive_keypairs or min_age.total_seconds() <= 0: - to_delete.append(hit) - else: - hit.detail = (hit.detail + " [SKIP_UNKNOWN_AGE_KEYPAIR]").strip() - skipped.append(hit) - continue - eligible, age = _age_ok(hit.created_at, min_age, now) - hit.age = age - if eligible: - to_delete.append(hit) - else: - hit.detail = (hit.detail + " [TOO_YOUNG]").strip() - skipped.append(hit) - return to_delete, skipped - - -def build_parser() -> argparse.ArgumentParser: - p = argparse.ArgumentParser( - description=( - "Cleanup dangling OpenStack CI resources from github-runner and " - "github-runner-image-builder integration tests." - ), - formatter_class=argparse.RawDescriptionHelpFormatter, - epilog=__doc__, - ) - p.add_argument( - "--apply", - action="store_true", - help="Actually delete matched resources. Without this flag, only list them.", - ) - p.add_argument( - "--min-age-hours", - type=float, - default=6.0, - help=( - "Only touch resources older than this many hours " - "(default: 6). Avoids racing active CI. Keypairs without timestamps " - "are skipped unless --aggressive-keypairs or min-age is 0." - ), - ) - p.add_argument( - "--aggressive-keypairs", - action="store_true", - help="Delete matching keypairs even when created_at is unavailable.", - ) - p.add_argument( - "--kinds", - default="server,keypair,image,security_group", - help="Comma-separated resource kinds to consider (default: all).", - ) - p.add_argument( - "--os-cloud", - default=os.environ.get("OS_CLOUD"), - help="Cloud name from clouds.yaml (or OS_CLOUD).", - ) - p.add_argument( - "--clouds-file", - default=None, - help="Path to clouds.yaml if not in the default search path.", - ) - p.add_argument( - "--wait-timeout", - type=int, - default=300, - help="Seconds to wait for server/image delete (default: 300).", - ) - p.add_argument( - "--max-deletes", - type=int, - default=0, - help="Safety cap on deletes this run (0 = unlimited).", - ) - p.add_argument( - "-v", - "--verbose", - action="count", - default=0, - help="Increase log verbosity (-v, -vv).", - ) - p.add_argument( - "--json-summary", - action="store_true", - help="Print a machine-readable summary line on stdout at the end.", - ) - return p - - -def main(argv: list[str] | None = None) -> int: - args = build_parser().parse_args(argv) - logging.basicConfig( - level=logging.DEBUG if args.verbose >= 2 else logging.INFO if args.verbose else logging.WARNING, - format="%(asctime)s %(levelname)s %(message)s", - datefmt="%Y-%m-%dT%H:%M:%S", - ) - # Always show cleanup decisions at INFO when not quiet — promote default to INFO. - if args.verbose == 0: - logging.getLogger().setLevel(logging.INFO) - - kinds = {k.strip() for k in args.kinds.split(",") if k.strip()} - valid = {"server", "keypair", "image", "security_group"} - unknown = kinds - valid - if unknown: - LOG.error("unknown --kinds values: %s (valid: %s)", unknown, sorted(valid)) - return 2 - - min_age = timedelta(hours=args.min_age_hours) - now = datetime.now(tz=timezone.utc) - - try: - conn = _connect(args) - except Exception as exc: # noqa: BLE001 - top-level auth failure - LOG.error("failed to connect to OpenStack: %s", exc) - return 2 - - project = None - try: - project = conn.current_project_id - except Exception: # noqa: BLE001 - pass - LOG.info( - "connected project_id=%s cloud=%s apply=%s min_age_hours=%s kinds=%s", - project, - args.os_cloud or "(env/default)", - args.apply, - args.min_age_hours, - ",".join(sorted(kinds)), - ) - - scanners: dict[str, Callable[[Connection, timedelta, datetime], list[ResourceHit]]] = { - "server": _scan_servers, - "keypair": _scan_keypairs, - "image": _scan_images, - "security_group": _scan_security_groups, - } - - all_hits: list[ResourceHit] = [] - for kind in ("server", "keypair", "image", "security_group"): - if kind not in kinds: - continue - try: - hits = scanners[kind](conn, min_age, now) - except Exception as exc: # noqa: BLE001 - LOG.error("failed listing %s: %s", kind, exc) - return 1 - all_hits.extend(hits) - - # Delete order: servers first (may hold SG/keypair refs), then images, keypairs, SGs. - order = {"server": 0, "image": 1, "keypair": 2, "security_group": 3} - all_hits.sort(key=lambda h: (order.get(h.kind, 9), h.name)) - - to_delete, skipped = _filter_eligible( - all_hits, - min_age=min_age, - now=now, - aggressive_keypairs=args.aggressive_keypairs, - ) - - stats = Stats(considered=len(all_hits), matched=len(all_hits)) - - for hit in skipped: - stats.too_young += 1 - LOG.info( - "SKIP %s name=%s id=%s %s %s", - hit.kind, - hit.name, - hit.resource_id, - _format_age(hit.age), - hit.detail, - ) - - if args.max_deletes and len(to_delete) > args.max_deletes: - LOG.warning( - "truncating candidate list from %s to --max-deletes=%s", - len(to_delete), - args.max_deletes, - ) - to_delete = to_delete[: args.max_deletes] - - mode = "DELETE" if args.apply else "DRY-RUN" - for hit in to_delete: - LOG.info( - "%s %s name=%s id=%s %s %s", - mode, - hit.kind, - hit.name, - hit.resource_id, - _format_age(hit.age), - hit.detail, - ) - if not args.apply: - continue - try: - _delete_hit(conn, hit, wait_timeout=args.wait_timeout) - stats.deleted += 1 - except Exception as exc: # noqa: BLE001 - stats.failed += 1 - LOG.error( - "FAILED %s name=%s id=%s: %s", - hit.kind, - hit.name, - hit.resource_id, - exc, - ) - - LOG.info( - "summary considered=%s candidates=%s skipped_young_or_policy=%s deleted=%s failed=%s apply=%s", - stats.considered, - len(to_delete), - stats.too_young, - stats.deleted, - stats.failed, - args.apply, - ) - if args.json_summary: - import json - - print( - json.dumps( - { - "project_id": project, - "apply": args.apply, - "min_age_hours": args.min_age_hours, - "considered": stats.considered, - "candidates": len(to_delete), - "skipped": stats.too_young, - "deleted": stats.deleted, - "failed": stats.failed, - "candidates_detail": [ - { - "kind": h.kind, - "name": h.name, - "id": h.resource_id, - "age_seconds": None if h.age is None else h.age.total_seconds(), - } - for h in to_delete - ], - } - ) - ) - - try: - conn.close() - except Exception: # noqa: BLE001 - pass - - return 1 if stats.failed else 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 0d0714de6..5378e323d 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -444,6 +444,10 @@ def openstack_connection_fixture( clouds_yaml_path.write_text(data=openstack_config.clouds_yaml_contents, encoding="utf-8") first_cloud = next(iter(clouds_yaml["clouds"].keys())) with openstack.connect(first_cloud) as connection: + # Reclaim leftovers from force-cancelled previous CI runs before creating new ones. + from tests.integration.helpers.orphan_cleanup import cleanup_stale_openstack_resources + + cleanup_stale_openstack_resources(connection) yield connection servers = connection.list_servers(filters={"name": app_name}) diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py new file mode 100644 index 000000000..a928d71e2 --- /dev/null +++ b/tests/integration/helpers/orphan_cleanup.py @@ -0,0 +1,162 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Delete stale OpenStack resources left by interrupted CI integration runs. + +Called at the start of integration tests so force-cancelled previous jobs still +get cleaned up the next time a suite runs against the tenant. +""" + +from __future__ import annotations + +import logging +import re +from datetime import datetime, timedelta, timezone +from typing import Iterable + +from openstack.connection import Connection + +logger = logging.getLogger(__name__) + +# Permanent SGs used by runtime code — never touch. +_PROTECTED_NAMES = frozenset( + { + "github-runner-v1", + "github-runner-image-builder-v1", + "default", + } +) + +# CI leftovers from this repository's charm + manager integration tests. +_NAME_PATTERNS: tuple[re.Pattern[str], ...] = tuple( + re.compile(p) + for p in ( + r"^test-runner-[a-z0-9]{6,12}($|-)", # manager TestConfig.vm_prefix + r"^test-[a-z0-9]{6,12}($|-)", # charm app_name + r"^github-runner-image-builder-[a-z0-9]{6,12}($|-)", # image-builder deploy in charm IT + ) +) + +_SG_PATTERNS: tuple[re.Pattern[str], ...] = tuple( + re.compile(p) + for p in ( + r"^test-runner-[a-z0-9]{6,12}-", + r"^test-[a-z0-9]{6,12}-", + ) +) + + +def _parse_created_at(value: object) -> datetime | None: + if value is None: + return None + if isinstance(value, datetime): + return value if value.tzinfo else value.replace(tzinfo=timezone.utc) + text = str(value).strip().replace("Z", "+00:00") + if not text: + return None + try: + dt = datetime.fromisoformat(text) + except ValueError: + return None + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) + return dt.astimezone(timezone.utc) + + +def _matches(name: str | None, patterns: Iterable[re.Pattern[str]]) -> bool: + if not name or name in _PROTECTED_NAMES: + return False + return any(p.search(name) for p in patterns) + + +def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: + """True if resource should be deleted. + + Missing created_at: treat as stale (keypair API often lacks timestamps). + """ + created = _parse_created_at(created_at) + if created is None: + return True + return now - created >= min_age + + +def _safe_delete(label: str, name: str, delete_fn) -> None: + try: + delete_fn() + logger.info("Orphan cleanup deleted %s %s", label, name) + except Exception as exc: # noqa: BLE001 - best-effort, already-gone is fine + logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc) + + +def cleanup_stale_openstack_resources( + connection: Connection, + min_age: timedelta = timedelta(hours=6), +) -> None: + """Remove CI-named OpenStack resources older than ``min_age``. + + Order: servers (may hold SG/keypair refs) → images → keypairs → security groups. + """ + now = datetime.now(tz=timezone.utc) + logger.info( + "OpenStack orphan cleanup starting (min_age=%sh)", + min_age.total_seconds() / 3600.0, + ) + + for server in connection.list_servers(bare=True) or []: + name = getattr(server, "name", None) + if not _matches(name, _NAME_PATTERNS): + continue + if not _is_stale( + getattr(server, "created_at", None) or getattr(server, "created", None), + min_age, + now, + ): + continue + _safe_delete( + "server", + name or server.id, + lambda s=server: connection.delete_server(s.id, wait=False), + ) + + for image in connection.list_images() or []: + name = getattr(image, "name", None) + visibility = str(getattr(image, "visibility", "") or "").lower() + if visibility in {"public", "community"}: + continue + if not _matches(name, _NAME_PATTERNS): + continue + if not _is_stale(getattr(image, "created_at", None), min_age, now): + continue + _safe_delete( + "image", + name or image.id, + lambda im=image: connection.delete_image(im.id, wait=False), + ) + + for keypair in connection.list_keypairs() or []: + name = getattr(keypair, "name", None) + if not _matches(name, _NAME_PATTERNS): + continue + if not _is_stale(getattr(keypair, "created_at", None), min_age, now): + continue + _safe_delete( + "keypair", + name or "", + lambda n=name: connection.delete_keypair(n), + ) + + for sg in connection.list_security_groups() or []: + name = getattr(sg, "name", None) + if name in _PROTECTED_NAMES: + continue + if not (_matches(name, _SG_PATTERNS) or _matches(name, _NAME_PATTERNS)): + continue + if not _is_stale(getattr(sg, "created_at", None), min_age, now): + continue + _safe_delete( + "security_group", + name or sg.id, + lambda g=sg: connection.delete_security_group(g.id), + ) + + logger.info("OpenStack orphan cleanup finished") From 0e63df0ef422ae171aed003bcf5666caaeccd8fb Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 16 Jul 2026 13:56:39 +0000 Subject: [PATCH 03/20] refactor: step-down orphan cleanup helpers Public cleanup entry point first; private helpers below. Module-level imports in conftest instead of inline. --- .../tests/integration/conftest.py | 3 +- .../tests/integration/orphan_cleanup.py | 78 ++++++++--------- tests/integration/conftest.py | 3 +- tests/integration/helpers/orphan_cleanup.py | 86 +++++++++---------- 4 files changed, 84 insertions(+), 86 deletions(-) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index c665aee5c..3bf76c2d2 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -17,6 +17,7 @@ from github.Repository import Repository from .factories import GitHubConfig, OpenStackConfig, ProxyConfig, TestConfig +from .orphan_cleanup import cleanup_stale_openstack_resources from .planner_stub import PlannerStub, PlannerStubConfig logger = logging.getLogger(__name__) @@ -183,8 +184,6 @@ def openstack_connection( region_name=openstack_config.region_name, ) # Reclaim leftovers from force-cancelled previous CI runs before creating new ones. - from .orphan_cleanup import cleanup_stale_openstack_resources - cleanup_stale_openstack_resources(conn) yield conn conn.close() diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index 7bb901f61..69f1a1aa6 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -8,7 +8,7 @@ import logging import re from datetime import datetime, timedelta, timezone -from typing import Iterable +from collections.abc import Callable, Iterable from openstack.connection import Connection @@ -26,44 +26,6 @@ ) -def _parse_created_at(value: object) -> datetime | None: - if value is None: - return None - if isinstance(value, datetime): - return value if value.tzinfo else value.replace(tzinfo=timezone.utc) - text = str(value).strip().replace("Z", "+00:00") - if not text: - return None - try: - dt = datetime.fromisoformat(text) - except ValueError: - return None - if dt.tzinfo is None: - dt = dt.replace(tzinfo=timezone.utc) - return dt.astimezone(timezone.utc) - - -def _matches(name: str | None, patterns: Iterable[re.Pattern[str]]) -> bool: - if not name or name in _PROTECTED_NAMES: - return False - return any(p.search(name) for p in patterns) - - -def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: - created = _parse_created_at(created_at) - if created is None: - return True - return now - created >= min_age - - -def _safe_delete(label: str, name: str, delete_fn) -> None: - try: - delete_fn() - logger.info("Orphan cleanup deleted %s %s", label, name) - except Exception as exc: # noqa: BLE001 - logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc) - - def cleanup_stale_openstack_resources( connection: Connection, min_age: timedelta = timedelta(hours=6), @@ -114,3 +76,41 @@ def cleanup_stale_openstack_resources( ) logger.info("OpenStack orphan cleanup finished") + + +def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None: + try: + delete_fn() + logger.info("Orphan cleanup deleted %s %s", label, name) + except Exception as exc: # noqa: BLE001 + logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc) + + +def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: + created = _parse_created_at(created_at) + if created is None: + return True + return now - created >= min_age + + +def _matches(name: str | None, patterns: Iterable[re.Pattern[str]]) -> bool: + if not name or name in _PROTECTED_NAMES: + return False + return any(p.search(name) for p in patterns) + + +def _parse_created_at(value: object) -> datetime | None: + if value is None: + return None + if isinstance(value, datetime): + return value if value.tzinfo else value.replace(tzinfo=timezone.utc) + text = str(value).strip().replace("Z", "+00:00") + if not text: + return None + try: + dt = datetime.fromisoformat(text) + except ValueError: + return None + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) + return dt.astimezone(timezone.utc) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 5378e323d..741987dfa 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -43,6 +43,7 @@ wait_for_runner_ready, ) from tests.integration.helpers.openstack import OpenStackInstanceHelper +from tests.integration.helpers.orphan_cleanup import cleanup_stale_openstack_resources DEFAULT_RECONCILE_INTERVAL = 2 @@ -445,8 +446,6 @@ def openstack_connection_fixture( first_cloud = next(iter(clouds_yaml["clouds"].keys())) with openstack.connect(first_cloud) as connection: # Reclaim leftovers from force-cancelled previous CI runs before creating new ones. - from tests.integration.helpers.orphan_cleanup import cleanup_stale_openstack_resources - cleanup_stale_openstack_resources(connection) yield connection diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index a928d71e2..99d581af6 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -12,7 +12,7 @@ import logging import re from datetime import datetime, timedelta, timezone -from typing import Iterable +from collections.abc import Callable, Iterable from openstack.connection import Connection @@ -46,48 +46,6 @@ ) -def _parse_created_at(value: object) -> datetime | None: - if value is None: - return None - if isinstance(value, datetime): - return value if value.tzinfo else value.replace(tzinfo=timezone.utc) - text = str(value).strip().replace("Z", "+00:00") - if not text: - return None - try: - dt = datetime.fromisoformat(text) - except ValueError: - return None - if dt.tzinfo is None: - dt = dt.replace(tzinfo=timezone.utc) - return dt.astimezone(timezone.utc) - - -def _matches(name: str | None, patterns: Iterable[re.Pattern[str]]) -> bool: - if not name or name in _PROTECTED_NAMES: - return False - return any(p.search(name) for p in patterns) - - -def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: - """True if resource should be deleted. - - Missing created_at: treat as stale (keypair API often lacks timestamps). - """ - created = _parse_created_at(created_at) - if created is None: - return True - return now - created >= min_age - - -def _safe_delete(label: str, name: str, delete_fn) -> None: - try: - delete_fn() - logger.info("Orphan cleanup deleted %s %s", label, name) - except Exception as exc: # noqa: BLE001 - best-effort, already-gone is fine - logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc) - - def cleanup_stale_openstack_resources( connection: Connection, min_age: timedelta = timedelta(hours=6), @@ -160,3 +118,45 @@ def cleanup_stale_openstack_resources( ) logger.info("OpenStack orphan cleanup finished") + + +def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None: + try: + delete_fn() + logger.info("Orphan cleanup deleted %s %s", label, name) + except Exception as exc: # noqa: BLE001 - best-effort, already-gone is fine + logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc) + + +def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: + """True if resource should be deleted. + + Missing created_at: treat as stale (keypair API often lacks timestamps). + """ + created = _parse_created_at(created_at) + if created is None: + return True + return now - created >= min_age + + +def _matches(name: str | None, patterns: Iterable[re.Pattern[str]]) -> bool: + if not name or name in _PROTECTED_NAMES: + return False + return any(p.search(name) for p in patterns) + + +def _parse_created_at(value: object) -> datetime | None: + if value is None: + return None + if isinstance(value, datetime): + return value if value.tzinfo else value.replace(tzinfo=timezone.utc) + text = str(value).strip().replace("Z", "+00:00") + if not text: + return None + try: + dt = datetime.fromisoformat(text) + except ValueError: + return None + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) + return dt.astimezone(timezone.utc) From 9db642429b6518893a29a84d3e0ab879808f9509 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 16 Jul 2026 14:03:51 +0000 Subject: [PATCH 04/20] fix: skip orphan cleanup when resource age is unknown Keypairs often lack created_at; treating unknown age as stale risked deleting in-progress CI resources. Skip those instead. --- github-runner-manager/tests/integration/orphan_cleanup.py | 7 ++++++- tests/integration/helpers/orphan_cleanup.py | 5 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index 69f1a1aa6..d7e0c2741 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -87,9 +87,14 @@ def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: + """True if resource is dated and older than ``min_age``. + + Missing/unparseable created_at: skip. Unknown age must not delete + concurrent or in-progress CI resources (e.g. keypairs without timestamps). + """ created = _parse_created_at(created_at) if created is None: - return True + return False return now - created >= min_age diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index 99d581af6..45a2127bc 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -131,11 +131,12 @@ def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: """True if resource should be deleted. - Missing created_at: treat as stale (keypair API often lacks timestamps). + Missing/unparseable created_at: skip (not stale). Keypair APIs often lack + timestamps; treating unknown as stale can delete in-progress CI resources. """ created = _parse_created_at(created_at) if created is None: - return True + return False return now - created >= min_age From c92d80940c552470b2b1ba06c26658b3f6e8e3d2 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 16 Jul 2026 14:35:23 +0000 Subject: [PATCH 05/20] fix: address Copilot round-2 orphan cleanup nits Wait for server delete before SGs, keep OpenStack teardown inside the connection context, best-effort start-up reaping, and tighten test-id regexes to the 8-char CI ids we generate. --- .../tests/integration/conftest.py | 5 ++- .../tests/integration/orphan_cleanup.py | 6 +-- tests/integration/conftest.py | 37 ++++++++++--------- tests/integration/helpers/orphan_cleanup.py | 12 +++--- 4 files changed, 33 insertions(+), 27 deletions(-) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index 3bf76c2d2..a19cc4779 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -184,7 +184,10 @@ def openstack_connection( region_name=openstack_config.region_name, ) # Reclaim leftovers from force-cancelled previous CI runs before creating new ones. - cleanup_stale_openstack_resources(conn) + try: + cleanup_stale_openstack_resources(conn) + except Exception as exc: # noqa: BLE001 - best-effort hygiene must not block the suite + logger.warning("OpenStack orphan cleanup failed: %s", exc) yield conn conn.close() diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index d7e0c2741..63d115470 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -18,11 +18,11 @@ # Matches TestConfig.vm_prefix = test-runner-{8 alnum} _NAME_PATTERNS: tuple[re.Pattern[str], ...] = ( - re.compile(r"^test-runner-[a-z0-9]{6,12}($|-)"), + re.compile(r"^test-runner-[a-z0-9]{8}($|-)"), ) _SG_PATTERNS: tuple[re.Pattern[str], ...] = ( - re.compile(r"^test-runner-[a-z0-9]{6,12}-"), + re.compile(r"^test-runner-[a-z0-9]{8}-"), ) @@ -50,7 +50,7 @@ def cleanup_stale_openstack_resources( _safe_delete( "server", name or server.id, - lambda s=server: connection.delete_server(s.id, wait=False), + lambda s=server: connection.delete_server(s.id, wait=True), ) for keypair in connection.list_keypairs() or []: diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 741987dfa..7440d4e70 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -446,26 +446,29 @@ def openstack_connection_fixture( first_cloud = next(iter(clouds_yaml["clouds"].keys())) with openstack.connect(first_cloud) as connection: # Reclaim leftovers from force-cancelled previous CI runs before creating new ones. - cleanup_stale_openstack_resources(connection) + try: + cleanup_stale_openstack_resources(connection) + except Exception as exc: # noqa: BLE001 - best-effort hygiene must not block the suite + logging.warning("OpenStack orphan cleanup failed: %s", exc) yield connection - servers = connection.list_servers(filters={"name": app_name}) + servers = connection.list_servers(filters={"name": app_name}) - if request.session.testsfailed: - logging.info("OpenStack servers: %s", servers) - for server in servers: - console_log = connection.get_server_console(server=server) - logging.info("Server %s console log:\n%s", server.name, console_log) - - if not existing_app_suffix: - for server in servers: - server_name: str = server.name - if server_name.startswith(app_name): - connection.delete_server(server_name) - for key in connection.list_keypairs(): - key_name: str = key.name - if key_name.startswith(app_name): - connection.delete_keypair(key_name) + if request.session.testsfailed: + logging.info("OpenStack servers: %s", servers) + for server in servers: + console_log = connection.get_server_console(server=server) + logging.info("Server %s console log:\n%s", server.name, console_log) + + if not existing_app_suffix: + for server in servers: + server_name: str = server.name + if server_name.startswith(app_name): + connection.delete_server(server_name) + for key in connection.list_keypairs(): + key_name: str = key.name + if key_name.startswith(app_name): + connection.delete_keypair(key_name) @pytest.fixture(scope="module") diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index 45a2127bc..a2292620c 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -31,17 +31,17 @@ _NAME_PATTERNS: tuple[re.Pattern[str], ...] = tuple( re.compile(p) for p in ( - r"^test-runner-[a-z0-9]{6,12}($|-)", # manager TestConfig.vm_prefix - r"^test-[a-z0-9]{6,12}($|-)", # charm app_name - r"^github-runner-image-builder-[a-z0-9]{6,12}($|-)", # image-builder deploy in charm IT + r"^test-runner-[a-z0-9]{8}($|-)", # manager TestConfig.vm_prefix + r"^test-[a-z0-9]{8}($|-)", # charm app_name + r"^github-runner-image-builder-[a-z0-9]{8}($|-)", # image-builder deploy in charm IT ) ) _SG_PATTERNS: tuple[re.Pattern[str], ...] = tuple( re.compile(p) for p in ( - r"^test-runner-[a-z0-9]{6,12}-", - r"^test-[a-z0-9]{6,12}-", + r"^test-runner-[a-z0-9]{8}-", + r"^test-[a-z0-9]{8}-", ) ) @@ -73,7 +73,7 @@ def cleanup_stale_openstack_resources( _safe_delete( "server", name or server.id, - lambda s=server: connection.delete_server(s.id, wait=False), + lambda s=server: connection.delete_server(s.id, wait=True), ) for image in connection.list_images() or []: From 7e74041dd658ee62b6840ba60447b91864567b7a Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 16 Jul 2026 14:41:22 +0000 Subject: [PATCH 06/20] fix: delete servers by id and keypairs by name= Align OpenStack teardown/orphan helpers with existing integration suite call conventions. --- github-runner-manager/tests/integration/orphan_cleanup.py | 2 +- tests/integration/conftest.py | 4 ++-- tests/integration/helpers/orphan_cleanup.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index 63d115470..9dc1132bd 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -59,7 +59,7 @@ def cleanup_stale_openstack_resources( continue if not _is_stale(getattr(keypair, "created_at", None), min_age, now): continue - _safe_delete("keypair", name or "", lambda n=name: connection.delete_keypair(n)) + _safe_delete("keypair", name or "", lambda n=name: connection.delete_keypair(name=n)) for sg in connection.list_security_groups() or []: name = getattr(sg, "name", None) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 7440d4e70..be8fcfcc4 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -464,11 +464,11 @@ def openstack_connection_fixture( for server in servers: server_name: str = server.name if server_name.startswith(app_name): - connection.delete_server(server_name) + connection.delete_server(server.id, wait=True) for key in connection.list_keypairs(): key_name: str = key.name if key_name.startswith(app_name): - connection.delete_keypair(key_name) + connection.delete_keypair(name=key_name) @pytest.fixture(scope="module") diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index a2292620c..7f29f519d 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -100,7 +100,7 @@ def cleanup_stale_openstack_resources( _safe_delete( "keypair", name or "", - lambda n=name: connection.delete_keypair(n), + lambda n=name: connection.delete_keypair(name=n), ) for sg in connection.list_security_groups() or []: From 9859b86bdd01fe543bd060804b0de774b4846e23 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 16 Jul 2026 14:45:18 +0000 Subject: [PATCH 07/20] fix: call delete_keypair positionally like the rest of GRO Keep suite call style consistent; openstacksdk accepts the name positionally as used elsewhere in this repository. --- github-runner-manager/tests/integration/orphan_cleanup.py | 2 +- tests/integration/conftest.py | 2 +- tests/integration/helpers/orphan_cleanup.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index 9dc1132bd..63d115470 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -59,7 +59,7 @@ def cleanup_stale_openstack_resources( continue if not _is_stale(getattr(keypair, "created_at", None), min_age, now): continue - _safe_delete("keypair", name or "", lambda n=name: connection.delete_keypair(name=n)) + _safe_delete("keypair", name or "", lambda n=name: connection.delete_keypair(n)) for sg in connection.list_security_groups() or []: name = getattr(sg, "name", None) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index be8fcfcc4..a723507a0 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -468,7 +468,7 @@ def openstack_connection_fixture( for key in connection.list_keypairs(): key_name: str = key.name if key_name.startswith(app_name): - connection.delete_keypair(name=key_name) + connection.delete_keypair(key_name) @pytest.fixture(scope="module") diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index 7f29f519d..a2292620c 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -100,7 +100,7 @@ def cleanup_stale_openstack_resources( _safe_delete( "keypair", name or "", - lambda n=name: connection.delete_keypair(name=n), + lambda n=name: connection.delete_keypair(n), ) for sg in connection.list_security_groups() or []: From 482c3889ebea4e9ed59a6556c64603a389ba54b0 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 16 Jul 2026 14:50:09 +0000 Subject: [PATCH 08/20] fix: more Copilot nits for orphan cleanup Drop wait= on image delete (not supported), materialize list_servers, and log orphan cleanup failures with exc_info. --- github-runner-manager/tests/integration/conftest.py | 2 +- github-runner-manager/tests/integration/orphan_cleanup.py | 2 +- tests/integration/conftest.py | 4 ++-- tests/integration/helpers/orphan_cleanup.py | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index a19cc4779..32320615b 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -187,7 +187,7 @@ def openstack_connection( try: cleanup_stale_openstack_resources(conn) except Exception as exc: # noqa: BLE001 - best-effort hygiene must not block the suite - logger.warning("OpenStack orphan cleanup failed: %s", exc) + logger.warning("OpenStack orphan cleanup failed: %s", exc, exc_info=True) yield conn conn.close() diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index 63d115470..144c92b12 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -83,7 +83,7 @@ def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None delete_fn() logger.info("Orphan cleanup deleted %s %s", label, name) except Exception as exc: # noqa: BLE001 - logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc) + logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc, exc_info=True) def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index a723507a0..dea66aac1 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -449,10 +449,10 @@ def openstack_connection_fixture( try: cleanup_stale_openstack_resources(connection) except Exception as exc: # noqa: BLE001 - best-effort hygiene must not block the suite - logging.warning("OpenStack orphan cleanup failed: %s", exc) + logging.warning("OpenStack orphan cleanup failed: %s", exc, exc_info=True) yield connection - servers = connection.list_servers(filters={"name": app_name}) + servers = list(connection.list_servers(filters={"name": app_name}) or []) if request.session.testsfailed: logging.info("OpenStack servers: %s", servers) diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index a2292620c..6b9294bb4 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -88,7 +88,7 @@ def cleanup_stale_openstack_resources( _safe_delete( "image", name or image.id, - lambda im=image: connection.delete_image(im.id, wait=False), + lambda im=image: connection.delete_image(im.id), ) for keypair in connection.list_keypairs() or []: @@ -125,7 +125,7 @@ def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None delete_fn() logger.info("Orphan cleanup deleted %s %s", label, name) except Exception as exc: # noqa: BLE001 - best-effort, already-gone is fine - logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc) + logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc, exc_info=True) def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: From 13bd30bed5f3b22bff5404cff36defa20f29dc8a Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 16 Jul 2026 14:58:17 +0000 Subject: [PATCH 09/20] fix: skip orphan cleanup when reusing existing app --use-existing-app-suffix is for local runs that keep long-lived resources; do not reap them at suite start. --- tests/integration/conftest.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index dea66aac1..5f3a7c36f 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -446,10 +446,13 @@ def openstack_connection_fixture( first_cloud = next(iter(clouds_yaml["clouds"].keys())) with openstack.connect(first_cloud) as connection: # Reclaim leftovers from force-cancelled previous CI runs before creating new ones. - try: - cleanup_stale_openstack_resources(connection) - except Exception as exc: # noqa: BLE001 - best-effort hygiene must not block the suite - logging.warning("OpenStack orphan cleanup failed: %s", exc, exc_info=True) + # Skip when reusing an existing app (local --use-existing-app-suffix); those resources + # may legitimately be older than min_age. + if not existing_app_suffix: + try: + cleanup_stale_openstack_resources(connection) + except Exception as exc: # noqa: BLE001 - best-effort hygiene must not block the suite + logging.warning("OpenStack orphan cleanup failed: %s", exc, exc_info=True) yield connection servers = list(connection.list_servers(filters={"name": app_name}) or []) From 4c71e77576071cb7336c3d5524cf24ea4a53b0d5 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Jul 2026 06:27:48 +0000 Subject: [PATCH 10/20] refactor: share OpenStack CI naming; drop unused orphan bits Single source of truth for test resource names (charm naming.py, manager factories). Orphan cleanup consumes those matchers, no longer redefines regexes, deletes SGs, or uses future annotations. Only servers/images/keypairs that round-trips create are reaped. --- .../tests/integration/factories.py | 54 +++++++++++-- .../tests/integration/orphan_cleanup.py | 56 ++++--------- tests/integration/conftest.py | 14 ++-- tests/integration/helpers/orphan_cleanup.py | 80 ++++--------------- tests/integration/naming.py | 61 ++++++++++++++ 5 files changed, 148 insertions(+), 117 deletions(-) create mode 100644 tests/integration/naming.py diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index d060b6384..da7db8056 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -10,13 +10,55 @@ from typing import Any -def _generate_test_id() -> str: +# Single source of truth for OpenStack resource names in this suite. +# Orphan cleanup imports these so patterns cannot drift from producers. +TEST_ID_LENGTH = 8 +TEST_ID_ALPHABET = string.ascii_lowercase + string.digits +MANAGER_VM_PREFIX = "test-runner-" +MANAGER_RUNNER_NAME_PREFIX = "test-manager-" +# Labels use "test-{id}" (not the openstack prefix); kept as a formatter only. + +OPENSTACK_RESOURCE_PREFIXES: tuple[str, ...] = (MANAGER_VM_PREFIX,) + + +def generate_test_id() -> str: """Generate a unique test identifier. Returns: A random 8-character alphanumeric string. """ - return "".join(secrets.choice(string.ascii_lowercase + string.digits) for _ in range(8)) + return "".join(secrets.choice(TEST_ID_ALPHABET) for _ in range(TEST_ID_LENGTH)) + + +def manager_vm_prefix(test_id: str) -> str: + """OpenStack VM / keypair name prefix for a suite run.""" + return f"{MANAGER_VM_PREFIX}{test_id}" + + +def manager_runner_name(test_id: str) -> str: + """Runner-manager process name for a suite run.""" + return f"{MANAGER_RUNNER_NAME_PREFIX}{test_id}" + + +def manager_labels(test_id: str) -> list[str]: + """Extra GitHub runner labels for a suite run.""" + return [f"test-{test_id}"] + + +def is_manager_openstack_resource_name(name: str | None) -> bool: + """True if *name* is a manager-IT OpenStack resource from this suite's scheme.""" + if not name: + return False + for prefix in OPENSTACK_RESOURCE_PREFIXES: + if not name.startswith(prefix): + continue + rest = name[len(prefix) :] + if not rest: + continue + test_id = rest.split("-", 1)[0] + if len(test_id) == TEST_ID_LENGTH and all(c in TEST_ID_ALPHABET for c in test_id): + return True + return False @dataclass @@ -107,16 +149,16 @@ class TestConfig: """ debug_log_dir: Path = Path("/tmp/github-runner-manager-test-logs") - test_id: str = field(default_factory=_generate_test_id) + test_id: str = field(default_factory=generate_test_id) runner_name: str = field(init=False) labels: list[str] = field(init=False) vm_prefix: str = field(init=False) def __post_init__(self) -> None: """Initialize derived fields based on test_id.""" - self.runner_name = f"test-manager-{self.test_id}" - self.labels = [f"test-{self.test_id}"] - self.vm_prefix = f"test-runner-{self.test_id}" + self.runner_name = manager_runner_name(self.test_id) + self.labels = manager_labels(self.test_id) + self.vm_prefix = manager_vm_prefix(self.test_id) @dataclass diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index 144c92b12..48c143a1c 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -3,34 +3,26 @@ """Delete stale OpenStack resources left by interrupted manager integration runs.""" -from __future__ import annotations - import logging -import re +from collections.abc import Callable from datetime import datetime, timedelta, timezone -from collections.abc import Callable, Iterable from openstack.connection import Connection -logger = logging.getLogger(__name__) - -_PROTECTED_NAMES = frozenset({"github-runner-v1", "default"}) +from .factories import is_manager_openstack_resource_name -# Matches TestConfig.vm_prefix = test-runner-{8 alnum} -_NAME_PATTERNS: tuple[re.Pattern[str], ...] = ( - re.compile(r"^test-runner-[a-z0-9]{8}($|-)"), -) - -_SG_PATTERNS: tuple[re.Pattern[str], ...] = ( - re.compile(r"^test-runner-[a-z0-9]{8}-"), -) +logger = logging.getLogger(__name__) def cleanup_stale_openstack_resources( connection: Connection, min_age: timedelta = timedelta(hours=6), ) -> None: - """Remove manager-IT OpenStack leftovers older than ``min_age``.""" + """Remove manager-IT OpenStack leftovers older than ``min_age``. + + Only servers and keypairs. Security groups are not suite-scoped; runtime + reuses the permanent ``github-runner-v1`` group. + """ now = datetime.now(tz=timezone.utc) logger.info( "OpenStack orphan cleanup starting (min_age=%sh)", @@ -39,7 +31,7 @@ def cleanup_stale_openstack_resources( for server in connection.list_servers(bare=True) or []: name = getattr(server, "name", None) - if not _matches(name, _NAME_PATTERNS): + if not is_manager_openstack_resource_name(name): continue if not _is_stale( getattr(server, "created_at", None) or getattr(server, "created", None), @@ -55,26 +47,12 @@ def cleanup_stale_openstack_resources( for keypair in connection.list_keypairs() or []: name = getattr(keypair, "name", None) - if not _matches(name, _NAME_PATTERNS): + if not is_manager_openstack_resource_name(name): continue if not _is_stale(getattr(keypair, "created_at", None), min_age, now): continue _safe_delete("keypair", name or "", lambda n=name: connection.delete_keypair(n)) - for sg in connection.list_security_groups() or []: - name = getattr(sg, "name", None) - if name in _PROTECTED_NAMES: - continue - if not (_matches(name, _SG_PATTERNS) or _matches(name, _NAME_PATTERNS)): - continue - if not _is_stale(getattr(sg, "created_at", None), min_age, now): - continue - _safe_delete( - "security_group", - name or sg.id, - lambda g=sg: connection.delete_security_group(g.id), - ) - logger.info("OpenStack orphan cleanup finished") @@ -83,14 +61,16 @@ def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None delete_fn() logger.info("Orphan cleanup deleted %s %s", label, name) except Exception as exc: # noqa: BLE001 - logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc, exc_info=True) + logger.warning( + "Orphan cleanup failed deleting %s %s: %s", label, name, exc, exc_info=True + ) def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: """True if resource is dated and older than ``min_age``. - Missing/unparseable created_at: skip. Unknown age must not delete - concurrent or in-progress CI resources (e.g. keypairs without timestamps). + Missing/unparseable created_at: skip. Unknown age must not delete concurrent + or in-progress CI resources (e.g. keypairs without timestamps). """ created = _parse_created_at(created_at) if created is None: @@ -98,12 +78,6 @@ def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: return now - created >= min_age -def _matches(name: str | None, patterns: Iterable[re.Pattern[str]]) -> bool: - if not name or name in _PROTECTED_NAMES: - return False - return any(p.search(name) for p in patterns) - - def _parse_created_at(value: object) -> datetime | None: if value is None: return None diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 5f3a7c36f..c3f9402a9 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -44,6 +44,11 @@ ) from tests.integration.helpers.openstack import OpenStackInstanceHelper from tests.integration.helpers.orphan_cleanup import cleanup_stale_openstack_resources +from tests.integration.naming import ( + app_name_from_suffix, + generate_app_suffix, + image_builder_app_name_from_suffix, +) DEFAULT_RECONCILE_INTERVAL = 2 @@ -280,22 +285,19 @@ def existing_app_suffix(pytestconfig: pytest.Config) -> Optional[str]: def random_app_name_suffix(existing_app_suffix: Optional[str]) -> str: """Randomized application name.""" # Randomized suffix name to avoid collision when runner is connecting to GitHub. - return existing_app_suffix or ( - random.choice(string.ascii_lowercase) - + "".join(random.choices(string.ascii_lowercase + string.digits, k=7)) - ) + return existing_app_suffix or generate_app_suffix() @pytest.fixture(scope="module") def app_name(random_app_name_suffix: str) -> str: """Randomized application name.""" - return f"test-{random_app_name_suffix}" + return app_name_from_suffix(random_app_name_suffix) @pytest.fixture(scope="module") def image_builder_app_name(random_app_name_suffix: str) -> str: """Randomized application name.""" - return f"github-runner-image-builder-{random_app_name_suffix}" + return image_builder_app_name_from_suffix(random_app_name_suffix) @pytest.fixture(scope="module") diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index 6b9294bb4..017ebae4e 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -7,43 +7,15 @@ get cleaned up the next time a suite runs against the tenant. """ -from __future__ import annotations - import logging -import re +from collections.abc import Callable from datetime import datetime, timedelta, timezone -from collections.abc import Callable, Iterable from openstack.connection import Connection -logger = logging.getLogger(__name__) +from tests.integration.naming import is_ci_openstack_resource_name -# Permanent SGs used by runtime code — never touch. -_PROTECTED_NAMES = frozenset( - { - "github-runner-v1", - "github-runner-image-builder-v1", - "default", - } -) - -# CI leftovers from this repository's charm + manager integration tests. -_NAME_PATTERNS: tuple[re.Pattern[str], ...] = tuple( - re.compile(p) - for p in ( - r"^test-runner-[a-z0-9]{8}($|-)", # manager TestConfig.vm_prefix - r"^test-[a-z0-9]{8}($|-)", # charm app_name - r"^github-runner-image-builder-[a-z0-9]{8}($|-)", # image-builder deploy in charm IT - ) -) - -_SG_PATTERNS: tuple[re.Pattern[str], ...] = tuple( - re.compile(p) - for p in ( - r"^test-runner-[a-z0-9]{8}-", - r"^test-[a-z0-9]{8}-", - ) -) +logger = logging.getLogger(__name__) def cleanup_stale_openstack_resources( @@ -52,7 +24,9 @@ def cleanup_stale_openstack_resources( ) -> None: """Remove CI-named OpenStack resources older than ``min_age``. - Order: servers (may hold SG/keypair refs) → images → keypairs → security groups. + Order: servers (hold keypairs/refs) → images → keypairs. + Security groups are not touched: runtime uses a permanent docker-project + group (``github-runner-v1``) that is get-or-create, never suite-scoped. """ now = datetime.now(tz=timezone.utc) logger.info( @@ -62,7 +36,7 @@ def cleanup_stale_openstack_resources( for server in connection.list_servers(bare=True) or []: name = getattr(server, "name", None) - if not _matches(name, _NAME_PATTERNS): + if not is_ci_openstack_resource_name(name): continue if not _is_stale( getattr(server, "created_at", None) or getattr(server, "created", None), @@ -81,7 +55,7 @@ def cleanup_stale_openstack_resources( visibility = str(getattr(image, "visibility", "") or "").lower() if visibility in {"public", "community"}: continue - if not _matches(name, _NAME_PATTERNS): + if not is_ci_openstack_resource_name(name): continue if not _is_stale(getattr(image, "created_at", None), min_age, now): continue @@ -93,29 +67,11 @@ def cleanup_stale_openstack_resources( for keypair in connection.list_keypairs() or []: name = getattr(keypair, "name", None) - if not _matches(name, _NAME_PATTERNS): + if not is_ci_openstack_resource_name(name): continue if not _is_stale(getattr(keypair, "created_at", None), min_age, now): continue - _safe_delete( - "keypair", - name or "", - lambda n=name: connection.delete_keypair(n), - ) - - for sg in connection.list_security_groups() or []: - name = getattr(sg, "name", None) - if name in _PROTECTED_NAMES: - continue - if not (_matches(name, _SG_PATTERNS) or _matches(name, _NAME_PATTERNS)): - continue - if not _is_stale(getattr(sg, "created_at", None), min_age, now): - continue - _safe_delete( - "security_group", - name or sg.id, - lambda g=sg: connection.delete_security_group(g.id), - ) + _safe_delete("keypair", name or "", lambda n=name: connection.delete_keypair(n)) logger.info("OpenStack orphan cleanup finished") @@ -125,14 +81,16 @@ def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None delete_fn() logger.info("Orphan cleanup deleted %s %s", label, name) except Exception as exc: # noqa: BLE001 - best-effort, already-gone is fine - logger.warning("Orphan cleanup failed deleting %s %s: %s", label, name, exc, exc_info=True) + logger.warning( + "Orphan cleanup failed deleting %s %s: %s", label, name, exc, exc_info=True + ) def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: - """True if resource should be deleted. + """True if resource is dated and older than ``min_age``. - Missing/unparseable created_at: skip (not stale). Keypair APIs often lack - timestamps; treating unknown as stale can delete in-progress CI resources. + Missing/unparseable created_at: skip. Unknown age must not delete concurrent + or in-progress CI resources (e.g. keypairs without timestamps). """ created = _parse_created_at(created_at) if created is None: @@ -140,12 +98,6 @@ def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: return now - created >= min_age -def _matches(name: str | None, patterns: Iterable[re.Pattern[str]]) -> bool: - if not name or name in _PROTECTED_NAMES: - return False - return any(p.search(name) for p in patterns) - - def _parse_created_at(value: object) -> datetime | None: if value is None: return None diff --git a/tests/integration/naming.py b/tests/integration/naming.py new file mode 100644 index 000000000..285890680 --- /dev/null +++ b/tests/integration/naming.py @@ -0,0 +1,61 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Shared naming for charm integration tests and OpenStack orphan cleanup. + +Producers (fixtures) and consumers (orphan cleanup) both import from here so +resource name formats cannot drift. +""" + +import random +import string + +# Suffixes for app names are 8 characters: 1 lowercase letter + 7 alnum +# (see random_app_name_suffix historical shape). +TEST_ID_LENGTH = 8 +TEST_ID_ALPHABET = string.ascii_lowercase + string.digits + +# Longest-first matters for matching: "test-runner-" before "test-". +OPENSTACK_RESOURCE_PREFIXES: tuple[str, ...] = ( + "github-runner-image-builder-", # image builder charm app in charm IT + "test-runner-", # github-runner-manager TestConfig.vm_prefix (shared tenant) + "test-", # github-runner charm app_name +) + + +def generate_app_suffix() -> str: + """Return a fresh 8-char application name suffix for a suite run.""" + return random.choice(string.ascii_lowercase) + "".join( + random.choices(TEST_ID_ALPHABET, k=TEST_ID_LENGTH - 1) + ) + + +def app_name_from_suffix(suffix: str) -> str: + """github-runner application name for a given suffix.""" + return f"test-{suffix}" + + +def image_builder_app_name_from_suffix(suffix: str) -> str: + """github-runner-image-builder application name for a given suffix.""" + return f"github-runner-image-builder-{suffix}" + + +def is_ci_openstack_resource_name(name: str | None) -> bool: + """True if *name* matches a known CI/OpenStack resource prefix + test id. + + A resource is considered CI-created when it starts with one of + :data:`OPENSTACK_RESOURCE_PREFIXES` followed by an 8-char test id, optionally + with further ``-...`` segments (InstanceID-style suffixes). + """ + if not name: + return False + for prefix in OPENSTACK_RESOURCE_PREFIXES: + if not name.startswith(prefix): + continue + rest = name[len(prefix) :] + if not rest: + continue + test_id = rest.split("-", 1)[0] + if len(test_id) == TEST_ID_LENGTH and all(c in TEST_ID_ALPHABET for c in test_id): + return True + return False From c68d8abc56235a22fce1ae7763bd6c78ebabca20 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Jul 2026 07:50:05 +0000 Subject: [PATCH 11/20] docs: spell out orphan cleanup suite scopes Replace fuzzy shorthand (manager-IT, CI-named) with explicit references to github-runner-manager vs charm integration suites and what they create. --- .../tests/integration/conftest.py | 5 ++- .../tests/integration/factories.py | 26 ++++++----- .../tests/integration/orphan_cleanup.py | 33 +++++++++++--- tests/integration/conftest.py | 7 +-- tests/integration/helpers/orphan_cleanup.py | 40 ++++++++++++----- tests/integration/naming.py | 45 ++++++++++++------- 6 files changed, 106 insertions(+), 50 deletions(-) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index 32320615b..441ed6311 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -183,7 +183,10 @@ def openstack_connection( project_domain_name=openstack_config.project_domain_name, region_name=openstack_config.region_name, ) - # Reclaim leftovers from force-cancelled previous CI runs before creating new ones. + # Previous force-cancelled github-runner-manager integration jobs can leave OpenStack + # servers/keypairs named test-runner-{id}…. Delete ones older than the default min age + # so they cannot accumulate across runs. Failures are logged and ignored so a flaky + # OpenStack API cannot block the suite. try: cleanup_stale_openstack_resources(conn) except Exception as exc: # noqa: BLE001 - best-effort hygiene must not block the suite diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index da7db8056..7b4b37b9e 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -10,43 +10,45 @@ from typing import Any -# Single source of truth for OpenStack resource names in this suite. -# Orphan cleanup imports these so patterns cannot drift from producers. +# Single source of truth for resource names created by +# github-runner-manager/tests/integration/. Orphan cleanup imports the matchers +# and formatters so names cannot drift from what the suite actually creates. TEST_ID_LENGTH = 8 TEST_ID_ALPHABET = string.ascii_lowercase + string.digits MANAGER_VM_PREFIX = "test-runner-" MANAGER_RUNNER_NAME_PREFIX = "test-manager-" -# Labels use "test-{id}" (not the openstack prefix); kept as a formatter only. +# GitHub extra labels use "test-{id}" — not an OpenStack name prefix. OPENSTACK_RESOURCE_PREFIXES: tuple[str, ...] = (MANAGER_VM_PREFIX,) def generate_test_id() -> str: - """Generate a unique test identifier. - - Returns: - A random 8-character alphanumeric string. - """ + """Return a random 8-character id distinguishing one suite run from another.""" return "".join(secrets.choice(TEST_ID_ALPHABET) for _ in range(TEST_ID_LENGTH)) def manager_vm_prefix(test_id: str) -> str: - """OpenStack VM / keypair name prefix for a suite run.""" + """Return the OpenStack server/keypair name prefix for this suite run.""" return f"{MANAGER_VM_PREFIX}{test_id}" def manager_runner_name(test_id: str) -> str: - """Runner-manager process name for a suite run.""" + """Return the github-runner-manager unit/process name for this suite run.""" return f"{MANAGER_RUNNER_NAME_PREFIX}{test_id}" def manager_labels(test_id: str) -> list[str]: - """Extra GitHub runner labels for a suite run.""" + """Return extra GitHub runner labels that identify this suite run.""" return [f"test-{test_id}"] def is_manager_openstack_resource_name(name: str | None) -> bool: - """True if *name* is a manager-IT OpenStack resource from this suite's scheme.""" + """Return True if *name* is an OpenStack resource from this manager test suite. + + Matches names that start with ``test-runner-`` followed by an 8-character + test id (and optional further ``-…`` segments). Used by orphan cleanup so + only leftovers from github-runner-manager integration tests are considered. + """ if not name: return False for prefix in OPENSTACK_RESOURCE_PREFIXES: diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index 48c143a1c..4812932fa 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -1,7 +1,18 @@ # Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. -"""Delete stale OpenStack resources left by interrupted manager integration runs.""" +"""Delete leftover OpenStack resources from github-runner-manager integration tests. + +This module is used by the suite under ``github-runner-manager/tests/integration/``. +That suite runs the github-runner-manager application against a real OpenStack +cloud (servers and SSH keypairs named with the suite's ``test-runner-{id}`` +prefix from ``factories.TestConfig``). + +When a previous CI job is force-cancelled, those OpenStack resources can be +left behind. The next time this suite starts, it calls +:func:`cleanup_stale_openstack_resources` so older leftovers are removed +before new ones are created. +""" import logging from collections.abc import Callable @@ -18,10 +29,17 @@ def cleanup_stale_openstack_resources( connection: Connection, min_age: timedelta = timedelta(hours=6), ) -> None: - """Remove manager-IT OpenStack leftovers older than ``min_age``. + """Delete old OpenStack servers and keypairs from previous manager test runs. + + Only considers names that match this suite's naming + (``test-runner-{8-char-id}…`` — see :func:`is_manager_openstack_resource_name`). + Resources younger than ``min_age`` are left alone so an in-progress job + is not damaged. Resources without a parseable creation timestamp are also + left alone (OpenStack keypairs often omit timestamps). - Only servers and keypairs. Security groups are not suite-scoped; runtime - reuses the permanent ``github-runner-v1`` group. + Does not delete security groups: the application uses a single permanent + project security group named ``github-runner-v1`` (get-or-create), not a + per-test group. """ now = datetime.now(tz=timezone.utc) logger.info( @@ -67,10 +85,11 @@ def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: - """True if resource is dated and older than ``min_age``. + """Return True only when the resource has a known age older than ``min_age``. - Missing/unparseable created_at: skip. Unknown age must not delete concurrent - or in-progress CI resources (e.g. keypairs without timestamps). + If ``created_at`` is missing or cannot be parsed, return False so concurrent + or in-progress CI resources (especially keypairs without timestamps) are + never deleted by guesswork. """ created = _parse_created_at(created_at) if created is None: diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index c3f9402a9..c12be6bcd 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -447,9 +447,10 @@ def openstack_connection_fixture( clouds_yaml_path.write_text(data=openstack_config.clouds_yaml_contents, encoding="utf-8") first_cloud = next(iter(clouds_yaml["clouds"].keys())) with openstack.connect(first_cloud) as connection: - # Reclaim leftovers from force-cancelled previous CI runs before creating new ones. - # Skip when reusing an existing app (local --use-existing-app-suffix); those resources - # may legitimately be older than min_age. + # Previous force-cancelled CI jobs can leave OpenStack servers/images/keypairs + # under our CI name prefixes. Delete ones older than the default min age so they + # cannot accumulate across runs. Skip when --use-existing-app-suffix reuses a + # long-lived local app whose resources may legitimately be older than min_age. if not existing_app_suffix: try: cleanup_stale_openstack_resources(connection) diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index 017ebae4e..3faea7331 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -1,10 +1,19 @@ # Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. -"""Delete stale OpenStack resources left by interrupted CI integration runs. - -Called at the start of integration tests so force-cancelled previous jobs still -get cleaned up the next time a suite runs against the tenant. +"""Delete leftover OpenStack resources from github-runner charm integration tests. + +This module is used by the suite under ``tests/integration/`` (the Juju charm +suite). That suite deploys the github-runner charm (and optionally related +helpers) and creates OpenStack servers, images, and keypairs whose names +follow the shared prefixes in :mod:`tests.integration.naming` +(for example ``test-{id}…``, ``test-runner-{id}…``, +``github-runner-image-builder-{id}…``). + +When a previous CI job is force-cancelled, those OpenStack resources can be +left behind. The next time this suite starts, it calls +:func:`cleanup_stale_openstack_resources` so older leftovers are removed +before new ones are created. """ import logging @@ -22,11 +31,19 @@ def cleanup_stale_openstack_resources( connection: Connection, min_age: timedelta = timedelta(hours=6), ) -> None: - """Remove CI-named OpenStack resources older than ``min_age``. + """Delete old OpenStack servers, images, and keypairs from previous CI runs. + + Only considers names recognized by :func:`is_ci_openstack_resource_name`. + Resources younger than ``min_age`` are left alone so an in-progress job + is not damaged. Resources without a parseable creation timestamp are also + left alone (OpenStack keypairs often omit timestamps). + + Deletion order is servers, then images, then keypairs (servers may still + reference keypairs until they are gone). - Order: servers (hold keypairs/refs) → images → keypairs. - Security groups are not touched: runtime uses a permanent docker-project - group (``github-runner-v1``) that is get-or-create, never suite-scoped. + Does not delete security groups: github-runner-manager uses a single + permanent project security group named ``github-runner-v1`` (get-or-create), + not a per-test group. """ now = datetime.now(tz=timezone.utc) logger.info( @@ -87,10 +104,11 @@ def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: - """True if resource is dated and older than ``min_age``. + """Return True only when the resource has a known age older than ``min_age``. - Missing/unparseable created_at: skip. Unknown age must not delete concurrent - or in-progress CI resources (e.g. keypairs without timestamps). + If ``created_at`` is missing or cannot be parsed, return False so concurrent + or in-progress CI resources (especially keypairs without timestamps) are + never deleted by guesswork. """ created = _parse_created_at(created_at) if created is None: diff --git a/tests/integration/naming.py b/tests/integration/naming.py index 285890680..c74e21365 100644 --- a/tests/integration/naming.py +++ b/tests/integration/naming.py @@ -1,51 +1,64 @@ # Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. -"""Shared naming for charm integration tests and OpenStack orphan cleanup. +"""Shared application/OpenStack resource naming for charm integration tests. -Producers (fixtures) and consumers (orphan cleanup) both import from here so -resource name formats cannot drift. +Who uses this module +-------------------- +- Producers: fixtures in ``tests/integration/conftest.py`` that choose Juju app + names and related OpenStack resource names for a suite run. +- Consumers: ``tests/integration/helpers/orphan_cleanup.py``, which deletes + leftover OpenStack resources from force-cancelled previous runs. + +Keeping both sides on the same helpers means renaming a resource format only +requires changing this file once. """ import random import string -# Suffixes for app names are 8 characters: 1 lowercase letter + 7 alnum -# (see random_app_name_suffix historical shape). +# Application name suffixes are 8 characters: 1 lowercase letter + 7 alnum. +# Matches the historical ``random_app_name_suffix`` shape used by the charm suite. TEST_ID_LENGTH = 8 TEST_ID_ALPHABET = string.ascii_lowercase + string.digits -# Longest-first matters for matching: "test-runner-" before "test-". +# OpenStack / app name prefixes produced by this repository's integration tests. +# Longest-first so matching does not stop early at the shorter ``test-`` prefix +# when the real name starts with ``test-runner-``. OPENSTACK_RESOURCE_PREFIXES: tuple[str, ...] = ( - "github-runner-image-builder-", # image builder charm app in charm IT - "test-runner-", # github-runner-manager TestConfig.vm_prefix (shared tenant) - "test-", # github-runner charm app_name + "github-runner-image-builder-", # image-builder charm app name in the charm suite + "test-runner-", # github-runner-manager VM/keypair prefix (shared CI tenant) + "test-", # github-runner charm application name ) def generate_app_suffix() -> str: - """Return a fresh 8-char application name suffix for a suite run.""" + """Return a new 8-character application name suffix for one suite run.""" return random.choice(string.ascii_lowercase) + "".join( random.choices(TEST_ID_ALPHABET, k=TEST_ID_LENGTH - 1) ) def app_name_from_suffix(suffix: str) -> str: - """github-runner application name for a given suffix.""" + """Return the github-runner Juju application name for *suffix*.""" return f"test-{suffix}" def image_builder_app_name_from_suffix(suffix: str) -> str: - """github-runner-image-builder application name for a given suffix.""" + """Return the github-runner-image-builder Juju application name for *suffix*.""" return f"github-runner-image-builder-{suffix}" def is_ci_openstack_resource_name(name: str | None) -> bool: - """True if *name* matches a known CI/OpenStack resource prefix + test id. + """Return True if *name* looks like an OpenStack resource from our CI suites. + + A name matches when it starts with one of :data:`OPENSTACK_RESOURCE_PREFIXES`, + then an 8-character test id from :data:`TEST_ID_ALPHABET`, optionally followed + by further ``-...`` segments (e.g. per-instance suffixes). - A resource is considered CI-created when it starts with one of - :data:`OPENSTACK_RESOURCE_PREFIXES` followed by an 8-char test id, optionally - with further ``-...`` segments (InstanceID-style suffixes). + Examples that match: ``test-abcdef12``, ``test-runner-abcdef12-0``, + ``github-runner-image-builder-abcdef12``. + Examples that do not: ``github-runner-v1``, ``prod-runner``, ``test-short``. """ if not name: return False From 47418a76fdfd2d9efb7d511d9a58dfae642e1c39 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Jul 2026 07:57:59 +0000 Subject: [PATCH 12/20] docs: keep public docstrings to intent only Drop matching algorithms and implementation notes from interface comments; the code already encodes those details. --- .../tests/integration/factories.py | 15 +++++-------- .../tests/integration/orphan_cleanup.py | 20 ++--------------- tests/integration/helpers/orphan_cleanup.py | 22 ++----------------- tests/integration/naming.py | 17 ++++---------- 4 files changed, 13 insertions(+), 61 deletions(-) diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index 7b4b37b9e..8efc344d9 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -23,32 +23,27 @@ def generate_test_id() -> str: - """Return a random 8-character id distinguishing one suite run from another.""" + """Return a unique id for one suite run.""" return "".join(secrets.choice(TEST_ID_ALPHABET) for _ in range(TEST_ID_LENGTH)) def manager_vm_prefix(test_id: str) -> str: - """Return the OpenStack server/keypair name prefix for this suite run.""" + """Return the OpenStack resource name prefix for this suite run.""" return f"{MANAGER_VM_PREFIX}{test_id}" def manager_runner_name(test_id: str) -> str: - """Return the github-runner-manager unit/process name for this suite run.""" + """Return the github-runner-manager name for this suite run.""" return f"{MANAGER_RUNNER_NAME_PREFIX}{test_id}" def manager_labels(test_id: str) -> list[str]: - """Return extra GitHub runner labels that identify this suite run.""" + """Return extra GitHub runner labels for this suite run.""" return [f"test-{test_id}"] def is_manager_openstack_resource_name(name: str | None) -> bool: - """Return True if *name* is an OpenStack resource from this manager test suite. - - Matches names that start with ``test-runner-`` followed by an 8-character - test id (and optional further ``-…`` segments). Used by orphan cleanup so - only leftovers from github-runner-manager integration tests are considered. - """ + """Return True if *name* belongs to this suite's OpenStack resources.""" if not name: return False for prefix in OPENSTACK_RESOURCE_PREFIXES: diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index 4812932fa..4c60c1111 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -29,18 +29,7 @@ def cleanup_stale_openstack_resources( connection: Connection, min_age: timedelta = timedelta(hours=6), ) -> None: - """Delete old OpenStack servers and keypairs from previous manager test runs. - - Only considers names that match this suite's naming - (``test-runner-{8-char-id}…`` — see :func:`is_manager_openstack_resource_name`). - Resources younger than ``min_age`` are left alone so an in-progress job - is not damaged. Resources without a parseable creation timestamp are also - left alone (OpenStack keypairs often omit timestamps). - - Does not delete security groups: the application uses a single permanent - project security group named ``github-runner-v1`` (get-or-create), not a - per-test group. - """ + """Delete leftover OpenStack resources from previous suite runs older than ``min_age``.""" now = datetime.now(tz=timezone.utc) logger.info( "OpenStack orphan cleanup starting (min_age=%sh)", @@ -85,12 +74,7 @@ def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: - """Return True only when the resource has a known age older than ``min_age``. - - If ``created_at`` is missing or cannot be parsed, return False so concurrent - or in-progress CI resources (especially keypairs without timestamps) are - never deleted by guesswork. - """ + """Return True if the resource is older than ``min_age``.""" created = _parse_created_at(created_at) if created is None: return False diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index 3faea7331..77146ce49 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -31,20 +31,7 @@ def cleanup_stale_openstack_resources( connection: Connection, min_age: timedelta = timedelta(hours=6), ) -> None: - """Delete old OpenStack servers, images, and keypairs from previous CI runs. - - Only considers names recognized by :func:`is_ci_openstack_resource_name`. - Resources younger than ``min_age`` are left alone so an in-progress job - is not damaged. Resources without a parseable creation timestamp are also - left alone (OpenStack keypairs often omit timestamps). - - Deletion order is servers, then images, then keypairs (servers may still - reference keypairs until they are gone). - - Does not delete security groups: github-runner-manager uses a single - permanent project security group named ``github-runner-v1`` (get-or-create), - not a per-test group. - """ + """Delete leftover OpenStack resources from previous suite runs older than ``min_age``.""" now = datetime.now(tz=timezone.utc) logger.info( "OpenStack orphan cleanup starting (min_age=%sh)", @@ -104,12 +91,7 @@ def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: - """Return True only when the resource has a known age older than ``min_age``. - - If ``created_at`` is missing or cannot be parsed, return False so concurrent - or in-progress CI resources (especially keypairs without timestamps) are - never deleted by guesswork. - """ + """Return True if the resource is older than ``min_age``.""" created = _parse_created_at(created_at) if created is None: return False diff --git a/tests/integration/naming.py b/tests/integration/naming.py index c74e21365..35af70cf4 100644 --- a/tests/integration/naming.py +++ b/tests/integration/naming.py @@ -33,33 +33,24 @@ def generate_app_suffix() -> str: - """Return a new 8-character application name suffix for one suite run.""" + """Return a unique application name suffix for one suite run.""" return random.choice(string.ascii_lowercase) + "".join( random.choices(TEST_ID_ALPHABET, k=TEST_ID_LENGTH - 1) ) def app_name_from_suffix(suffix: str) -> str: - """Return the github-runner Juju application name for *suffix*.""" + """Return the github-runner application name for *suffix*.""" return f"test-{suffix}" def image_builder_app_name_from_suffix(suffix: str) -> str: - """Return the github-runner-image-builder Juju application name for *suffix*.""" + """Return the github-runner-image-builder application name for *suffix*.""" return f"github-runner-image-builder-{suffix}" def is_ci_openstack_resource_name(name: str | None) -> bool: - """Return True if *name* looks like an OpenStack resource from our CI suites. - - A name matches when it starts with one of :data:`OPENSTACK_RESOURCE_PREFIXES`, - then an 8-character test id from :data:`TEST_ID_ALPHABET`, optionally followed - by further ``-...`` segments (e.g. per-instance suffixes). - - Examples that match: ``test-abcdef12``, ``test-runner-abcdef12-0``, - ``github-runner-image-builder-abcdef12``. - Examples that do not: ``github-runner-v1``, ``prod-runner``, ``test-short``. - """ + """Return True if *name* belongs to this repository's CI OpenStack resources.""" if not name: return False for prefix in OPENSTACK_RESOURCE_PREFIXES: From d1f60c2666b8fbb041d4bf1390dca612bc59c9c7 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Jul 2026 08:10:23 +0000 Subject: [PATCH 13/20] docs: clarify OpenStack prefix list ordering comment Longer prefixes first is for readability, not early-exit matching. --- tests/integration/naming.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/naming.py b/tests/integration/naming.py index 35af70cf4..ea1fc4717 100644 --- a/tests/integration/naming.py +++ b/tests/integration/naming.py @@ -23,8 +23,8 @@ TEST_ID_ALPHABET = string.ascii_lowercase + string.digits # OpenStack / app name prefixes produced by this repository's integration tests. -# Longest-first so matching does not stop early at the shorter ``test-`` prefix -# when the real name starts with ``test-runner-``. +# Longer prefixes are listed first so readers see the more specific form before +# the shorter ``test-`` prefix (``test-runner-`` vs ``test-``). OPENSTACK_RESOURCE_PREFIXES: tuple[str, ...] = ( "github-runner-image-builder-", # image-builder charm app name in the charm suite "test-runner-", # github-runner-manager VM/keypair prefix (shared CI tenant) From 6cf81c47e9ede840cd0e1c0c457d03a95c2e5d30 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Jul 2026 08:25:21 +0000 Subject: [PATCH 14/20] fix: require letter-leading ids for charm OpenStack prefixes Charm app suffixes always start with a lowercase letter; keep manager test-runner- ids free to begin with a digit. --- tests/integration/naming.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/integration/naming.py b/tests/integration/naming.py index ea1fc4717..2062809d8 100644 --- a/tests/integration/naming.py +++ b/tests/integration/naming.py @@ -60,6 +60,13 @@ def is_ci_openstack_resource_name(name: str | None) -> bool: if not rest: continue test_id = rest.split("-", 1)[0] - if len(test_id) == TEST_ID_LENGTH and all(c in TEST_ID_ALPHABET for c in test_id): - return True + if len(test_id) != TEST_ID_LENGTH: + continue + if not all(c in TEST_ID_ALPHABET for c in test_id): + continue + # Charm app suffixes always start with a letter; manager test ids may + # start with a digit. + if prefix != "test-runner-" and not test_id[0].isalpha(): + continue + return True return False From f97672485482d2613a878390289fc9991d34e413 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Jul 2026 08:46:21 +0000 Subject: [PATCH 15/20] fix: list OpenStack servers by app prefix for teardown Exact name filters miss runner VMs named test--N. Also soften naming module wording around suite scope and random suffixes. --- tests/integration/conftest.py | 6 +++++- tests/integration/naming.py | 13 +++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index c12be6bcd..bf3366476 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -458,7 +458,11 @@ def openstack_connection_fixture( logging.warning("OpenStack orphan cleanup failed: %s", exc, exc_info=True) yield connection - servers = list(connection.list_servers(filters={"name": app_name}) or []) + servers = [ + server + for server in (connection.list_servers() or []) + if str(getattr(server, "name", "") or "").startswith(app_name) + ] if request.session.testsfailed: logging.info("OpenStack servers: %s", servers) diff --git a/tests/integration/naming.py b/tests/integration/naming.py index 2062809d8..815a7b144 100644 --- a/tests/integration/naming.py +++ b/tests/integration/naming.py @@ -1,17 +1,18 @@ # Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. -"""Shared application/OpenStack resource naming for charm integration tests. +"""Shared application/OpenStack resource naming for integration tests. Who uses this module -------------------- - Producers: fixtures in ``tests/integration/conftest.py`` that choose Juju app - names and related OpenStack resource names for a suite run. + names for a suite run. - Consumers: ``tests/integration/helpers/orphan_cleanup.py``, which deletes - leftover OpenStack resources from force-cancelled previous runs. + leftover OpenStack resources from force-cancelled previous runs, including + names produced by github-runner-manager on shared CI tenants. -Keeping both sides on the same helpers means renaming a resource format only -requires changing this file once. +Keeping producers and orphan cleanup on the same helpers means renaming a +resource format only requires changing this file once. """ import random @@ -33,7 +34,7 @@ def generate_app_suffix() -> str: - """Return a unique application name suffix for one suite run.""" + """Return a random application name suffix for one suite run.""" return random.choice(string.ascii_lowercase) + "".join( random.choices(TEST_ID_ALPHABET, k=TEST_ID_LENGTH - 1) ) From e460b9a2af8df7ba9ea79ebf831df0aa74e39f19 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Jul 2026 08:58:30 +0000 Subject: [PATCH 16/20] docs: call generate_test_id random not unique --- github-runner-manager/tests/integration/factories.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index 8efc344d9..1701983e6 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -23,7 +23,7 @@ def generate_test_id() -> str: - """Return a unique id for one suite run.""" + """Return a random id for one suite run.""" return "".join(secrets.choice(TEST_ID_ALPHABET) for _ in range(TEST_ID_LENGTH)) From c45fb30b8c4215427b67af5eea201464bf7ce509 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Jul 2026 09:31:34 +0000 Subject: [PATCH 17/20] fix: narrow orphan cleanup exceptions, drop _safe_delete Catch only ResourceNotFound and ConflictException instead of broad Exception. Unexpected failures (auth, bugs) now propagate. Inline the try/except at each resource loop instead of a shared helper. --- .../tests/integration/orphan_cleanup.py | 32 +++++++------- tests/integration/helpers/orphan_cleanup.py | 44 +++++++++---------- 2 files changed, 37 insertions(+), 39 deletions(-) diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index 4c60c1111..a899c98f9 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -15,9 +15,9 @@ """ import logging -from collections.abc import Callable from datetime import datetime, timedelta, timezone +import openstack.exceptions from openstack.connection import Connection from .factories import is_manager_openstack_resource_name @@ -46,11 +46,13 @@ def cleanup_stale_openstack_resources( now, ): continue - _safe_delete( - "server", - name or server.id, - lambda s=server: connection.delete_server(s.id, wait=True), - ) + try: + connection.delete_server(server.id, wait=True) + logger.info("Orphan cleanup deleted server %s", name or server.id) + except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): + logger.warning( + "Orphan cleanup failed deleting server %s", name or server.id, exc_info=True + ) for keypair in connection.list_keypairs() or []: name = getattr(keypair, "name", None) @@ -58,21 +60,17 @@ def cleanup_stale_openstack_resources( continue if not _is_stale(getattr(keypair, "created_at", None), min_age, now): continue - _safe_delete("keypair", name or "", lambda n=name: connection.delete_keypair(n)) + try: + connection.delete_keypair(name) + logger.info("Orphan cleanup deleted keypair %s", name) + except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): + logger.warning( + "Orphan cleanup failed deleting keypair %s", name, exc_info=True + ) logger.info("OpenStack orphan cleanup finished") -def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None: - try: - delete_fn() - logger.info("Orphan cleanup deleted %s %s", label, name) - except Exception as exc: # noqa: BLE001 - logger.warning( - "Orphan cleanup failed deleting %s %s: %s", label, name, exc, exc_info=True - ) - - def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: """Return True if the resource is older than ``min_age``.""" created = _parse_created_at(created_at) diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index 77146ce49..79bcc562f 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -17,9 +17,9 @@ """ import logging -from collections.abc import Callable from datetime import datetime, timedelta, timezone +import openstack.exceptions from openstack.connection import Connection from tests.integration.naming import is_ci_openstack_resource_name @@ -48,11 +48,13 @@ def cleanup_stale_openstack_resources( now, ): continue - _safe_delete( - "server", - name or server.id, - lambda s=server: connection.delete_server(s.id, wait=True), - ) + try: + connection.delete_server(server.id, wait=True) + logger.info("Orphan cleanup deleted server %s", name or server.id) + except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): + logger.warning( + "Orphan cleanup failed deleting server %s", name or server.id, exc_info=True + ) for image in connection.list_images() or []: name = getattr(image, "name", None) @@ -63,11 +65,13 @@ def cleanup_stale_openstack_resources( continue if not _is_stale(getattr(image, "created_at", None), min_age, now): continue - _safe_delete( - "image", - name or image.id, - lambda im=image: connection.delete_image(im.id), - ) + try: + connection.delete_image(image.id) + logger.info("Orphan cleanup deleted image %s", name or image.id) + except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): + logger.warning( + "Orphan cleanup failed deleting image %s", name or image.id, exc_info=True + ) for keypair in connection.list_keypairs() or []: name = getattr(keypair, "name", None) @@ -75,21 +79,17 @@ def cleanup_stale_openstack_resources( continue if not _is_stale(getattr(keypair, "created_at", None), min_age, now): continue - _safe_delete("keypair", name or "", lambda n=name: connection.delete_keypair(n)) + try: + connection.delete_keypair(name) + logger.info("Orphan cleanup deleted keypair %s", name) + except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): + logger.warning( + "Orphan cleanup failed deleting keypair %s", name, exc_info=True + ) logger.info("OpenStack orphan cleanup finished") -def _safe_delete(label: str, name: str, delete_fn: Callable[[], object]) -> None: - try: - delete_fn() - logger.info("Orphan cleanup deleted %s %s", label, name) - except Exception as exc: # noqa: BLE001 - best-effort, already-gone is fine - logger.warning( - "Orphan cleanup failed deleting %s %s: %s", label, name, exc, exc_info=True - ) - - def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: """Return True if the resource is older than ``min_age``.""" created = _parse_created_at(created_at) From e80d894e2454aeb844d9715c6d579928617a6931 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Jul 2026 09:38:55 +0000 Subject: [PATCH 18/20] fix: split cleanup into per-resource helpers, add docstrings Satisfies flake8 complexity (C901) and docstring (DCO010) rules. --- .../tests/integration/orphan_cleanup.py | 65 +++++++++------ tests/integration/helpers/orphan_cleanup.py | 82 ++++++++++++------- 2 files changed, 92 insertions(+), 55 deletions(-) diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index a899c98f9..5ee4ce104 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -25,17 +25,17 @@ logger = logging.getLogger(__name__) -def cleanup_stale_openstack_resources( - connection: Connection, - min_age: timedelta = timedelta(hours=6), -) -> None: - """Delete leftover OpenStack resources from previous suite runs older than ``min_age``.""" - now = datetime.now(tz=timezone.utc) - logger.info( - "OpenStack orphan cleanup starting (min_age=%sh)", - min_age.total_seconds() / 3600.0, - ) +def _delete_resource(connection: Connection, label: str, name: str, delete_fn): + """Delete a resource and log success or expected failure.""" + try: + delete_fn() + logger.info("Orphan cleanup deleted %s %s", label, name) + except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): + logger.warning("Orphan cleanup failed deleting %s %s", label, name, exc_info=True) + +def _cleanup_servers(connection: Connection, now: datetime, min_age: timedelta): + """Delete stale servers matching manager naming.""" for server in connection.list_servers(bare=True) or []: name = getattr(server, "name", None) if not is_manager_openstack_resource_name(name): @@ -46,28 +46,44 @@ def cleanup_stale_openstack_resources( now, ): continue - try: - connection.delete_server(server.id, wait=True) - logger.info("Orphan cleanup deleted server %s", name or server.id) - except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): - logger.warning( - "Orphan cleanup failed deleting server %s", name or server.id, exc_info=True - ) + _delete_resource( + connection, + "server", + name or server.id, + lambda s=server: connection.delete_server(s.id, wait=True), + ) + +def _cleanup_keypairs(connection: Connection, now: datetime, min_age: timedelta): + """Delete stale keypairs matching manager naming.""" for keypair in connection.list_keypairs() or []: name = getattr(keypair, "name", None) if not is_manager_openstack_resource_name(name): continue if not _is_stale(getattr(keypair, "created_at", None), min_age, now): continue - try: - connection.delete_keypair(name) - logger.info("Orphan cleanup deleted keypair %s", name) - except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): - logger.warning( - "Orphan cleanup failed deleting keypair %s", name, exc_info=True - ) + _delete_resource( + connection, + "keypair", + name or "", + lambda n=name: connection.delete_keypair(n), + ) + +def cleanup_stale_openstack_resources( + connection: Connection, + min_age: timedelta = timedelta(hours=6), +) -> None: + """Delete leftover OpenStack resources from previous suite runs. + + Args: + connection: Authenticated OpenStack connection. + min_age: Age threshold; resources newer than this are left alone. + """ + now = datetime.now(tz=timezone.utc) + logger.info("OpenStack orphan cleanup starting (min_age=%sh)", min_age.total_seconds() / 3600.0) + _cleanup_servers(connection, now, min_age) + _cleanup_keypairs(connection, now, min_age) logger.info("OpenStack orphan cleanup finished") @@ -80,6 +96,7 @@ def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: def _parse_created_at(value: object) -> datetime | None: + """Parse a created_at value into a UTC datetime, or return None.""" if value is None: return None if isinstance(value, datetime): diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index 79bcc562f..3b1c47b40 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -27,17 +27,17 @@ logger = logging.getLogger(__name__) -def cleanup_stale_openstack_resources( - connection: Connection, - min_age: timedelta = timedelta(hours=6), -) -> None: - """Delete leftover OpenStack resources from previous suite runs older than ``min_age``.""" - now = datetime.now(tz=timezone.utc) - logger.info( - "OpenStack orphan cleanup starting (min_age=%sh)", - min_age.total_seconds() / 3600.0, - ) +def _delete_resource(connection: Connection, label: str, name: str, delete_fn): + """Delete a resource and log success or expected failure.""" + try: + delete_fn() + logger.info("Orphan cleanup deleted %s %s", label, name) + except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): + logger.warning("Orphan cleanup failed deleting %s %s", label, name, exc_info=True) + +def _cleanup_servers(connection: Connection, now: datetime, min_age: timedelta): + """Delete stale servers matching CI naming.""" for server in connection.list_servers(bare=True) or []: name = getattr(server, "name", None) if not is_ci_openstack_resource_name(name): @@ -48,14 +48,16 @@ def cleanup_stale_openstack_resources( now, ): continue - try: - connection.delete_server(server.id, wait=True) - logger.info("Orphan cleanup deleted server %s", name or server.id) - except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): - logger.warning( - "Orphan cleanup failed deleting server %s", name or server.id, exc_info=True - ) + _delete_resource( + connection, + "server", + name or server.id, + lambda s=server: connection.delete_server(s.id, wait=True), + ) + +def _cleanup_images(connection: Connection, now: datetime, min_age: timedelta): + """Delete stale images matching CI naming.""" for image in connection.list_images() or []: name = getattr(image, "name", None) visibility = str(getattr(image, "visibility", "") or "").lower() @@ -65,28 +67,45 @@ def cleanup_stale_openstack_resources( continue if not _is_stale(getattr(image, "created_at", None), min_age, now): continue - try: - connection.delete_image(image.id) - logger.info("Orphan cleanup deleted image %s", name or image.id) - except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): - logger.warning( - "Orphan cleanup failed deleting image %s", name or image.id, exc_info=True - ) + _delete_resource( + connection, + "image", + name or image.id, + lambda im=image: connection.delete_image(im.id), + ) + +def _cleanup_keypairs(connection: Connection, now: datetime, min_age: timedelta): + """Delete stale keypairs matching CI naming.""" for keypair in connection.list_keypairs() or []: name = getattr(keypair, "name", None) if not is_ci_openstack_resource_name(name): continue if not _is_stale(getattr(keypair, "created_at", None), min_age, now): continue - try: - connection.delete_keypair(name) - logger.info("Orphan cleanup deleted keypair %s", name) - except (openstack.exceptions.ResourceNotFound, openstack.exceptions.ConflictException): - logger.warning( - "Orphan cleanup failed deleting keypair %s", name, exc_info=True - ) + _delete_resource( + connection, + "keypair", + name or "", + lambda n=name: connection.delete_keypair(n), + ) + + +def cleanup_stale_openstack_resources( + connection: Connection, + min_age: timedelta = timedelta(hours=6), +) -> None: + """Delete leftover OpenStack resources from previous suite runs. + Args: + connection: Authenticated OpenStack connection. + min_age: Age threshold; resources newer than this are left alone. + """ + now = datetime.now(tz=timezone.utc) + logger.info("OpenStack orphan cleanup starting (min_age=%sh)", min_age.total_seconds() / 3600.0) + _cleanup_servers(connection, now, min_age) + _cleanup_images(connection, now, min_age) + _cleanup_keypairs(connection, now, min_age) logger.info("OpenStack orphan cleanup finished") @@ -99,6 +118,7 @@ def _is_stale(created_at: object, min_age: timedelta, now: datetime) -> bool: def _parse_created_at(value: object) -> datetime | None: + """Parse a created_at value into a UTC datetime, or return None.""" if value is None: return None if isinstance(value, datetime): From 545c5aa988392eca7986860a09c40126c0ab8b94 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Jul 2026 09:49:21 +0000 Subject: [PATCH 19/20] fix: extract conftest teardown, fix docstrings, drop unused connection arg - Extract suite teardown from openstack_connection_fixture to reduce flake8 complexity below 10. - Add Args/Returns docstring sections to satisfy DCO020/DCO030. - Remove unused connection param from _delete_resource helper. --- .../tests/integration/orphan_cleanup.py | 8 ++-- tests/integration/conftest.py | 45 +++++++++++-------- tests/integration/helpers/orphan_cleanup.py | 9 ++-- tests/integration/naming.py | 33 ++++++++++++-- 4 files changed, 63 insertions(+), 32 deletions(-) diff --git a/github-runner-manager/tests/integration/orphan_cleanup.py b/github-runner-manager/tests/integration/orphan_cleanup.py index 5ee4ce104..e0049a0d7 100644 --- a/github-runner-manager/tests/integration/orphan_cleanup.py +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -25,7 +25,7 @@ logger = logging.getLogger(__name__) -def _delete_resource(connection: Connection, label: str, name: str, delete_fn): +def _delete_resource(label: str, name: str, delete_fn): """Delete a resource and log success or expected failure.""" try: delete_fn() @@ -47,7 +47,6 @@ def _cleanup_servers(connection: Connection, now: datetime, min_age: timedelta): ): continue _delete_resource( - connection, "server", name or server.id, lambda s=server: connection.delete_server(s.id, wait=True), @@ -63,7 +62,6 @@ def _cleanup_keypairs(connection: Connection, now: datetime, min_age: timedelta) if not _is_stale(getattr(keypair, "created_at", None), min_age, now): continue _delete_resource( - connection, "keypair", name or "", lambda n=name: connection.delete_keypair(n), @@ -81,7 +79,9 @@ def cleanup_stale_openstack_resources( min_age: Age threshold; resources newer than this are left alone. """ now = datetime.now(tz=timezone.utc) - logger.info("OpenStack orphan cleanup starting (min_age=%sh)", min_age.total_seconds() / 3600.0) + logger.info( + "OpenStack orphan cleanup starting (min_age=%sh)", min_age.total_seconds() / 3600.0 + ) _cleanup_servers(connection, now, min_age) _cleanup_keypairs(connection, now, min_age) logger.info("OpenStack orphan cleanup finished") diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index bf3366476..12f9c72be 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -457,28 +457,35 @@ def openstack_connection_fixture( except Exception as exc: # noqa: BLE001 - best-effort hygiene must not block the suite logging.warning("OpenStack orphan cleanup failed: %s", exc, exc_info=True) yield connection + _teardown_charm_suite_openstack(connection, app_name, existing_app_suffix, request) - servers = [ - server - for server in (connection.list_servers() or []) - if str(getattr(server, "name", "") or "").startswith(app_name) - ] - if request.session.testsfailed: - logging.info("OpenStack servers: %s", servers) - for server in servers: - console_log = connection.get_server_console(server=server) - logging.info("Server %s console log:\n%s", server.name, console_log) +def _teardown_charm_suite_openstack( + connection: Connection, + app_name: str, + existing_app_suffix: Optional[str], + request: pytest.FixtureRequest, +) -> None: + """Log and delete OpenStack resources created during this suite run.""" + servers = [ + server + for server in (connection.list_servers() or []) + if str(getattr(server, "name", "") or "").startswith(app_name) + ] - if not existing_app_suffix: - for server in servers: - server_name: str = server.name - if server_name.startswith(app_name): - connection.delete_server(server.id, wait=True) - for key in connection.list_keypairs(): - key_name: str = key.name - if key_name.startswith(app_name): - connection.delete_keypair(key_name) + if request.session.testsfailed: + logging.info("OpenStack servers: %s", servers) + for server in servers: + console_log = connection.get_server_console(server=server) + logging.info("Server %s console log:\n%s", server.name, console_log) + + if not existing_app_suffix: + for server in servers: + connection.delete_server(server.id, wait=True) + for key in connection.list_keypairs(): + key_name: str = key.name + if key_name.startswith(app_name): + connection.delete_keypair(key_name) @pytest.fixture(scope="module") diff --git a/tests/integration/helpers/orphan_cleanup.py b/tests/integration/helpers/orphan_cleanup.py index 3b1c47b40..6ea3a50b1 100644 --- a/tests/integration/helpers/orphan_cleanup.py +++ b/tests/integration/helpers/orphan_cleanup.py @@ -27,7 +27,7 @@ logger = logging.getLogger(__name__) -def _delete_resource(connection: Connection, label: str, name: str, delete_fn): +def _delete_resource(label: str, name: str, delete_fn): """Delete a resource and log success or expected failure.""" try: delete_fn() @@ -49,7 +49,6 @@ def _cleanup_servers(connection: Connection, now: datetime, min_age: timedelta): ): continue _delete_resource( - connection, "server", name or server.id, lambda s=server: connection.delete_server(s.id, wait=True), @@ -68,7 +67,6 @@ def _cleanup_images(connection: Connection, now: datetime, min_age: timedelta): if not _is_stale(getattr(image, "created_at", None), min_age, now): continue _delete_resource( - connection, "image", name or image.id, lambda im=image: connection.delete_image(im.id), @@ -84,7 +82,6 @@ def _cleanup_keypairs(connection: Connection, now: datetime, min_age: timedelta) if not _is_stale(getattr(keypair, "created_at", None), min_age, now): continue _delete_resource( - connection, "keypair", name or "", lambda n=name: connection.delete_keypair(n), @@ -102,7 +99,9 @@ def cleanup_stale_openstack_resources( min_age: Age threshold; resources newer than this are left alone. """ now = datetime.now(tz=timezone.utc) - logger.info("OpenStack orphan cleanup starting (min_age=%sh)", min_age.total_seconds() / 3600.0) + logger.info( + "OpenStack orphan cleanup starting (min_age=%sh)", min_age.total_seconds() / 3600.0 + ) _cleanup_servers(connection, now, min_age) _cleanup_images(connection, now, min_age) _cleanup_keypairs(connection, now, min_age) diff --git a/tests/integration/naming.py b/tests/integration/naming.py index 815a7b144..d09647d30 100644 --- a/tests/integration/naming.py +++ b/tests/integration/naming.py @@ -34,24 +34,49 @@ def generate_app_suffix() -> str: - """Return a random application name suffix for one suite run.""" + """Return a random application name suffix for one suite run. + + Returns: + An 8-character suffix starting with a lowercase letter. + """ return random.choice(string.ascii_lowercase) + "".join( random.choices(TEST_ID_ALPHABET, k=TEST_ID_LENGTH - 1) ) def app_name_from_suffix(suffix: str) -> str: - """Return the github-runner application name for *suffix*.""" + """Return the github-runner application name for *suffix*. + + Args: + suffix: The application name suffix. + + Returns: + The full Juju application name. + """ return f"test-{suffix}" def image_builder_app_name_from_suffix(suffix: str) -> str: - """Return the github-runner-image-builder application name for *suffix*.""" + """Return the github-runner-image-builder application name for *suffix*. + + Args: + suffix: The application name suffix. + + Returns: + The full Juju application name. + """ return f"github-runner-image-builder-{suffix}" def is_ci_openstack_resource_name(name: str | None) -> bool: - """Return True if *name* belongs to this repository's CI OpenStack resources.""" + """Return True if *name* belongs to this repository's CI OpenStack resources. + + Args: + name: The OpenStack resource name to check. + + Returns: + Whether the name matches a known CI prefix and test id format. + """ if not name: return False for prefix in OPENSTACK_RESOURCE_PREFIXES: From 78392686228adeef636cb91162560b0c9f465a3b Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 17 Jul 2026 11:55:24 +0200 Subject: [PATCH 20/20] fix(test): remove additional new line --- github-runner-manager/tests/integration/factories.py | 1 - 1 file changed, 1 deletion(-) diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index 1701983e6..9ec3a7045 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -9,7 +9,6 @@ from pathlib import Path from typing import Any - # Single source of truth for resource names created by # github-runner-manager/tests/integration/. Orphan cleanup imports the matchers # and formatters so names cannot drift from what the suite actually creates.