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..3d649b9e 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,83 @@ 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: A stale app status and a reconcile that will succeed. + act: Call _reconcile_runners(). + assert: Both unit and app status refresh to ActiveStatus. + """ + 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: A stale app status and a missing garm-configurator relation. + act: Call restart(). + assert: Both unit and app status become "Waiting for garm-configurator relation". + """ + 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/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( 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