From 9811f28fe4626f4ac864d4981a7765050f4a63f8 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 21 Aug 2026 07:41:58 +0000 Subject: [PATCH 01/14] test(charms): share the fixtures and GitHub helpers the E2E needs Extracts what the end-to-end suite has to reuse from the integration suite rather than forking it: the configurator and image-builder deploys become parameterised helpers, GARM login is split out of first-run, and the GitHub App env is described by a GithubAppEnv record so a second app can be addressed without duplicating the accessors. Adds dispatch_workflow and wait_for_completion, ported from the equivalents in github-runner-operator, and widens the diagnostics redaction: it now takes sentinel values -- the credential strings themselves -- so juju debug-log and show-unit output is filtered on what a credential actually is rather than on a guessed key name. test_redaction.py guards that, since it is the only thing that stops a later diagnostics change from regressing it. github_app_private_key accepts a pasted PEM as well as a base64 blob, matching how the key is stored in the repository secret. --- charms/tests/integration/conftest.py | 215 +++++++++++++++------ charms/tests/integration/helpers.py | 153 ++++++++++++++- charms/tests/integration/test_helpers.py | 49 +++++ charms/tests/integration/test_redaction.py | 77 ++++++++ 4 files changed, 421 insertions(+), 73 deletions(-) create mode 100644 charms/tests/integration/test_helpers.py create mode 100644 charms/tests/integration/test_redaction.py diff --git a/charms/tests/integration/conftest.py b/charms/tests/integration/conftest.py index 1f713888..f396683a 100644 --- a/charms/tests/integration/conftest.py +++ b/charms/tests/integration/conftest.py @@ -1,7 +1,9 @@ # Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. +import base64 import json import logging +import os import secrets import string import subprocess @@ -24,7 +26,14 @@ ) from urllib3.util.retry import Retry -from tests.integration.helpers import GITHUB_PATH_ENV_VAR, create_github_app_client, required_env, TEST_RSA_PRIVATE_KEY +from tests.integration.helpers import ( + E2E_APP_ENV, + GITHUB_APP_PRIVATE_KEY_ENV_VAR, + GITHUB_PATH_ENV_VAR, + create_github_app_client, + required_env, + TEST_RSA_PRIVATE_KEY, +) GARM_API_PORT = 8080 GARM_ADMIN_CREDENTIALS_LABEL = "garm-admin-credentials" @@ -33,17 +42,31 @@ logger = logging.getLogger(__name__) -def _redact_pebble_output(output: str) -> str: - """Remove environment mappings before logging structured Pebble output.""" +def _redact_sentinels(text: str, sentinel_values: list[str] | None = None) -> str: + """Replace cleartext credential values with a redaction marker.""" + if not sentinel_values: + return text + for val in sentinel_values: + if val: + text = text.replace(val, "[REDACTED]") + return text + + +def _redact_pebble_output(output: str, sentinel_values: list[str] | None = None) -> str: + """Remove environment mappings and redact sensitive values before logging output.""" + res = _redact_sentinels(output, sentinel_values) + try: - parsed = yaml.safe_load(output) + parsed = yaml.safe_load(res) except yaml.YAMLError: - return output + return res def redact(value: Any) -> Any: if isinstance(value, dict): return { - key: "[REDACTED]" if key == "environment" else redact(item) + key: "[REDACTED]" + if key == "environment" + else redact(item) for key, item in value.items() } if isinstance(value, list): @@ -52,7 +75,7 @@ def redact(value: Any) -> Any: if isinstance(parsed, (dict, list)): return yaml.safe_dump(redact(parsed), sort_keys=False) - return output + return res @pytest.fixture(scope="module") @@ -389,6 +412,7 @@ def _pre_pull_garm_image(image: str) -> None: def _collect_debug_info(juju: jubilant.Juju, app_name: str) -> None: """Collect k8s, Juju, and Pebble debug information after a deployment failure.""" + sentinel_values = _credential_sentinels() unit = f"{app_name}/0" logger.error("=== Debug info for failed GARM deployment: unit=%s ===", unit) for cmd in [ @@ -406,7 +430,14 @@ def _collect_debug_info(juju: jubilant.Juju, app_name: str) -> None: ]: try: out = subprocess.run(cmd, capture_output=True, text=True, timeout=30) - logger.error("$ %s\n%s%s", " ".join(cmd), out.stdout, out.stderr) + # describe pod renders the container spec, so this output can carry the + # workload's environment: redact it like the Juju and Pebble output below. + logger.error( + "$ %s\n%s%s", + " ".join(cmd), + _redact_sentinels(out.stdout, sentinel_values), + _redact_sentinels(out.stderr, sentinel_values), + ) except Exception as exc: logger.error("Failed to run %s: %s", cmd, exc) @@ -416,7 +447,7 @@ def _collect_debug_info(juju: jubilant.Juju, app_name: str) -> None: ("debug-log", "--replay", f"--include={unit}"), ): try: - output = juju.cli(*args) + output = _redact_sentinels(juju.cli(*args), sentinel_values) logger.error("$ juju %s\n%s", " ".join(args), output) except Exception: logger.exception("Failed to collect: juju %s", " ".join(args)) @@ -431,13 +462,28 @@ def _collect_debug_info(juju: jubilant.Juju, app_name: str) -> None: "$ juju exec --unit %s -- %s\n%s%s", unit, command, - _redact_pebble_output(result.stdout), - _redact_pebble_output(result.stderr), + _redact_pebble_output(result.stdout, sentinel_values=sentinel_values), + _redact_pebble_output(result.stderr, sentinel_values=sentinel_values), ) except Exception: logger.exception("Failed to collect workload command: %s", command) +def _credential_sentinels() -> list[str]: + """Cleartext credential values that must never reach a log line.""" + values = [os.environ.get("OS_PASSWORD", "")] + for key_env in (GITHUB_APP_PRIVATE_KEY_ENV_VAR, E2E_APP_ENV.private_key): + encoded_key = os.environ.get(key_env, "") + if not encoded_key: + continue + values.append(encoded_key) + try: + values.append(base64.b64decode(encoded_key).decode()) + except ValueError: + pass + return [value for value in values if value] + + def _assert_garm_unit_healthy(juju: jubilant.Juju, app_name: str) -> None: """Fail immediately and collect diagnostics when the GARM unit is errored.""" unit_name = f"{app_name}/0" @@ -499,7 +545,6 @@ def deploy_garm_app_no_integration_fixture( delay=10, ) except TimeoutError: - logger.error("GARM app '%s' did not reach blocked status within 600s", app_name) _collect_debug_info(juju, app_name) raise @@ -556,7 +601,6 @@ def integrate_garm_with_postgresql_fixture( delay=10, ) except TimeoutError: - logger.error("GARM app '%s' did not reach active status within 600s", app_name) _collect_debug_info(juju, app_name) raise @@ -564,21 +608,16 @@ def integrate_garm_with_postgresql_fixture( return app_name -@pytest.fixture(scope="module", name="any_charm_image_builder_app") -def deploy_any_charm_image_builder_app_fixture(juju: jubilant.Juju) -> str: - """Deploy any-charm as a fake image builder providing github_runner_image_v0. - - On relation joined, the fake builder immediately writes a synthetic image UUID - to its unit relation data, allowing the configurator to transition to Active. - """ - app_name = "fake-image-builder" - +def _deploy_image_builder( + juju: jubilant.Juju, app_name: str, image_id: str, tags: str = "x64,noble" +) -> str: + """Deploy any-charm as an image builder providing github_runner_image_v0.""" any_charm_src_overwrite = { - "any_charm.py": textwrap.dedent("""\ + "any_charm.py": textwrap.dedent(f"""\ from any_charm_base import AnyCharmBase - FAKE_IMAGE_ID = "fake-openstack-image-uuid" - FAKE_IMAGE_TAGS = "x64,noble" + IMAGE_ID = {json.dumps(image_id)} + IMAGE_TAGS = {json.dumps(tags)} class AnyCharm(AnyCharmBase): def __init__(self, *args, **kwargs): @@ -589,8 +628,8 @@ def __init__(self, *args, **kwargs): ) def _on_image_relation_joined(self, event): - event.relation.data[self.unit]["id"] = FAKE_IMAGE_ID - event.relation.data[self.unit]["tags"] = FAKE_IMAGE_TAGS + event.relation.data[self.unit]["id"] = IMAGE_ID + event.relation.data[self.unit]["tags"] = IMAGE_TAGS """), } juju.deploy( @@ -607,6 +646,44 @@ def _on_image_relation_joined(self, event): return app_name +def _deploy_configurator( + juju: jubilant.Juju, + charm_file: str, + app_name: str, + config: dict[str, Any], + secret_uris: list[str] | None = None, +) -> str: + """Deploy garm-configurator, grant required secrets, and configure it.""" + juju.deploy(charm=charm_file, app=app_name) + juju.wait( + lambda status: jubilant.all_blocked(status, app_name), + timeout=6 * 60, + delay=10, + ) + + if secret_uris: + for secret_uri in secret_uris: + juju.grant_secret(secret_uri, app_name) + + juju.config(app_name, values=config) + return app_name + + +@pytest.fixture(scope="module", name="any_charm_image_builder_app") +def deploy_any_charm_image_builder_app_fixture(juju: jubilant.Juju) -> str: + """Deploy any-charm as a fake image builder providing github_runner_image_v0. + + On relation joined, the fake builder immediately writes a synthetic image UUID + to its unit relation data, allowing the configurator to transition to Active. + """ + return _deploy_image_builder( + juju=juju, + app_name="fake-image-builder", + image_id="fake-openstack-image-uuid", + tags="x64,noble", + ) + + @pytest.fixture(scope="module", name="any_charm_debug_ssh_app") def deploy_any_charm_debug_ssh_app_fixture(juju: jubilant.Juju) -> str: """Deploy any-charm as a fake tmate server providing the debug-ssh relation.""" @@ -674,37 +751,32 @@ def deploy_configurator_with_image_fixture( content={"value": TEST_RSA_PRIVATE_KEY}, ) - juju.deploy(charm=garm_configurator_charm_file, app=app_name) - juju.wait( - lambda status: jubilant.all_blocked(status, app_name), - timeout=6 * 60, - delay=10, - ) - - juju.grant_secret(password_secret, app_name) - juju.grant_secret(private_key_secret, app_name) + config_values = { + "openstack-auth-url": "https://keystone.example.com:5000/v3", + "openstack-username": "admin", + "openstack-password": password_secret, + "openstack-project-name": "test-project", + "openstack-user-domain-name": "Default", + "openstack-project-domain-name": "Default", + "openstack-region-name": "RegionOne", + "openstack-network": "external-net", + "github-app-id": "12345", + "github-app-installation-id": "67890", + "github-app-private-key": private_key_secret, + "name": "test-scaleset", + "flavor": "m1.large", + "os-arch": "amd64", + "min-idle-runner": "0", + "max-runner": "5", + "org": "test-org", + } - juju.config( + _deploy_configurator( + juju, + garm_configurator_charm_file, app_name, - values={ - "openstack-auth-url": "https://keystone.example.com:5000/v3", - "openstack-username": "admin", - "openstack-password": password_secret, - "openstack-project-name": "test-project", - "openstack-user-domain-name": "Default", - "openstack-project-domain-name": "Default", - "openstack-region-name": "RegionOne", - "openstack-network": "external-net", - "github-app-id": "12345", - "github-app-installation-id": "67890", - "github-app-private-key": private_key_secret, - "name": "test-scaleset", - "flavor": "m1.large", - "os-arch": "amd64", - "min-idle-runner": "0", - "max-runner": "5", - "org": "test-org", - }, + config_values, + secret_uris=[password_secret, private_key_secret], ) juju.integrate(app_name, any_charm_image_builder_app) @@ -1005,13 +1077,12 @@ def _get_admin_credentials(juju: jubilant.Juju) -> dict[str, str]: return content -def _garm_first_run(juju: jubilant.Juju, address: str) -> str: +def _garm_login(juju: jubilant.Juju, address: str) -> str: """Log in to GARM with charm-managed credentials and return an admin JWT. The charm creates the admin user automatically via _maybe_first_run(). This function reads credentials from the garm-admin-credentials Juju secret, - logs in to obtain a JWT, and configures required controller URLs so GARM will - serve operational API endpoints. + and logs in to obtain a JWT. Retries with backoff to allow GARM time to finish starting and the charm's first-run initialization to complete. @@ -1047,18 +1118,34 @@ def _do_login() -> str: json={"username": creds["username"], "password": creds["password"]}, timeout=30, ) - logger.info( - "login response: status=%d body=%s", resp.status_code, resp.text[:500] - ) if resp.status_code != 200: + # Only the failure body is safe to log: a 200 carries the admin JWT. + logger.info( + "login response: status=%d body=%s", resp.status_code, resp.text[:200] + ) raise _LoginRetryable( f"Unexpected login status {resp.status_code}: {resp.text[:200]}" ) + logger.info("login response: status=%d", resp.status_code) token = resp.json().get("token", "") assert token, "Expected non-empty JWT token from login" return token - token = _do_login() + return _do_login() + + +def _garm_first_run(juju: jubilant.Juju, address: str) -> str: + """Log in to GARM and configure required controller URLs for standalone integration tests. + + Args: + juju: Jubilant Juju handle (used to read admin credentials from Juju secret). + address: GARM unit IP address. + + Returns: + JWT token string for authenticated API calls. + """ + token = _garm_login(juju, address) + base_url = f"http://{address}:{GARM_API_PORT}/api/v1" # Configure controller URLs — GARM requires metadata_url and callback_url # before it will serve operational endpoints (returns 409 otherwise) @@ -1068,7 +1155,7 @@ def _do_login() -> str: "callback_url": f"http://{address}:{GARM_API_PORT}/api/v1/callbacks", "webhook_url": f"http://{address}:{GARM_API_PORT}/webhooks", } - resp = session.put( + resp = requests.put( f"{base_url}/controller", json=controller_payload, headers=headers, timeout=30 ) logger.info( diff --git a/charms/tests/integration/helpers.py b/charms/tests/integration/helpers.py index 2d27100d..f3852c6b 100644 --- a/charms/tests/integration/helpers.py +++ b/charms/tests/integration/helpers.py @@ -1,14 +1,16 @@ # Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. import base64 +import datetime import json import os import time +from dataclasses import dataclass from typing import Any import jubilant import pytest -from github import Github +from github import Github, GithubException from github.Auth import AppAuth, AppInstallationAuth GITHUB_APP_ID_ENV_VAR = "TEST_GITHUB_APP_ID" @@ -16,6 +18,40 @@ GITHUB_APP_PRIVATE_KEY_ENV_VAR = "TEST_GITHUB_APP_PRIVATE_KEY" GITHUB_PATH_ENV_VAR = "TEST_GITHUB_PATH" +E2E_GITHUB_APP_ID_ENV_VAR = "E2E_GITHUB_APP_ID" +E2E_GITHUB_APP_INSTALLATION_ID_ENV_VAR = "E2E_GITHUB_APP_INSTALLATION_ID" +E2E_GITHUB_APP_PRIVATE_KEY_ENV_VAR = "E2E_GITHUB_APP_PRIVATE_KEY" + +# Set by GitHub Actions to "owner/repo"; the GARM E2E targets the repository it runs +# from, so the scaleset entity, the dispatch target and the workflow file agree by +# construction and no separate path setting can drift out of sync with them. +GITHUB_REPOSITORY_ENV_VAR = "GITHUB_REPOSITORY" + + +@dataclass(frozen=True) +class GithubAppEnv: + """Names of the environment variables holding one GitHub App's credentials. + + The integration suite and the GARM E2E authenticate as different Apps, installed on + different repositories with different permissions, so each needs its own set. + """ + + app_id: str + installation_id: str + private_key: str + + +INTEGRATION_APP_ENV = GithubAppEnv( + app_id=GITHUB_APP_ID_ENV_VAR, + installation_id=GITHUB_APP_INSTALLATION_ID_ENV_VAR, + private_key=GITHUB_APP_PRIVATE_KEY_ENV_VAR, +) +E2E_APP_ENV = GithubAppEnv( + app_id=E2E_GITHUB_APP_ID_ENV_VAR, + installation_id=E2E_GITHUB_APP_INSTALLATION_ID_ENV_VAR, + private_key=E2E_GITHUB_APP_PRIVATE_KEY_ENV_VAR, +) + # A throwaway RSA key used only to satisfy GARM's GitHub App credential parsing in # integration tests. Not a real secret. TEST_RSA_PRIVATE_KEY = """-----BEGIN RSA PRIVATE KEY----- @@ -63,7 +99,7 @@ def required_env(name: str) -> str: """Return a required environment variable or fail the running test.""" value = os.environ.get(name) if not value: - pytest.fail(f"{name} is required for webhook redelivery integration test") + pytest.fail(f"{name} is required but was empty or unset") return value @@ -76,17 +112,39 @@ def required_int_env(name: str) -> int: pytest.fail(f"{name} must be an integer") -def create_github_app_client() -> Github: - """Create a GitHub client authenticated as the test app installation.""" - # Private key is stored base64-encoded in CI secrets to avoid GITHUB_ENV multiline issues. - private_key = base64.b64decode(required_env(GITHUB_APP_PRIVATE_KEY_ENV_VAR)).decode() +def github_app_private_key(env: GithubAppEnv = INTEGRATION_APP_ENV) -> str: + """Return the GitHub App private key as a PEM string. + + Accepts either the PEM as GitHub issues it or its base64 encoding, so the same + secret works wherever it is set from. + """ + # CI carries the key base64-encoded because the channels it crosses take one + # KEY=value per line, but a developer exporting it locally has no such constraint + # and should not have to discover the encoding. + value = required_env(env.private_key) + if "-----BEGIN" in value: + return value + try: + # validate=True: the default discards characters outside the alphabet, which + # would turn a mangled key into plausible-looking bytes instead of an error. + return base64.b64decode(value, validate=True).decode() + except ValueError: + pytest.fail( + f"{env.private_key} is neither a PEM nor valid base64. Set it to the key " + f"file's contents." + ) + + +def create_github_app_client(env: GithubAppEnv = INTEGRATION_APP_ENV) -> Github: + """Create a GitHub client authenticated as the given App's installation.""" + private_key = github_app_private_key(env) app_auth = AppAuth( - app_id=required_int_env(GITHUB_APP_ID_ENV_VAR), + app_id=required_int_env(env.app_id), private_key=private_key, ) installation_auth = AppInstallationAuth( app_auth=app_auth, - installation_id=required_int_env(GITHUB_APP_INSTALLATION_ID_ENV_VAR), + installation_id=required_int_env(env.installation_id), ) return Github(auth=installation_auth) @@ -99,4 +157,81 @@ def trigger_failed_workflow_job_delivery( github_client = create_github_app_client() repo = github_client.get_repo(repo_path) workflow = repo.get_workflow(workflow_path) - workflow.create_dispatch(ref=repo.default_branch) + # throw=True: the default returns False on error, so a dispatch the App is not + # permitted to make would leave the test asserting against the webhook ping + # delivery alone and still passing. + workflow.create_dispatch(ref=repo.default_branch, throw=True) + + +def dispatch_workflow( + github_client: Github, + repo_path: str, + workflow_path: str, + ref: str, + inputs: dict[str, Any], +) -> int: + """Dispatch a workflow run and return its run ID. + + Args: + github_client: Authenticated PyGithub instance. + repo_path: ``org/repo`` string. + workflow_path: Path in the repository to the workflow file. + ref: Git ref (branch or tag) to run the workflow on. + inputs: Workflow dispatch inputs. + + Returns: + The workflow run ID. + """ + repo = github_client.get_repo(repo_path) + workflow = repo.get_workflow(workflow_path) + # Snapshot existing run IDs to disambiguate concurrent or recent runs + existing_run_ids = {run.id for run in workflow.get_runs(branch=ref, event="workflow_dispatch")} + try: + # throw=True: the default returns False on error, which would surface a + # permissions failure as an unrelated "no new run appeared" timeout below. + workflow.create_dispatch(ref=ref, inputs=inputs, throw=True) + except GithubException as e: + pytest.fail(f"dispatch_workflow failed: {e.status} {e.data}") + + # After dispatch, poll for the run ID that is not in the initial snapshot. + for _ in range(30): + time.sleep(2) + runs = workflow.get_runs(branch=ref, event="workflow_dispatch") + for run in runs: + if run.id not in existing_run_ids: + return run.id + + pytest.fail( + f"Workflow {workflow_path} did not produce a new run on {ref} after dispatch" + ) + + +def wait_for_completion( + github_client: Github, + repo_path: str, + run_id: int, + poll_interval: int = 15, + timeout: int = 600, +) -> str | None: + """Poll a workflow run until it completes, returning the conclusion. + + Args: + github_client: Authenticated PyGithub instance. + repo_path: ``org/repo`` string. + run_id: Workflow run ID to monitor. + poll_interval: Seconds between polls. + timeout: Max seconds to wait. + + Returns: + The run conclusion string (e.g. ``"success"``, ``"failure"``) or None. + """ + repo = github_client.get_repo(repo_path) + deadline = datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta( + seconds=timeout + ) + while datetime.datetime.now(datetime.timezone.utc) < deadline: + run = repo.get_workflow_run(run_id) + if run.status == "completed": + return run.conclusion + time.sleep(poll_interval) + pytest.fail(f"Workflow run {run_id} did not complete within {timeout}s") diff --git a/charms/tests/integration/test_helpers.py b/charms/tests/integration/test_helpers.py new file mode 100644 index 00000000..752a9ab3 --- /dev/null +++ b/charms/tests/integration/test_helpers.py @@ -0,0 +1,49 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Unit tests for the shared integration helpers.""" + +import base64 + +import pytest + +from tests.integration.helpers import ( + INTEGRATION_APP_ENV, + TEST_RSA_PRIVATE_KEY, + github_app_private_key, +) + + +@pytest.mark.parametrize( + "stored", + [ + pytest.param(TEST_RSA_PRIVATE_KEY, id="pem-as-issued"), + pytest.param( + base64.b64encode(TEST_RSA_PRIVATE_KEY.encode()).decode(), id="base64-encoded" + ), + ], +) +def test_github_app_private_key_accepts_either_form(monkeypatch, stored: str): + """ + arrange: The private key set in the environment, once as the PEM GitHub issues and + once base64-encoded as CI carries it. + act: Read it back through github_app_private_key. + assert: Both yield the same PEM, so a key pasted directly and a key encoded for + transport authenticate identically. + """ + monkeypatch.setenv(INTEGRATION_APP_ENV.private_key, stored) + + assert github_app_private_key(INTEGRATION_APP_ENV) == TEST_RSA_PRIVATE_KEY + + +def test_github_app_private_key_rejects_a_mangled_value(monkeypatch): + """ + arrange: A private key that is neither a PEM nor valid base64. + act: Read it back through github_app_private_key. + assert: It fails naming the variable, rather than decoding to plausible bytes that + would surface later as an opaque authentication error. + """ + monkeypatch.setenv(INTEGRATION_APP_ENV.private_key, "not a key!!") + + with pytest.raises(pytest.fail.Exception, match=INTEGRATION_APP_ENV.private_key): + github_app_private_key(INTEGRATION_APP_ENV) diff --git a/charms/tests/integration/test_redaction.py b/charms/tests/integration/test_redaction.py new file mode 100644 index 00000000..59214a65 --- /dev/null +++ b/charms/tests/integration/test_redaction.py @@ -0,0 +1,77 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. +"""Unit tests for pebble output and diagnostic redaction helpers.""" + +import base64 + +import yaml + +from tests.integration.conftest import _credential_sentinels, _redact_pebble_output + + +def test_redact_pebble_output_removes_environment(): + """ + arrange: A sample Pebble plan YAML containing an environment block. + act: Run _redact_pebble_output on the YAML string. + assert: The environment entry is replaced with [REDACTED]. + """ + sample_plan = yaml.safe_dump({ + "services": { + "garm": { + "override": "replace", + "command": "/charm/bin/garm", + "environment": { + "GARM_PROVIDERS_JSON": '[{"password": "secret-openstack-password"}]', + "SOME_OTHER_KEY": "some-value", + }, + } + } + }) + + redacted = _redact_pebble_output(sample_plan) + assert "secret-openstack-password" not in redacted + assert "[REDACTED]" in redacted + + +def test_redact_pebble_output_with_sentinel_values(): + """ + arrange: A sample output containing cleartext sentinel passwords in arbitrary text. + act: Run _redact_pebble_output with sentinel_values specified. + assert: All occurrences of the sentinel passwords are replaced with [REDACTED]. + """ + sample_output = "Connected with password secret-tenant-pass123 in log line." + redacted = _redact_pebble_output( + sample_output, sentinel_values=["secret-tenant-pass123"] + ) + assert "secret-tenant-pass123" not in redacted + assert "[REDACTED]" in redacted + + +def test_credential_sentinels_reads_env(monkeypatch): + """ + arrange: OS_PASSWORD and a base64-encoded TEST_GITHUB_APP_PRIVATE_KEY set in the environment. + act: Call _credential_sentinels. + assert: The raw password, the base64 form, and its decoded PEM form are all returned. + """ + pem_body = "-----BEGIN RSA PRIVATE KEY-----\nfake\n-----END RSA PRIVATE KEY-----" + encoded_key = base64.b64encode(pem_body.encode()).decode() + monkeypatch.setenv("OS_PASSWORD", "super-secret-pw") + monkeypatch.setenv("TEST_GITHUB_APP_PRIVATE_KEY", encoded_key) + + sentinels = _credential_sentinels() + + assert "super-secret-pw" in sentinels + assert encoded_key in sentinels + assert pem_body in sentinels + + +def test_credential_sentinels_excludes_unset_vars(monkeypatch): + """ + arrange: Neither OS_PASSWORD nor TEST_GITHUB_APP_PRIVATE_KEY set in the environment. + act: Call _credential_sentinels. + assert: No sentinel values are returned, so unset credentials never masquerade as redaction targets. + """ + monkeypatch.delenv("OS_PASSWORD", raising=False) + monkeypatch.delenv("TEST_GITHUB_APP_PRIVATE_KEY", raising=False) + + assert _credential_sentinels() == [] From 3d6dcdbd83d1138fad998c767bd1ff81d82d66f2 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 21 Aug 2026 07:44:05 +0000 Subject: [PATCH 02/14] ci(garm-e2e): wire the end-to-end suite into spread and tox Adds an e2e-test backend pinned to the private-endpoint runner and a charms/tests/e2e/ suite, kept separate from the integration suite so opcli does not discover the end-to-end test as part of the pull-request matrix. Restricting charms_integration.yaml to 'integration-test-ci:*' is what keeps it off the merge gate: the filter defaults to empty, which would schedule every backend in spread.yaml, ProdStack included, on every pull request. concierge-e2e.yaml.tmpl adds a MetalLB addon whose range the workflow fills in with the runner's own address. traefik-k8s needs a LoadBalancer service microk8s cannot otherwise satisfy, and that address is what makes GARM's callback and metadata URLs reachable from a runner VM on the tenant. --- .github/workflows/charms_integration.yaml | 4 ++ concierge-e2e.yaml.tmpl | 26 +++++++++++ spread.yaml | 54 +++++++++++++++++++++++ tox.ini | 31 ++++++------- 4 files changed, 100 insertions(+), 15 deletions(-) create mode 100644 concierge-e2e.yaml.tmpl diff --git a/.github/workflows/charms_integration.yaml b/.github/workflows/charms_integration.yaml index 67e0a77d..b6363d53 100644 --- a/.github/workflows/charms_integration.yaml +++ b/.github/workflows/charms_integration.yaml @@ -16,6 +16,10 @@ jobs: actions: read with: working-directory: . + # Restrict the matrix to the integration backend. The filter defaults to empty, + # which would include every backend in spread.yaml — and so run the ProdStack + # e2e suite on every pull request. + spread-jobs-include: 'integration-test-ci:*' test-secret-1-name: TEST_GITHUB_APP_ID test-secret-2-name: TEST_GITHUB_APP_INSTALLATION_ID test-secret-3-name: TEST_GITHUB_APP_PRIVATE_KEY diff --git a/concierge-e2e.yaml.tmpl b/concierge-e2e.yaml.tmpl new file mode 100644 index 00000000..1f454c2e --- /dev/null +++ b/concierge-e2e.yaml.tmpl @@ -0,0 +1,26 @@ +juju: + channel: 3.6/stable + model-defaults: + test-mode: "true" + automatically-retry-hooks: "false" + +providers: + lxd: + enable: true + microk8s: + channel: 1.34-strict + enable: true + bootstrap: true + addons: + - hostpath-storage + - dns + - rbac + # traefik-k8s asks for a LoadBalancer Service, which microk8s cannot satisfy on + # its own; MetalLB supplies the external address that makes GARM's callback and + # metadata URLs reachable from a runner VM on the tenant. The workflow fills in + # the range before invoking concierge. + - metallb:@HOST_IP@-@HOST_IP@ + +host: + snaps: + rockcraft: diff --git a/spread.yaml b/spread.yaml index a9ef0ec6..cca47b58 100644 --- a/spread.yaml +++ b/spread.yaml @@ -11,13 +11,39 @@ backends: cpu: 4 memory: 8 disk: 20 + e2e-test: + type: integration-test + systems: + - ubuntu-24.04-e2e: + runner: [self-hosted-linux-amd64-noble-private-endpoint-medium] + cpu: 4 + memory: 8 + disk: 40 environment: CONCIERGE: '$(HOST: echo "${CONCIERGE:-concierge.yaml}")' OPCLI_GIT_REF: '$(HOST: echo "${OPCLI_GIT_REF:-main}")' + OPCLI_ARTIFACTS_BUILD_YAML: '$(HOST: echo "${OPCLI_ARTIFACTS_BUILD_YAML:-}")' + GITHUB_RUN_ID: '$(HOST: echo "${GITHUB_RUN_ID:-}")' + GITHUB_REF_NAME: '$(HOST: echo "${GITHUB_REF_NAME:-}")' + GITHUB_REPOSITORY: '$(HOST: echo "${GITHUB_REPOSITORY:-}")' + E2E_GITHUB_APP_ID: '$(HOST: echo "${E2E_GITHUB_APP_ID:-}")' + E2E_GITHUB_APP_INSTALLATION_ID: '$(HOST: echo "${E2E_GITHUB_APP_INSTALLATION_ID:-}")' + E2E_GITHUB_APP_PRIVATE_KEY: '$(HOST: echo "${E2E_GITHUB_APP_PRIVATE_KEY:-}")' TEST_GITHUB_APP_ID: '$(HOST: echo "${TEST_GITHUB_APP_ID:-}")' TEST_GITHUB_APP_INSTALLATION_ID: '$(HOST: echo "${TEST_GITHUB_APP_INSTALLATION_ID:-}")' TEST_GITHUB_APP_PRIVATE_KEY: '$(HOST: echo "${TEST_GITHUB_APP_PRIVATE_KEY:-}")' TEST_GITHUB_PATH: '$(HOST: echo "${TEST_GITHUB_PATH:-}")' + OS_AUTH_URL: '$(HOST: echo "${OS_AUTH_URL:-}")' + OS_USERNAME: '$(HOST: echo "${OS_USERNAME:-}")' + OS_PASSWORD: '$(HOST: echo "${OS_PASSWORD:-}")' + OS_PROJECT_NAME: '$(HOST: echo "${OS_PROJECT_NAME:-}")' + OS_USER_DOMAIN_NAME: '$(HOST: echo "${OS_USER_DOMAIN_NAME:-}")' + OS_PROJECT_DOMAIN_NAME: '$(HOST: echo "${OS_PROJECT_DOMAIN_NAME:-}")' + OS_REGION_NAME: '$(HOST: echo "${OS_REGION_NAME:-}")' + OS_NETWORK: '$(HOST: echo "${OS_NETWORK:-}")' + E2E_RUNNER_IMAGE_NAME: '$(HOST: echo "${E2E_RUNNER_IMAGE_NAME:-}")' + E2E_RUNNER_HTTP_PROXY: '$(HOST: echo "${E2E_RUNNER_HTTP_PROXY:-}")' + E2E_OPENSTACK_FLAVOR: '$(HOST: echo "${E2E_OPENSTACK_FLAVOR:-}")' exclude: - .git - .tox @@ -32,7 +58,35 @@ integration-suites: environment: TOX_ENV: charms-integration pytest-environment-template: | + OPCLI_ARTIFACTS_BUILD_YAML={{ env.get("OPCLI_ARTIFACTS_BUILD_YAML", "") }} TEST_GITHUB_APP_ID={{ env.get("TEST_GITHUB_APP_ID", "") }} TEST_GITHUB_APP_INSTALLATION_ID={{ env.get("TEST_GITHUB_APP_INSTALLATION_ID", "") }} TEST_GITHUB_APP_PRIVATE_KEY={{ env.get("TEST_GITHUB_APP_PRIVATE_KEY", "") }} TEST_GITHUB_PATH={{ env.get("TEST_GITHUB_PATH", "") }} + charms/tests/e2e/: + summary: GARM end-to-end test on ProdStack + kill-timeout: 120m + working-dir: ./ + backends: + - e2e-test + environment: + TOX_ENV: garm-e2e + pytest-environment-template: | + OPCLI_ARTIFACTS_BUILD_YAML={{ env.get("OPCLI_ARTIFACTS_BUILD_YAML", "") }} + GITHUB_RUN_ID={{ env.get("GITHUB_RUN_ID", "") }} + GITHUB_REF_NAME={{ env.get("GITHUB_REF_NAME", "") }} + GITHUB_REPOSITORY={{ env.get("GITHUB_REPOSITORY", "") }} + E2E_GITHUB_APP_ID={{ env.get("E2E_GITHUB_APP_ID", "") }} + E2E_GITHUB_APP_INSTALLATION_ID={{ env.get("E2E_GITHUB_APP_INSTALLATION_ID", "") }} + E2E_GITHUB_APP_PRIVATE_KEY={{ env.get("E2E_GITHUB_APP_PRIVATE_KEY", "") }} + OS_AUTH_URL={{ env.get("OS_AUTH_URL", "") }} + OS_USERNAME={{ env.get("OS_USERNAME", "") }} + OS_PASSWORD={{ env.get("OS_PASSWORD", "") }} + OS_PROJECT_NAME={{ env.get("OS_PROJECT_NAME", "") }} + OS_USER_DOMAIN_NAME={{ env.get("OS_USER_DOMAIN_NAME", "") }} + OS_PROJECT_DOMAIN_NAME={{ env.get("OS_PROJECT_DOMAIN_NAME", "") }} + OS_REGION_NAME={{ env.get("OS_REGION_NAME", "") }} + OS_NETWORK={{ env.get("OS_NETWORK", "") }} + E2E_RUNNER_IMAGE_NAME={{ env.get("E2E_RUNNER_IMAGE_NAME", "") }} + E2E_RUNNER_HTTP_PROXY={{ env.get("E2E_RUNNER_HTTP_PROXY", "") }} + E2E_OPENSTACK_FLAVOR={{ env.get("E2E_OPENSTACK_FLAVOR", "") }} diff --git a/tox.ini b/tox.ini index e6a08579..62ef38bb 100644 --- a/tox.ini +++ b/tox.ini @@ -10,6 +10,17 @@ min_version = 4.0.0 [vars] tests_path = {tox_root}/charms/tests actions_path = {tox_root}/actions +# Shared by charms-integration and garm-e2e: both drive pytest through opcli +# against a live Juju model, so both need the opcli-managed artifact/CI plumbing. +opcli_pass_env = + PYTEST_ADDOPTS + OPCLI_ARTIFACTS_BUILD_YAML + SPREAD_JOB + JUJU_* + KUBECONFIG +opcli_deps = + opcli @ git+https://github.com/canonical/charm-ci.git@main + -r {[vars]tests_path}/integration/requirements.txt [testenv] setenv = @@ -90,11 +101,7 @@ commands = [testenv:charms-integration] pass_env = - PYTEST_ADDOPTS - OPCLI_ARTIFACTS_BUILD_YAML - SPREAD_JOB - JUJU_* - KUBECONFIG + {[vars]opcli_pass_env} TEST_GITHUB_APP_ID TEST_GITHUB_APP_INSTALLATION_ID TEST_GITHUB_APP_PRIVATE_KEY @@ -102,9 +109,7 @@ pass_env = description = Run combined charm integration tests set_env = PYTHONPATH = {tox_root}/charms -deps = - opcli @ git+https://github.com/canonical/charm-ci.git@main - -r {[vars]tests_path}/integration/requirements.txt +deps = {[vars]opcli_deps} commands = pytest -v \ -s \ @@ -114,20 +119,16 @@ commands = [testenv:garm-e2e] pass_env = - PYTEST_ADDOPTS - OPCLI_ARTIFACTS_BUILD_YAML - SPREAD_JOB - JUJU_* - KUBECONFIG + {[vars]opcli_pass_env} OS_* E2E_* GITHUB_RUN_ID GITHUB_REF_NAME + GITHUB_REPOSITORY description = Run the GARM end-to-end test against ProdStack set_env = PYTHONPATH = {tox_root}/charms -deps = - -r {[vars]tests_path}/integration/requirements.txt +deps = {[vars]opcli_deps} commands = pytest -v \ -s \ From 401a31fcb1e9fc2658746db58db5789f059fc72c Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 21 Aug 2026 11:12:55 +0000 Subject: [PATCH 03/14] ci(garm-e2e): pass the runner image, flavor and proxy through to the suite spread.yaml and tox.ini already carried these three onward, but nothing put them into the environment in the first place, so the fixtures saw them empty. The image name is required and fails fast when absent, since a blank one surfaces much later as an image lookup that finds nothing. The flavor and proxy are written only when set: the fixtures supply their own defaults, and an exported empty string is not the same as absent -- it would win over the default rather than yield to it. --- .github/workflows/garm_e2e.yaml | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/garm_e2e.yaml b/.github/workflows/garm_e2e.yaml index f25f1200..bdce820c 100644 --- a/.github/workflows/garm_e2e.yaml +++ b/.github/workflows/garm_e2e.yaml @@ -52,13 +52,16 @@ jobs: E2E_GITHUB_APP_ID: ${{ secrets.E2E_GITHUB_APP_ID }} E2E_GITHUB_APP_INSTALLATION_ID: ${{ secrets.E2E_GITHUB_APP_INSTALLATION_ID }} E2E_GITHUB_APP_PRIVATE_KEY: ${{ secrets.E2E_GITHUB_APP_PRIVATE_KEY }} + E2E_RUNNER_IMAGE_NAME: ${{ secrets.E2E_RUNNER_IMAGE_NAME }} + E2E_OPENSTACK_FLAVOR: ${{ secrets.E2E_OPENSTACK_FLAVOR }} + E2E_RUNNER_HTTP_PROXY: ${{ secrets.E2E_RUNNER_HTTP_PROXY }} run: | set -euo pipefail for KEY in OS_AUTH_URL OS_PROJECT_NAME OS_USER_DOMAIN_NAME \ OS_PROJECT_DOMAIN_NAME OS_REGION_NAME OS_NETWORK \ E2E_GITHUB_APP_ID E2E_GITHUB_APP_INSTALLATION_ID \ - E2E_GITHUB_APP_PRIVATE_KEY; do + E2E_GITHUB_APP_PRIVATE_KEY E2E_RUNNER_IMAGE_NAME; do if [ -z "${!KEY:-}" ]; then echo "::error::Missing repository secret: $KEY (Settings > Secrets > Actions)." exit 1 @@ -68,10 +71,20 @@ jobs: # Everything but the key is single-line already. for KEY in OS_AUTH_URL OS_PROJECT_NAME OS_USER_DOMAIN_NAME \ OS_PROJECT_DOMAIN_NAME OS_REGION_NAME OS_NETWORK \ - E2E_GITHUB_APP_ID E2E_GITHUB_APP_INSTALLATION_ID; do + E2E_GITHUB_APP_ID E2E_GITHUB_APP_INSTALLATION_ID \ + E2E_RUNNER_IMAGE_NAME; do echo "${KEY}=${!KEY}" >> "$GITHUB_ENV" done + # Written only when set: the fixtures fall back to their own defaults for + # these two, and an exported empty string is not absent -- it would win + # over the default rather than yield to it. + for KEY in E2E_OPENSTACK_FLAVOR E2E_RUNNER_HTTP_PROXY; do + if [ -n "${!KEY:-}" ]; then + echo "${KEY}=${!KEY}" >> "$GITHUB_ENV" + fi + done + # The key travels through two channels that take one KEY=value per line — # this file, and later opcli's pytest-environment-template, which rejects a # line without an '='. Normalising here rather than requiring a pre-encoded From 4369aededb90fab495200924bd0cec6001fc1028 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 21 Aug 2026 11:16:13 +0000 Subject: [PATCH 04/14] ci(garm-e2e): re-add the label trigger to verify the new configuration path The image name, flavor and proxy were never exported before, so nothing has run with them present. Assert the image name reaches pytest as well, since a value that loads but does not arrive is the failure this is meant to catch. The trigger is temporary and comes out before review. --- .github/workflows/garm_e2e.yaml | 9 +++++++++ charms/tests/e2e/test_garm_e2e.py | 1 + 2 files changed, 10 insertions(+) diff --git a/.github/workflows/garm_e2e.yaml b/.github/workflows/garm_e2e.yaml index bdce820c..32551126 100644 --- a/.github/workflows/garm_e2e.yaml +++ b/.github/workflows/garm_e2e.yaml @@ -10,6 +10,10 @@ name: GARM E2E on: workflow_dispatch: + # Temporary: label a pull request `run-e2e` to exercise this workflow from its + # branch. Removed again once the run has served its purpose. + pull_request: + types: [opened, synchronize, labeled] permissions: contents: read @@ -17,6 +21,11 @@ permissions: jobs: e2e: name: GARM E2E test + # `pull_request`, never `pull_request_target`: a fork's code must not run with + # access to these secrets. A fork PR gets none, so it can only fail fast. + if: >- + github.event_name == 'workflow_dispatch' + || contains(github.event.pull_request.labels.*.name, 'run-e2e') runs-on: self-hosted-linux-amd64-noble-private-endpoint-medium timeout-minutes: 15 steps: diff --git a/charms/tests/e2e/test_garm_e2e.py b/charms/tests/e2e/test_garm_e2e.py index 0cd545df..b74f6022 100644 --- a/charms/tests/e2e/test_garm_e2e.py +++ b/charms/tests/e2e/test_garm_e2e.py @@ -33,6 +33,7 @@ "E2E_GITHUB_APP_ID", "E2E_GITHUB_APP_INSTALLATION_ID", "E2E_GITHUB_APP_PRIVATE_KEY", + "E2E_RUNNER_IMAGE_NAME", ) From 039d64026796770613f7d1dff3f4b59ae53aec27 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 21 Aug 2026 13:45:59 +0000 Subject: [PATCH 05/14] test(garm-e2e): dispatch the runner workflow and assert the job succeeds Deploys GARM behind a traefik ingress with the configurator holding real tenant credentials, waits for a runner to register, dispatches garm_e2e_test_run.yaml at the scale set's label and asserts the run concludes successfully -- which is only reachable if GARM authenticated to OpenStack, booted a VM from the published image, and the runner registered, claimed the job and exited clean. Waiting on runner_status rather than the provider's status: the VM reports running well before the agent inside it has registered, so following the provider's view dispatches into a label with nothing listening. Instances are matched by scale set rather than taken from the model at large. The dispatch wait outlasts the dispatched job's own ceiling, so a wedged runner surfaces as that job timing out rather than as this wait giving up first, and it covers queue time, which timeout-minutes does not span. Teardown waits for the drain instead of sleeping through it: GARM rejects the delete while the scale set still owns instances, and a rejected delete is a VM left running on the tenant. --- charms/tests/e2e/conftest.py | 294 ++++++++++++++++++++++++++++++ charms/tests/e2e/test_garm_e2e.py | 174 +++++++++++++----- 2 files changed, 423 insertions(+), 45 deletions(-) create mode 100644 charms/tests/e2e/conftest.py diff --git a/charms/tests/e2e/conftest.py b/charms/tests/e2e/conftest.py new file mode 100644 index 00000000..1769af5a --- /dev/null +++ b/charms/tests/e2e/conftest.py @@ -0,0 +1,294 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. +"""E2E-only fixtures for the GARM end-to-end test. + +Reuses credential-agnostic fixtures from integration conftest: +``juju``, ``garm_charm_file``, ``garm_app_image``, ``garm_configurator_charm_file``, +``postgresql``, ``garm_app_deployed``, ``garm_app``. +""" + +import logging +import os +import re +import time +import uuid +from typing import Iterator + +import jubilant +import pytest +import requests + +# Registers these fixtures with pytest: importing a function decorated with +# @pytest.fixture is enough for pytest to pick it up under its own name. +# `pytest_plugins` would do the same but hard-errors as soon as a sibling +# suite (e.g. charms/tests/integration) is collected in the same session. +from tests.integration.conftest import ( # noqa: F401 + _collect_debug_info, + _deploy_configurator, + _deploy_image_builder, + _garm_login, + _get_garm_address, + deploy_garm_app_no_integration_fixture, + deploy_postgresql_server_fixture, + garm_app_image_fixture, + garm_charm_file_fixture, + garm_configurator_charm_file_fixture, + integrate_garm_with_postgresql_fixture, + juju, +) +from tests.integration.helpers import ( + E2E_APP_ENV, + GITHUB_REPOSITORY_ENV_VAR, + github_app_private_key, + required_env, + required_int_env, +) + +logger = logging.getLogger(__name__) + +GARM_API_PORT = 8080 +SCALESET_DRAIN_TIMEOUT = 10 * 60 +TRAEFIK_CHANNEL = "2/stable" + + +@pytest.fixture(scope="module", name="openstack_credentials") +def openstack_credentials_fixture() -> dict[str, str]: + """Read real ProdStack OpenStack credentials from the environment. + + Never a pytest CLI option — argv shows up in logs and in ``ps``. + """ + return { + "auth_url": required_env("OS_AUTH_URL"), + "username": required_env("OS_USERNAME"), + "password": required_env("OS_PASSWORD"), + "project_name": required_env("OS_PROJECT_NAME"), + "user_domain_name": required_env("OS_USER_DOMAIN_NAME"), + "project_domain_name": required_env("OS_PROJECT_DOMAIN_NAME"), + "region_name": required_env("OS_REGION_NAME"), + "network": required_env("OS_NETWORK"), + } + + +@pytest.fixture(scope="module", name="traefik") +def deploy_traefik_fixture(juju: jubilant.Juju) -> str: + """Deploy traefik-k8s with trust and wait for active. + + The E2E needs ingress at all because a runner VM on the tenant has to reach GARM's + callback and metadata URLs, which the in-cluster service address cannot serve. + + ``ingress`` is a standard charm relation interface with several providers; the charm + requires it via the ``charms.traefik_k8s.v2.ingress`` library the go-framework + extension vendors. traefik-k8s is picked here as the usual provider on microk8s, not + because it is the only one that would satisfy the relation. + """ + app_name = "traefik-k8s" + juju.deploy(app_name, channel=TRAEFIK_CHANNEL, trust=True) + juju.wait( + lambda status: jubilant.all_active(status, app_name), + error=lambda status: jubilant.any_error(status, app_name), + timeout=10 * 60, + delay=10, + ) + return app_name + + +@pytest.fixture(scope="module", name="garm_with_ingress") +def integrate_garm_ingress_fixture( + juju: jubilant.Juju, + garm_app: str, + traefik: str, +) -> str: + """Integrate GARM with traefik and assert controller URLs resolve to the LB IP. + + This is the guard that the callback path is actually live before a VM is spawned. + """ + app_name = garm_app + juju.integrate(f"{app_name}:ingress", traefik) + juju.wait( + lambda status: jubilant.all_active(status, app_name, traefik), + error=lambda status: jubilant.any_error(status, app_name, traefik), + timeout=10 * 60, + delay=10, + ) + + address = _get_garm_address(juju, app_name) + token = _garm_login(juju, address) + headers = {"Authorization": f"Bearer {token}"} + + traefik_status = juju.status() + traefik_unit = f"{traefik}/0" + traefik_ip = traefik_status.apps[traefik].units[traefik_unit].address + + # Assert GARM's controller-info reports metadata_url with the Traefik LB IP + resp = requests.get( + f"http://{address}:{GARM_API_PORT}/api/v1/controller-info", + headers=headers, + timeout=30, + ) + resp.raise_for_status() + controller = resp.json() + metadata_url = controller.get("metadata_url", "") + logger.info("GARM controller metadata_url: %s (expected host: %s)", metadata_url, traefik_ip) + assert traefik_ip in metadata_url, ( + f"Expected metadata_url to contain Traefik LB IP {traefik_ip}, got: {metadata_url}" + ) + assert not re.search(r"\.svc\.", metadata_url), ( + f"Expected metadata_url to be routable (not .svc), got: {metadata_url}" + ) + return app_name + + +@pytest.fixture(scope="module", name="real_image_builder") +def deploy_real_image_builder_fixture(juju: jubilant.Juju) -> str: + """Deploy any-charm as an image builder publishing a real image name.""" + image_name = required_env("E2E_RUNNER_IMAGE_NAME") + return _deploy_image_builder( + juju=juju, + app_name="image-builder", + image_id=image_name, + tags="x64,noble", + ) + + +@pytest.fixture(scope="module", name="e2e_scaleset") +def deploy_e2e_scaleset_fixture( + juju: jubilant.Juju, + garm_with_ingress: str, + openstack_credentials: dict[str, str], + real_image_builder: str, + garm_configurator_charm_file: str, +) -> Iterator[str]: + """Deploy garm-configurator with real tenant values and a unique run label. + + Creates Juju secrets for the password and private key, deploys the configurator, + integrates with the image builder and GARM, and waits for the scaleset to register. + Returns the unique runner label that runners will register with. + """ + app_name = "garm-configurator" + run_id = os.environ.get("GITHUB_RUN_ID", uuid.uuid4().hex[:8]) + label = f"garm-e2e-{run_id}" + garm_app = garm_with_ingress + creds = openstack_credentials + repo = required_env(GITHUB_REPOSITORY_ENV_VAR) + + private_key_decoded = github_app_private_key(E2E_APP_ENV) + runner_http_proxy = os.environ.get("E2E_RUNNER_HTTP_PROXY", "") + + # Create secrets + password_secret = juju.add_secret( + name="e2e-os-password", + content={"value": creds["password"]}, + ) + private_key_secret = juju.add_secret( + name="e2e-github-private-key", + content={"value": private_key_decoded}, + ) + + config_values = { + "openstack-auth-url": creds["auth_url"], + "openstack-username": creds["username"], + "openstack-password": password_secret, + "openstack-project-name": creds["project_name"], + "openstack-user-domain-name": creds["user_domain_name"], + "openstack-project-domain-name": creds["project_domain_name"], + "openstack-region-name": creds["region_name"], + "openstack-network": creds["network"], + "github-app-id": str(required_int_env(E2E_APP_ENV.app_id)), + "github-app-installation-id": str(required_int_env(E2E_APP_ENV.installation_id)), + "github-app-private-key": private_key_secret, + "name": label, + "labels": label, + "flavor": os.environ.get("E2E_OPENSTACK_FLAVOR", "m1.small"), + "os-arch": "amd64", + "min-idle-runner": "1", + "max-runner": "1", + "repo": repo, + } + if runner_http_proxy: + config_values["runner-http-proxy"] = runner_http_proxy + + _deploy_configurator( + juju, + garm_configurator_charm_file, + app_name, + config_values, + secret_uris=[password_secret, private_key_secret], + ) + + # Integrate with image builder first + juju.integrate(app_name, real_image_builder) + try: + juju.wait( + lambda status: jubilant.all_active(status, app_name), + error=lambda status: jubilant.any_error(status, app_name), + timeout=6 * 60, + delay=10, + ) + except (TimeoutError, jubilant.WaitError): + _collect_debug_info(juju, app_name) + raise + + # Then integrate with GARM + juju.integrate(app_name, garm_app) + try: + juju.wait( + lambda status: jubilant.all_active(status, app_name) + and jubilant.all_agents_idle(status, garm_app), + error=lambda status: jubilant.any_error(status, app_name), + timeout=10 * 60, + delay=10, + ) + except (TimeoutError, jubilant.WaitError): + _collect_debug_info(juju, garm_app) + raise + + yield label + + # Best effort only: the workflow's own sweep is what guarantees no VM is left + # behind, since a fixture cannot run if the model or the runner dies mid-test. + try: + address = _get_garm_address(juju, garm_app) + headers = {"Authorization": f"Bearer {_garm_login(juju, address)}"} + base_url = f"http://{address}:{GARM_API_PORT}/api/v1" + + response = requests.get(f"{base_url}/scalesets", headers=headers, timeout=30) + response.raise_for_status() + scaleset = next((s for s in response.json() or [] if s.get("name") == label), None) + if scaleset is not None: + scaleset_id = scaleset["id"] + logger.info("Draining E2E scale set %s (%s)", scaleset_id, label) + # Disabling stops replacement; min_idle_runners=0 lets the existing ones go. + requests.patch( + f"{base_url}/scalesets/{scaleset_id}", + json={"enabled": False, "min_idle_runners": 0}, + headers=headers, + timeout=30, + ).raise_for_status() + + # GARM rejects the delete while the scale set still owns instances, so wait + # for the drain rather than racing it -- a failed delete here is a VM left + # running on the tenant. + deadline = time.time() + SCALESET_DRAIN_TIMEOUT + while time.time() < deadline: + instances = requests.get( + f"{base_url}/scalesets/{scaleset_id}/instances", headers=headers, timeout=30 + ) + instances.raise_for_status() + remaining = instances.json() or [] + if not remaining: + break + logger.info("Waiting for %d instance(s) to drain", len(remaining)) + time.sleep(10) + else: + logger.warning( + "Scale set %s still had instances after %ds; deleting anyway", + scaleset_id, + SCALESET_DRAIN_TIMEOUT, + ) + + requests.delete( + f"{base_url}/scalesets/{scaleset_id}", headers=headers, timeout=30 + ).raise_for_status() + logger.info("Deleted E2E scale set %s", scaleset_id) + except (requests.RequestException, ValueError, KeyError) as exc: + logger.warning("Best-effort scale set teardown did not complete: %s", exc) diff --git a/charms/tests/e2e/test_garm_e2e.py b/charms/tests/e2e/test_garm_e2e.py index b74f6022..128a093f 100644 --- a/charms/tests/e2e/test_garm_e2e.py +++ b/charms/tests/e2e/test_garm_e2e.py @@ -1,57 +1,141 @@ # Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. +"""GARM end-to-end test on ProdStack.""" -"""GARM end-to-end test. +import logging +import time -TODO: This module is deliberately incomplete. It exists so that the workflows have -something to run; the end-to-end implementation lands in a follow-up. What is here -asserts only that the credentials reach pytest. +import jubilant +import pytest +import requests -Still to come: deploy postgresql, GARM, a traefik ingress and garm-configurator; assert -the GARM API is reachable and the provider authenticates to OpenStack; assert a VM is -created from the runner image and the runner registers with GitHub; then dispatch -``garm_e2e_test_run.yaml`` at the scale set's label and assert the job is picked up and -exits clean. -""" +from tests.integration.conftest import _garm_login, _get_garm_address +from tests.integration.helpers import ( + E2E_APP_ENV, + GITHUB_REPOSITORY_ENV_VAR, + create_github_app_client, + dispatch_workflow, + required_env, + wait_for_completion, +) -import os +logger = logging.getLogger(__name__) -import pytest +WORKFLOW_PATH = ".github/workflows/garm_e2e_test_run.yaml" +GARM_API_PORT = 8080 -# Settings the deployment fixtures will need. The OpenStack username and password come -# from Vault, the rest from repository secrets; both routes converge on the job -# environment, which tox forwards to pytest. -REQUIRED_SETTINGS = ( - "OS_AUTH_URL", - "OS_USERNAME", - "OS_PASSWORD", - "OS_PROJECT_NAME", - "OS_USER_DOMAIN_NAME", - "OS_PROJECT_DOMAIN_NAME", - "OS_REGION_NAME", - "OS_NETWORK", - "E2E_GITHUB_APP_ID", - "E2E_GITHUB_APP_INSTALLATION_ID", - "E2E_GITHUB_APP_PRIVATE_KEY", - "E2E_RUNNER_IMAGE_NAME", -) +# GARM tracks two independent lifecycles. `status` is the provider's view -- the VM +# exists and is running -- which is reached well before the agent inside it has +# registered. Only `runner_status` reflects GitHub having seen the runner, so it is +# what a dispatch can safely follow; `pending` is registered-but-not-yet-usable. +# Values from params.RunnerStatus in GARM. +REGISTERED_RUNNER_STATUSES = ("idle", "active") -@pytest.mark.parametrize("setting", REQUIRED_SETTINGS) -def test_setting_reaches_pytest(setting: str): +def test_garm_e2e(juju: jubilant.Juju, garm_with_ingress: str, e2e_scaleset: str): """ - arrange: The workflow has resolved the settings, taking the OpenStack username and - password from Vault and the rest from repository secrets, and exported them. - act: Read the setting pytest inherited through tox. - assert: It is present, so the deployment fixtures can authenticate to the tenant and - to GitHub. - Only the name is reported on failure — printing the value would defeat the - masking the workflow applied. + arrange: GARM deployed with postgresql + traefik ingress; garm-configurator holding real + ProdStack credentials, a stable runner-image name, and a unique run label; + GARM's controller metadata_url resolved to the routable LB address. + act: Dispatch garm_e2e_test_run.yaml against the run label and wait for completion. + assert: The workflow run concludes 'success' — which is only reachable if GARM + authenticated to OpenStack, booted a VM on the published image, the runner + registered with GitHub, picked up the job, and exited clean. + """ + repo_path = required_env(GITHUB_REPOSITORY_ENV_VAR) + label = e2e_scaleset # Unique runner label returned by e2e_scaleset fixture + ref = required_env("GITHUB_REF_NAME") + + # Wait for runner VM to spawn and register before dispatching + _wait_for_runner_online(juju, garm_with_ingress, label) + + github_client = create_github_app_client(E2E_APP_ENV) + + logger.info("Dispatching %s on %s with label %s", WORKFLOW_PATH, ref, label) + run_id = dispatch_workflow( + github_client=github_client, + repo_path=repo_path, + workflow_path=WORKFLOW_PATH, + ref=ref, + inputs={"runner-label": label}, + ) + + logger.info("Workflow run %d dispatched, waiting for completion", run_id) + # Longer than garm_e2e_test_run.yaml's own timeout-minutes, so a wedged runner + # surfaces as that job timing out -- which names the step that hung -- rather than + # as this wait expiring first and reporting only that nothing finished. It also has + # to cover queue time: the job does not start until a VM has booted and registered, + # and timeout-minutes does not span that. + conclusion = wait_for_completion( + github_client=github_client, + repo_path=repo_path, + run_id=run_id, + poll_interval=15, + timeout=45 * 60, + ) + + assert conclusion == "success", ( + f"Workflow run {run_id} concluded as '{conclusion}', expected 'success'" + ) + logger.info("GARM E2E test passed: workflow run %s concluded as 'success'", run_id) + + +def _wait_for_runner_online( + juju: jubilant.Juju, + garm_app: str, + scaleset_name: str, + timeout: int = 15 * 60, + poll_interval: int = 15, +) -> None: + """Block until the named scale set has a runner GitHub has registered. + + Args: + juju: Juju client for the model GARM is deployed in. + garm_app: Name of the deployed GARM application. + scaleset_name: Name of the scale set whose runners to wait for. + timeout: Seconds to wait before failing the test. + poll_interval: Seconds between polls. """ - # pytest.fail rather than assert: assertion rewriting would introspect the - # expression and dump the whole of os.environ into the failure output. - if not os.environ.get(setting): - pytest.fail( - f"{setting} did not reach pytest. Check that the workflow exports it and " - f"that tox passes it through in the garm-e2e environment." - ) + address = _get_garm_address(juju, garm_app) + base_url = f"http://{address}:{GARM_API_PORT}/api/v1" + token = _garm_login(juju, address) + deadline = time.time() + timeout + logger.info("Waiting for a registered runner in scale set %r", scaleset_name) + + while time.time() < deadline: + try: + headers = {"Authorization": f"Bearer {token}"} + scalesets = requests.get(f"{base_url}/scalesets", headers=headers, timeout=30) + if scalesets.status_code == 401: + # The poll window outlives the JWT; renew and retry on the next pass. + token = _garm_login(juju, address) + time.sleep(poll_interval) + continue + scalesets.raise_for_status() + scaleset = next( + (s for s in scalesets.json() or [] if s.get("name") == scaleset_name), None + ) + if scaleset is not None: + instances = requests.get( + f"{base_url}/scalesets/{scaleset['id']}/instances", + headers=headers, + timeout=30, + ) + instances.raise_for_status() + for instance in instances.json() or []: + if instance.get("runner_status") in REGISTERED_RUNNER_STATUSES: + logger.info( + "Runner %s registered (runner_status=%s)", + instance.get("name"), + instance.get("runner_status"), + ) + return + except (requests.RequestException, ValueError) as exc: + logger.warning("Transient error polling GARM, retrying: %s", exc) + + time.sleep(poll_interval) + + pytest.fail( + f"No runner in scale set {scaleset_name!r} reached a registered state " + f"({' or '.join(REGISTERED_RUNNER_STATUSES)}) within {timeout}s." + ) From 05b79f82a951b12ea98f236b90c3ce876ae2a879 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 21 Aug 2026 13:57:45 +0000 Subject: [PATCH 06/14] ci(garm-e2e): build the artifacts and run the suite through spread Adds the build job the suite deploys from and swaps the direct tox call for opcli spread run, which is what supplies the charm_paths and resource-image fixtures and fetches the build's artifacts. The e2e job waits for the build rather than provisioning alongside it, as charm-ci's reusable workflow does: overlapping them only holds the private-endpoint runner idle until the artifacts exist, and that runner is scarce. concierge is pointed at a MetalLB pool of this host's own address. traefik-k8s asks for a LoadBalancer service microk8s cannot satisfy unaided, and that address is what lets a runner VM on the tenant reach GARM's callback and metadata URLs. Runners left on the tenant are deleted whatever the outcome, matched on the garm-controller-id GARM stamps on every server it creates so the sweep cannot reach anything else. A fixture cannot cover this: it does not run if the model or the runner dies mid-test, which is exactly when VMs are left behind. No juju debug-log or status dump on failure, unlike charm-ci's template: both replay charm output carrying the rendered GARM config, and so the tenant password. The suite's own diagnostics go through the sentinel redactor. --- .github/workflows/garm_e2e.yaml | 93 +++++++++++++++++++++++++++++++-- 1 file changed, 90 insertions(+), 3 deletions(-) diff --git a/.github/workflows/garm_e2e.yaml b/.github/workflows/garm_e2e.yaml index 32551126..671a62a7 100644 --- a/.github/workflows/garm_e2e.yaml +++ b/.github/workflows/garm_e2e.yaml @@ -19,15 +19,42 @@ permissions: contents: read jobs: + # Builds the rocks and charms the suite deploys, and publishes artifacts.build.yaml + # for opcli to fetch during the spread prepare. + build: + if: >- + github.event_name == 'workflow_dispatch' + || contains(github.event.pull_request.labels.*.name, 'run-e2e') + uses: canonical/charm-ci/.github/workflows/build-artifacts.yml@v0.0.1-alpha.10 + permissions: + contents: read + packages: write + actions: read + with: + working-directory: . + e2e: name: GARM E2E test + # Waits for the build rather than provisioning alongside it: the private-endpoint + # runner is scarce, and starting early would only hold it idle until the artifacts + # the spread prepare fetches actually exist. + needs: [build] # `pull_request`, never `pull_request_target`: a fork's code must not run with # access to these secrets. A fork PR gets none, so it can only fail fast. if: >- github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'run-e2e') + permissions: + contents: read + packages: read + # opcli fetches the build job's artifacts from this workflow run. + actions: read runs-on: self-hosted-linux-amd64-noble-private-endpoint-medium - timeout-minutes: 15 + # Longer than the suite's own kill-timeout in spread.yaml, so spread stops the run + # first and says which task hung, rather than the job being cut from underneath it. + # Generous on purpose: it is not worth tuning before a real run says how long a + # deploy, a spawn and a dispatch actually take here. + timeout-minutes: 130 steps: - uses: actions/checkout@v7.0.1 @@ -224,12 +251,72 @@ jobs: set -euo pipefail uv tool install python-openstackclient + # uv installs into ~/.local/bin, which later steps do not inherit; the + # cleanup step needs the client too. + echo "$HOME/.local/bin" >> "$GITHUB_PATH" TOKEN_EXPIRES=$(openstack token issue -f value -c expires) echo "OpenStack token issued (expires: ${TOKEN_EXPIRES}). Tenant reachable." + - name: Install opcli and spread + run: | + set -euo pipefail + uv tool install "opcli[cli] @ git+https://github.com/canonical/charm-ci.git@main" + sudo snap install go --classic + go install github.com/canonical/spread/cmd/spread@latest + sudo ln -sf ~/go/bin/spread /usr/local/bin/spread + + - name: Point concierge at a MetalLB range on this host + run: | + set -euo pipefail + + # traefik-k8s asks for a LoadBalancer service, which microk8s cannot satisfy + # unaided. Pinning the pool to this host's own address is what makes GARM's + # callback and metadata URLs resolve to something a runner VM on the tenant + # can actually reach -- an in-cluster service address cannot serve them. + HOST_IP=$(ip route get 1.1.1.1 | grep -oP 'src \K\S+') + echo "MetalLB pool pinned to ${HOST_IP}" + sed "s/@HOST_IP@/${HOST_IP}/g" concierge-e2e.yaml.tmpl > concierge-e2e.yaml + echo "CONCIERGE=concierge-e2e.yaml" >> "$GITHUB_ENV" + - name: Run the E2E suite + env: + CI: "true" + # Spread reads this from the host environment and the prepare script uses it + # to download the build job's artifacts. Actions does not export it on its own. + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + + # Ask opcli for the selector rather than writing it out here, so a rename in + # spread.yaml cannot leave this step silently selecting nothing. + SELECTOR=$(opcli spread jobs --include 'e2e-test-ci:*' \ + | python3 -c 'import json,sys; print(json.load(sys.stdin)["include"][0]["selector"])') + echo "Running ${SELECTOR}" + opcli spread run -- -vv "${SELECTOR}" + + # Deliberately no juju debug-log or status dump here: both replay charm output + # that carries the rendered GARM config, and so the tenant password. The suite + # collects its own diagnostics through the sentinel redactor instead, and a + # re-run with debug logging drops into the tmate session above. + - name: Delete runners left behind on the tenant + if: always() run: | set -euo pipefail - uv tool install tox --with tox-uv - tox -e garm-e2e + + # GARM stamps every server it creates with garm-controller-id. Matching that + # alongside the runner image scopes the sweep to VMs this suite's GARM + # created, so a failure that skips the suite's own teardown cannot leave them + # running and billing. + ORPHANS=$(openstack server list --long --image "$E2E_RUNNER_IMAGE_NAME" -f json \ + | python3 -c "import json,sys; print('\n'.join(s['ID'] for s in json.load(sys.stdin) if 'garm-controller-id' in (s.get('Properties') or {})))") + + if [ -z "$ORPHANS" ]; then + echo "No runners left on the tenant." + exit 0 + fi + + echo "$ORPHANS" | while read -r ID; do + echo "Deleting orphaned runner ${ID}" + openstack server delete --wait "$ID" || echo "::warning::Could not delete ${ID}" + done From 48d34ab9e9255c84ce66aba837d931cc1ce42caf Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 21 Aug 2026 14:40:08 +0000 Subject: [PATCH 07/14] fix(garm-e2e): deploy traefik from a channel that exists traefik-k8s has no 2 track: juju refused the deploy with "charm or bundle not found for channel 2/stable". Charmhub publishes 1.0 and latest only. --- charms/tests/e2e/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charms/tests/e2e/conftest.py b/charms/tests/e2e/conftest.py index 1769af5a..f66c6a10 100644 --- a/charms/tests/e2e/conftest.py +++ b/charms/tests/e2e/conftest.py @@ -48,7 +48,7 @@ GARM_API_PORT = 8080 SCALESET_DRAIN_TIMEOUT = 10 * 60 -TRAEFIK_CHANNEL = "2/stable" +TRAEFIK_CHANNEL = "latest/stable" @pytest.fixture(scope="module", name="openstack_credentials") From 253171272be3ebd6940215f46a67705c686c5317 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 21 Aug 2026 15:19:09 +0000 Subject: [PATCH 08/14] fix(garm-e2e): do not wait for GARM to be active before the configurator exists GARM reports "Waiting for garm-configurator relation" until the configurator is related, and the fixture that deploys the configurator depends on this one, so waiting for active here could only ever time out. Wait for traefik to serve and for GARM's hook to settle instead: that is what says the ingress has been taken up, which is all this fixture needs before it checks the controller URLs. --- charms/tests/e2e/conftest.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/charms/tests/e2e/conftest.py b/charms/tests/e2e/conftest.py index f66c6a10..7c68d8cd 100644 --- a/charms/tests/e2e/conftest.py +++ b/charms/tests/e2e/conftest.py @@ -104,8 +104,13 @@ def integrate_garm_ingress_fixture( """ app_name = garm_app juju.integrate(f"{app_name}:ingress", traefik) + # GARM cannot reach active here: it reports "Waiting for garm-configurator relation" + # until the configurator arrives, and the fixture that deploys the configurator + # depends on this one. Waiting for traefik to serve and for GARM's hook to settle is + # what says the ingress has actually been taken up. juju.wait( - lambda status: jubilant.all_active(status, app_name, traefik), + lambda status: jubilant.all_active(status, traefik) + and jubilant.all_agents_idle(status, app_name), error=lambda status: jubilant.any_error(status, app_name, traefik), timeout=10 * 60, delay=10, From bbf951e8801acbe409b1b03035e651c0c1751686 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 21 Aug 2026 15:29:51 +0000 Subject: [PATCH 09/14] ci(garm-e2e): gate the temporary trigger on the branch, not a label The repository's labels are managed and run-e2e was deleted from the repo, so a label gate cannot be relied on here. Gate on the development branch instead, which is self-limiting and needs nothing set up by hand. Runs for a ref now cancel any earlier one still going: several full runs queueing against a single private-endpoint runner helps nobody. Still temporary, and still removed before review. --- .github/workflows/garm_e2e.yaml | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/garm_e2e.yaml b/.github/workflows/garm_e2e.yaml index 671a62a7..94c2756d 100644 --- a/.github/workflows/garm_e2e.yaml +++ b/.github/workflows/garm_e2e.yaml @@ -10,21 +10,27 @@ name: GARM E2E on: workflow_dispatch: - # Temporary: label a pull request `run-e2e` to exercise this workflow from its - # branch. Removed again once the run has served its purpose. + # Temporary: runs this workflow from its own development branch, so it can be + # exercised before it exists on the default branch and can be dispatched. + # Removed again once it has served its purpose. A label gate would be tidier but + # the repository's labels are managed, and `run-e2e` was deleted from the repo. pull_request: - types: [opened, synchronize, labeled] + types: [opened, synchronize] permissions: contents: read +concurrency: + group: garm-e2e-${{ github.ref }} + cancel-in-progress: true + jobs: # Builds the rocks and charms the suite deploys, and publishes artifacts.build.yaml # for opcli to fetch during the spread prepare. build: if: >- github.event_name == 'workflow_dispatch' - || contains(github.event.pull_request.labels.*.name, 'run-e2e') + || github.head_ref == 'feat/garm-e2e-implementation-ISD-5876' uses: canonical/charm-ci/.github/workflows/build-artifacts.yml@v0.0.1-alpha.10 permissions: contents: read @@ -43,7 +49,7 @@ jobs: # access to these secrets. A fork PR gets none, so it can only fail fast. if: >- github.event_name == 'workflow_dispatch' - || contains(github.event.pull_request.labels.*.name, 'run-e2e') + || github.head_ref == 'feat/garm-e2e-implementation-ISD-5876' permissions: contents: read packages: read From 269630a1410b0eeeb1a93a1ab570745543501eb1 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 21 Aug 2026 16:52:02 +0000 Subject: [PATCH 10/14] fix(garm-e2e): check the controller URLs once GARM is actually serving The charm's restart() returns before starting the workload while no configurator has supplied provider configs, so GARM's API is not listening when the ingress relation is made. Asking it for controller-info there could only ever retry against a refused connection. Relating the ingress and checking what it produced are now separate: the fixture relates and waits for traefik, and the scale set fixture makes the assertion once the configurator has brought the workload up -- the first moment GARM is serving, and still before any runner has been asked for. --- charms/tests/e2e/conftest.py | 79 ++++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/charms/tests/e2e/conftest.py b/charms/tests/e2e/conftest.py index 7c68d8cd..50aae769 100644 --- a/charms/tests/e2e/conftest.py +++ b/charms/tests/e2e/conftest.py @@ -98,49 +98,59 @@ def integrate_garm_ingress_fixture( garm_app: str, traefik: str, ) -> str: - """Integrate GARM with traefik and assert controller URLs resolve to the LB IP. + """Relate GARM to traefik so its controller URLs become routable. - This is the guard that the callback path is actually live before a VM is spawned. + Args: + juju: Juju client for the model GARM is deployed in. + garm_app: Name of the deployed GARM application. + traefik: Name of the deployed traefik application. + + Returns: + The GARM application name. """ - app_name = garm_app - juju.integrate(f"{app_name}:ingress", traefik) - # GARM cannot reach active here: it reports "Waiting for garm-configurator relation" - # until the configurator arrives, and the fixture that deploys the configurator - # depends on this one. Waiting for traefik to serve and for GARM's hook to settle is - # what says the ingress has actually been taken up. + juju.integrate(f"{garm_app}:ingress", traefik) + # GARM cannot reach active here, and its API is not up either: the charm's restart() + # returns before starting the workload while no configurator has supplied provider + # configs. So this waits for traefik to serve and for GARM's hook to settle, and the + # controller URLs are checked once the configurator has brought the workload up. juju.wait( lambda status: jubilant.all_active(status, traefik) - and jubilant.all_agents_idle(status, app_name), - error=lambda status: jubilant.any_error(status, app_name, traefik), + and jubilant.all_agents_idle(status, garm_app), + error=lambda status: jubilant.any_error(status, garm_app, traefik), timeout=10 * 60, delay=10, ) + return garm_app + - address = _get_garm_address(juju, app_name) - token = _garm_login(juju, address) - headers = {"Authorization": f"Bearer {token}"} +def assert_controller_urls_routable(juju: jubilant.Juju, garm_app: str, traefik: str) -> None: + """Assert GARM advertises callback URLs a runner VM on the tenant can reach. - traefik_status = juju.status() - traefik_unit = f"{traefik}/0" - traefik_ip = traefik_status.apps[traefik].units[traefik_unit].address + Args: + juju: Juju client for the model GARM is deployed in. + garm_app: Name of the deployed GARM application. + traefik: Name of the deployed traefik application. + """ + address = _get_garm_address(juju, garm_app) + headers = {"Authorization": f"Bearer {_garm_login(juju, address)}"} + traefik_ip = juju.status().apps[traefik].units[f"{traefik}/0"].address - # Assert GARM's controller-info reports metadata_url with the Traefik LB IP - resp = requests.get( - f"http://{address}:{GARM_API_PORT}/api/v1/controller-info", - headers=headers, - timeout=30, + response = requests.get( + f"http://{address}:{GARM_API_PORT}/api/v1/controller-info", headers=headers, timeout=30 ) - resp.raise_for_status() - controller = resp.json() - metadata_url = controller.get("metadata_url", "") - logger.info("GARM controller metadata_url: %s (expected host: %s)", metadata_url, traefik_ip) + response.raise_for_status() + metadata_url = response.json().get("metadata_url", "") + logger.info("GARM metadata_url: %s (expecting host %s)", metadata_url, traefik_ip) + + # A spawned VM reaches GARM over the load balancer; the in-cluster service name it + # would otherwise advertise does not resolve outside the cluster, so a runner would + # boot and then never call back. assert traefik_ip in metadata_url, ( - f"Expected metadata_url to contain Traefik LB IP {traefik_ip}, got: {metadata_url}" + f"Expected metadata_url on the traefik LB address {traefik_ip}, got: {metadata_url}" ) assert not re.search(r"\.svc\.", metadata_url), ( - f"Expected metadata_url to be routable (not .svc), got: {metadata_url}" + f"Expected a routable metadata_url, got the in-cluster address: {metadata_url}" ) - return app_name @pytest.fixture(scope="module", name="real_image_builder") @@ -159,6 +169,7 @@ def deploy_real_image_builder_fixture(juju: jubilant.Juju) -> str: def deploy_e2e_scaleset_fixture( juju: jubilant.Juju, garm_with_ingress: str, + traefik: str, openstack_credentials: dict[str, str], real_image_builder: str, garm_configurator_charm_file: str, @@ -233,12 +244,12 @@ def deploy_e2e_scaleset_fixture( _collect_debug_info(juju, app_name) raise - # Then integrate with GARM + # Then integrate with GARM. This is what starts the workload: until provider configs + # arrive from the configurator, the charm's restart() returns before starting it. juju.integrate(app_name, garm_app) try: juju.wait( - lambda status: jubilant.all_active(status, app_name) - and jubilant.all_agents_idle(status, garm_app), + lambda status: jubilant.all_active(status, app_name, garm_app), error=lambda status: jubilant.any_error(status, app_name), timeout=10 * 60, delay=10, @@ -247,6 +258,12 @@ def deploy_e2e_scaleset_fixture( _collect_debug_info(juju, garm_app) raise + # Checked here rather than when the ingress relation is made, which is the first + # moment GARM is serving and still before any runner has been asked for: a VM that + # boots against an unroutable callback URL never reports back, and the failure + # surfaces much later as a runner that simply never registers. + assert_controller_urls_routable(juju, garm_app, traefik) + yield label # Best effort only: the workflow's own sweep is what guarantees no VM is left From 0ca9cd48316ac04d2400f0f7b541937c2528e0cb Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 21 Aug 2026 17:26:04 +0000 Subject: [PATCH 11/14] fix(garm-e2e): compare the callback URL against traefik's serving address The assertion read traefik's unit address, which is its pod IP. GARM advertises the address MetalLB handed traefik, so the check compared two addresses that were never going to match even when everything was wired correctly. Read the address traefik reports serving on instead, and fail clearly if that cannot be parsed rather than silently comparing against nothing. --- charms/tests/e2e/conftest.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/charms/tests/e2e/conftest.py b/charms/tests/e2e/conftest.py index 50aae769..73155837 100644 --- a/charms/tests/e2e/conftest.py +++ b/charms/tests/e2e/conftest.py @@ -133,7 +133,15 @@ def assert_controller_urls_routable(juju: jubilant.Juju, garm_app: str, traefik: """ address = _get_garm_address(juju, garm_app) headers = {"Authorization": f"Bearer {_garm_login(juju, address)}"} - traefik_ip = juju.status().apps[traefik].units[f"{traefik}/0"].address + + # traefik reports the address it serves on, which is the one MetalLB handed it. Its + # unit address is the pod IP, which is not reachable from outside the cluster and so + # is not what GARM should be advertising. + message = juju.status().apps[traefik].app_status.message + serving = re.search(r"https?://(?P[^/\s]+)", message) + if serving is None: + pytest.fail(f"Could not read traefik's serving address from its status: {message!r}") + traefik_ip = serving.group("host") response = requests.get( f"http://{address}:{GARM_API_PORT}/api/v1/controller-info", headers=headers, timeout=30 From d40216854e0e7042afbd855ebed981175d3074ac Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Mon, 24 Aug 2026 07:24:44 +0000 Subject: [PATCH 12/14] fix(garm-e2e): drain the scale set with PUT and force-remove leftovers The teardown sent PATCH to the scaleset update endpoint, which GARM routes only for PUT, so every teardown aborted with a 405 and the model destroy left the scale set, its instances and their JIT runners behind on GitHub. Disable via PUT, force-remove non-running instances (the post-disable scale-down only reclaims running idle runners), and delete the scale set -- which GARM also removes from GitHub. --- charms/tests/e2e/conftest.py | 150 +++++++++++++++++++++++++---------- 1 file changed, 107 insertions(+), 43 deletions(-) diff --git a/charms/tests/e2e/conftest.py b/charms/tests/e2e/conftest.py index 73155837..7e74e188 100644 --- a/charms/tests/e2e/conftest.py +++ b/charms/tests/e2e/conftest.py @@ -277,48 +277,112 @@ def deploy_e2e_scaleset_fixture( # Best effort only: the workflow's own sweep is what guarantees no VM is left # behind, since a fixture cannot run if the model or the runner dies mid-test. try: - address = _get_garm_address(juju, garm_app) - headers = {"Authorization": f"Bearer {_garm_login(juju, address)}"} - base_url = f"http://{address}:{GARM_API_PORT}/api/v1" - - response = requests.get(f"{base_url}/scalesets", headers=headers, timeout=30) - response.raise_for_status() - scaleset = next((s for s in response.json() or [] if s.get("name") == label), None) - if scaleset is not None: - scaleset_id = scaleset["id"] - logger.info("Draining E2E scale set %s (%s)", scaleset_id, label) - # Disabling stops replacement; min_idle_runners=0 lets the existing ones go. - requests.patch( - f"{base_url}/scalesets/{scaleset_id}", - json={"enabled": False, "min_idle_runners": 0}, - headers=headers, - timeout=30, - ).raise_for_status() - - # GARM rejects the delete while the scale set still owns instances, so wait - # for the drain rather than racing it -- a failed delete here is a VM left - # running on the tenant. - deadline = time.time() + SCALESET_DRAIN_TIMEOUT - while time.time() < deadline: - instances = requests.get( - f"{base_url}/scalesets/{scaleset_id}/instances", headers=headers, timeout=30 - ) - instances.raise_for_status() - remaining = instances.json() or [] - if not remaining: - break - logger.info("Waiting for %d instance(s) to drain", len(remaining)) - time.sleep(10) - else: - logger.warning( - "Scale set %s still had instances after %ds; deleting anyway", - scaleset_id, - SCALESET_DRAIN_TIMEOUT, - ) - - requests.delete( - f"{base_url}/scalesets/{scaleset_id}", headers=headers, timeout=30 - ).raise_for_status() - logger.info("Deleted E2E scale set %s", scaleset_id) + _drain_and_delete_scaleset(juju, garm_app, label) except (requests.RequestException, ValueError, KeyError) as exc: logger.warning("Best-effort scale set teardown did not complete: %s", exc) + + +def _drain_and_delete_scaleset(juju: jubilant.Juju, garm_app: str, label: str) -> None: + """Drain and delete the E2E scale set, on GARM and on GitHub. + + Args: + juju: Juju client for the model GARM is deployed in. + garm_app: Name of the deployed GARM application. + label: Name of the scale set to drain and delete. + """ + address = _get_garm_address(juju, garm_app) + headers = {"Authorization": f"Bearer {_garm_login(juju, address)}"} + base_url = f"http://{address}:{GARM_API_PORT}/api/v1" + + response = requests.get(f"{base_url}/scalesets", headers=headers, timeout=30) + response.raise_for_status() + scaleset = next((s for s in response.json() or [] if s.get("name") == label), None) + if scaleset is None: + return + scaleset_id = scaleset["id"] + logger.info("Draining E2E scale set %s (%s)", scaleset_id, label) + + # Disabling stops replacement; min_idle_runners=0 lets the existing ones go. + # PUT, not PATCH: GARM routes only PUT to the scale set update handler, so a + # PATCH is answered with a 405 -- which used to abort the whole teardown here, + # leaving the scale set, its instances and their GitHub runners behind when + # the model was destroyed. + requests.put( + f"{base_url}/scalesets/{scaleset_id}", + json={"enabled": False, "min_idle_runners": 0}, + headers=headers, + timeout=30, + ).raise_for_status() + + # GARM rejects the delete while the scale set still owns instances, so wait + # for the drain rather than racing it -- a failed delete here is a VM left + # running on the tenant. + deadline = time.time() + SCALESET_DRAIN_TIMEOUT + force_removed: set[str] = set() + while time.time() < deadline: + instances = requests.get( + f"{base_url}/scalesets/{scaleset_id}/instances", headers=headers, timeout=30 + ) + instances.raise_for_status() + remaining = instances.json() or [] + if not remaining: + break + # The post-disable scale-down only reclaims *running* idle runners, so + # force-remove anything else once: that covers instances a failed spawn + # left in error, and deleting an instance also removes its JIT runner + # from GitHub -- the source of the offline garm-* leftovers this suite + # used to leave behind. + for instance in remaining: + _force_remove_instance(base_url, headers, instance, force_removed) + logger.info("Waiting for %d instance(s) to drain", len(remaining)) + time.sleep(10) + else: + logger.warning( + "Scale set %s still had instances after %ds; deleting anyway", + scaleset_id, + SCALESET_DRAIN_TIMEOUT, + ) + + # Deletes the scale set on GitHub too, not just in GARM's database. + requests.delete( + f"{base_url}/scalesets/{scaleset_id}", headers=headers, timeout=30 + ).raise_for_status() + logger.info("Deleted E2E scale set %s", scaleset_id) + + +def _force_remove_instance( + base_url: str, + headers: dict[str, str], + instance: dict, + force_removed: set[str], +) -> None: + """Force-remove one scale set instance, at most once, best-effort. + + Args: + base_url: GARM API base URL, ending in ``/api/v1``. + headers: Authorization headers for the GARM API. + instance: Instance payload as returned by the GARM API. + force_removed: Instance names already attempted, so a still-draining + instance is not re-attempted on every poll. + """ + name = instance.get("name") + if not name or name in force_removed: + return + force_removed.add(name) + response = requests.delete( + f"{base_url}/instances/{name}", + params={"forceRemove": "true"}, + headers=headers, + timeout=30, + ) + if response.ok: + logger.info("Force-removing leftover instance %s", name) + else: + # Expected for states GARM refuses to delete (e.g. pending_create); + # the drain timeout below is what bounds those. + logger.warning( + "Could not force-remove instance %s (status=%s): HTTP %d", + name, + instance.get("status"), + response.status_code, + ) From 41b95ebff2dff29983223464b413d24ff2ecaf30 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Mon, 24 Aug 2026 07:24:50 +0000 Subject: [PATCH 13/14] test(garm-e2e): outlast the bootstrap timeout and log why a runner never came up The 900s wait was shorter than GARM's own 20-minute runner bootstrap timeout, failing the test on runners GARM still considered booting. Raise it to 25 minutes, log a trail of instance state transitions while polling, and on timeout dump the instance list and the redacted GARM workload logs -- without them, a spawn failure surfaced only as 'no runner registered', with no way to tell GARM, the provider or OpenStack apart. --- charms/tests/e2e/test_garm_e2e.py | 50 +++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/charms/tests/e2e/test_garm_e2e.py b/charms/tests/e2e/test_garm_e2e.py index 128a093f..5167404d 100644 --- a/charms/tests/e2e/test_garm_e2e.py +++ b/charms/tests/e2e/test_garm_e2e.py @@ -9,7 +9,11 @@ import pytest import requests -from tests.integration.conftest import _garm_login, _get_garm_address +from tests.integration.conftest import ( + _collect_debug_info, + _garm_login, + _get_garm_address, +) from tests.integration.helpers import ( E2E_APP_ENV, GITHUB_REPOSITORY_ENV_VAR, @@ -84,7 +88,7 @@ def _wait_for_runner_online( juju: jubilant.Juju, garm_app: str, scaleset_name: str, - timeout: int = 15 * 60, + timeout: int = 25 * 60, poll_interval: int = 15, ) -> None: """Block until the named scale set has a runner GitHub has registered. @@ -96,10 +100,16 @@ def _wait_for_runner_online( timeout: Seconds to wait before failing the test. poll_interval: Seconds between polls. """ + # Must outlast GARM's own runner bootstrap timeout -- 20 minutes by default, + # and not configurable through garm-configurator. Waiting less fails the test + # on a runner GARM still considers booting, instead of letting GARM reap a + # stuck one and spawn a replacement. address = _get_garm_address(juju, garm_app) base_url = f"http://{address}:{GARM_API_PORT}/api/v1" token = _garm_login(juju, address) deadline = time.time() + timeout + instances: list[dict] = [] + last_summary: list[str] | None = None logger.info("Waiting for a registered runner in scale set %r", scaleset_name) while time.time() < deadline: @@ -116,13 +126,25 @@ def _wait_for_runner_online( (s for s in scalesets.json() or [] if s.get("name") == scaleset_name), None ) if scaleset is not None: - instances = requests.get( + instances_response = requests.get( f"{base_url}/scalesets/{scaleset['id']}/instances", headers=headers, timeout=30, ) - instances.raise_for_status() - for instance in instances.json() or []: + instances_response.raise_for_status() + instances = instances_response.json() or [] + summary = sorted( + f"{i.get('name')}: status={i.get('status')} " + f"runner_status={i.get('runner_status')}" + for i in instances + ) + if summary != last_summary: + # Log on change only: the wait spans many polls, and this trail + # is what shows whether instances are appearing, failing, or + # never being created at all. + last_summary = summary + logger.info("Scale set instances: %s", summary) + for instance in instances: if instance.get("runner_status") in REGISTERED_RUNNER_STATUSES: logger.info( "Runner %s registered (runner_status=%s)", @@ -135,6 +157,24 @@ def _wait_for_runner_online( time.sleep(poll_interval) + # Leave the evidence in the log before failing: the instance state says which + # stage stalled, since GARM only reaches "registered" after spawning an + # instance, booting its VM, and installing the runner against the callback + # URL. An empty list means GARM never spawned an instance at all; + # pending_create means the provider never picked one up; error means the + # provider tried and failed, with GARM's own logs -- collected next, through + # the sentinel redactor -- carrying the reason. + logger.error( + "Instances in scale set %s at timeout: %s", + scaleset_name, + sorted( + f"{i.get('name')}: status={i.get('status')} " + f"runner_status={i.get('runner_status')} provider_id={i.get('provider_id')!r}" + for i in instances + ) + or "none (GARM never spawned an instance)", + ) + _collect_debug_info(juju, garm_app) pytest.fail( f"No runner in scale set {scaleset_name!r} reached a registered state " f"({' or '.join(REGISTERED_RUNNER_STATUSES)}) within {timeout}s." From 6f1cbe3fbf13e69739a81a16bec28166c1d45592 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Mon, 24 Aug 2026 15:10:53 +0000 Subject: [PATCH 14/14] ci(garm-e2e): fail fast when the tenant lacks the image, flavor or network The provider resolves these only at instance creation, so a miss cost the whole build and deploy before surfacing as an opaque error in the redacted workload logs. Resolve them up front with the same OS_* scope the provider runs under -- which can differ from the one a local openstack CLI query uses -- and print the names the tenant does offer when one is missing. --- .github/workflows/garm_e2e.yaml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/.github/workflows/garm_e2e.yaml b/.github/workflows/garm_e2e.yaml index 94c2756d..c636f342 100644 --- a/.github/workflows/garm_e2e.yaml +++ b/.github/workflows/garm_e2e.yaml @@ -264,6 +264,35 @@ jobs: TOKEN_EXPIRES=$(openstack token issue -f value -c expires) echo "OpenStack token issued (expires: ${TOKEN_EXPIRES}). Tenant reachable." + - name: Assert the tenant provides the image, flavor and network + # The provider resolves these only at instance creation, deep into the run: + # a miss there costs the whole build and deploy before surfacing as an + # opaque provider error in the redacted logs. Resolving them here -- with + # the same OS_* scope the provider will use, which may differ from the one + # a local `openstack flavor list` runs under -- fails the job in minutes + # and prints what the tenant does offer. + run: | + set -euo pipefail + + check() { + local KIND="$1" NAME="$2" + if openstack "$KIND" show "$NAME" > /dev/null 2>&1; then + echo "$KIND '$NAME' resolves on the tenant." + return 0 + fi + echo "::error::The $KIND configured for the E2E does not resolve on this tenant. Check that the value exists in the project and region the OS_* secrets point at, and that it is shared with that project if private." + echo "Available ${KIND}s on this tenant (name only):" + openstack "$KIND" list -c Name -f value | sed 's/^/ /' || true + return 1 + } + + check image "$E2E_RUNNER_IMAGE_NAME" + # Optional: unset means the suite falls back to its own default flavor. + if [ -n "${E2E_OPENSTACK_FLAVOR:-}" ]; then + check flavor "$E2E_OPENSTACK_FLAVOR" + fi + check network "$OS_NETWORK" + - name: Install opcli and spread run: | set -euo pipefail