fix(garm): make teardown lifecycle safe

[yanksyoon]

Summary

This PR makes GARM teardown lifecycle-safe by stopping normal reconciliation before paas-charm can reconstruct charm state when local Juju reports app.planned_units() == 0.

• Keep GARM-owned and inherited paas-charm callbacks behind the pre-state teardown gate.
• Preserve active reconciliation, including rerun_migrations=True for PostgreSQL events.
• Reject rotate-secret-key before the inherited decorator can mutate state during teardown.
• Fail loudly before base observer registration if the pinned paas-charm lifecycle hook contract changes.
• Keep the four guard-only inherited callbacks in a separate test group; they are intercepted for safety but are not added to the explicit teardown cleanup fallthrough.
• Rebase onto current main and validate against paas-charm==1.12.3.

The architecture diagram is generated locally at ~/tmp/garm-teardown-lifecycle.html and is intentionally not included in this PR.

Related to #332.

Behavior

Active application (planned_units() > 0)

• Normal events use the existing decorated reconciliation path.
• PostgreSQL database-created and endpoints-changed retain the migration rerun flag.
• update-status and rotate-secret-key delegate to the inherited behavior.

Local teardown (planned_units() == 0)

• Normal and migration events return through _teardown() before block_if_invalid_data or normal GARM state construction.
• update-status returns before the inherited ingress refresh.
• rotate-secret-key fails before state construction, secret reset, or restart.
• restart() retains a second defensive planned-unit check.

The aliases are an adapter for the released paas-charm 1.12.x API. _validate_paas_charm_hook_contract() checks the expected base hooks before paas-charm registers observers, so an incompatible dependency fails instead of silently bypassing the gate.

Test coverage

• Scenario coverage for active and teardown PostgreSQL migration events.
• Scenario coverage for the teardown action rejection.
• Guard-only teardown coverage for secret-storage changed/departed and ingress ready/revoked.
• Existing GARM-owned teardown cases remain covered.

Validation

• tox -c tox.toml -e unit — 323 passed
• tox -c tox.toml -e lint — passed
• tox -c tox.toml -e complexity — passed
• tox -c tox.toml -e static — passed
• tox -c tox.toml -e coverage-report — passed
• git diff --check — passed

Scope and follow-ups

This change protects GARM and paas-charm normal reconciliation. It does not change runner cleanup, secret migration, raw relation teardown, or callbacks registered directly by third-party helper libraries. Those require separate lifecycle review and removal integration coverage.

[yanksyoon]

Lifecycle funnel safety audit

I audited the current funnel against the unpatched origin/main behavior using the pinned paas-charm==1.12.2 and ops==3.8.0 sources.

Safety criterion: when planned_units() == 0, the hook must not construct normal charm state, read teardown-time relation data, or run normal reconciliation. Active behavior must remain equivalent to the original implementation.

block_if_invalid_data calls _create_charm_state() before the handler body and catches only CharmConfigInvalidError and RelationDataError (paas_charm/charm_utils.py:36-71). Other errors such as ModelError, KeyError, and IndexError fail the hook.

GARM-owned hooks

Hook	Original operation	Patched operation	Active operation preserved?	Decision
install	Decorated _reconcile() then restart().	Gate first, then decorated normal reconcile while active.	Yes.	Safe.
leader-elected	Decorated _reconcile() then restart().	Same active path; teardown returns through _teardown().	Yes.	Safe.
update-status GARM observer	Decorated _reconcile() then restart(); base update-status observer also ran.	GARM observer uses the router; base observer has its own active/teardown guard.	Yes.	Safe for GARM/base paths.
garm-configurator joined/changed	Decorated _reconcile() then restart().	Same active path; teardown returns.	Yes.	Safe.
garm-configurator departed/broken	Normal restart; departure may also converge orphaned scalesets.	Active restart preserved; teardown skips normal reconciliation.	Active: yes. Teardown orphan cleanup: intentionally skipped.	Safe for gate-only scope; cleanup is separate.
debug-ssh joined/changed	Decorated _reconcile() then restart().	Same active path; teardown returns.	Yes.	Safe.
debug-ssh departed/broken	Normal restart; departure re-renders current template.	Active restart preserved; teardown skips normal reconciliation.	Active: yes. Teardown template cleanup: intentionally skipped.	Safe for gate-only scope.

