diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index 81257d7b22..441ed6311a 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__) @@ -182,6 +183,14 @@ def openstack_connection( project_domain_name=openstack_config.project_domain_name, region_name=openstack_config.region_name, ) + # 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 + logger.warning("OpenStack orphan cleanup failed: %s", exc, exc_info=True) yield conn conn.close() diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index d060b63844..1701983e6c 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -10,13 +10,52 @@ from typing import Any -def _generate_test_id() -> str: - """Generate a unique test identifier. +# 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-" +# GitHub extra labels use "test-{id}" — not an OpenStack name prefix. - Returns: - A random 8-character alphanumeric string. - """ - return "".join(secrets.choice(string.ascii_lowercase + string.digits) for _ in range(8)) +OPENSTACK_RESOURCE_PREFIXES: tuple[str, ...] = (MANAGER_VM_PREFIX,) + + +def generate_test_id() -> str: + """Return a random 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 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 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 for this suite run.""" + return [f"test-{test_id}"] + + +def is_manager_openstack_resource_name(name: str | None) -> bool: + """Return True if *name* belongs to this suite's OpenStack resources.""" + 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 +146,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 new file mode 100644 index 0000000000..5ee4ce1049 --- /dev/null +++ b/github-runner-manager/tests/integration/orphan_cleanup.py @@ -0,0 +1,113 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""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 datetime import datetime, timedelta, timezone + +import openstack.exceptions +from openstack.connection import Connection + +from .factories import is_manager_openstack_resource_name + +logger = logging.getLogger(__name__) + + +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): + continue + if not _is_stale( + getattr(server, "created_at", None) or getattr(server, "created", None), + min_age, + now, + ): + continue + _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 + _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") + + +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) + if created is None: + return False + return now - created >= min_age + + +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): + 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 0d0714de67..bf3366476e 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -43,6 +43,12 @@ wait_for_runner_ready, ) 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 @@ -279,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") @@ -444,25 +447,38 @@ 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: + # 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) + 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 = connection.list_servers(filters={"name": app_name}) + 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) - - 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.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 new file mode 100644 index 0000000000..3b1c47b40b --- /dev/null +++ b/tests/integration/helpers/orphan_cleanup.py @@ -0,0 +1,135 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""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 +from datetime import datetime, timedelta, timezone + +import openstack.exceptions +from openstack.connection import Connection + +from tests.integration.naming import is_ci_openstack_resource_name + +logger = logging.getLogger(__name__) + + +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): + continue + if not _is_stale( + getattr(server, "created_at", None) or getattr(server, "created", None), + min_age, + now, + ): + continue + _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() + if visibility in {"public", "community"}: + continue + if not is_ci_openstack_resource_name(name): + continue + 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), + ) + + +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 + _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") + + +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) + if created is None: + return False + return now - created >= min_age + + +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): + 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/naming.py b/tests/integration/naming.py new file mode 100644 index 0000000000..815a7b1440 --- /dev/null +++ b/tests/integration/naming.py @@ -0,0 +1,73 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Shared application/OpenStack resource naming for integration tests. + +Who uses this module +-------------------- +- Producers: fixtures in ``tests/integration/conftest.py`` that choose Juju app + names for a suite run. +- Consumers: ``tests/integration/helpers/orphan_cleanup.py``, which deletes + leftover OpenStack resources from force-cancelled previous runs, including + names produced by github-runner-manager on shared CI tenants. + +Keeping producers and orphan cleanup on the same helpers means renaming a +resource format only requires changing this file once. +""" + +import random +import string + +# 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 + +# OpenStack / app name prefixes produced by this repository's integration tests. +# 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) + "test-", # github-runner charm application name +) + + +def generate_app_suffix() -> str: + """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) + ) + + +def app_name_from_suffix(suffix: str) -> str: + """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 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* belongs to this repository's CI OpenStack resources.""" + 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: + 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