fix(garm): keep a departing configurator's provider until its scaleset is gone (ISD-7262)

charms/garm/AGENTS.md

@@ -30,6 +30,28 @@ charm conventions; this file lists only what's specific to `garm`.
   `github_reconciler.py` base64-encodes the raw PEM private key (held as `CredentialSpec.private_key`)
   when building the `GithubApp` model. To fix a generated-type issue, bump `GARM_COMMIT` and
   re-run the script, never hand-edit the output.
+- **Provider set is published, not merely derived.** `restart()` reads the `garm-providers`
+  secret, adds every provider the relations publish, restarts GARM with that set, reconciles
+  scalesets, and only then trims and re-records what no relation and no surviving scaleset
+  names. **DON'T** reduce the published set to the current relations: GARM fixes its provider
+  map per process, so dropping a provider whose scaleset still exists leaves that scaleset
+  with no worker that can delete its runners or the scaleset — the teardown deadlocks. The
+  provider leaves the record on the hook after its last scaleset is gone, and GARM's next
+  start drops it.
+  - The record is published even with **no** configurator left, so a pod replaced mid-drain
+    still starts GARM with the providers that drain needs. The hook waits for the relation
+    only when nothing is related **and** nothing is recorded.
+  - A configurator that is related but has published nothing yet is a waiting state, not a
+    reason to reconcile: the sweep would prune the scalesets that configurator is about to
+    describe.
+  - The trim takes effect at the **following** replan; that is deliberate, so a retirement
+    does not restart GARM twice.
+  - A reconcile pass that cannot establish which scalesets GARM holds returns `None`, and
+    `None` trims nothing. **DON'T** default it to an empty set — that deletes every provider.
+  - A record that cannot be read or written raises `ProviderRecordError` out of the hook and
+    puts the unit in error state: the charm cannot deduce the provider set, and guessing is
+    what strands a scaleset. The leader is the only writer, and readers use
+    `peek_content()`, so the superseded revision is removed after each write.
 - GARM serves its API and `/metrics` on one fixed port (`GARM_PORT`); the `app-port` /
   `metrics-port` / `metrics-path` config options have no effect (the charm logs a warning
   rather than blocking). The port is pinned in the `_workload_config` property.

charms/garm/src/charm_state.py

@@ -385,3 +385,43 @@ def from_charm(cls, charm: ops.CharmBase) -> "CharmState":
                 charm.model.relations.get(GARM_CONFIGURATOR_RELATION_NAME, [])
             ),
         )
+
+
+@dataclasses.dataclass(frozen=True)
+class ProviderConfig:
+    """One OpenStack provider, as the charm publishes it to GARM.
+
+    The provider's name is its configurator's unit name with ``/`` replaced by ``-``, which is
+    also the key GARM's provider registry and the scale sets' ``provider_name`` use. The field
+    names are the wire contract: ``as_dict()`` is what the workload entrypoint reads out of
+    ``GARM_PROVIDERS_JSON`` and what the record keeps, so renaming a field changes both.
+
+    Attributes:
+        unit_name: The provider name.
+        auth_url: Keystone authentication URL.
+        username: OpenStack username.
+        password: OpenStack password, in plaintext — it goes into the provider's clouds.yaml.
+        project_name: OpenStack project (tenant).
+        user_domain_name: Domain of the user.
+        project_domain_name: Domain of the project.
+        region_name: OpenStack region.
+        network: Network name or ID instances are attached to.
+    """
+
+    unit_name: str
+    auth_url: str
+    username: str
+    password: str
+    project_name: str
+    user_domain_name: str
+    project_domain_name: str
+    region_name: str
+    network: str
+
+    def as_dict(self) -> dict[str, str]:
+        """Return the wire form the entrypoint and the record read.
+
+        Returns:
+            The provider's fields, keyed by the names in ``GARM_PROVIDERS_JSON``.
+        """
+        return dataclasses.asdict(self)

charms/garm/src/garm_entrypoint.py

@@ -35,7 +35,7 @@
     "GARM_PASSPHRASE",
     "GARM_PROVIDERS_JSON",
 )