Inherited paas-charm hooks

The base class already registers these observers in paas_charm/charm.py:195-225. The subclass overrides the method names used by those registrations; no duplicate observer registration is added.

Hook	Unpatched operation	Teardown risk before patch	Patched active behavior	Decision
config-changed	Build state, then restart().	State/config/relation reads occur before restart().	Routes through GARM normal reconcile.	Safe.
secret-changed	Build state, then restart().	Secret and relation reads occur before restart().	Routes through GARM normal reconcile.	Safe.
secret-storage-relation-changed	Build state, then restart().	Peer state is read before the restart gate.	Routes through GARM normal reconcile.	Safe.
secret-storage-relation-departed	Build state, then restart().	Peer relation may be departing before state construction.	Routes through GARM normal reconcile.	Safe.
PostgreSQL database-created	Build state, then restart(rerun_migrations=True).	PostgreSQL state is read before the gate.	Uses a separate gated migration path.	Safe; migration flag preserved.
PostgreSQL endpoints-changed	Same as database-created.	Same PostgreSQL state-read risk.	Uses the same gated migration path.	Safe; migration flag preserved.
PostgreSQL relation-broken	Build state, then restart().	The relation is no longer valid. The data-platform helper documents that fetch_relation_data() cannot be used in relation-broken; its legacy PostgreSQL fetch path assumes a relation exists (data_interfaces.py:1696-1712, 2649-2653).	Gate runs before the decorator.	Necessary and safe.
Ingress ready	Build state, then restart().	Late ingress event can arrive after relation/state teardown starts.	Routes through GARM normal reconcile.	Safe for the base callback.
Ingress revoked	Build state, then restart().	Same late relation/state risk.	Routes through GARM normal reconcile.	Safe for the base callback.
App pebble-ready	Build state, then restart().	A queued event may arrive after teardown starts.	Routes through GARM normal reconcile.	Safe for the base callback.
rotate-secret-key action	Build state, reset the peer secret, report success, then restart.	It could mutate peer state during teardown.	Rejects before calling the base decorator; delegates to base while active.	Safe if teardown must reject rotation.
update-status base observer	Build state, retry failed migrations, then call _ingress._publish_auto_data().	Ingress refresh can read/write relation data even if restart() returns.	Calls base only while active.	Necessary and safe.

Evidence and retained operations

• Base observer registration: paas_charm/charm.py:195-225.
• Decorator behavior: paas_charm/charm_utils.py:36-71.
• Framework state reconstruction: paas_charm/charm.py:748-784.
• Config validation errors: paas_charm/charm_state.py:126-132 and paas_charm/charm.py:466-491.
• PostgreSQL relation conversion: paas_charm/databases.py:42-76.
• PostgreSQL relation-broken has no known remote units: ops/charm.py:775-789.
• planned_units() is local goal state current at hook start: ops/model.py:459-479.
• Current GARM router and overrides: charms/garm/src/charm.py:155-257.
• Current GARM restart sequence is unchanged while active; the only new behavior is the early teardown return: charms/garm/src/charm.py:348-421.
• Scenario tests patch GarmCharm._create_charm_state to fail and cover teardown events; the current suite passes 257 tests.

Explicit limits

This patch protects GARM-owned and paas-charm-owned callbacks. It does not intercept direct callbacks registered on helper objects:

• Traefik ingress: charms/garm/lib/charms/traefik_k8s/v2/ingress.py:386-392.
• Prometheus metrics: charms/garm/lib/charms/prometheus_k8s/v0/prometheus_scrape.py:1594-1623.
• Loki logging: charms/garm/lib/charms/loki_k8s/v1/loki_push_api.py:2383-2400.
• Grafana dashboards: charms/garm/lib/charms/grafana_k8s/v0/grafana_dashboard.py:1187-1198.
• Data-platform PostgreSQL raw relation callbacks: charms/garm/lib/charms/data_platform_libs/v0/data_interfaces.py:1801-1826.

The patch also does not add a raw postgresql-relation-departed handler. Early runner cleanup and helper-library lifecycle changes remain separate follow-up work.

[yhaliaw]

Major changes needed

---

🤝 Human review with AI assistance.