Skip to content

refactor(garm): make teardown the shared cleanup coordinator - #334

Draft
yanksyoon wants to merge 1 commit into
feat/garm-teardown-lifecyclefrom
feat/garm-teardown-coordinator
Draft

refactor(garm): make teardown the shared cleanup coordinator#334
yanksyoon wants to merge 1 commit into
feat/garm-teardown-lifecyclefrom
feat/garm-teardown-coordinator

Conversation

@yanksyoon

@yanksyoon yanksyoon commented Aug 27, 2026

Copy link
Copy Markdown
Member

Summary

This child PR is stacked on #333 and refactors the existing removal cleanup into the shared _teardown() orchestrator.

  • Rename the removal observer target from _on_remove() to _teardown().
  • Use the same orchestrator for local GARM teardown routed by _reconcile(), PostgreSQL relation-departed, stop, and remove.
  • Keep cleanup independent of PostgreSQL relation data and event application/unit fields.
  • Keep active behavior unchanged when planned_units() > 0.

Relation-independent teardown

_teardown() uses only:

  • local planned_units() and leadership;
  • the application-owned GARM admin secret;
  • the local GARM API; and
  • the existing GarmResourceCleanup API coordinator.

The old fallback that called _get_postgresql_config() when GARM was unreachable has been removed. Late teardown now logs an unconfirmed result and returns instead of reading a relation that may already be broken.

Pre-break relation-departed failures remain strict and raise GarmCleanupError so Juju can retry. Late stop, remove, and other teardown attempts tolerate unavailable API/database/credentials.

Idempotency

  • _teardown_claimed prevents duplicate cleanup attempts from multiple observers in one hook process, notably the two update-status observers.
  • A new hook process resets the claim and re-observes current GARM API state, so a killed hook can retry safely.
  • GarmResourceCleanup disables resources before deletion, polls asynchronous runner removal, and treats already-missing resources as success.
  • No force-removal or GitHub authorization bypass is enabled.

Validation

  • tox -c tox.toml -e unit — 266 passed
  • Focused teardown tests — 7 passed
  • Ruff check and formatting — passed
  • Complexity checks — passed
  • Pyright — passed
  • git diff --check — passed

Review focus

  • _teardown() is intentionally global and leader-only, matching the existing _on_remove() behavior.
  • The child PR does not add persistent-key migration or direct COS-helper lifecycle changes; those remain separate work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant