From ac2e6734a82434481b0e1f49ec263eb0d8961160 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Mon, 6 Jul 2026 14:00:15 +0000 Subject: [PATCH 1/3] fix(garm): sync app status with unit status The charm's own status writes used self.unit.status directly, which updates only the unit status. Only the paas_charm base's update_app_and_unit_status also sets self.app.status (on the leader), and restart()'s config-unchanged fast path returns before super().restart() ever calls it. So in steady state the application status froze on whatever the framework last set it to (e.g. a stale "Waiting for pebble ready" from an earlier can_connect blip) while the unit status stayed live and correct. Route the six status writes in restart()'s readiness gates and _reconcile_runners through update_app_and_unit_status so the app status tracks the unit status. --- charms/garm/src/charm.py | 14 +++-- charms/garm/tests/unit/test_charm.py | 87 ++++++++++++++++++++++++++++ docs/changelog.md | 2 + 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/charms/garm/src/charm.py b/charms/garm/src/charm.py index 0d83453e..ac5aa352 100755 --- a/charms/garm/src/charm.py +++ b/charms/garm/src/charm.py @@ -438,18 +438,20 @@ def restart(self, rerun_migrations: bool = False) -> None: postgresql_config = self._get_postgresql_config() if not postgresql_config: logger.info("PostgreSQL relation data not yet available; blocking") - self.unit.status = ops.WaitingStatus("Waiting for postgresql relation") + self.update_app_and_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") - self.unit.status = ops.WaitingStatus("Waiting for GARM secrets") + self.update_app_and_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.update_app_and_unit_status( + ops.WaitingStatus("Waiting for garm-configurator relation") + ) return proxy_env = _proxy_environment() @@ -918,13 +920,13 @@ def _reconcile_runners(self) -> None: EntityReconciler(auth_client).reconcile(charm_state.desired_entities) template_id = _apply_garm_template(auth_client, charm_state.ssh_debug_connections) ScalesetReconciler(auth_client).reconcile(self._build_desired_scalesets(template_id)) - self.unit.status = ops.ActiveStatus() + self.update_app_and_unit_status(ops.ActiveStatus()) except CharmedTemplateError as exc: logger.warning("GARM charmed template error during reconcile: %s", exc) - self.unit.status = ops.WaitingStatus(str(exc)) + self.update_app_and_unit_status(ops.WaitingStatus(str(exc))) except GarmApiError as exc: logger.warning("GARM API error during reconcile: %s", exc) - self.unit.status = ops.WaitingStatus("GARM sync failed") + self.update_app_and_unit_status(ops.WaitingStatus("GARM sync failed")) def _ensure_controller_urls(self, auth_client: GarmAuthenticatedClient) -> None: """Configure the GARM controller URLs that gate its operational API. diff --git a/charms/garm/tests/unit/test_charm.py b/charms/garm/tests/unit/test_charm.py index de52d96a..9013064d 100644 --- a/charms/garm/tests/unit/test_charm.py +++ b/charms/garm/tests/unit/test_charm.py @@ -862,10 +862,12 @@ def test_reconcile_runners_reconciles_credentials_entities_then_scalesets(): patch("charm._apply_garm_template", return_value=99) as mock_apply, patch("charm.CharmState") as mock_charm_state_cls, patch.object(GarmCharm, "unit", new_callable=PropertyMock) as mock_unit, + patch.object(GarmCharm, "app", new_callable=PropertyMock) as mock_app, ): mock_charm_state_cls.from_charm.return_value.ssh_debug_connections = [] mock_charm_state_cls.from_charm.return_value.desired_entities = entities mock_unit.return_value = MagicMock() + mock_app.return_value = MagicMock() order = MagicMock() order.attach_mock(mock_github_cls.return_value.reconcile, "github") order.attach_mock(mock_entity_cls.return_value.reconcile, "entity") @@ -916,9 +918,11 @@ def test_reconcile_runners_charmed_template_error_sets_waiting_status(): ), patch("charm.CharmState") as mock_state, patch.object(GarmCharm, "unit", new_callable=PropertyMock) as mock_unit, + patch.object(GarmCharm, "app", new_callable=PropertyMock) as mock_app, ): mock_state.from_charm.return_value.ssh_debug_connections = [] mock_unit.return_value = MagicMock() + mock_app.return_value = MagicMock() charm._reconcile_runners() # must not raise status = mock_unit.return_value.status @@ -928,6 +932,89 @@ def test_reconcile_runners_charmed_template_error_sets_waiting_status(): assert status.message == "base template missing" +def test_reconcile_runners_success_refreshes_stale_app_status(): + """ + arrange: Admin credentials are available and the reconcile succeeds; the app status is + stale from an earlier reconcile ("Waiting for pebble ready") -- this is the steady + state restart() reaches via its config-unchanged fast path, which calls + _reconcile_runners() and returns before super().restart() ever runs. + act: Call _reconcile_runners(). + assert: Both unit and app status are refreshed to ActiveStatus, not left stale. + """ + charm = object.__new__(GarmCharm) + charm._get_admin_credentials = MagicMock( + return_value={"username": "admin", "password": "TestPass-123!"} + ) + charm._build_desired_credentials = MagicMock(return_value=[]) + charm._build_desired_scalesets = MagicMock(return_value=[]) + charm._ensure_controller_urls = MagicMock() + + with ( + patch("charm.GarmAuthenticatedClient"), + patch("charm.GithubReconciler"), + patch("charm.EntityReconciler"), + patch("charm.ScalesetReconciler"), + patch("charm._apply_garm_template", return_value=99), + patch("charm.CharmState") as mock_charm_state_cls, + patch.object(GarmCharm, "unit", new_callable=PropertyMock) as mock_unit, + patch.object(GarmCharm, "app", new_callable=PropertyMock) as mock_app, + ): + mock_charm_state_cls.from_charm.return_value.ssh_debug_connections = [] + mock_charm_state_cls.from_charm.return_value.desired_entities = [] + mock_unit.return_value = MagicMock() + mock_unit.return_value.is_leader.return_value = True + mock_unit.return_value.status = ops.WaitingStatus("Waiting for pebble ready") + mock_app.return_value = MagicMock() + mock_app.return_value.status = ops.WaitingStatus("Waiting for pebble ready") + + charm._reconcile_runners() + + unit_status = mock_unit.return_value.status + app_status = mock_app.return_value.status + + assert unit_status == ops.ActiveStatus() + assert app_status == ops.ActiveStatus() + + +def test_restart_missing_configurator_relation_refreshes_stale_app_status(): + """ + arrange: PostgreSQL and GARM secrets are available but the garm-configurator relation has + not sent provider data yet; the app status is stale from an earlier reconcile + ("Waiting for pebble ready"). + act: Call restart(). + assert: Both unit and app degrade to the same "Waiting for garm-configurator relation" + status -- the app status is not left stale. + """ + charm = object.__new__(GarmCharm) + charm._ensure_secrets = MagicMock() + charm.is_ready = MagicMock(return_value=True) + charm._get_postgresql_config = MagicMock(return_value=_DEFAULT_PG_CONFIG) + charm._get_secrets = MagicMock( + return_value={"jwt-secret": "test-jwt-secret", "db-passphrase": "a" * 32} + ) + charm._get_configurator_provider_configs = MagicMock(return_value=[]) + + with ( + patch.object(GarmCharm, "config", new_callable=PropertyMock) as mock_config, + patch.object(GarmCharm, "unit", new_callable=PropertyMock) as mock_unit, + patch.object(GarmCharm, "app", new_callable=PropertyMock) as mock_app, + ): + mock_config.return_value = {} + mock_unit.return_value = MagicMock() + mock_unit.return_value.is_leader.return_value = True + mock_unit.return_value.status = ops.WaitingStatus("Waiting for pebble ready") + mock_app.return_value = MagicMock() + mock_app.return_value.status = ops.WaitingStatus("Waiting for pebble ready") + + charm.restart() + + unit_status = mock_unit.return_value.status + app_status = mock_app.return_value.status + + assert unit_status == ops.WaitingStatus("Waiting for garm-configurator relation") + assert app_status == ops.WaitingStatus("Waiting for garm-configurator relation") + + def test_restart_ensures_secrets_before_readiness_gate(): """ arrange: A GarmCharm whose workload is not ready (pebble not up yet). diff --git a/docs/changelog.md b/docs/changelog.md index a8ae22ce..ea8b5074 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -10,6 +10,8 @@ Each revision is versioned by the date of the revision. ## 2026-07-06 +- `garm`: fix the application status freezing on a stale value. The charm wrote the unit status directly on its own reconcile paths, so the leader's application status was never refreshed and could stay stuck (for example showing `Waiting for pebble ready` while the unit was `active`). Status writes now go through the shared unit/application status helper so both stay in sync. + - `garm-configurator`: reject `max-runner=0` during config validation. GARM rejects scale set creation with `max_runners cannot be 0`, so the charm now surfaces the invalid configuration before publishing unusable scale set data to GARM. ## 2026-07-02 From bbc0b137269174365e3c6bd45cd762b3933cbd27 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Mon, 6 Jul 2026 14:11:59 +0000 Subject: [PATCH 2/3] test(garm): tighten docstrings on app-status sync tests --- charms/garm/tests/unit/test_charm.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/charms/garm/tests/unit/test_charm.py b/charms/garm/tests/unit/test_charm.py index 9013064d..3d649b9e 100644 --- a/charms/garm/tests/unit/test_charm.py +++ b/charms/garm/tests/unit/test_charm.py @@ -934,12 +934,9 @@ def test_reconcile_runners_charmed_template_error_sets_waiting_status(): def test_reconcile_runners_success_refreshes_stale_app_status(): """ - arrange: Admin credentials are available and the reconcile succeeds; the app status is - stale from an earlier reconcile ("Waiting for pebble ready") -- this is the steady - state restart() reaches via its config-unchanged fast path, which calls - _reconcile_runners() and returns before super().restart() ever runs. + arrange: A stale app status and a reconcile that will succeed. act: Call _reconcile_runners(). - assert: Both unit and app status are refreshed to ActiveStatus, not left stale. + assert: Both unit and app status refresh to ActiveStatus. """ charm = object.__new__(GarmCharm) charm._get_admin_credentials = MagicMock( @@ -978,12 +975,9 @@ def test_reconcile_runners_success_refreshes_stale_app_status(): def test_restart_missing_configurator_relation_refreshes_stale_app_status(): """ - arrange: PostgreSQL and GARM secrets are available but the garm-configurator relation has - not sent provider data yet; the app status is stale from an earlier reconcile - ("Waiting for pebble ready"). + arrange: A stale app status and a missing garm-configurator relation. act: Call restart(). - assert: Both unit and app degrade to the same "Waiting for garm-configurator relation" - status -- the app status is not left stale. + assert: Both unit and app status become "Waiting for garm-configurator relation". """ charm = object.__new__(GarmCharm) charm._ensure_secrets = MagicMock() From 75076448a65e9e23e21e871af2e0a84f633ee195 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Mon, 6 Jul 2026 17:24:39 +0000 Subject: [PATCH 3/3] test(garm): accept synced app status in reaches-active test The app-status sync fix makes the leader's application status track the unit status, so on integration GARM's application status now honestly reflects the pending _reconcile_runners() (waiting/GARM sync) instead of a stale active set once by paas_charm. Loosen test_garm_charm_reaches_active to accept active or waiting, and drop the now-obsolete split-state note. --- charms/tests/integration/conftest.py | 10 +++++----- charms/tests/integration/test_garm.py | 20 ++++++++++++-------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/charms/tests/integration/conftest.py b/charms/tests/integration/conftest.py index 0fde00b7..7e2be7d1 100644 --- a/charms/tests/integration/conftest.py +++ b/charms/tests/integration/conftest.py @@ -641,11 +641,11 @@ def integrate_configurator_with_garm_fixture( Returns the garm app name. """ juju.integrate(configurator_with_image, garm_app) - # GARM may end up in a split state: app_status=active (set by paas_charm) - # while unit workload_status=waiting (set by _reconcile_runners when - # credentials/providers are not yet configured). all_active and all_waiting - # both require BOTH app and unit to match, so neither would pass. Using - # all_agents_idle checks only that hooks have finished running, which is + # The charm keeps app and unit status in sync, so after integration GARM + # settles into a consistent state that is usually waiting (both app and unit) + # because _reconcile_runners() is still pending until credentials/providers + # are configured -- not active. Waiting for all_active would therefore time + # out; all_agents_idle checks only that hooks have finished running, which is # sufficient to confirm the integration event was processed. juju.wait( lambda status: jubilant.all_agents_idle(status, garm_app) diff --git a/charms/tests/integration/test_garm.py b/charms/tests/integration/test_garm.py index 83dd64a1..f60325b2 100644 --- a/charms/tests/integration/test_garm.py +++ b/charms/tests/integration/test_garm.py @@ -128,18 +128,22 @@ def test_garm_charm_reaches_active( """ arrange: The GARM charm is deployed with postgresql & garm-configurator integrated. act: Observe the Juju application status. - assert: The application is in active status, confirming a successful install. + assert: The application is up (``active``, or ``waiting`` on the GARM runner + reconcile) rather than ``blocked`` or errored, confirming a successful install. - Note: the unit workload status may be ``waiting`` at this point because - _reconcile_runners() runs on integration and fails until first-run - initialises GARM via the API. The app-level status is what paas_charm - sets, confirming the service is up. + The charm keeps the application status in sync with the unit status, so while + _reconcile_runners() is still pending (GARM not yet fully initialised via the + API) the application legitimately reports ``waiting`` instead of a stale + ``active``. Either state confirms the workload came up cleanly. """ status = juju.status() - current = status.apps[configurator_garm].app_status.current - logger.info("GARM app status: %s", current) + app_status = status.apps[configurator_garm].app_status + logger.info("GARM app status: %s (%s)", app_status.current, app_status.message) - assert current == "active", f"Expected {configurator_garm} to be active, got: {current}" + assert app_status.current in ( + "active", + "waiting", + ), f"Expected {configurator_garm} to be active or waiting, got: {app_status.current}" def test_garm_api_controller_info(