-_PROVIDER_REQUIRED_FIELDS: Final[tuple[str, ...]] = (
+PROVIDER_REQUIRED_FIELDS: Final[tuple[str, ...]] = (
     "unit_name",
     "network",
     "auth_url",
@@ -226,7 +226,7 @@ def _build_provider(
     unit_name = provider.get("unit_name")
     if not isinstance(unit_name, str) or not unit_name or Path(unit_name).name != unit_name:
         raise InvalidConfigurationError(f"Invalid provider unit name: {unit_name!r}")
-    missing = [name for name in _PROVIDER_REQUIRED_FIELDS if name not in provider]
+    missing = [name for name in PROVIDER_REQUIRED_FIELDS if name not in provider]
     if missing:
         raise InvalidConfigurationError(
             f"GARM provider is missing required fields: {', '.join(missing)}"

charms/garm/src/scaleset_reconciler.py

@@ -162,7 +162,9 @@ def __init__(self, client: GarmAuthenticatedClient) -> None:
         """
         self._client = client
 
-    def reconcile(self, desired: list[ScalesetSpec]) -> list[ScalesetProgress]:
+    def reconcile(
+        self, desired: list[ScalesetSpec]
+    ) -> tuple[list[ScalesetProgress], list[ScaleSet]]:
         """Sync GARM scalesets to match *desired*.
 
         Performs the minimum set of CREATE / UPDATE / DELETE operations, and
@@ -192,7 +194,11 @@ def reconcile(self, desired: list[ScalesetSpec]) -> list[ScalesetProgress]:
             desired: The full desired set of scalesets.
 
         Returns:
-            One entry per replacement still in flight; empty once converged.
+            The replacements still in flight (empty once converged), and every scaleset GARM
+            holds after this pass — listed and kept, newly created, or not deleted. Callers
+            decide what those mean: the charm keeps a provider published while a scaleset
+            still names it, so this reports live state rather than concluding anything about
+            the desired state.
 
         Raises:
             GarmApiError: If any spec failed to reconcile, re-raised once the rest
@@ -217,12 +223,18 @@ def reconcile(self, desired: list[ScalesetSpec]) -> list[ScalesetProgress]:
             claimed.update(family)
 
         progress: list[ScalesetProgress] = []
+        created: dict[str, ScaleSet] = {}
         failure: GarmApiError | None = None
         for spec in desired:
             try:
                 progress.extend(
                     self._reconcile_one(
-                        spec, providers, observed, templates, families.get(spec.name, [])
+                        spec,
+                        providers,
+                        observed,
+                        templates,
+                        families.get(spec.name, []),
+                        created,
                     )
                 )
             except (GarmConnectionError, GarmUnauthorizedError):
@@ -231,12 +243,40 @@ def reconcile(self, desired: list[ScalesetSpec]) -> list[ScalesetProgress]:
                 logger.warning("Failed to reconcile scaleset %s: %s", spec.name, exc)
                 failure = failure or exc
 
+        deleted: set[str] = set()
         for name, scaleset in observed.items():
             if name not in claimed and self._delete_orphaned(scaleset):
                 self._delete_custom_template(name, templates)
+                deleted.add(name)
         if failure is not None:
             raise failure
-        return progress
+        return progress, self._live_scalesets(observed, deleted, created)
+
+    @staticmethod
+    def _live_scalesets(
+        observed: dict[str, ScaleSet], deleted: set[str], created: dict[str, ScaleSet]
+    ) -> list[ScaleSet]:
+        """Return every scaleset GARM holds after this pass.
+
+        The live set is what GARM answered, adjusted by what this pass did to it: the
+        scalesets it listed and kept, the ones it created, minus the ones whose delete GARM
+        accepted. Reporting the state after the pass rather than the observation before it
+        means a caller never has to reason about which half of a pass a scaleset came from.
+
+        A scaleset whose delete was refused, deferred, or never attempted is still live, and
+        a provider dropped while its scaleset survives can never be handed back.
+
+        Args:
+            observed: The scalesets GARM listed before this pass, by name.
+            deleted: Names of scalesets GARM accepted a delete for during this pass.
+            created: Scalesets this pass created, by name.
+
+        Returns:
+            The live scalesets, ordered by name so a caller's decisions are stable.
+        """
+        live = {name: scaleset for name, scaleset in observed.items() if name not in deleted}
+        live.update(created)
+        return [live[name] for name in sorted(live)]
 
     @staticmethod
     def _dedupe(desired: list[ScalesetSpec]) -> list[ScalesetSpec]:
@@ -324,6 +364,7 @@ def _reconcile_one(
         observed: dict[str, ScaleSet],
         templates: dict[str, Template],
         family: list[str],
+        created: dict[str, ScaleSet],
     ) -> list[ScalesetProgress]:
         """Reconcile a single desired scaleset: validate, create or update, and sync its template.
 
@@ -333,6 +374,7 @@ def _reconcile_one(
             observed: Observed scalesets keyed by name.
             templates: Observed templates keyed by name.
             family: Every live generation of this spec, plus its target name.
+            created: Scalesets this pass created, filled in so the caller sees them as live.
 
         Returns:
             One entry per replaced generation of this spec still draining.
@@ -370,7 +412,9 @@ def _reconcile_one(
         if active_name in observed:
             self._maybe_update(observed[active_name], spec, template_id)
         else:
-            self._create(spec, active_name, entity_id, create_params, template_id)
+            created[active_name] = self._create(
+                spec, active_name, entity_id, create_params, template_id
+            )
 
         if not spec.runner_config.has_config():
             # Runner options were cleared (or the system template is
@@ -1065,14 +1109,25 @@ def _create(
         entity_id: str,
         params: CreateScaleSetParams,
         template_id: int,
-    ) -> None:
+    ) -> ScaleSet:
+        """Create the scaleset and return it as GARM reports it.
+
+        Args:
+            spec: The desired scaleset.
+            scaleset_name: The generation name to create.
+            entity_id: The GARM entity UUID to create it under.
+            params: Creation parameters.
+            template_id: The runner template to reference, if any.
+
+        Returns:
+            The created scaleset.
+        """
         if template_id:
             params.template_id = template_id
         logger.info("Creating scaleset %s under %s %s", scaleset_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)
+            return self._client.create_org_scaleset(entity_id, params)
+        return self._client.create_repo_scaleset(entity_id, params)
 
     def _maybe_update(self, observed: ScaleSet, spec: ScalesetSpec, template_id: int) -> None:
         observed_labels = _observed_labels(observed)