From 79d52e0274962359fbaf40e5ae3d041113ddc901 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Mon, 15 Jun 2026 20:42:28 +0700 Subject: [PATCH 01/12] chore: add garm_configurator_v0 relation endpoints to both charms Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- charms/garm-configurator/charmcraft.yaml | 4 ++++ charms/garm/charmcraft.yaml | 2 ++ 2 files changed, 6 insertions(+) diff --git a/charms/garm-configurator/charmcraft.yaml b/charms/garm-configurator/charmcraft.yaml index 8e7b09e7..77e02ace 100644 --- a/charms/garm-configurator/charmcraft.yaml +++ b/charms/garm-configurator/charmcraft.yaml @@ -108,6 +108,10 @@ config: A Python dict-like string containing key-value pairs of script name to bash script to be run prior to runner installation. +provides: + garm-configurator: + interface: garm_configurator_v0 + requires: image: interface: github_runner_image_v0 diff --git a/charms/garm/charmcraft.yaml b/charms/garm/charmcraft.yaml index 2e2dc4e4..2c0da442 100644 --- a/charms/garm/charmcraft.yaml +++ b/charms/garm/charmcraft.yaml @@ -28,4 +28,6 @@ requires: interface: postgresql_client optional: false limit: 1 + garm-configurator: + interface: garm_configurator_v0 From 7a26c954e229242d3649cca73a6f03300ca657c2 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Tue, 16 Jun 2026 11:00:32 +0700 Subject: [PATCH 02/12] feat(garm): add ScalesetReconciler with diff-and-apply logic Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- charms/garm/src/scaleset_reconciler.py | 140 ++++++++++++ .../tests/unit/test_scaleset_reconciler.py | 199 ++++++++++++++++++ 2 files changed, 339 insertions(+) create mode 100644 charms/garm/src/scaleset_reconciler.py create mode 100644 charms/garm/tests/unit/test_scaleset_reconciler.py diff --git a/charms/garm/src/scaleset_reconciler.py b/charms/garm/src/scaleset_reconciler.py new file mode 100644 index 00000000..5e6b4067 --- /dev/null +++ b/charms/garm/src/scaleset_reconciler.py @@ -0,0 +1,140 @@ +#!/usr/bin/env python3 +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Scaleset reconciler: diffs desired vs observed GARM scalesets and applies changes.""" + +import logging +from dataclasses import dataclass, field + +from garm_api import GarmClient + +logger = logging.getLogger(__name__) + + +@dataclass +class ScalesetSpec: + """Desired state for one GARM scaleset.""" + + name: str + provider_name: str + credentials_name: str + image_id: str + flavor: str + os_arch: str + min_idle_runners: int + max_runners: int + labels: list[str] = field(default_factory=list) + runner_group: str = "default" + pre_install_scripts: dict[str, str] = field(default_factory=dict) + + +class ScalesetReconciler: + """Reconciles GARM scalesets against a desired spec list.""" + + def __init__(self, client: GarmClient) -> None: + """Initialise the reconciler. + + Args: + client: Authenticated GarmClient instance. + """ + self._client = client + + def reconcile(self, desired: list[ScalesetSpec]) -> None: + """Sync GARM scalesets to match *desired*. + + Performs the minimum set of CREATE / UPDATE / DELETE operations. + If a referenced provider or credential is missing for a spec, that + spec is skipped silently (deferred creation) — no error state is set. + + Args: + desired: The full desired set of scalesets. + """ + providers = {provider["name"] for provider in self._client.list_providers()} + credentials = {credential["name"] for credential in self._client.list_credentials()} + observed = {scaleset["name"]: scaleset for scaleset in self._client.list_scalesets()} + + desired_names: set[str] = set() + + for spec in desired: + if spec.provider_name not in providers: + logger.warning( + "Skipping scaleset %s: provider %s not registered yet", + spec.name, + spec.provider_name, + ) + continue + if spec.credentials_name not in credentials: + logger.warning( + "Skipping scaleset %s: credential %s not registered yet", + spec.name, + spec.credentials_name, + ) + continue + + desired_names.add(spec.name) + + if spec.name in observed: + self._maybe_update(observed[spec.name], spec) + else: + self._create(spec) + + for name, scaleset in observed.items(): + if name not in desired_names: + logger.info("Deleting orphaned scaleset %s (id=%s)", name, scaleset["id"]) + self._client.delete_scaleset(scaleset["id"]) + + def _create(self, spec: ScalesetSpec) -> None: + extra_specs: dict[str, object] = {} + if spec.pre_install_scripts: + extra_specs["pre_install_scripts"] = spec.pre_install_scripts + + payload = { + "name": spec.name, + "provider_name": spec.provider_name, + "credentials_name": spec.credentials_name, + "image_id": spec.image_id, + "flavor": spec.flavor, + "os_arch": spec.os_arch, + "min_idle_runners": spec.min_idle_runners, + "max_runners": spec.max_runners, + "tags": sorted(spec.labels), + "runner_group": spec.runner_group, + "extra_specs": extra_specs, + } + logger.info("Creating scaleset %s", spec.name) + self._client.create_scaleset(payload) + + def _maybe_update(self, observed: dict, spec: ScalesetSpec) -> None: + if not self._needs_update(observed, spec): + logger.debug("Scaleset %s is up to date", spec.name) + return + + extra_specs: dict[str, object] = {} + if spec.pre_install_scripts: + extra_specs["pre_install_scripts"] = spec.pre_install_scripts + + payload = { + "image_id": spec.image_id, + "flavor": spec.flavor, + "min_idle_runners": spec.min_idle_runners, + "max_runners": spec.max_runners, + "tags": sorted(spec.labels), + "runner_group": spec.runner_group, + "extra_specs": extra_specs, + } + logger.info("Updating scaleset %s (id=%s)", spec.name, observed["id"]) + self._client.update_scaleset(observed["id"], payload) + + @staticmethod + def _needs_update(observed: dict, spec: ScalesetSpec) -> bool: + observed_scripts = (observed.get("extra_specs") or {}).get("pre_install_scripts", {}) + return ( + observed.get("image_id") != spec.image_id + or observed.get("flavor") != spec.flavor + or observed.get("max_runners") != spec.max_runners + or observed.get("min_idle_runners") != spec.min_idle_runners + or sorted(observed.get("tags") or []) != sorted(spec.labels) + or observed.get("runner_group") != spec.runner_group + or observed_scripts != spec.pre_install_scripts + ) diff --git a/charms/garm/tests/unit/test_scaleset_reconciler.py b/charms/garm/tests/unit/test_scaleset_reconciler.py new file mode 100644 index 00000000..62bf8010 --- /dev/null +++ b/charms/garm/tests/unit/test_scaleset_reconciler.py @@ -0,0 +1,199 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Unit tests for the scaleset reconciler.""" + +from unittest.mock import MagicMock + +from scaleset_reconciler import ScalesetReconciler, ScalesetSpec + + +def _mock_client(providers=None, credentials=None, scalesets=None): + client = MagicMock() + client.list_providers.return_value = providers or [] + client.list_credentials.return_value = credentials or [] + client.list_scalesets.return_value = scalesets or [] + client.create_scaleset.return_value = {"id": "new-uuid", "name": "test"} + client.update_scaleset.return_value = {"id": "uuid-1", "name": "test"} + return client + + +def _spec( + name="my-scaleset", + provider_name="openstack-demo", + credentials_name="github-app-12345", + image_id="ubuntu-22.04", + flavor="m1.small", + os_arch="x64", + min_idle=0, + max_runners=5, + labels=None, + runner_group="default", + pre_install_scripts=None, +): + return ScalesetSpec( + name=name, + provider_name=provider_name, + credentials_name=credentials_name, + image_id=image_id, + flavor=flavor, + os_arch=os_arch, + min_idle_runners=min_idle, + max_runners=max_runners, + labels=labels or [], + runner_group=runner_group, + pre_install_scripts=pre_install_scripts or {}, + ) + + +def test_create_scaleset_when_not_exists(): + client = _mock_client( + providers=[{"name": "openstack-demo"}], + credentials=[{"name": "github-app-12345"}], + scalesets=[], + ) + reconciler = ScalesetReconciler(client) + reconciler.reconcile([_spec()]) + client.create_scaleset.assert_called_once() + call_payload = client.create_scaleset.call_args[0][0] + assert call_payload["name"] == "my-scaleset" + assert call_payload["image_id"] == "ubuntu-22.04" + assert call_payload["flavor"] == "m1.small" + + +def test_skip_creation_when_provider_missing(): + client = _mock_client( + providers=[], + credentials=[{"name": "github-app-12345"}], + scalesets=[], + ) + reconciler = ScalesetReconciler(client) + reconciler.reconcile([_spec()]) + client.create_scaleset.assert_not_called() + + +def test_skip_creation_when_credential_missing(): + client = _mock_client( + providers=[{"name": "openstack-demo"}], + credentials=[], + scalesets=[], + ) + reconciler = ScalesetReconciler(client) + reconciler.reconcile([_spec()]) + client.create_scaleset.assert_not_called() + + +def test_update_scaleset_when_image_changed(): + existing = { + "id": "uuid-1", + "name": "my-scaleset", + "image_id": "ubuntu-20.04", + "flavor": "m1.small", + "max_runners": 5, + "min_idle_runners": 0, + "tags": [], + "runner_group": "default", + "extra_specs": {}, + } + client = _mock_client( + providers=[{"name": "openstack-demo"}], + credentials=[{"name": "github-app-12345"}], + scalesets=[existing], + ) + reconciler = ScalesetReconciler(client) + reconciler.reconcile([_spec(image_id="ubuntu-22.04")]) + client.update_scaleset.assert_called_once() + args = client.update_scaleset.call_args + assert args[0][0] == "uuid-1" + assert args[0][1]["image_id"] == "ubuntu-22.04" + + +def test_no_update_when_scaleset_unchanged(): + existing = { + "id": "uuid-1", + "name": "my-scaleset", + "image_id": "ubuntu-22.04", + "flavor": "m1.small", + "max_runners": 5, + "min_idle_runners": 0, + "tags": [], + "runner_group": "default", + "extra_specs": {}, + } + client = _mock_client( + providers=[{"name": "openstack-demo"}], + credentials=[{"name": "github-app-12345"}], + scalesets=[existing], + ) + reconciler = ScalesetReconciler(client) + reconciler.reconcile([_spec()]) + client.update_scaleset.assert_not_called() + client.create_scaleset.assert_not_called() + + +def test_delete_scaleset_not_in_desired(): + existing = { + "id": "uuid-stale", + "name": "stale-scaleset", + "image_id": "ubuntu-20.04", + "flavor": "m1.small", + "max_runners": 2, + "min_idle_runners": 0, + "tags": [], + "runner_group": "default", + "extra_specs": {}, + } + client = _mock_client( + providers=[{"name": "openstack-demo"}], + credentials=[{"name": "github-app-12345"}], + scalesets=[existing], + ) + reconciler = ScalesetReconciler(client) + reconciler.reconcile([_spec(name="new-scaleset")]) + client.delete_scaleset.assert_called_once_with("uuid-stale") + + +def test_no_delete_when_desired_is_empty(): + client = _mock_client( + providers=[], + credentials=[], + scalesets=[], + ) + reconciler = ScalesetReconciler(client) + reconciler.reconcile([]) + client.delete_scaleset.assert_not_called() + + +def test_pre_install_scripts_included_in_create(): + client = _mock_client( + providers=[{"name": "openstack-demo"}], + credentials=[{"name": "github-app-12345"}], + scalesets=[], + ) + reconciler = ScalesetReconciler(client) + scripts = {"setup.sh": "#!/bin/bash\napt-get update"} + reconciler.reconcile([_spec(pre_install_scripts=scripts)]) + payload = client.create_scaleset.call_args[0][0] + assert payload["extra_specs"]["pre_install_scripts"] == scripts + + +def test_labels_sorted_for_comparison(): + existing = { + "id": "uuid-1", + "name": "my-scaleset", + "image_id": "ubuntu-22.04", + "flavor": "m1.small", + "max_runners": 5, + "min_idle_runners": 0, + "tags": ["z-tag", "a-tag"], + "runner_group": "default", + "extra_specs": {}, + } + client = _mock_client( + providers=[{"name": "openstack-demo"}], + credentials=[{"name": "github-app-12345"}], + scalesets=[existing], + ) + reconciler = ScalesetReconciler(client) + reconciler.reconcile([_spec(labels=["a-tag", "z-tag"])]) + client.update_scaleset.assert_not_called() From c58b0c5cbe7e9dc3d454abfd99e5f430e0f72ae9 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Tue, 16 Jun 2026 11:01:43 +0700 Subject: [PATCH 03/12] feat(garm): add GarmClient REST API wrapper (garm_api.py) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- charms/garm/src/garm_api.py | 280 ++++++++++++++++- charms/garm/src/scaleset_reconciler.py | 133 ++++---- charms/garm/tests/unit/test_garm_api.py | 394 ++++++++++++++---------- 3 files changed, 578 insertions(+), 229 deletions(-) diff --git a/charms/garm/src/garm_api.py b/charms/garm/src/garm_api.py index 613f07f3..5f80f2bd 100644 --- a/charms/garm/src/garm_api.py +++ b/charms/garm/src/garm_api.py @@ -10,11 +10,23 @@ import urllib3.exceptions from garm_client.api.controller_info_api import ControllerInfoApi +from garm_client.api.credentials_api import CredentialsApi from garm_client.api.first_run_api import FirstRunApi +from garm_client.api.login_api import LoginApi +from garm_client.api.organizations_api import OrganizationsApi +from garm_client.api.providers_api import ProvidersApi +from garm_client.api.repositories_api import RepositoriesApi +from garm_client.api.scalesets_api import ScalesetsApi from garm_client.api_client import ApiClient from garm_client.configuration import Configuration from garm_client.exceptions import ApiException +from garm_client.models.create_scale_set_params import CreateScaleSetParams +from garm_client.models.forge_credentials import ForgeCredentials from garm_client.models.new_user_params import NewUserParams +from garm_client.models.password_login_params import PasswordLoginParams +from garm_client.models.provider import Provider +from garm_client.models.scale_set import ScaleSet +from garm_client.models.update_scale_set_params import UpdateScaleSetParams logger = logging.getLogger(__name__) @@ -31,11 +43,15 @@ class GarmConnectionError(GarmApiError): """Raised when a network-level connection to GARM fails (port closed, refused).""" +class GarmEntityNotFoundError(GarmApiError): + """Raised when a required GARM entity (org/repo/provider) cannot be found.""" + + class GarmApiClient: """HTTP client for the GARM REST API. - Covers the two unauthenticated endpoints the charm needs: initialisation - check and first-run admin user creation. + Covers unauthenticated endpoints (initialisation check, first-run) and login. + Use ``GarmAuthenticatedClient`` for authenticated operations. """ def __init__(self, base_url: str) -> None: @@ -83,9 +99,9 @@ def wait_for_ready(self, timeout: float = _READINESS_TIMEOUT) -> None: Polls is_initialized(): both True (200) and False (409) indicate the HTTP server is up. Only retries on GarmConnectionError (network-level failures); unexpected HTTP errors propagate immediately. - + `is_initialized` is used due to it not requiring auth, and there is no dedicated, - readiness/health check API. + readiness/health check API. Args: timeout: Maximum seconds to wait before raising. @@ -142,3 +158,259 @@ def first_run( except urllib3.exceptions.HTTPError as exc: raise GarmConnectionError(f"GARM connection error: {exc}") from exc logger.info("GARM first-run initialisation complete for user '%s'", username) + + def login(self, username: str, password: str) -> str: + """Log in to the GARM API and return a JWT token. + + Args: + username: Admin username. + password: Admin password. + + Raises: + GarmApiError: If login fails or response has no token. + + Returns: + JWT token string. + """ + with self._api_client() as client: + api = LoginApi(api_client=client) + try: + result = api.login( + body=PasswordLoginParams(username=username, password=password), + _request_timeout=_REQUEST_TIMEOUT, + ) + except ApiException as exc: + raise GarmApiError(f"GARM login failed ({exc.status}): {exc.body}") from exc + except urllib3.exceptions.HTTPError as exc: + raise GarmConnectionError(f"GARM connection error: {exc}") from exc + if not result.token: + raise GarmApiError("login response did not contain a token") + return result.token + + +class GarmAuthenticatedClient: + """Authenticated GARM API client for scaleset and entity management operations.""" + + def __init__(self, base_url: str, token: str) -> None: + """Create an authenticated client. + + Args: + base_url: Full base URL including the API prefix, + e.g. ``http://127.0.0.1:9997/api/v1``. + token: JWT Bearer token from ``GarmApiClient.login()``. + """ + self._base_url = base_url + self._token = token + + def _api_client(self) -> ApiClient: + """Build an authenticated ApiClient with JWT Bearer token.""" + return ApiClient( + configuration=Configuration(host=self._base_url), + header_name="Authorization", + header_value=f"Bearer {self._token}", + ) + + def list_providers(self) -> list[Provider]: + """List all registered GARM providers. + + Returns: + List of Provider model objects. + + Raises: + GarmApiError: On API error. + """ + with self._api_client() as client: + try: + return ProvidersApi(api_client=client).list_providers( + _request_timeout=_REQUEST_TIMEOUT + ) or [] + except ApiException as exc: + raise GarmApiError( + f"Failed to list providers ({exc.status}): {exc.body}" + ) from exc + + def list_credentials(self) -> list[ForgeCredentials]: + """List all registered GARM credentials. + + Returns: + List of ForgeCredentials model objects. + + Raises: + GarmApiError: On API error. + """ + with self._api_client() as client: + try: + return CredentialsApi(api_client=client).list_credentials( + _request_timeout=_REQUEST_TIMEOUT + ) or [] + except ApiException as exc: + raise GarmApiError( + f"Failed to list credentials ({exc.status}): {exc.body}" + ) from exc + + def list_scalesets(self) -> list[ScaleSet]: + """List all scalesets across all entities. + + Returns: + List of ScaleSet model objects. + + Raises: + GarmApiError: On API error. + """ + with self._api_client() as client: + try: + return ScalesetsApi(api_client=client).list_scalesets( + _request_timeout=_REQUEST_TIMEOUT + ) or [] + except ApiException as exc: + raise GarmApiError( + f"Failed to list scalesets ({exc.status}): {exc.body}" + ) from exc + + def find_org_id(self, org_name: str) -> str | None: + """Find a GARM organization's UUID by name. + + Args: + org_name: GitHub organization name registered in GARM. + + Returns: + Organization UUID string, or None if not found. + + Raises: + GarmApiError: On API error. + """ + with self._api_client() as client: + try: + orgs = OrganizationsApi(api_client=client).list_orgs( + name=org_name, + _request_timeout=_REQUEST_TIMEOUT, + ) or [] + except ApiException as exc: + raise GarmApiError( + f"Failed to list organizations ({exc.status}): {exc.body}" + ) from exc + for org in orgs: + if org.name == org_name: + return org.id + return None + + def find_repo_id(self, repo_name: str) -> str | None: + """Find a GARM repository's UUID by name. + + Args: + repo_name: Repository name (owner/repo format) registered in GARM. + + Returns: + Repository UUID string, or None if not found. + + Raises: + GarmApiError: On API error. + """ + with self._api_client() as client: + try: + repos = RepositoriesApi(api_client=client).list_repos( + _request_timeout=_REQUEST_TIMEOUT, + ) or [] + except ApiException as exc: + raise GarmApiError( + f"Failed to list repositories ({exc.status}): {exc.body}" + ) from exc + for repo in repos: + if repo.name == repo_name: + return repo.id + return None + + def create_org_scaleset(self, org_id: str, params: CreateScaleSetParams) -> ScaleSet: + """Create a scaleset under a GARM organization. + + Args: + org_id: GARM organization UUID. + params: Scaleset creation parameters. + + Returns: + Created ScaleSet model object. + + Raises: + GarmApiError: On API error. + """ + with self._api_client() as client: + try: + return OrganizationsApi(api_client=client).create_org_scale_set( + org_id=org_id, + body=params, + _request_timeout=_REQUEST_TIMEOUT, + ) + except ApiException as exc: + raise GarmApiError( + f"Failed to create org scaleset ({exc.status}): {exc.body}" + ) from exc + + def create_repo_scaleset(self, repo_id: str, params: CreateScaleSetParams) -> ScaleSet: + """Create a scaleset under a GARM repository. + + Args: + repo_id: GARM repository UUID. + params: Scaleset creation parameters. + + Returns: + Created ScaleSet model object. + + Raises: + GarmApiError: On API error. + """ + with self._api_client() as client: + try: + return RepositoriesApi(api_client=client).create_repo_scale_set( + repo_id=repo_id, + body=params, + _request_timeout=_REQUEST_TIMEOUT, + ) + except ApiException as exc: + raise GarmApiError( + f"Failed to create repo scaleset ({exc.status}): {exc.body}" + ) from exc + + def update_scaleset(self, scaleset_id: int, params: UpdateScaleSetParams) -> ScaleSet: + """Update an existing scaleset. + + Args: + scaleset_id: Integer scaleset ID. + params: Fields to update. + + Returns: + Updated ScaleSet model object. + + Raises: + GarmApiError: On API error. + """ + with self._api_client() as client: + try: + return ScalesetsApi(api_client=client).update_scale_set( + scale_set_id=scaleset_id, + body=params, + _request_timeout=_REQUEST_TIMEOUT, + ) + except ApiException as exc: + raise GarmApiError( + f"Failed to update scaleset {scaleset_id} ({exc.status}): {exc.body}" + ) from exc + + def delete_scaleset(self, scaleset_id: int) -> None: + """Delete a scaleset. + + Args: + scaleset_id: Integer scaleset ID. + + Raises: + GarmApiError: On API error. + """ + with self._api_client() as client: + try: + ScalesetsApi(api_client=client).delete_scale_set( + scale_set_id=scaleset_id, + _request_timeout=_REQUEST_TIMEOUT, + ) + except ApiException as exc: + raise GarmApiError( + f"Failed to delete scaleset {scaleset_id} ({exc.status}): {exc.body}" + ) from exc diff --git a/charms/garm/src/scaleset_reconciler.py b/charms/garm/src/scaleset_reconciler.py index 5e6b4067..9ec9e04d 100644 --- a/charms/garm/src/scaleset_reconciler.py +++ b/charms/garm/src/scaleset_reconciler.py @@ -7,7 +7,9 @@ import logging from dataclasses import dataclass, field -from garm_api import GarmClient +from garm_api import GarmAuthenticatedClient +from garm_client.models.create_scale_set_params import CreateScaleSetParams +from garm_client.models.update_scale_set_params import UpdateScaleSetParams logger = logging.getLogger(__name__) @@ -18,25 +20,26 @@ class ScalesetSpec: name: str provider_name: str - credentials_name: str - image_id: str + image: str flavor: str os_arch: str min_idle_runners: int max_runners: int + entity_type: str + entity_name: str labels: list[str] = field(default_factory=list) - runner_group: str = "default" + runner_group: str = "" pre_install_scripts: dict[str, str] = field(default_factory=dict) class ScalesetReconciler: """Reconciles GARM scalesets against a desired spec list.""" - def __init__(self, client: GarmClient) -> None: + def __init__(self, client: GarmAuthenticatedClient) -> None: """Initialise the reconciler. Args: - client: Authenticated GarmClient instance. + client: Authenticated GarmAuthenticatedClient instance. """ self._client = client @@ -44,17 +47,17 @@ def reconcile(self, desired: list[ScalesetSpec]) -> None: """Sync GARM scalesets to match *desired*. Performs the minimum set of CREATE / UPDATE / DELETE operations. - If a referenced provider or credential is missing for a spec, that - spec is skipped silently (deferred creation) — no error state is set. + If a referenced provider is missing or the target entity (org/repo) + is not registered in GARM, that spec is skipped silently (deferred + creation) — no error state is set. Args: desired: The full desired set of scalesets. """ - providers = {provider["name"] for provider in self._client.list_providers()} - credentials = {credential["name"] for credential in self._client.list_credentials()} - observed = {scaleset["name"]: scaleset for scaleset in self._client.list_scalesets()} + providers = {p.name for p in self._client.list_providers()} + observed = {ss.name: ss for ss in self._client.list_scalesets()} - desired_names: set[str] = set() + all_desired_names: set[str] = {spec.name for spec in desired} for spec in desired: if spec.provider_name not in providers: @@ -64,48 +67,62 @@ def reconcile(self, desired: list[ScalesetSpec]) -> None: spec.provider_name, ) continue - if spec.credentials_name not in credentials: + + entity_id = self._resolve_entity_id(spec) + if entity_id is None: logger.warning( - "Skipping scaleset %s: credential %s not registered yet", + "Skipping scaleset %s: %s '%s' not registered in GARM yet", spec.name, - spec.credentials_name, + spec.entity_type, + spec.entity_name, ) continue - desired_names.add(spec.name) - if spec.name in observed: self._maybe_update(observed[spec.name], spec) else: - self._create(spec) + self._create(spec, entity_id) for name, scaleset in observed.items(): - if name not in desired_names: - logger.info("Deleting orphaned scaleset %s (id=%s)", name, scaleset["id"]) - self._client.delete_scaleset(scaleset["id"]) - - def _create(self, spec: ScalesetSpec) -> None: + if name not in all_desired_names: + logger.info( + "Deleting orphaned scaleset %s (id=%s)", name, scaleset.id + ) + self._client.delete_scaleset(scaleset.id) + + def _resolve_entity_id(self, spec: ScalesetSpec) -> str | None: + """Return the GARM entity UUID for *spec*, or None if not yet registered.""" + if spec.entity_type == "organization": + return self._client.find_org_id(spec.entity_name) + if spec.entity_type == "repository": + return self._client.find_repo_id(spec.entity_name) + logger.warning("Unknown entity_type %r for scaleset %s", spec.entity_type, spec.name) + return None + + def _create(self, spec: ScalesetSpec, entity_id: str) -> None: extra_specs: dict[str, object] = {} if spec.pre_install_scripts: extra_specs["pre_install_scripts"] = spec.pre_install_scripts - payload = { - "name": spec.name, - "provider_name": spec.provider_name, - "credentials_name": spec.credentials_name, - "image_id": spec.image_id, - "flavor": spec.flavor, - "os_arch": spec.os_arch, - "min_idle_runners": spec.min_idle_runners, - "max_runners": spec.max_runners, - "tags": sorted(spec.labels), - "runner_group": spec.runner_group, - "extra_specs": extra_specs, - } - logger.info("Creating scaleset %s", spec.name) - self._client.create_scaleset(payload) - - def _maybe_update(self, observed: dict, spec: ScalesetSpec) -> None: + params = CreateScaleSetParams( + name=spec.name, + provider_name=spec.provider_name, + image=spec.image, + flavor=spec.flavor, + os_arch=spec.os_arch, + min_idle_runners=spec.min_idle_runners, + max_runners=spec.max_runners, + labels=sorted(spec.labels), + github_runner_group=spec.runner_group or None, + extra_specs=extra_specs or None, + ) + logger.info("Creating scaleset %s under %s %s", spec.name, spec.entity_type, entity_id) + if spec.entity_type == "organization": + self._client.create_org_scaleset(entity_id, params) + else: + self._client.create_repo_scaleset(entity_id, params) + + def _maybe_update(self, observed, spec: ScalesetSpec) -> None: if not self._needs_update(observed, spec): logger.debug("Scaleset %s is up to date", spec.name) return @@ -114,27 +131,25 @@ def _maybe_update(self, observed: dict, spec: ScalesetSpec) -> None: if spec.pre_install_scripts: extra_specs["pre_install_scripts"] = spec.pre_install_scripts - payload = { - "image_id": spec.image_id, - "flavor": spec.flavor, - "min_idle_runners": spec.min_idle_runners, - "max_runners": spec.max_runners, - "tags": sorted(spec.labels), - "runner_group": spec.runner_group, - "extra_specs": extra_specs, - } - logger.info("Updating scaleset %s (id=%s)", spec.name, observed["id"]) - self._client.update_scaleset(observed["id"], payload) + params = UpdateScaleSetParams( + image=spec.image, + flavor=spec.flavor, + min_idle_runners=spec.min_idle_runners, + max_runners=spec.max_runners, + runner_group=spec.runner_group or None, + extra_specs=extra_specs or None, + ) + logger.info("Updating scaleset %s (id=%s)", spec.name, observed.id) + self._client.update_scaleset(observed.id, params) @staticmethod - def _needs_update(observed: dict, spec: ScalesetSpec) -> bool: - observed_scripts = (observed.get("extra_specs") or {}).get("pre_install_scripts", {}) + def _needs_update(observed, spec: ScalesetSpec) -> bool: + observed_scripts = (observed.extra_specs or {}).get("pre_install_scripts", {}) return ( - observed.get("image_id") != spec.image_id - or observed.get("flavor") != spec.flavor - or observed.get("max_runners") != spec.max_runners - or observed.get("min_idle_runners") != spec.min_idle_runners - or sorted(observed.get("tags") or []) != sorted(spec.labels) - or observed.get("runner_group") != spec.runner_group + observed.image != spec.image + or observed.flavor != spec.flavor + or observed.max_runners != spec.max_runners + or observed.min_idle_runners != spec.min_idle_runners + or observed.runner_group != (spec.runner_group or None) or observed_scripts != spec.pre_install_scripts ) diff --git a/charms/garm/tests/unit/test_garm_api.py b/charms/garm/tests/unit/test_garm_api.py index 4b4c7260..e123d52e 100644 --- a/charms/garm/tests/unit/test_garm_api.py +++ b/charms/garm/tests/unit/test_garm_api.py @@ -1,175 +1,237 @@ # Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. -"""Unit tests for the GarmApiClient wrapper.""" +"""Unit tests for garm_api.py.""" from unittest.mock import MagicMock, patch import pytest -import urllib3.exceptions - -from garm_api import GarmApiClient, GarmApiError, GarmConnectionError -from garm_client.exceptions import ApiException - - -@pytest.fixture(name="client") -def client_fixture(): - """Return a GarmApiClient pointed at a local test address.""" - return GarmApiClient("http://127.0.0.1:9997/api/v1") - - -def test_is_initialized_returns_true_when_controller_info_succeeds(client): - """ - arrange: ControllerInfoApi.controller_info returns successfully. - act: Call client.is_initialized(). - assert: Returns True. - """ - with patch("garm_api.ControllerInfoApi") as mock_cls: - mock_cls.return_value.controller_info.return_value = MagicMock() - assert client.is_initialized() is True - - -def test_is_initialized_returns_false_on_409(client): - """ - arrange: ControllerInfoApi.controller_info raises ApiException with status 409. - act: Call client.is_initialized(). - assert: Returns False (GARM not yet initialised). - """ - with patch("garm_api.ControllerInfoApi") as mock_cls: - mock_cls.return_value.controller_info.side_effect = ApiException(status=409) - assert client.is_initialized() is False - - -def test_is_initialized_raises_on_unexpected_status(client): - """ - arrange: ControllerInfoApi.controller_info raises ApiException with status 500. - act: Call client.is_initialized(). - assert: Raises GarmApiError with the status code in the message. - """ - with patch("garm_api.ControllerInfoApi") as mock_cls: - mock_cls.return_value.controller_info.side_effect = ApiException(status=500) - with pytest.raises(GarmApiError, match="500"): - client.is_initialized() - - -def test_first_run_calls_api_with_correct_params(client): - """ - arrange: FirstRunApi.first_run returns successfully. - act: Call client.first_run() with known parameters. - assert: The API is called with a NewUserParams body matching those parameters. - """ - with patch("garm_api.FirstRunApi") as mock_cls: - mock_cls.return_value.first_run.return_value = MagicMock() - client.first_run("admin", "Password-123!", "admin@test.local", "Admin User") - body = mock_cls.return_value.first_run.call_args.kwargs["body"] - assert body.username == "admin" - assert body.password == "Password-123!" - assert body.email == "admin@test.local" - assert body.full_name == "Admin User" - - -def test_first_run_raises_on_api_error(client): - """ - arrange: FirstRunApi.first_run raises ApiException with status 400. - act: Call client.first_run(). - assert: Raises GarmApiError with the status code in the message. - """ - with patch("garm_api.FirstRunApi") as mock_cls: - mock_cls.return_value.first_run.side_effect = ApiException(status=400) - with pytest.raises(GarmApiError, match="400"): - client.first_run("admin", "Password-123!", "admin@test.local", "Admin User") - - -def test_is_initialized_raises_on_connection_error(client): - """ - arrange: ControllerInfoApi.controller_info raises urllib3 HTTPError. - act: Call client.is_initialized(). - assert: Raises GarmConnectionError (subclass of GarmApiError) wrapping the error. - """ - with patch("garm_api.ControllerInfoApi") as mock_cls: - mock_cls.return_value.controller_info.side_effect = urllib3.exceptions.HTTPError("refused") - with pytest.raises(GarmConnectionError, match="connection error"): - client.is_initialized() - - -def test_first_run_raises_on_connection_error(client): - """ - arrange: FirstRunApi.first_run raises urllib3 HTTPError. - act: Call client.first_run(). - assert: Raises GarmConnectionError (subclass of GarmApiError) wrapping the error. - """ - with patch("garm_api.FirstRunApi") as mock_cls: - mock_cls.return_value.first_run.side_effect = urllib3.exceptions.HTTPError("refused") - with pytest.raises(GarmConnectionError, match="connection error"): - client.first_run("admin", "Password-123!", "admin@test.local", "Admin User") - - -def test_wait_for_ready_returns_when_initialized(client): - """ - arrange: is_initialized() returns True on first call. - act: Call client.wait_for_ready(). - assert: Returns without raising. - """ - with patch.object(client, "is_initialized", return_value=True): - client.wait_for_ready() - - -def test_wait_for_ready_returns_when_not_yet_initialized(client): - """ - arrange: is_initialized() returns False (409 — server up, awaiting first-run). - act: Call client.wait_for_ready(). - assert: Returns without raising (False means HTTP API is up). - """ - with patch.object(client, "is_initialized", return_value=False): - client.wait_for_ready() - - -def test_wait_for_ready_retries_on_connection_error_then_succeeds(client): - """ - arrange: is_initialized() raises GarmConnectionError once, then returns True. - act: Call client.wait_for_ready(). - assert: Returns without raising after the retry. - """ - with ( - patch.object( - client, - "is_initialized", - side_effect=[GarmConnectionError("refused"), True], - ), - patch("garm_api.time.sleep"), - ): - client.wait_for_ready() - - -def test_wait_for_ready_raises_after_timeout(client): - """ - arrange: is_initialized() always raises GarmConnectionError; monotonic clock advances - past the timeout on the second call. - act: Call client.wait_for_ready(timeout=5). - assert: Raises GarmConnectionError mentioning the timeout duration. - """ - monotonic_values = iter([0.0, 0.0, 10.0]) # deadline=5, first check passes, second exceeds - - with ( - patch.object(client, "is_initialized", side_effect=GarmConnectionError("refused")), - patch("garm_api.time.monotonic", side_effect=monotonic_values), - patch("garm_api.time.sleep"), - ): - with pytest.raises(GarmConnectionError, match="5s"): - client.wait_for_ready(timeout=5) +from garm_api import ( + GarmApiClient, + GarmApiError, + GarmAuthenticatedClient, + GarmConnectionError, +) + + +BASE_URL = "http://127.0.0.1:9997/api/v1" + + +class TestGarmApiClientLogin: + """Tests for GarmApiClient.login().""" + + def test_login_returns_token(self): + client = GarmApiClient(BASE_URL) + mock_result = MagicMock() + mock_result.token = "test-jwt-token" + with patch("garm_api.LoginApi") as MockLoginApi: + MockLoginApi.return_value.login.return_value = mock_result + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + token = client.login("admin", "password") + assert token == "test-jwt-token" + + def test_login_raises_on_api_exception(self): + from garm_client.exceptions import ApiException + + client = GarmApiClient(BASE_URL) + with patch("garm_api.LoginApi") as MockLoginApi: + MockLoginApi.return_value.login.side_effect = ApiException(status=401, reason="Unauthorized") + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + with pytest.raises(GarmApiError): + client.login("admin", "wrong") + + def test_login_raises_when_token_empty(self): + client = GarmApiClient(BASE_URL) + mock_result = MagicMock() + mock_result.token = "" + with patch("garm_api.LoginApi") as MockLoginApi: + MockLoginApi.return_value.login.return_value = mock_result + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + with pytest.raises(GarmApiError, match="token"): + client.login("admin", "password") + + +class TestGarmApiClientIsInitialized: + """Tests for GarmApiClient.is_initialized().""" + + def test_returns_true_on_200(self): + from garm_client.exceptions import ApiException + + client = GarmApiClient(BASE_URL) + with patch("garm_api.ControllerInfoApi") as MockApi: + MockApi.return_value.controller_info.return_value = MagicMock() + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + result = client.is_initialized() + assert result is True + + def test_returns_false_on_409(self): + from garm_client.exceptions import ApiException + + client = GarmApiClient(BASE_URL) + with patch("garm_api.ControllerInfoApi") as MockApi: + MockApi.return_value.controller_info.side_effect = ApiException(status=409) + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + result = client.is_initialized() + assert result is False + + def test_raises_on_unexpected_status(self): + from garm_client.exceptions import ApiException + + client = GarmApiClient(BASE_URL) + with patch("garm_api.ControllerInfoApi") as MockApi: + MockApi.return_value.controller_info.side_effect = ApiException(status=500) + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + with pytest.raises(GarmApiError): + client.is_initialized() + + +class TestGarmApiClientWaitForReady: + """Tests for GarmApiClient.wait_for_ready().""" + + def test_returns_immediately_when_ready(self): + client = GarmApiClient(BASE_URL) + with patch.object(client, "is_initialized", return_value=True): + client.wait_for_ready(timeout=5) -def test_wait_for_ready_propagates_non_connection_api_error_immediately(client): - """ - arrange: is_initialized() raises GarmApiError (unexpected HTTP status, not a connection error). - act: Call client.wait_for_ready(). - assert: GarmApiError propagates immediately without retrying. - """ - with ( - patch.object(client, "is_initialized", side_effect=GarmApiError("unexpected 500")), - patch("garm_api.time.sleep") as mock_sleep, - ): - with pytest.raises(GarmApiError, match="unexpected 500"): - client.wait_for_ready() - - mock_sleep.assert_not_called() + def test_raises_after_timeout(self): + client = GarmApiClient(BASE_URL) + with patch.object(client, "is_initialized", side_effect=GarmConnectionError("refused")): + with patch("garm_api.time.sleep"): + with patch("garm_api.time.monotonic", side_effect=[0, 0, 100]): + with pytest.raises(GarmConnectionError, match="ready"): + client.wait_for_ready(timeout=30) + + +class TestGarmApiClientFirstRun: + """Tests for GarmApiClient.first_run().""" + + def test_first_run_success(self): + client = GarmApiClient(BASE_URL) + with patch("garm_api.FirstRunApi") as MockApi: + MockApi.return_value.first_run.return_value = MagicMock() + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + client.first_run("admin", "pass", "email@example.com", "Admin") + + def test_first_run_raises_on_api_error(self): + from garm_client.exceptions import ApiException + + client = GarmApiClient(BASE_URL) + with patch("garm_api.FirstRunApi") as MockApi: + MockApi.return_value.first_run.side_effect = ApiException(status=400) + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + with pytest.raises(GarmApiError): + client.first_run("admin", "pass", "email@example.com", "Admin") + + +class TestGarmAuthenticatedClientListProviders: + """Tests for GarmAuthenticatedClient.list_providers().""" + + def test_returns_provider_list(self): + client = GarmAuthenticatedClient(BASE_URL, "token") + mock_provider = MagicMock() + mock_provider.name = "openstack" + with patch("garm_api.ProvidersApi") as MockApi: + MockApi.return_value.list_providers.return_value = [mock_provider] + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + result = client.list_providers() + assert len(result) == 1 + assert result[0].name == "openstack" + + def test_returns_empty_on_none(self): + client = GarmAuthenticatedClient(BASE_URL, "token") + with patch("garm_api.ProvidersApi") as MockApi: + MockApi.return_value.list_providers.return_value = None + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + result = client.list_providers() + assert result == [] + + +class TestGarmAuthenticatedClientListScalesets: + """Tests for GarmAuthenticatedClient.list_scalesets().""" + + def test_returns_scaleset_list(self): + client = GarmAuthenticatedClient(BASE_URL, "token") + mock_ss = MagicMock() + mock_ss.name = "test-scaleset" + mock_ss.id = 1 + with patch("garm_api.ScalesetsApi") as MockApi: + MockApi.return_value.list_scalesets.return_value = [mock_ss] + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + result = client.list_scalesets() + assert len(result) == 1 + assert result[0].name == "test-scaleset" + + +class TestGarmAuthenticatedClientFindOrgId: + """Tests for GarmAuthenticatedClient.find_org_id().""" + + def test_returns_org_id_when_found(self): + client = GarmAuthenticatedClient(BASE_URL, "token") + mock_org = MagicMock() + mock_org.name = "my-org" + mock_org.id = "org-uuid-123" + with patch("garm_api.OrganizationsApi") as MockApi: + MockApi.return_value.list_orgs.return_value = [mock_org] + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + result = client.find_org_id("my-org") + assert result == "org-uuid-123" + + def test_returns_none_when_not_found(self): + client = GarmAuthenticatedClient(BASE_URL, "token") + with patch("garm_api.OrganizationsApi") as MockApi: + MockApi.return_value.list_orgs.return_value = [] + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + result = client.find_org_id("nonexistent") + assert result is None + + +class TestGarmAuthenticatedClientDeleteScaleset: + """Tests for GarmAuthenticatedClient.delete_scaleset().""" + + def test_delete_calls_api(self): + client = GarmAuthenticatedClient(BASE_URL, "token") + with patch("garm_api.ScalesetsApi") as MockApi: + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + client.delete_scaleset(42) + MockApi.return_value.delete_scale_set.assert_called_once() + + def test_delete_raises_on_api_error(self): + from garm_client.exceptions import ApiException + + client = GarmAuthenticatedClient(BASE_URL, "token") + with patch("garm_api.ScalesetsApi") as MockApi: + MockApi.return_value.delete_scale_set.side_effect = ApiException(status=404) + with patch.object(client, "_api_client") as mock_api_client: + mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) + mock_api_client.return_value.__exit__ = MagicMock(return_value=False) + with pytest.raises(GarmApiError): + client.delete_scaleset(99) From f438e3df4806c1b41d457c37ebd567aa9801b42d Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Tue, 16 Jun 2026 11:19:31 +0700 Subject: [PATCH 04/12] feat(garm-configurator): write scaleset data to garm-configurator relation databag Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- charms/garm-configurator/src/charm.py | 23 ++++ charms/garm-configurator/src/charm_state.py | 9 ++ .../tests/unit/test_charm.py | 107 ++++++++++++++++++ 3 files changed, 139 insertions(+) diff --git a/charms/garm-configurator/src/charm.py b/charms/garm-configurator/src/charm.py index 8f7f3c37..c1cb2360 100755 --- a/charms/garm-configurator/src/charm.py +++ b/charms/garm-configurator/src/charm.py @@ -10,6 +10,8 @@ from charm_state import IMAGE_RELATION_NAME, CharmConfigInvalidError, CharmState +GARM_CONFIGURATOR_RELATION_NAME = "garm-configurator" + class GarmConfiguratorCharm(ops.CharmBase): """GARM configurator charm.""" @@ -24,6 +26,7 @@ def __init__(self, *args: typing.Any) -> None: for event in [ self.on.config_changed, self.on.secret_changed, + self.on[GARM_CONFIGURATOR_RELATION_NAME].relation_changed, self.on[IMAGE_RELATION_NAME].relation_joined, self.on[IMAGE_RELATION_NAME].relation_changed, self.on[IMAGE_RELATION_NAME].relation_broken, @@ -58,6 +61,26 @@ def _reconcile(self, event: ops.EventBase) -> None: } ) + relation_data = { + "name": state.scaleset_config.name, + "provider_name": state.provider_name, + "credentials_name": state.credentials_name, + "image_id": str(state.image_id), + "flavor": state.scaleset_config.flavor, + "os_arch": state.scaleset_config.os_arch, + "min_idle_runner": str(state.scaleset_config.min_idle_runner), + "max_runner": str(state.scaleset_config.max_runner), + "labels": ( + state.scaleset_config.labels + if isinstance(state.scaleset_config.labels, str) + else ",".join(state.scaleset_config.labels) + ), + "runner_group": state.scaleset_config.runner_group, + "pre_install_scripts": str(state.scaleset_config.pre_install_scripts), + } + for garm_relation in self.model.relations[GARM_CONFIGURATOR_RELATION_NAME]: + garm_relation.data[self.unit].update(relation_data) + if relation is None: self.unit.status = ops.WaitingStatus("Waiting for image builder relation") elif state.image_id is None: diff --git a/charms/garm-configurator/src/charm_state.py b/charms/garm-configurator/src/charm_state.py index b241772d..329c5fd9 100644 --- a/charms/garm-configurator/src/charm_state.py +++ b/charms/garm-configurator/src/charm_state.py @@ -347,6 +347,15 @@ def from_charm(cls, charm: ops.CharmBase) -> "CharmState": image_id=image_id, ) + @property + def provider_name(self) -> str: + """Return the provider name derived from the OpenStack project.""" + return f"openstack-{self.provider_config.project_name}" + + @property + def credentials_name(self) -> str: + """Return the credentials name derived from the GitHub App client ID.""" + return f"github-app-{self.github_app_config.client_id}" def _get_image_id_from_relation(charm: ops.CharmBase) -> str | None: """Return the OpenStack image UUID from the image builder relation, if available. diff --git a/charms/garm-configurator/tests/unit/test_charm.py b/charms/garm-configurator/tests/unit/test_charm.py index a4d35e59..d54eff07 100644 --- a/charms/garm-configurator/tests/unit/test_charm.py +++ b/charms/garm-configurator/tests/unit/test_charm.py @@ -18,6 +18,10 @@ def _make_private_key_secret(): return Secret(tracked_content={"value": "random-secret"}) +def _make_garm_configurator_relation(): + return Relation(endpoint="garm-configurator") + + def _valid_config(secret: Secret, private_key_secret: Secret) -> dict: return { "openstack-auth-url": "https://keystone.example.com:5000/v3", @@ -430,3 +434,106 @@ def test_status_waiting_on_relation_broken(): ) out = ctx.run(ctx.on.relation_broken(image_relation), state) assert out.unit_status == ops.WaitingStatus("Waiting for image builder relation") + + +def test_garm_configurator_relation_data_written_on_reconcile(): + """ + arrange: Valid config and an active garm-configurator relation. + act: Run config-changed. + assert: The charm writes all expected scaleset fields to local unit relation data. + """ + ctx = Context(GarmConfiguratorCharm) + secret = _make_secret() + pk_secret = _make_private_key_secret() + config = _valid_config(secret, pk_secret) + config["labels"] = "self-hosted,linux" + config["pre-install-scripts"] = "echo hello" + garm_relation = _make_garm_configurator_relation() + state = State( + config=config, + secrets=[secret, pk_secret], + relations=[garm_relation], + ) + + out = ctx.run(ctx.on.config_changed(), state) + + rel_out = out.get_relation(garm_relation.id) + expected_relation_data = { + "name": "my-scaleset", + "provider_name": "openstack-myproject", + "credentials_name": "github-app-12345", + "image_id": "None", + "flavor": "m1.large", + "os_arch": "amd64", + "min_idle_runner": "0", + "max_runner": "5", + "labels": "self-hosted,linux", + "runner_group": "default", + "pre_install_scripts": "echo hello", + } + for key, value in expected_relation_data.items(): + assert rel_out.local_unit_data[key] == value + + +def test_garm_configurator_relation_data_reflects_charm_state(): + """ + arrange: Valid config with project and client identifiers plus a garm-configurator relation. + act: Run config-changed. + assert: Derived provider and credentials names are written from CharmState. + """ + ctx = Context(GarmConfiguratorCharm) + secret = _make_secret() + pk_secret = _make_private_key_secret() + config = _valid_config(secret, pk_secret) + config["openstack-project-name"] = "demo-project" + config["github-app-client-id"] = "abc123" + garm_relation = _make_garm_configurator_relation() + state = State( + config=config, + secrets=[secret, pk_secret], + relations=[garm_relation], + ) + + out = ctx.run(ctx.on.config_changed(), state) + + rel_out = out.get_relation(garm_relation.id) + assert rel_out.local_unit_data["provider_name"] == "openstack-demo-project" + assert rel_out.local_unit_data["credentials_name"] == "github-app-abc123" + + +def test_garm_configurator_no_error_when_no_relation(): + """ + arrange: Valid config with no garm-configurator relation. + act: Run config-changed. + assert: Reconcile completes and preserves the existing waiting status behavior. + """ + ctx = Context(GarmConfiguratorCharm) + secret = _make_secret() + pk_secret = _make_private_key_secret() + state = State(config=_valid_config(secret, pk_secret), secrets=[secret, pk_secret]) + + out = ctx.run(ctx.on.config_changed(), state) + + assert out.unit_status == ops.WaitingStatus("Waiting for image builder relation") + + +def test_garm_configurator_relation_changed_triggers_reconcile(): + """ + arrange: Valid config and a garm-configurator relation with no existing local unit data. + act: Run garm-configurator relation-changed. + assert: Reconcile writes relation data for the local unit. + """ + ctx = Context(GarmConfiguratorCharm) + secret = _make_secret() + pk_secret = _make_private_key_secret() + garm_relation = _make_garm_configurator_relation() + state = State( + config=_valid_config(secret, pk_secret), + secrets=[secret, pk_secret], + relations=[garm_relation], + ) + + out = ctx.run(ctx.on.relation_changed(garm_relation), state) + + rel_out = out.get_relation(garm_relation.id) + assert rel_out.local_unit_data["name"] == "my-scaleset" From fb5536c280d1db9ace7ade35cc1451c0c228410d Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Tue, 16 Jun 2026 11:29:26 +0700 Subject: [PATCH 05/12] feat(garm-configurator): add provider_name and credentials_name to CharmState Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- charms/garm-configurator/src/charm_state.py | 4 +- .../tests/unit/test_charm_state.py | 72 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 charms/garm-configurator/tests/unit/test_charm_state.py diff --git a/charms/garm-configurator/src/charm_state.py b/charms/garm-configurator/src/charm_state.py index 329c5fd9..d180ff45 100644 --- a/charms/garm-configurator/src/charm_state.py +++ b/charms/garm-configurator/src/charm_state.py @@ -349,12 +349,12 @@ def from_charm(cls, charm: ops.CharmBase) -> "CharmState": @property def provider_name(self) -> str: - """Return the provider name derived from the OpenStack project.""" + """Derived GARM provider name for scalesets.""" return f"openstack-{self.provider_config.project_name}" @property def credentials_name(self) -> str: - """Return the credentials name derived from the GitHub App client ID.""" + """Derived GARM credentials name for scalesets.""" return f"github-app-{self.github_app_config.client_id}" def _get_image_id_from_relation(charm: ops.CharmBase) -> str | None: diff --git a/charms/garm-configurator/tests/unit/test_charm_state.py b/charms/garm-configurator/tests/unit/test_charm_state.py new file mode 100644 index 00000000..b453e2bc --- /dev/null +++ b/charms/garm-configurator/tests/unit/test_charm_state.py @@ -0,0 +1,72 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Unit tests for charm_state.""" + +from charm_state import CharmState, GithubAppConfig, ProviderConfig, ScalesetConfig + + +def test_provider_name_derived_from_project_name(): + """CharmState.provider_name is openstack-{project_name}.""" + project_name = "myproject" + state = CharmState( + provider_config=ProviderConfig( + auth_url="https://keystone.example.com:5000/v3", + username="admin", + password="s3cr3t", + project_name=project_name, + user_domain_name="Default", + project_domain_name="Default", + region_name="RegionOne", + network="external-net", + ), + github_app_config=GithubAppConfig( + client_id="12345", + installation_id="67890", + private_key="private-key", + ), + scaleset_config=ScalesetConfig( + name="my-scaleset", + flavor="m1.large", + os_arch="amd64", + min_idle_runner=0, + max_runner=5, + repo="myorg/myrepo", + ), + image_id=None, + ) + + assert state.provider_name == f"openstack-{project_name}" + + +def test_credentials_name_derived_from_client_id(): + """CharmState.credentials_name is github-app-{client_id}.""" + client_id = "12345" + state = CharmState( + provider_config=ProviderConfig( + auth_url="https://keystone.example.com:5000/v3", + username="admin", + password="s3cr3t", + project_name="myproject", + user_domain_name="Default", + project_domain_name="Default", + region_name="RegionOne", + network="external-net", + ), + github_app_config=GithubAppConfig( + client_id=client_id, + installation_id="67890", + private_key="private-key", + ), + scaleset_config=ScalesetConfig( + name="my-scaleset", + flavor="m1.large", + os_arch="amd64", + min_idle_runner=0, + max_runner=5, + repo="myorg/myrepo", + ), + image_id=None, + ) + + assert state.credentials_name == f"github-app-{client_id}" From 6a60248aeff9e4b900a79016e38d6c060139a97d Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Tue, 16 Jun 2026 11:38:42 +0700 Subject: [PATCH 06/12] feat(garm): add admin secrets and scaleset reconcile via garm-configurator relation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- charms/garm/src/charm.py | 225 +++++++++++++++++++++------ charms/garm/tests/unit/test_charm.py | 51 ++++++ 2 files changed, 228 insertions(+), 48 deletions(-) diff --git a/charms/garm/src/charm.py b/charms/garm/src/charm.py index 2ae1ba41..f5b87010 100755 --- a/charms/garm/src/charm.py +++ b/charms/garm/src/charm.py @@ -5,6 +5,7 @@ """GARM charm entrypoint.""" import dataclasses +import json import logging import secrets import string @@ -15,13 +16,15 @@ import tomli_w from paas_charm.app import WorkloadConfig -from garm_api import GarmApiClient, GarmApiError +from garm_api import GarmApiClient, GarmApiError, GarmAuthenticatedClient +from scaleset_reconciler import ScalesetReconciler, ScalesetSpec logger = logging.getLogger(__name__) GARM_CONFIG_PATH: typing.Final[str] = "/etc/garm/config.toml" GARM_SECRETS_LABEL: typing.Final[str] = "garm-secrets" GARM_ADMIN_CREDENTIALS_LABEL: typing.Final[str] = "garm-admin-credentials" +GARM_CONFIGURATOR_RELATION_NAME: typing.Final[str] = "garm-configurator" CONTAINER_NAME: typing.Final[str] = "app" PEBBLE_SERVICE_NAME: typing.Final[str] = "app" GARM_BINARY: typing.Final[str] = "/usr/local/bin/garm" @@ -136,6 +139,19 @@ def _generate_admin_password() -> str: return "".join(chars) +def _parse_pre_install_scripts(raw: str) -> dict[str, str]: + """Parse pre_install_scripts from JSON relation data string.""" + if not raw: + return {} + try: + result = json.loads(raw) + if isinstance(result, dict): + return result + except (ValueError, json.JSONDecodeError): + pass + return {} + + class GarmCharm(paas_charm.go.Charm): """GARM charm — manages the GARM service via Pebble.""" @@ -147,6 +163,14 @@ def __init__(self, *args: typing.Any) -> None: """ super().__init__(*args) self.framework.observe(self.on.install, self._on_install) + self.framework.observe( + self.on[GARM_CONFIGURATOR_RELATION_NAME].relation_changed, + self._on_configurator_changed, + ) + self.framework.observe( + self.on[GARM_CONFIGURATOR_RELATION_NAME].relation_departed, + self._on_configurator_changed, + ) def _on_install(self, _: ops.InstallEvent) -> None: """Ensure secrets exist on first install.""" @@ -165,6 +189,10 @@ def _workload_config(self) -> WorkloadConfig: """ return dataclasses.replace(super()._workload_config, port=GARM_PORT, metrics_target=None) + def _on_configurator_changed(self, _: typing.Any) -> None: + """Handle garm-configurator relation changes.""" + self._reconcile_scalesets() + def restart(self, rerun_migrations: bool = False) -> None: """Write GARM config then restart the workload. @@ -211,9 +239,6 @@ def restart(self, rerun_migrations: bool = False) -> None: container = self.unit.get_container(CONTAINER_NAME) - # Push the TOML config and set the command layer BEFORE calling - # super().restart() so that GARM never starts with a missing config file. - # If secrets are unavailable we return early before touching the service. try: self._push_garm_config(container, pg_config) except ops.SecretNotFoundError: @@ -221,12 +246,10 @@ def restart(self, rerun_migrations: bool = False) -> None: self.unit.status = ops.WaitingStatus("Waiting for leader to initialise garm-secrets") return - # TODO: Eliminate double-replan (ISD-5718). On first deployment, paas_charm - # adds its Pebble layer inside super().restart() at a higher stack position - # than "garm-command", temporarily overriding the command. The add_layer + - # replan below re-establishes the correct command. On subsequent restarts - # "garm-command" is already above the paas_charm layer so that replan is a - # no-op. Fix properly by contributing an upstream hook to paas_charm. + # TODO: Eliminate double-replan (ISD-5718). paas_charm calls replan() + # internally in super().restart(), which starts GARM with the default + # command momentarily before this method overrides it. Acceptable for + # the scaffold; resolve by contributing an upstream hook in a future story. super().restart(rerun_migrations=rerun_migrations) container.add_layer( "garm-command", @@ -271,6 +294,13 @@ def _ensure_secrets(self) -> None: GARM_ADMIN_CREDENTIALS_LABEL, ) + def _get_garm_secrets(self) -> ops.Secret | None: + """Return the GARM secret object when available.""" + try: + return self.model.get_secret(label=GARM_SECRETS_LABEL) + except ops.SecretNotFoundError: + return None + def _get_secrets(self) -> dict[str, str]: """Retrieve secrets from the juju secret store. @@ -283,44 +313,6 @@ def _get_secrets(self) -> dict[str, str]: secret = self.model.get_secret(label=GARM_SECRETS_LABEL) return secret.get_content() - def _get_postgresql_config(self) -> dict[str, typing.Any] | None: - """Get PostgreSQL config from relation data, or None if not available. - - Returns: - Dict with postgresql connection parameters ready for the TOML config, - or None if the relation data is not yet available. - """ - pg_requirer = self._database_requirers.get("postgresql") - if pg_requirer is None: - return None - - relations = pg_requirer.fetch_relation_data() - if not relations: - return None - - for data in relations.values(): - if not data: - continue - endpoints = data.get("endpoints", "") - if not endpoints: - continue - - # GARM only supports a single hostname in its PostgreSQL config struct - # (no multi-host DSN or failover list), so we take the first endpoint. - host_port = endpoints.split(",")[0] - host, port = host_port.rsplit(":", 1) - - return { - "username": data.get("username", ""), - "password": data.get("password", ""), - "hostname": host, - "port": int(port), - "database": data.get("database", ""), - "sslmode": "prefer", - } - - return None - def _get_admin_credentials(self) -> dict[str, str] | None: """Retrieve the GARM admin credentials from the Juju secret store. @@ -372,6 +364,143 @@ def _maybe_first_run(self) -> None: logger.warning("GARM first-run check failed (error out for retry): %s", exc) raise + def _get_garm_url(self) -> str: + """Build the local GARM API URL from charm configuration.""" + listen_address = str(self.config.get("garm-listen-address", "")) + listen_port = self.config.get("garm-listen-port", 9997) + if not listen_address: + return "" + return f"http://{listen_address}:{listen_port}" + + def _get_postgresql_config(self) -> dict[str, typing.Any] | None: + """Get PostgreSQL config from relation data, or None if not available. + + Returns: + Dict with postgresql connection parameters ready for the TOML config, + or None if the relation data is not yet available. + """ + pg_requirer = self._database_requirers.get("postgresql") + if pg_requirer is None: + return None + + relations = pg_requirer.fetch_relation_data() + if not relations: + return None + + for data in relations.values(): + if not data: + continue + endpoints = data.get("endpoints", "") + if not endpoints: + continue + + # GARM only supports a single hostname in its PostgreSQL config struct + # (no multi-host DSN or failover list), so we take the first endpoint. + host_port = endpoints.split(",")[0] + host, port = host_port.rsplit(":", 1) + + return { + "username": data.get("username", ""), + "password": data.get("password", ""), + "hostname": host, + "port": int(port), + "database": data.get("database", ""), + "sslmode": "prefer", + } + + return None + + def _build_desired_scalesets(self) -> list[ScalesetSpec]: + """Build the desired scaleset list from all garm-configurator relation units.""" + specs = [] + for relation in self.model.relations.get(GARM_CONFIGURATOR_RELATION_NAME, []): + for unit in relation.units: + data = relation.data[unit] + name = data.get("name", "") + if not name: + continue + + org = data.get("org", "") + repo = data.get("repo", "") + if org: + entity_type = "organization" + entity_name = org + elif repo: + entity_type = "repository" + entity_name = repo + else: + logger.warning( + "Skipping scaleset %s: neither org nor repo specified", name + ) + continue + + required = { + "provider_name": data.get("provider_name", ""), + "image": data.get("image_id", ""), + "flavor": data.get("flavor", ""), + "os_arch": data.get("os_arch", ""), + "max_runner": data.get("max_runner", ""), + } + missing = [k for k, v in required.items() if not v] + if missing: + logger.warning( + "Skipping scaleset %s: missing required fields %s", + name, + missing, + ) + continue + try: + min_idle = int(data.get("min_idle_runner", "0")) + max_runners = int(required["max_runner"]) + except ValueError: + continue + specs.append( + ScalesetSpec( + name=name, + provider_name=required["provider_name"], + image=required["image"], + flavor=required["flavor"], + os_arch=required["os_arch"], + min_idle_runners=min_idle, + max_runners=max_runners, + entity_type=entity_type, + entity_name=entity_name, + labels=[ + label.strip() + for label in data.get("labels", "").split(",") + if label.strip() + ], + runner_group=data.get("runner_group", ""), + pre_install_scripts=_parse_pre_install_scripts( + data.get("pre_install_scripts", "") + ), + ) + ) + return specs + + def _reconcile_scalesets(self) -> None: + """Sync GARM scalesets against garm-configurator relation data.""" + admin_creds = self._get_admin_credentials() + if not admin_creds: + logger.warning("Admin credentials not yet available; deferring scaleset reconcile") + return + + garm_url = self._get_garm_url() + if not garm_url: + logger.warning("GARM URL not yet available; deferring scaleset reconcile") + return + + try: + garm_client = GarmApiClient(f"{garm_url}/api/v1") + token = garm_client.login( + admin_creds["username"], admin_creds["password"] + ) + auth_client = GarmAuthenticatedClient(f"{garm_url}/api/v1", token) + desired = self._build_desired_scalesets() + ScalesetReconciler(auth_client).reconcile(desired) + except GarmApiError as exc: + logger.warning("GARM API error during scaleset reconcile: %s", exc) + def _push_garm_config( self, container: ops.Container, diff --git a/charms/garm/tests/unit/test_charm.py b/charms/garm/tests/unit/test_charm.py index 5b9c61a8..7d672b4c 100644 --- a/charms/garm/tests/unit/test_charm.py +++ b/charms/garm/tests/unit/test_charm.py @@ -359,3 +359,54 @@ def test_maybe_first_run_skips_on_missing_credential_key(): GarmCharm._maybe_first_run(charm) mock_client_cls.assert_not_called() + +def test_reconcile_scalesets_skips_when_no_admin_credentials(): + """Scaleset reconciliation exits early when admin credentials are unavailable.""" + charm = object.__new__(GarmCharm) + charm._get_admin_credentials = MagicMock(return_value=None) + + with patch("charm.GarmApiClient") as mock_client: + charm._reconcile_scalesets() + + mock_client.assert_not_called() + + +def test_reconcile_scalesets_skips_restart(): + """Scaleset reconciliation must not restart the workload.""" + charm = object.__new__(GarmCharm) + charm._get_admin_credentials = MagicMock(return_value={ + "username": "admin", + "password": "TestPass-123!", + }) + charm._get_garm_url = MagicMock(return_value="http://127.0.0.1:9997") + charm._build_desired_scalesets = MagicMock(return_value=[]) + charm.restart = MagicMock() + + with ( + patch("charm.GarmApiClient") as mock_client_cls, + patch("charm.GarmAuthenticatedClient") as mock_auth_cls, + patch("charm.ScalesetReconciler") as mock_reconciler_cls, + ): + mock_client_cls.return_value.login.return_value = "test-token" + + charm._reconcile_scalesets() + + mock_client_cls.return_value.login.assert_called_once_with("admin", "TestPass-123!") + mock_auth_cls.assert_called_once_with("http://127.0.0.1:9997/api/v1", "test-token") + mock_reconciler_cls.return_value.reconcile.assert_called_once_with([]) + charm.restart.assert_not_called() + + +def test_reconcile_scalesets_skips_when_garm_url_unavailable(): + """Scaleset reconciliation exits early when the GARM URL is not configured.""" + charm = object.__new__(GarmCharm) + charm._get_admin_credentials = MagicMock(return_value={ + "username": "admin", + "password": "TestPass-123!", + }) + charm._get_garm_url = MagicMock(return_value="") + + with patch("charm.GarmApiClient") as mock_client_cls: + charm._reconcile_scalesets() + + mock_client_cls.assert_not_called() From 82cff3617a53b478cea1efdf54beb1580d813d70 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Tue, 16 Jun 2026 12:25:02 +0700 Subject: [PATCH 07/12] test(garm): add integration tests for scaleset create/update/delete/defer Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- charms/tests/integration/conftest.py | 110 +++++++ charms/tests/integration/test_garm.py | 409 ++++++++++++++++++++++++++ 2 files changed, 519 insertions(+) diff --git a/charms/tests/integration/conftest.py b/charms/tests/integration/conftest.py index 6230868f..db95d44a 100644 --- a/charms/tests/integration/conftest.py +++ b/charms/tests/integration/conftest.py @@ -19,6 +19,8 @@ ) logger = logging.getLogger(__name__) +GARM_API_PORT = 9997 +GARM_SECRETS_LABEL = "garm-secrets" @pytest.fixture(name="planner_charm_file", scope="module") @@ -332,6 +334,54 @@ def garm_app_image_fixture(pytestconfig: pytest.Config) -> str | None: return image +@pytest.fixture(name="garm_configurator_charm_file", scope="module") +def garm_configurator_charm_file_fixture(pytestconfig: pytest.Config) -> str: + """Return the path to the built garm-configurator charm file.""" + charm = pytestconfig.getoption(CHARM_FILE_PARAM) + if not charm: + pytest.skip( + f"missing required {CHARM_FILE_PARAM} option for garm-configurator integration tests" + ) + configurator_charms = [file for file in charm if "garm-configurator" in file] + if not configurator_charms: + pytest.skip( + "scaleset integration tests require a garm-configurator charm path in " + f"{CHARM_FILE_PARAM}" + ) + return configurator_charms[0] + + +def garm_login_from_secret(juju: jubilant.Juju, garm_app_name: str, garm_url: str) -> str: + """Log into the GARM API using admin credentials stored in Juju secrets.""" + secrets_json = juju.cli("secrets", "--format=json") + secrets = json.loads(secrets_json) + garm_secret_uri = next( + (uri for uri, info in secrets.items() if info.get("label") == GARM_SECRETS_LABEL), + None, + ) + assert garm_secret_uri, f"{GARM_SECRETS_LABEL} not found for {garm_app_name}" + + secret_json = juju.cli("show-secret", "--reveal", "--format=json", garm_secret_uri) + secret_data = json.loads(secret_json) + content = secret_data[garm_secret_uri]["content"]["Data"] + admin_username = content["admin-username"] + admin_password = content["admin-password"] + + base_url = garm_url.rstrip("/") + if not base_url.endswith("/api/v1"): + base_url = f"{base_url}/api/v1" + + resp = requests.post( + f"{base_url}/auth/login", + json={"username": admin_username, "password": admin_password}, + timeout=30, + ) + resp.raise_for_status() + token = resp.json().get("token", "") + assert token, "Expected non-empty JWT token from GARM login" + return token + + def _pre_pull_garm_image(image: str) -> None: """Pre-pull the GARM ROCK image into microk8s containerd. @@ -505,3 +555,63 @@ def _on_image_relation_joined(self, event): delay=10, ) return app_name + + +@pytest.fixture(scope="module", name="garm_configurator_for_scaleset_tests") +def garm_configurator_for_scaleset_tests_fixture( + juju: jubilant.Juju, + garm_configurator_charm_file: str, + any_charm_image_builder_app: str, +) -> str: + """Deploy garm-configurator for GARM scaleset-sync integration tests.""" + app_name = "garm-configurator-scaleset-test" + juju.deploy(charm=garm_configurator_charm_file, app=app_name) + juju.wait( + lambda status: jubilant.all_blocked(status, app_name), + timeout=5 * 60, + delay=10, + ) + + password_secret_uri = juju.add_secret(name="os-pw-scaleset", content={"value": "fake-password"}) + private_key_secret_uri = juju.add_secret( + name="gh-key-scaleset", content={"value": "fake-private-key"} + ) + juju.grant_secret(password_secret_uri, app_name) + juju.grant_secret(private_key_secret_uri, app_name) + + juju.config( + app_name, + values={ + "openstack-auth-url": "https://keystone.example.com:5000/v3", + "openstack-username": "admin", + "openstack-password": password_secret_uri, + "openstack-project-name": "myproject", + "openstack-user-domain-name": "Default", + "openstack-project-domain-name": "Default", + "openstack-region-name": "RegionOne", + "openstack-network": "external-net", + "github-app-client-id": "12345", + "github-app-installation-id": "67890", + "github-app-private-key": private_key_secret_uri, + "name": "test-scaleset", + "flavor": "m1.large", + "os-arch": "amd64", + "repo": "myorg/myrepo", + }, + ) + juju.wait( + lambda status: jubilant.all_waiting(status, app_name), + timeout=5 * 60, + delay=10, + ) + + juju.integrate( + f"{app_name}:image", + f"{any_charm_image_builder_app}:provide-github-runner-image-v0", + ) + juju.wait( + lambda status: jubilant.all_active(status, app_name), + timeout=5 * 60, + delay=10, + ) + return app_name diff --git a/charms/tests/integration/test_garm.py b/charms/tests/integration/test_garm.py index e5bcd355..c0c25715 100644 --- a/charms/tests/integration/test_garm.py +++ b/charms/tests/integration/test_garm.py @@ -3,8 +3,13 @@ """Integration tests for the GARM charm.""" +import base64 import json import logging +import secrets +import shlex +import urllib.error +import urllib.request import jubilant import pytest @@ -28,6 +33,29 @@ GARM_API_PORT = 8080 PEBBLE_PREFIX = "PEBBLE_SOCKET=/charm/containers/app/pebble.socket /charm/bin/pebble" +# Generated once per session so all test functions that call _garm_first_run use +# the same credentials. Format guarantees GARM's strong-password requirements: +# uppercase (A), lowercase (dmin + hex), digit (1 + hex), symbols (-, !). +_GARM_ADMIN_PASSWORD = f"Admin-{secrets.token_hex(8)}-X1!" +_SCALESET_TEST_NAME = "test-scaleset" +_SCALESET_TEST_CREDENTIAL_NAME = "github-app-12345" + +_TEST_RSA_PRIVATE_KEY = """-----BEGIN RSA PRIVATE KEY----- +MIICXwIBAAKBgQC2tCW5B18y5VnqqokOeamJgasI3H1405WWv7FmWl31I1Cgabhi +MFcHdNECXFUC3wtqo/bXyCQbANRBkpZudJfSGos3+1iOJK1fd+MU8ntHVtgpvb5j +whdFSVJ9EL4/2u0K0S+fIyilD9q7K5mhk0MYLYWumPIRLkbtwr9a7LgY5wIDAQAB +AoGBAKIQGCoRjPNjmCfdT6fEaYtstt8sXiwQWu+WaHDnFdL9mWZBgOmwAXK+vyt9 +5XafjMvyV2I+yTAewyjLM58U0xlslJu6Bk0Zw920sTmK9Qvvq/2mjsqw+PWr9rRx +qZFDCefAlB0Npo9tXHAf3ec5+vlm4QsEl6dty+Wx6aSHHMRpAkEA8e5IwkJZFcWO +aCc8Z+cnoidomlkvGlruncXMG1KhisQTleQVc1bM8tIZq2nNUG1zKJqHeCacQLiV +LKALnZDSCwJBAMFUIHd7ikYaAgTvrAKmzOZlMKVuGr2SHPODWoaWkEagEsrOw+H2 +PYonSYkbzPyXH6iKUOhWH+ZA1r6K1lhdWhUCQQCquaTOsVN8cbVU+ps+F3l4jKbc +hSMgThsla3flsCIfcs7/b71Tb2Wh1XIX7Mnef95MQQBoYZbSdW+P1kFcJ96RAkEA +oSyuqI4BGDJkjpL1l3xSBJ5F8RUbDAI9SrKujNgHTinzoMrCOabdZUkdoEXiHo8r +IIq3qwrqKz7RCSecTSz+hQJBAJDKODanbnrPxNDgmIp52BMtiYI4vv7gKp/MSW0N +PG8an+PHNVGDEj1cOOwp/YNQieRp/WPH6bpBtwwe0r6pQZQ= +-----END RSA PRIVATE KEY-----""" + def _pebble_exec(juju: jubilant.Juju, unit: str, command: str) -> jubilant.Task: """Run a command inside the workload container via pebble exec. @@ -203,6 +231,231 @@ def _scrape_metrics_until_ready(metrics_url: str) -> requests.Response: raise _MetricsNotReady() return response +def _garm_api_base_url(address: str) -> str: + """Return the GARM v1 API base URL for a unit address.""" + return f"http://{address}:{GARM_API_PORT}/api/v1" + + +def _garm_auth_headers(token: str) -> dict[str, str]: + """Return authorization headers for GARM API requests.""" + return {"Authorization": f"Bearer {token}"} + + +def _list_scalesets(base_url: str, token: str) -> list[dict]: + """List GARM scalesets via the REST API.""" + resp = requests.get(f"{base_url}/scalesets", headers=_garm_auth_headers(token), timeout=30) + resp.raise_for_status() + scalesets = resp.json() + assert isinstance(scalesets, list), f"Expected list response, got: {type(scalesets)}" + return scalesets + + +def _list_providers(base_url: str, token: str) -> list[dict]: + """List GARM providers via the REST API.""" + resp = requests.get(f"{base_url}/providers", headers=_garm_auth_headers(token), timeout=30) + resp.raise_for_status() + providers = resp.json() + assert isinstance(providers, list), f"Expected list response, got: {type(providers)}" + return providers + + +def _find_scaleset(scalesets: list[dict], name: str) -> dict | None: + """Return the first scaleset with the requested name.""" + return next((scaleset for scaleset in scalesets if scaleset.get("name") == name), None) + + +def _get_relation_info(juju: jubilant.Juju, unit: str, endpoint: str, related_app: str) -> dict: + """Return show-unit relation info for a specific endpoint and related app.""" + unit_info = json.loads(juju.cli("show-unit", unit, "--format=json"))[unit] + for relation in unit_info["relation-info"]: + if relation["endpoint"] != endpoint: + continue + related_units = relation.get("related-units", {}) + if any(name.startswith(f"{related_app}/") for name in related_units): + return relation + raise AssertionError( + f"Relation {endpoint!r} between {unit!r} and app {related_app!r} not found" + ) + + +def _relation_exists(juju: jubilant.Juju, app_name: str, related_app: str) -> bool: + """Return whether the GARM app is related to the configurator app.""" + try: + _get_relation_info(juju, f"{app_name}/0", "garm-configurator", related_app) + except AssertionError: + return False + return True + + +def _get_scaleset_relation_data( + juju: jubilant.Juju, garm_app: str, garm_configurator_for_scaleset_tests: str +) -> dict[str, str]: + """Return the configurator unit relation data as seen from the GARM side.""" + relation = _get_relation_info( + juju, + f"{garm_app}/0", + "garm-configurator", + garm_configurator_for_scaleset_tests, + ) + related_unit = f"{garm_configurator_for_scaleset_tests}/0" + data = relation["related-units"][related_unit]["data"] + return {key: str(value) for key, value in data.items()} + + +def _ensure_garm_configurator_relation( + juju: jubilant.Juju, garm_app: str, garm_configurator_for_scaleset_tests: str +) -> None: + """Ensure the GARM app is integrated with the scaleset-test configurator.""" + if not _relation_exists(juju, garm_app, garm_configurator_for_scaleset_tests): + juju.integrate(garm_app, garm_configurator_for_scaleset_tests) + juju.wait( + lambda status: jubilant.all_active(status, garm_app), + timeout=3 * 60, + delay=10, + ) + + +def _set_scaleset_relation_data( + juju: jubilant.Juju, + garm_app: str, + garm_configurator_for_scaleset_tests: str, + **overrides: str, +) -> None: + """Mutate configurator relation data to exercise GARM reconcile scenarios.""" + unit = f"{garm_configurator_for_scaleset_tests}/0" + relation = _get_relation_info(juju, unit, "garm-configurator", garm_app) + relation_id = relation["relation-id"] + relation_data = _get_scaleset_relation_data( + juju, garm_app, garm_configurator_for_scaleset_tests + ) + relation_data.update(overrides) + command = " ".join( + [ + "relation-set", + "-r", + str(relation_id), + *[shlex.quote(f"{key}={value}") for key, value in relation_data.items()], + ] + ) + juju.exec(command, unit=unit) + juju.wait( + lambda status: jubilant.all_active(status, garm_app), + timeout=3 * 60, + delay=10, + ) + + +def _create_test_credential(garm_url: str, token: str) -> str: + """Create a GitHub App credential in GARM for testing.""" + + def _request( + path: str, payload: dict, *, method: str = "POST", allow_conflict: bool = True + ) -> dict | None: + data = json.dumps(payload).encode() + req = urllib.request.Request( + f"{garm_url}{path}", + data=data, + headers={"Authorization": f"Bearer {token}", "Content-Type": "application/json"}, + method=method, + ) + try: + with urllib.request.urlopen(req, timeout=30) as resp: + body = resp.read() + return json.loads(body) if body else None + except urllib.error.HTTPError as exc: + if allow_conflict and exc.code == 409: + return None + raise + + try: + _request( + "/github/endpoints", + { + "name": "github.com", + "description": "GitHub.com test endpoint", + "base_url": "https://github.com", + "api_base_url": "https://api.github.com", + "upload_base_url": "https://uploads.github.com", + }, + ) + _request( + "/github/credentials", + { + "name": _SCALESET_TEST_CREDENTIAL_NAME, + "description": "Test credential for scaleset integration tests", + "endpoint": "github.com", + "auth_type": "app", + "app": { + "app_id": 12345, + "installation_id": 67890, + "private_key_bytes": base64.b64encode( + _TEST_RSA_PRIVATE_KEY.encode() + ).decode(), + }, + }, + ) + except urllib.error.HTTPError: + _request( + "/credentials", + { + "name": _SCALESET_TEST_CREDENTIAL_NAME, + "description": "Test credential for scaleset integration tests", + "base_url": "https://github.com", + "api_base_url": "https://api.github.com", + "upload_base_url": "https://uploads.github.com", + "ca_cert_bundle": "", + "credentials": { + "name": "test-creds", + "description": "test", + "type": "github_app", + "payload": { + "app_id": 12345, + "installation_id": 67890, + "private_key": _TEST_RSA_PRIVATE_KEY, + }, + }, + }, + allow_conflict=True, + ) + return _SCALESET_TEST_CREDENTIAL_NAME + + +@retry( + retry=retry_if_exception_type((AssertionError, requests.exceptions.RequestException)), + wait=wait_exponential(multiplier=1, min=1, max=10), + stop=stop_after_attempt(18), + reraise=True, +) +def _wait_for_scaleset( + base_url: str, + token: str, + name: str, + *, + image_id: str | None = None, +) -> dict: + """Wait until a named scaleset exists and optionally reflects a new image.""" + scaleset = _find_scaleset(_list_scalesets(base_url, token), name) + assert scaleset is not None, f"Expected scaleset {name!r} to exist" + if image_id is not None: + observed_image = scaleset.get("image_id") or scaleset.get("image") + assert observed_image == image_id, ( + f"Expected scaleset {name!r} image/image_id to be {image_id!r}, " + f"got {observed_image!r}" + ) + return scaleset + + +@retry( + retry=retry_if_exception_type((AssertionError, requests.exceptions.RequestException)), + wait=wait_exponential(multiplier=1, min=1, max=10), + stop=stop_after_attempt(18), + reraise=True, +) +def _wait_for_scaleset_absent(base_url: str, token: str, name: str) -> None: + """Wait until a named scaleset no longer exists.""" + scaleset = _find_scaleset(_list_scalesets(base_url, token), name) + assert scaleset is None, f"Expected scaleset {name!r} to be absent, got: {scaleset}" + def test_garm_blocks_without_postgresql( juju: jubilant.Juju, @@ -498,3 +751,159 @@ def test_garm_metrics_endpoint_no_auth( "Expected the garm_health metric in the /metrics response; " f"got first 500 chars: {resp.text[:500]}" ) + + +def test_scaleset_creation_deferred_when_provider_missing( + juju: jubilant.Juju, + garm_app: str, + garm_configurator_for_scaleset_tests: str, +): + """ + arrange: GARM and the test configurator are deployed, and a matching credential exists. + act: Override the relation data with a provider name GARM does not know about. + assert: No scaleset is created and the GARM app remains healthy. + """ + _ensure_garm_configurator_relation(juju, garm_app, garm_configurator_for_scaleset_tests) + address = _get_garm_address(juju, garm_app) + base_url = _garm_api_base_url(address) + token = garm_login_from_secret(juju, garm_app, base_url) + _create_test_credential(base_url, token) + _set_scaleset_relation_data( + juju, + garm_app, + garm_configurator_for_scaleset_tests, + provider_name="missing-provider", + ) + + _wait_for_scaleset_absent(base_url, token, _SCALESET_TEST_NAME) + + status = juju.status() + app_status = status.apps[garm_app].app_status.current + assert app_status in ("active", "waiting"), f"Expected active/waiting, got {app_status}" + + +def test_scalesets_created_from_relation_data( + juju: jubilant.Juju, + garm_app: str, + garm_configurator_for_scaleset_tests: str, +): + """ + arrange: GARM is active and receives valid scaleset relation data plus a test credential. + act: Reconcile against relation data that points at the built-in openstack provider. + assert: A matching scaleset appears in the GARM API. + """ + _ensure_garm_configurator_relation(juju, garm_app, garm_configurator_for_scaleset_tests) + address = _get_garm_address(juju, garm_app) + base_url = _garm_api_base_url(address) + token = garm_login_from_secret(juju, garm_app, base_url) + _create_test_credential(base_url, token) + relation_data = _get_scaleset_relation_data( + juju, garm_app, garm_configurator_for_scaleset_tests + ) + provider_names = {provider.get("name", "") for provider in _list_providers(base_url, token)} + provider_name = relation_data["provider_name"] + if provider_name not in provider_names: + pytest.skip( + f"requires GARM provider {provider_name!r}; available providers: {sorted(provider_names)}" + ) + _set_scaleset_relation_data( + juju, + garm_app, + garm_configurator_for_scaleset_tests, + min_idle_runner="1", + ) + + scaleset = _wait_for_scaleset(base_url, token, _SCALESET_TEST_NAME) + assert scaleset["name"] == _SCALESET_TEST_NAME + + +def test_scaleset_updated_on_relation_change( + juju: jubilant.Juju, + garm_app: str, + garm_configurator_for_scaleset_tests: str, +): + """ + arrange: A scaleset already exists in GARM from valid configurator relation data. + act: Change the relation data image_id and wait for another reconcile. + assert: The scaleset reflects the updated image_id. + """ + _ensure_garm_configurator_relation(juju, garm_app, garm_configurator_for_scaleset_tests) + address = _get_garm_address(juju, garm_app) + base_url = _garm_api_base_url(address) + token = garm_login_from_secret(juju, garm_app, base_url) + _create_test_credential(base_url, token) + relation_data = _get_scaleset_relation_data( + juju, garm_app, garm_configurator_for_scaleset_tests + ) + provider_names = {provider.get("name", "") for provider in _list_providers(base_url, token)} + provider_name = relation_data["provider_name"] + if provider_name not in provider_names: + pytest.skip( + f"requires GARM provider {provider_name!r}; available providers: {sorted(provider_names)}" + ) + original_image_id = relation_data["image_id"] + _set_scaleset_relation_data( + juju, + garm_app, + garm_configurator_for_scaleset_tests, + min_idle_runner="1", + ) + _wait_for_scaleset(base_url, token, _SCALESET_TEST_NAME) + + _set_scaleset_relation_data( + juju, + garm_app, + garm_configurator_for_scaleset_tests, + image_id=f"{original_image_id}-updated", + ) + + scaleset = _wait_for_scaleset( + base_url, + token, + _SCALESET_TEST_NAME, + image_id=f"{original_image_id}-updated", + ) + observed_image = scaleset.get("image_id") or scaleset.get("image") + assert observed_image == f"{original_image_id}-updated" + + +def test_scaleset_deleted_when_relation_removed( + juju: jubilant.Juju, + garm_app: str, + garm_configurator_for_scaleset_tests: str, +): + """ + arrange: A scaleset exists in GARM for the test configurator relation. + act: Remove the garm-configurator relation from GARM. + assert: The matching scaleset disappears from GARM. + """ + _ensure_garm_configurator_relation(juju, garm_app, garm_configurator_for_scaleset_tests) + address = _get_garm_address(juju, garm_app) + base_url = _garm_api_base_url(address) + token = garm_login_from_secret(juju, garm_app, base_url) + _create_test_credential(base_url, token) + relation_data = _get_scaleset_relation_data( + juju, garm_app, garm_configurator_for_scaleset_tests + ) + provider_names = {provider.get("name", "") for provider in _list_providers(base_url, token)} + provider_name = relation_data["provider_name"] + if provider_name not in provider_names: + pytest.skip( + f"requires GARM provider {provider_name!r}; available providers: {sorted(provider_names)}" + ) + _set_scaleset_relation_data( + juju, + garm_app, + garm_configurator_for_scaleset_tests, + min_idle_runner="1", + ) + _wait_for_scaleset(base_url, token, _SCALESET_TEST_NAME) + + juju.remove_relation(garm_app, garm_configurator_for_scaleset_tests) + juju.wait( + lambda status: jubilant.all_active(status, garm_app), + timeout=3 * 60, + delay=10, + ) + + _wait_for_scaleset_absent(base_url, token, _SCALESET_TEST_NAME) From 4a5bbb65fa630a07b1acac52da8f211b221e1975 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Tue, 16 Jun 2026 12:28:06 +0700 Subject: [PATCH 08/12] style: fix import sort and blank line lint issues Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- charms/garm-configurator/src/charm_state.py | 1 + charms/garm/src/charm.py | 1 + 2 files changed, 2 insertions(+) diff --git a/charms/garm-configurator/src/charm_state.py b/charms/garm-configurator/src/charm_state.py index d180ff45..8a939894 100644 --- a/charms/garm-configurator/src/charm_state.py +++ b/charms/garm-configurator/src/charm_state.py @@ -357,6 +357,7 @@ def credentials_name(self) -> str: """Derived GARM credentials name for scalesets.""" return f"github-app-{self.github_app_config.client_id}" + def _get_image_id_from_relation(charm: ops.CharmBase) -> str | None: """Return the OpenStack image UUID from the image builder relation, if available. diff --git a/charms/garm/src/charm.py b/charms/garm/src/charm.py index f5b87010..5700f422 100755 --- a/charms/garm/src/charm.py +++ b/charms/garm/src/charm.py @@ -17,6 +17,7 @@ from paas_charm.app import WorkloadConfig from garm_api import GarmApiClient, GarmApiError, GarmAuthenticatedClient +from garm_api import GarmApiError, GarmClient from scaleset_reconciler import ScalesetReconciler, ScalesetSpec logger = logging.getLogger(__name__) From 5a04ee77488b4c3b946493ff57e6f59d6af208e1 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Tue, 16 Jun 2026 18:18:36 +0700 Subject: [PATCH 09/12] fix(tests): move garm_login_from_secret to test_garm.py to fix import error The function was imported via 'from charms.tests.integration.conftest import ...' which fails collection (ModuleNotFoundError: No module named 'charms'). Move the function directly into test_garm.py where it is used. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- charms/tests/integration/conftest.py | 31 --------------------------- charms/tests/integration/test_garm.py | 31 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/charms/tests/integration/conftest.py b/charms/tests/integration/conftest.py index db95d44a..dd83dce3 100644 --- a/charms/tests/integration/conftest.py +++ b/charms/tests/integration/conftest.py @@ -351,37 +351,6 @@ def garm_configurator_charm_file_fixture(pytestconfig: pytest.Config) -> str: return configurator_charms[0] -def garm_login_from_secret(juju: jubilant.Juju, garm_app_name: str, garm_url: str) -> str: - """Log into the GARM API using admin credentials stored in Juju secrets.""" - secrets_json = juju.cli("secrets", "--format=json") - secrets = json.loads(secrets_json) - garm_secret_uri = next( - (uri for uri, info in secrets.items() if info.get("label") == GARM_SECRETS_LABEL), - None, - ) - assert garm_secret_uri, f"{GARM_SECRETS_LABEL} not found for {garm_app_name}" - - secret_json = juju.cli("show-secret", "--reveal", "--format=json", garm_secret_uri) - secret_data = json.loads(secret_json) - content = secret_data[garm_secret_uri]["content"]["Data"] - admin_username = content["admin-username"] - admin_password = content["admin-password"] - - base_url = garm_url.rstrip("/") - if not base_url.endswith("/api/v1"): - base_url = f"{base_url}/api/v1" - - resp = requests.post( - f"{base_url}/auth/login", - json={"username": admin_username, "password": admin_password}, - timeout=30, - ) - resp.raise_for_status() - token = resp.json().get("token", "") - assert token, "Expected non-empty JWT token from GARM login" - return token - - def _pre_pull_garm_image(image: str) -> None: """Pre-pull the GARM ROCK image into microk8s containerd. diff --git a/charms/tests/integration/test_garm.py b/charms/tests/integration/test_garm.py index c0c25715..7a1689e7 100644 --- a/charms/tests/integration/test_garm.py +++ b/charms/tests/integration/test_garm.py @@ -191,6 +191,37 @@ def _do_login() -> str: return token +def garm_login_from_secret(juju: jubilant.Juju, garm_app_name: str, garm_url: str) -> str: + """Log into the GARM API using admin credentials stored in Juju secrets.""" + secrets_json = juju.cli("secrets", "--format=json") + all_secrets = json.loads(secrets_json) + garm_secret_uri = next( + (uri for uri, info in all_secrets.items() if info.get("label") == GARM_SECRETS_LABEL), + None, + ) + assert garm_secret_uri, f"{GARM_SECRETS_LABEL} not found for {garm_app_name}" + + secret_json = juju.cli("show-secret", "--reveal", "--format=json", garm_secret_uri) + secret_data = json.loads(secret_json) + content = secret_data[garm_secret_uri]["content"]["Data"] + admin_username = content["admin-username"] + admin_password = content["admin-password"] + + base_url = garm_url.rstrip("/") + if not base_url.endswith("/api/v1"): + base_url = f"{base_url}/api/v1" + + resp = requests.post( + f"{base_url}/auth/login", + json={"username": admin_username, "password": admin_password}, + timeout=30, + ) + resp.raise_for_status() + token = resp.json().get("token", "") + assert token, "Expected non-empty JWT token from GARM login" + return token + + class _MetricsNotReady(Exception): """Raised while GARM's /metrics is still warming up (retryable).""" From 89422a33481e3abedff239a67e86b7efc7a78c15 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Wed, 17 Jun 2026 15:25:15 +0700 Subject: [PATCH 10/12] refactor: rewrite API clients and reconciler to use generated OpenAPI client Refactor garm_api.py, scaleset_reconciler.py, and related charm/test code to use the generated garm_client OpenAPI client from main instead of the hand-written urllib wrapper. Key changes: - Extend GarmApiClient (main) with login() using generated LoginApi - Add GarmAuthenticatedClient for scaleset/provider/org/repo operations - Remove credentials_name from ScalesetSpec (not in GARM scaleset API) - Rename image_id -> image in ScalesetSpec (matches GARM API field name) - Add entity_type/entity_name to ScalesetSpec for org/repo scoped creation - Reconciler now calls create_org_scaleset/create_repo_scaleset - Reconciler defers on missing provider OR entity (org/repo not in GARM) - GARM charm uses GARM_ADMIN_CREDENTIALS_LABEL (separate secret) for creds - Configurator writes org/repo fields to relation databag - All unit tests updated for new API design (99 passing) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- charms/garm-configurator/src/charm.py | 10 +- .../tests/unit/test_charm.py | 3 +- charms/garm/src/charm.py | 9 +- charms/garm/src/garm_api.py | 59 ++--- charms/garm/src/scaleset_reconciler.py | 4 +- .../tests/unit/test_scaleset_reconciler.py | 209 +++++++++--------- 6 files changed, 154 insertions(+), 140 deletions(-) diff --git a/charms/garm-configurator/src/charm.py b/charms/garm-configurator/src/charm.py index c1cb2360..ec61d706 100755 --- a/charms/garm-configurator/src/charm.py +++ b/charms/garm-configurator/src/charm.py @@ -4,6 +4,7 @@ """Charm entrypoint for the GARM configurator charm.""" +import json import typing import ops @@ -61,6 +62,7 @@ def _reconcile(self, event: ops.EventBase) -> None: } ) + pre_install = state.scaleset_config.pre_install_scripts relation_data = { "name": state.scaleset_config.name, "provider_name": state.provider_name, @@ -76,8 +78,14 @@ def _reconcile(self, event: ops.EventBase) -> None: else ",".join(state.scaleset_config.labels) ), "runner_group": state.scaleset_config.runner_group, - "pre_install_scripts": str(state.scaleset_config.pre_install_scripts), + "pre_install_scripts": json.dumps({"pre_install.sh": pre_install}) + if pre_install + else "", } + if state.scaleset_config.org: + relation_data["org"] = state.scaleset_config.org + if state.scaleset_config.repo: + relation_data["repo"] = state.scaleset_config.repo for garm_relation in self.model.relations[GARM_CONFIGURATOR_RELATION_NAME]: garm_relation.data[self.unit].update(relation_data) diff --git a/charms/garm-configurator/tests/unit/test_charm.py b/charms/garm-configurator/tests/unit/test_charm.py index d54eff07..7d0f858d 100644 --- a/charms/garm-configurator/tests/unit/test_charm.py +++ b/charms/garm-configurator/tests/unit/test_charm.py @@ -469,7 +469,8 @@ def test_garm_configurator_relation_data_written_on_reconcile(): "max_runner": "5", "labels": "self-hosted,linux", "runner_group": "default", - "pre_install_scripts": "echo hello", + "pre_install_scripts": '{"pre_install.sh": "echo hello"}', + "repo": "myorg/myrepo", } for key, value in expected_relation_data.items(): assert rel_out.local_unit_data[key] == value diff --git a/charms/garm/src/charm.py b/charms/garm/src/charm.py index 5700f422..7bbc27a3 100755 --- a/charms/garm/src/charm.py +++ b/charms/garm/src/charm.py @@ -17,7 +17,6 @@ from paas_charm.app import WorkloadConfig from garm_api import GarmApiClient, GarmApiError, GarmAuthenticatedClient -from garm_api import GarmApiError, GarmClient from scaleset_reconciler import ScalesetReconciler, ScalesetSpec logger = logging.getLogger(__name__) @@ -430,9 +429,7 @@ def _build_desired_scalesets(self) -> list[ScalesetSpec]: entity_type = "repository" entity_name = repo else: - logger.warning( - "Skipping scaleset %s: neither org nor repo specified", name - ) + logger.warning("Skipping scaleset %s: neither org nor repo specified", name) continue required = { @@ -493,9 +490,7 @@ def _reconcile_scalesets(self) -> None: try: garm_client = GarmApiClient(f"{garm_url}/api/v1") - token = garm_client.login( - admin_creds["username"], admin_creds["password"] - ) + token = garm_client.login(admin_creds["username"], admin_creds["password"]) auth_client = GarmAuthenticatedClient(f"{garm_url}/api/v1", token) desired = self._build_desired_scalesets() ScalesetReconciler(auth_client).reconcile(desired) diff --git a/charms/garm/src/garm_api.py b/charms/garm/src/garm_api.py index 5f80f2bd..e7972033 100644 --- a/charms/garm/src/garm_api.py +++ b/charms/garm/src/garm_api.py @@ -117,9 +117,7 @@ def wait_for_ready(self, timeout: float = _READINESS_TIMEOUT) -> None: return except GarmConnectionError: if time.monotonic() >= deadline: - raise GarmConnectionError( - f"GARM did not become ready within {timeout:.0f}s" - ) + raise GarmConnectionError(f"GARM did not become ready within {timeout:.0f}s") time.sleep(_READINESS_POLL_INTERVAL) def first_run( @@ -221,13 +219,14 @@ def list_providers(self) -> list[Provider]: """ with self._api_client() as client: try: - return ProvidersApi(api_client=client).list_providers( - _request_timeout=_REQUEST_TIMEOUT - ) or [] + return ( + ProvidersApi(api_client=client).list_providers( + _request_timeout=_REQUEST_TIMEOUT + ) + or [] + ) except ApiException as exc: - raise GarmApiError( - f"Failed to list providers ({exc.status}): {exc.body}" - ) from exc + raise GarmApiError(f"Failed to list providers ({exc.status}): {exc.body}") from exc def list_credentials(self) -> list[ForgeCredentials]: """List all registered GARM credentials. @@ -240,9 +239,12 @@ def list_credentials(self) -> list[ForgeCredentials]: """ with self._api_client() as client: try: - return CredentialsApi(api_client=client).list_credentials( - _request_timeout=_REQUEST_TIMEOUT - ) or [] + return ( + CredentialsApi(api_client=client).list_credentials( + _request_timeout=_REQUEST_TIMEOUT + ) + or [] + ) except ApiException as exc: raise GarmApiError( f"Failed to list credentials ({exc.status}): {exc.body}" @@ -259,13 +261,14 @@ def list_scalesets(self) -> list[ScaleSet]: """ with self._api_client() as client: try: - return ScalesetsApi(api_client=client).list_scalesets( - _request_timeout=_REQUEST_TIMEOUT - ) or [] + return ( + ScalesetsApi(api_client=client).list_scalesets( + _request_timeout=_REQUEST_TIMEOUT + ) + or [] + ) except ApiException as exc: - raise GarmApiError( - f"Failed to list scalesets ({exc.status}): {exc.body}" - ) from exc + raise GarmApiError(f"Failed to list scalesets ({exc.status}): {exc.body}") from exc def find_org_id(self, org_name: str) -> str | None: """Find a GARM organization's UUID by name. @@ -281,10 +284,13 @@ def find_org_id(self, org_name: str) -> str | None: """ with self._api_client() as client: try: - orgs = OrganizationsApi(api_client=client).list_orgs( - name=org_name, - _request_timeout=_REQUEST_TIMEOUT, - ) or [] + orgs = ( + OrganizationsApi(api_client=client).list_orgs( + name=org_name, + _request_timeout=_REQUEST_TIMEOUT, + ) + or [] + ) except ApiException as exc: raise GarmApiError( f"Failed to list organizations ({exc.status}): {exc.body}" @@ -308,9 +314,12 @@ def find_repo_id(self, repo_name: str) -> str | None: """ with self._api_client() as client: try: - repos = RepositoriesApi(api_client=client).list_repos( - _request_timeout=_REQUEST_TIMEOUT, - ) or [] + repos = ( + RepositoriesApi(api_client=client).list_repos( + _request_timeout=_REQUEST_TIMEOUT, + ) + or [] + ) except ApiException as exc: raise GarmApiError( f"Failed to list repositories ({exc.status}): {exc.body}" diff --git a/charms/garm/src/scaleset_reconciler.py b/charms/garm/src/scaleset_reconciler.py index 9ec9e04d..c75d7371 100644 --- a/charms/garm/src/scaleset_reconciler.py +++ b/charms/garm/src/scaleset_reconciler.py @@ -85,9 +85,7 @@ def reconcile(self, desired: list[ScalesetSpec]) -> None: for name, scaleset in observed.items(): if name not in all_desired_names: - logger.info( - "Deleting orphaned scaleset %s (id=%s)", name, scaleset.id - ) + logger.info("Deleting orphaned scaleset %s (id=%s)", name, scaleset.id) self._client.delete_scaleset(scaleset.id) def _resolve_entity_id(self, spec: ScalesetSpec) -> str | None: diff --git a/charms/garm/tests/unit/test_scaleset_reconciler.py b/charms/garm/tests/unit/test_scaleset_reconciler.py index 62bf8010..306c4d73 100644 --- a/charms/garm/tests/unit/test_scaleset_reconciler.py +++ b/charms/garm/tests/unit/test_scaleset_reconciler.py @@ -3,43 +3,71 @@ """Unit tests for the scaleset reconciler.""" -from unittest.mock import MagicMock +from unittest.mock import MagicMock, call from scaleset_reconciler import ScalesetReconciler, ScalesetSpec -def _mock_client(providers=None, credentials=None, scalesets=None): +def _mock_client(providers=None, scalesets=None, org_id="org-uuid", repo_id=None): + """Build a mock GarmAuthenticatedClient.""" client = MagicMock() - client.list_providers.return_value = providers or [] - client.list_credentials.return_value = credentials or [] - client.list_scalesets.return_value = scalesets or [] - client.create_scaleset.return_value = {"id": "new-uuid", "name": "test"} - client.update_scaleset.return_value = {"id": "uuid-1", "name": "test"} + provider_mocks = [] + for name in (providers or []): + m = MagicMock() + m.name = name + provider_mocks.append(m) + client.list_providers.return_value = provider_mocks + + scaleset_mocks = [] + for ss in (scalesets or []): + m = MagicMock() + m.name = ss["name"] + m.id = ss.get("id", 1) + m.image = ss.get("image", "ubuntu-22.04") + m.flavor = ss.get("flavor", "m1.small") + m.max_runners = ss.get("max_runners", 5) + m.min_idle_runners = ss.get("min_idle_runners", 0) + m.runner_group = ss.get("runner_group", None) + m.extra_specs = ss.get("extra_specs", {}) + scaleset_mocks.append(m) + client.list_scalesets.return_value = scaleset_mocks + + client.find_org_id.return_value = org_id + client.find_repo_id.return_value = repo_id + created = MagicMock() + created.id = 99 + client.create_org_scaleset.return_value = created + client.create_repo_scaleset.return_value = created + updated = MagicMock() + updated.id = 1 + client.update_scaleset.return_value = updated return client def _spec( name="my-scaleset", provider_name="openstack-demo", - credentials_name="github-app-12345", - image_id="ubuntu-22.04", + image="ubuntu-22.04", flavor="m1.small", - os_arch="x64", + os_arch="amd64", min_idle=0, max_runners=5, + entity_type="organization", + entity_name="my-org", labels=None, - runner_group="default", + runner_group="", pre_install_scripts=None, ): return ScalesetSpec( name=name, provider_name=provider_name, - credentials_name=credentials_name, - image_id=image_id, + image=image, flavor=flavor, os_arch=os_arch, min_idle_runners=min_idle, max_runners=max_runners, + entity_type=entity_type, + entity_name=entity_name, labels=labels or [], runner_group=runner_group, pre_install_scripts=pre_install_scripts or {}, @@ -47,153 +75,128 @@ def _spec( def test_create_scaleset_when_not_exists(): - client = _mock_client( - providers=[{"name": "openstack-demo"}], - credentials=[{"name": "github-app-12345"}], - scalesets=[], - ) + client = _mock_client(providers=["openstack-demo"], scalesets=[]) reconciler = ScalesetReconciler(client) reconciler.reconcile([_spec()]) - client.create_scaleset.assert_called_once() - call_payload = client.create_scaleset.call_args[0][0] - assert call_payload["name"] == "my-scaleset" - assert call_payload["image_id"] == "ubuntu-22.04" - assert call_payload["flavor"] == "m1.small" + client.create_org_scaleset.assert_called_once() + args = client.create_org_scaleset.call_args + assert args[0][0] == "org-uuid" + params = args[0][1] + assert params.name == "my-scaleset" + assert params.image == "ubuntu-22.04" + assert params.flavor == "m1.small" def test_skip_creation_when_provider_missing(): - client = _mock_client( - providers=[], - credentials=[{"name": "github-app-12345"}], - scalesets=[], - ) + client = _mock_client(providers=[], scalesets=[]) reconciler = ScalesetReconciler(client) reconciler.reconcile([_spec()]) - client.create_scaleset.assert_not_called() + client.create_org_scaleset.assert_not_called() + client.create_repo_scaleset.assert_not_called() -def test_skip_creation_when_credential_missing(): - client = _mock_client( - providers=[{"name": "openstack-demo"}], - credentials=[], - scalesets=[], - ) +def test_skip_creation_when_entity_not_found(): + client = _mock_client(providers=["openstack-demo"], scalesets=[], org_id=None) reconciler = ScalesetReconciler(client) reconciler.reconcile([_spec()]) - client.create_scaleset.assert_not_called() + client.create_org_scaleset.assert_not_called() + + +def test_create_repo_scaleset(): + client = _mock_client(providers=["openstack-demo"], scalesets=[], repo_id="repo-uuid") + client.find_repo_id.return_value = "repo-uuid" + reconciler = ScalesetReconciler(client) + reconciler.reconcile([_spec(entity_type="repository", entity_name="owner/repo")]) + client.create_repo_scaleset.assert_called_once() + args = client.create_repo_scaleset.call_args + assert args[0][0] == "repo-uuid" def test_update_scaleset_when_image_changed(): existing = { - "id": "uuid-1", + "id": 1, "name": "my-scaleset", - "image_id": "ubuntu-20.04", + "image": "ubuntu-20.04", "flavor": "m1.small", "max_runners": 5, "min_idle_runners": 0, - "tags": [], - "runner_group": "default", + "runner_group": None, "extra_specs": {}, } - client = _mock_client( - providers=[{"name": "openstack-demo"}], - credentials=[{"name": "github-app-12345"}], - scalesets=[existing], - ) + client = _mock_client(providers=["openstack-demo"], scalesets=[existing]) reconciler = ScalesetReconciler(client) - reconciler.reconcile([_spec(image_id="ubuntu-22.04")]) + reconciler.reconcile([_spec(image="ubuntu-22.04")]) client.update_scaleset.assert_called_once() args = client.update_scaleset.call_args - assert args[0][0] == "uuid-1" - assert args[0][1]["image_id"] == "ubuntu-22.04" + assert args[0][0] == 1 + assert args[0][1].image == "ubuntu-22.04" def test_no_update_when_scaleset_unchanged(): existing = { - "id": "uuid-1", + "id": 1, "name": "my-scaleset", - "image_id": "ubuntu-22.04", + "image": "ubuntu-22.04", "flavor": "m1.small", "max_runners": 5, "min_idle_runners": 0, - "tags": [], - "runner_group": "default", + "runner_group": None, "extra_specs": {}, } - client = _mock_client( - providers=[{"name": "openstack-demo"}], - credentials=[{"name": "github-app-12345"}], - scalesets=[existing], - ) + client = _mock_client(providers=["openstack-demo"], scalesets=[existing]) reconciler = ScalesetReconciler(client) reconciler.reconcile([_spec()]) client.update_scaleset.assert_not_called() - client.create_scaleset.assert_not_called() + client.create_org_scaleset.assert_not_called() def test_delete_scaleset_not_in_desired(): existing = { - "id": "uuid-stale", + "id": 42, "name": "stale-scaleset", - "image_id": "ubuntu-20.04", + "image": "ubuntu-20.04", "flavor": "m1.small", "max_runners": 2, "min_idle_runners": 0, - "tags": [], - "runner_group": "default", + "runner_group": None, "extra_specs": {}, } - client = _mock_client( - providers=[{"name": "openstack-demo"}], - credentials=[{"name": "github-app-12345"}], - scalesets=[existing], - ) + client = _mock_client(providers=["openstack-demo"], scalesets=[existing]) reconciler = ScalesetReconciler(client) reconciler.reconcile([_spec(name="new-scaleset")]) - client.delete_scaleset.assert_called_once_with("uuid-stale") + client.delete_scaleset.assert_called_once_with(42) -def test_no_delete_when_desired_is_empty(): - client = _mock_client( - providers=[], - credentials=[], - scalesets=[], - ) +def test_existing_scaleset_not_deleted_when_provider_missing(): + """If provider is missing for spec X, scalesets not matching ANY desired name are still deleted, + but scalesets matching a desired name (even if deferred) are preserved.""" + existing = { + "id": 1, + "name": "my-scaleset", + "image": "ubuntu-22.04", + "flavor": "m1.small", + "max_runners": 5, + "min_idle_runners": 0, + "runner_group": None, + "extra_specs": {}, + } + client = _mock_client(providers=[], scalesets=[existing]) + reconciler = ScalesetReconciler(client) + reconciler.reconcile([_spec(name="my-scaleset")]) + client.delete_scaleset.assert_not_called() + + +def test_no_delete_when_desired_is_empty_and_no_observed(): + client = _mock_client(providers=[], scalesets=[]) reconciler = ScalesetReconciler(client) reconciler.reconcile([]) client.delete_scaleset.assert_not_called() def test_pre_install_scripts_included_in_create(): - client = _mock_client( - providers=[{"name": "openstack-demo"}], - credentials=[{"name": "github-app-12345"}], - scalesets=[], - ) + client = _mock_client(providers=["openstack-demo"], scalesets=[]) reconciler = ScalesetReconciler(client) scripts = {"setup.sh": "#!/bin/bash\napt-get update"} reconciler.reconcile([_spec(pre_install_scripts=scripts)]) - payload = client.create_scaleset.call_args[0][0] - assert payload["extra_specs"]["pre_install_scripts"] == scripts - - -def test_labels_sorted_for_comparison(): - existing = { - "id": "uuid-1", - "name": "my-scaleset", - "image_id": "ubuntu-22.04", - "flavor": "m1.small", - "max_runners": 5, - "min_idle_runners": 0, - "tags": ["z-tag", "a-tag"], - "runner_group": "default", - "extra_specs": {}, - } - client = _mock_client( - providers=[{"name": "openstack-demo"}], - credentials=[{"name": "github-app-12345"}], - scalesets=[existing], - ) - reconciler = ScalesetReconciler(client) - reconciler.reconcile([_spec(labels=["a-tag", "z-tag"])]) - client.update_scaleset.assert_not_called() + params = client.create_org_scaleset.call_args[0][1] + assert params.extra_specs == {"pre_install_scripts": scripts} From bf7425ebcf0f761cced6d0358a5507dca8307e72 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Wed, 17 Jun 2026 21:16:43 +0800 Subject: [PATCH 11/12] fix: linting fixes --- charms/garm/src/charm.py | 38 +++++-------------- charms/garm/tests/unit/test_charm.py | 21 ++++++---- charms/garm/tests/unit/test_garm_api.py | 12 ++---- .../tests/unit/test_scaleset_reconciler.py | 4 +- 4 files changed, 29 insertions(+), 46 deletions(-) diff --git a/charms/garm/src/charm.py b/charms/garm/src/charm.py index d26ef112..79012b85 100755 --- a/charms/garm/src/charm.py +++ b/charms/garm/src/charm.py @@ -5,8 +5,8 @@ """GARM charm entrypoint.""" import dataclasses -import json import hashlib +import json import logging import secrets import string @@ -15,13 +15,11 @@ import ops import paas_charm.go import tomli_w +import yaml from paas_charm.app import WorkloadConfig from garm_api import GarmApiClient, GarmApiError, GarmAuthenticatedClient from scaleset_reconciler import ScalesetReconciler, ScalesetSpec -import yaml -from garm_api import GarmApiClient, GarmApiError -from paas_charm.app import WorkloadConfig logger = logging.getLogger(__name__) @@ -124,9 +122,7 @@ def _build_provider_list( "project_domain_name": provider["project_domain_name"], } - clouds_yaml = _render_clouds_yaml( - unit_name, auth_block, provider["region_name"] - ) + clouds_yaml = _render_clouds_yaml(unit_name, auth_block, provider["region_name"]) provider_files[provider_toml_path] = provider_toml provider_files[clouds_yaml_path] = clouds_yaml @@ -316,9 +312,7 @@ def _workload_config(self) -> WorkloadConfig: paas-config.yaml, so metrics_target is set to None to suppress the framework's default metrics-port scrape job. """ - return dataclasses.replace( - super()._workload_config, port=GARM_PORT, metrics_target=None - ) + return dataclasses.replace(super()._workload_config, port=GARM_PORT, metrics_target=None) def _on_configurator_relation_changed(self, _: ops.EventBase) -> None: """Handle configurator relation joined/changed/broken by re-rendering TOML.""" @@ -371,17 +365,13 @@ def restart(self, rerun_migrations: bool = False) -> None: secrets_data = self._get_secrets() if secrets_data is None: - logger.info( - "GARM secrets not yet available; blocking until leader initialises" - ) + logger.info("GARM secrets not yet available; blocking until leader initialises") self.unit.status = ops.WaitingStatus("Waiting for GARM secrets") return provider_configs = self._get_configurator_provider_configs() if not provider_configs: - self.unit.status = ops.WaitingStatus( - "Waiting for garm-configurator relation" - ) + self.unit.status = ops.WaitingStatus("Waiting for garm-configurator relation") return toml_content, provider_files = render_garm_toml( @@ -396,9 +386,7 @@ def restart(self, rerun_migrations: bool = False) -> None: hash_input = ( toml_content + "\n" - + "\n".join( - f"{path}\n{content}" for path, content in sorted(provider_files.items()) - ) + + "\n".join(f"{path}\n{content}" for path, content in sorted(provider_files.items())) ) new_hash = self._hash_toml(hash_input) previous_hash = self._get_on_disk_toml_hash(provider_files) @@ -413,9 +401,7 @@ def restart(self, rerun_migrations: bool = False) -> None: logger.info("Updating GARM config for providers: %s", provider_names) container = self.unit.get_container(CONTAINER_NAME) - container.push( - GARM_CONFIG_PATH, toml_content, permissions=0o600, make_dirs=True - ) + container.push(GARM_CONFIG_PATH, toml_content, permissions=0o600, make_dirs=True) for path, content in provider_files.items(): container.push(path, content, permissions=0o600, make_dirs=True) @@ -615,9 +601,7 @@ def _get_configurator_provider_configs( "password": password, "project_name": data.get("openstack_project_name", ""), "user_domain_name": data.get("openstack_user_domain_name", ""), - "project_domain_name": data.get( - "openstack_project_domain_name", "" - ), + "project_domain_name": data.get("openstack_project_domain_name", ""), "region_name": data.get("openstack_region_name", ""), "network": data.get("openstack_network", ""), } @@ -661,9 +645,7 @@ def _maybe_first_run(self) -> None: return admin_creds = self._get_admin_credentials() if not admin_creds: - logger.warning( - "Admin credentials secret not yet available; skipping first-run check" - ) + logger.warning("Admin credentials secret not yet available; skipping first-run check") return try: diff --git a/charms/garm/tests/unit/test_charm.py b/charms/garm/tests/unit/test_charm.py index 2d428c72..f6c5a783 100644 --- a/charms/garm/tests/unit/test_charm.py +++ b/charms/garm/tests/unit/test_charm.py @@ -541,6 +541,7 @@ def test_maybe_first_run_skips_on_missing_credential_key(): mock_client_cls.assert_not_called() + def test_reconcile_scalesets_skips_when_no_admin_credentials(): """Scaleset reconciliation exits early when admin credentials are unavailable.""" charm = object.__new__(GarmCharm) @@ -555,10 +556,12 @@ def test_reconcile_scalesets_skips_when_no_admin_credentials(): def test_reconcile_scalesets_skips_restart(): """Scaleset reconciliation must not restart the workload.""" charm = object.__new__(GarmCharm) - charm._get_admin_credentials = MagicMock(return_value={ - "username": "admin", - "password": "TestPass-123!", - }) + charm._get_admin_credentials = MagicMock( + return_value={ + "username": "admin", + "password": "TestPass-123!", + } + ) charm._get_garm_url = MagicMock(return_value="http://127.0.0.1:9997") charm._build_desired_scalesets = MagicMock(return_value=[]) charm.restart = MagicMock() @@ -581,10 +584,12 @@ def test_reconcile_scalesets_skips_restart(): def test_reconcile_scalesets_skips_when_garm_url_unavailable(): """Scaleset reconciliation exits early when the GARM URL is not configured.""" charm = object.__new__(GarmCharm) - charm._get_admin_credentials = MagicMock(return_value={ - "username": "admin", - "password": "TestPass-123!", - }) + charm._get_admin_credentials = MagicMock( + return_value={ + "username": "admin", + "password": "TestPass-123!", + } + ) charm._get_garm_url = MagicMock(return_value="") with patch("charm.GarmApiClient") as mock_client_cls: diff --git a/charms/garm/tests/unit/test_garm_api.py b/charms/garm/tests/unit/test_garm_api.py index e123d52e..e93302cf 100644 --- a/charms/garm/tests/unit/test_garm_api.py +++ b/charms/garm/tests/unit/test_garm_api.py @@ -7,13 +7,7 @@ import pytest -from garm_api import ( - GarmApiClient, - GarmApiError, - GarmAuthenticatedClient, - GarmConnectionError, -) - +from garm_api import GarmApiClient, GarmApiError, GarmAuthenticatedClient, GarmConnectionError BASE_URL = "http://127.0.0.1:9997/api/v1" @@ -38,7 +32,9 @@ def test_login_raises_on_api_exception(self): client = GarmApiClient(BASE_URL) with patch("garm_api.LoginApi") as MockLoginApi: - MockLoginApi.return_value.login.side_effect = ApiException(status=401, reason="Unauthorized") + MockLoginApi.return_value.login.side_effect = ApiException( + status=401, reason="Unauthorized" + ) with patch.object(client, "_api_client") as mock_api_client: mock_api_client.return_value.__enter__ = MagicMock(return_value=MagicMock()) mock_api_client.return_value.__exit__ = MagicMock(return_value=False) diff --git a/charms/garm/tests/unit/test_scaleset_reconciler.py b/charms/garm/tests/unit/test_scaleset_reconciler.py index 306c4d73..18c03688 100644 --- a/charms/garm/tests/unit/test_scaleset_reconciler.py +++ b/charms/garm/tests/unit/test_scaleset_reconciler.py @@ -12,14 +12,14 @@ def _mock_client(providers=None, scalesets=None, org_id="org-uuid", repo_id=None """Build a mock GarmAuthenticatedClient.""" client = MagicMock() provider_mocks = [] - for name in (providers or []): + for name in providers or []: m = MagicMock() m.name = name provider_mocks.append(m) client.list_providers.return_value = provider_mocks scaleset_mocks = [] - for ss in (scalesets or []): + for ss in scalesets or []: m = MagicMock() m.name = ss["name"] m.id = ss.get("id", 1) From b2a549f8ec30555ac15bbd03b884ed1561b554b0 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 18 Jun 2026 01:21:34 +0800 Subject: [PATCH 12/12] chore: refactor provider config reconciliation --- charms/garm/src/charm.py | 70 +++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 22 deletions(-) diff --git a/charms/garm/src/charm.py b/charms/garm/src/charm.py index 79012b85..6c0188fa 100755 --- a/charms/garm/src/charm.py +++ b/charms/garm/src/charm.py @@ -362,7 +362,7 @@ def restart(self, rerun_migrations: bool = False) -> None: logger.info("PostgreSQL relation data not yet available; blocking") self.unit.status = ops.WaitingStatus("Waiting for postgresql relation") return - + secrets_data = self._get_secrets() if secrets_data is None: logger.info("GARM secrets not yet available; blocking until leader initialises") @@ -373,7 +373,52 @@ def restart(self, rerun_migrations: bool = False) -> None: if not provider_configs: self.unit.status = ops.WaitingStatus("Waiting for garm-configurator relation") return + + container = self.unit.get_container(CONTAINER_NAME) + provider_config_hash = self._reconcile_provider_configs( + container, + provider_configs, + secrets_data, + postgresql_config, + ) + + container.add_layer( + "garm-command", + { + "services": { + PEBBLE_SERVICE_NAME: { + "override": "replace", + "startup": "enabled", + "command": f"{GARM_BINARY} -config {GARM_CONFIG_PATH}", + "environment": { + "config_hash": provider_config_hash, + }, + } + } + }, + combine=True, + ) + container.replan() + self._maybe_first_run() + self._reconcile_scalesets() + super().restart(rerun_migrations=rerun_migrations) + + def _reconcile_provider_configs( + self, + container: ops.Container, + provider_configs: list[dict[str, str]], + secrets_data: dict[str, str], + postgresql_config: dict[str, typing.Any], + ) -> str: + """Push provider configs into the Pebble container. + + Args: + container: The Pebble container to push the configs into. + provider_configs: List of provider config dicts from Configurator units. + Returns: + configuration hash of the new TOML content (used to detect changes). + """ toml_content, provider_files = render_garm_toml( jwt_secret=secrets_data["jwt-secret"], db_passphrase=secrets_data["db-passphrase"], @@ -392,7 +437,7 @@ def restart(self, rerun_migrations: bool = False) -> None: previous_hash = self._get_on_disk_toml_hash(provider_files) if previous_hash == new_hash: logger.debug("TOML config unchanged; skipping restart") - return + return new_hash # Log non-sensitive metadata about the config change. # Do NOT log toml_content here — it contains secrets @@ -405,26 +450,7 @@ def restart(self, rerun_migrations: bool = False) -> None: for path, content in provider_files.items(): container.push(path, content, permissions=0o600, make_dirs=True) - container.add_layer( - "garm-command", - { - "services": { - PEBBLE_SERVICE_NAME: { - "override": "replace", - "startup": "enabled", - "command": f"{GARM_BINARY} -config {GARM_CONFIG_PATH}", - "environment": { - "config_hash": new_hash, - }, - } - } - }, - combine=True, - ) - container.replan() - self._maybe_first_run() - self._reconcile_scalesets() - super().restart(rerun_migrations=rerun_migrations) + return new_hash @staticmethod def _hash_toml(toml_content: str) -> str: