Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
yanksyoon wants to merge 1 commit into from
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 100 additions & 66 deletions charms/garm/src/charm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
resolve_entity,
)
from entity_reconciler import EntityReconciler
from garm_api import GarmApiClient, GarmApiError, GarmAuthenticatedClient, GarmConnectionError
from garm_api import GarmApiClient, GarmApiError, GarmAuthenticatedClient
from garm_template import CharmedTemplateError
from garm_template import apply_charmed_template as _apply_garm_template
from github_reconciler import (
Expand Down Expand Up @@ -130,6 +130,10 @@ def __init__(self, *args: typing.Any) -> None:
args: Passed through to CharmBase.
"""
super().__init__(*args)
# Multiple observers can deliver the same hook (notably update-status). Claim one
# teardown attempt per hook process; a new hook instance re-observes GARM state so
# a killed process can safely retry.
self._teardown_claimed = False
for event in (
self.on.install,
self.on.leader_elected,
Expand All @@ -150,7 +154,9 @@ def __init__(self, *args: typing.Any) -> None:
self.framework.observe(event, self._reconcile)

self.framework.observe(self.on.get_credentials_action, self._on_get_credentials_action)
self.framework.observe(self.on.remove, self._on_remove)
self.framework.observe(self.on["postgresql"].relation_departed, self._teardown)
self.framework.observe(self.on.stop, self._teardown)
self.framework.observe(self.on.remove, self._teardown)

def _is_tearing_down(self) -> bool:
"""Return whether Juju plans no remaining units for the local application."""
Expand All @@ -170,13 +176,6 @@ def _reconcile_with_migrations(self, event: ops.EventBase) -> None:
return
self._normal_reconcile_with_migrations(event)

def _teardown(self, event: ops.EventBase) -> None:
"""Handle a local teardown event without normal reconciliation."""
logger.info(
"Skipping normal GARM reconciliation for %s during local teardown",
event.handle.kind,
)

@block_if_invalid_data
def _normal_reconcile(self, _: ops.EventBase) -> None:
"""Reconcile active GARM charm state."""
Expand Down Expand Up @@ -212,7 +211,7 @@ def _route_reconcile_with_migrations(self, event: ops.EventBase) -> None:
def _on_update_status(self, event: ops.HookEvent) -> None:
"""Run the framework update-status handler only while active."""
if self._is_tearing_down():
logger.info("Skipping update-status handling during local teardown")
self._teardown(event)
return
super()._on_update_status(event)

Expand All @@ -223,79 +222,114 @@ def _on_rotate_secret_key_action(self, event: ops.ActionEvent) -> None:
return
super()._on_rotate_secret_key_action(event)

def _on_remove(self, _: ops.RemoveEvent) -> None:
"""Drain GARM resources before Juju removes the application."""
if not self.unit.is_leader():
logger.info("Skipping GARM removal cleanup on a non-leader unit")
return
if self.app.planned_units() > 0:
logger.info(
"Skipping GARM removal cleanup while the application still has planned units"
)
return
def _teardown_failure(
self,
message: str,
*,
strict: bool,
cause: BaseException | None = None,
) -> None:
"""Raise an early cleanup error or record an unconfirmed late attempt."""
if strict:
logger.error(message)
if cause is None:
raise GarmCleanupError(message)
raise GarmCleanupError(message) from cause
logger.warning("Late GARM teardown is unconfirmed: %s", message)

initialization_client = GarmApiClient(GARM_LOCAL_API_BASE_URL)
def _teardown_credentials(self, *, strict: bool) -> tuple[str, str] | None:
"""Return local admin credentials without consulting any relation."""
try:
if not initialization_client.is_initialized():
logger.info(
"GARM has not completed first-run; no resources require removal cleanup"
)
return
except GarmConnectionError as exc:
if self._get_postgresql_config() is None:
logger.info(
"PostgreSQL is not configured and GARM is unreachable; "
"GARM cannot have been initialized, so no cleanup is required"
)
return
message = (
f"Cannot determine whether GARM was initialized: {exc}. "
"Operator action: restore GARM/API availability, then retry "
"Juju application removal."
)
logger.error(message)
raise GarmCleanupError(message) from exc
except GarmApiError as exc:
message = (
f"Cannot determine whether GARM was initialized: {exc}. "
"Operator action: restore GARM/API availability, then retry "
"Juju application removal."
admin_creds = self._get_admin_credentials()
except (ops.ModelError, ops.SecretNotFoundError) as exc:
self._teardown_failure(
"GARM admin credentials are unavailable; teardown cannot be confirmed. "
"Operator action: restore the labelled admin credentials secret, "
"then retry teardown.",
strict=strict,
cause=exc,
)
logger.error(message)
raise GarmCleanupError(message) from exc

admin_creds = self._get_admin_credentials()
return None
if not admin_creds:
message = (
"GARM admin credentials are unavailable; refusing removal. "
self._teardown_failure(
"GARM admin credentials are unavailable; teardown cannot be confirmed. "
"Operator action: restore the labelled admin credentials secret, "
"then retry Juju application removal."
"then retry teardown.",
strict=strict,
)
logger.error(message)
raise GarmCleanupError(message)
return None

username = admin_creds.get("username")
password = admin_creds.get("password")
if not username or not password:
message = (
"GARM admin credentials are incomplete; refusing removal. "
self._teardown_failure(
"GARM admin credentials are incomplete; teardown cannot be confirmed. "
"Operator action: restore username and password in the labelled "
"admin credentials secret, then retry Juju application removal."
"admin credentials secret, then retry teardown.",
strict=strict,
)
logger.error(message)
raise GarmCleanupError(message)
return None
return username, password

def _teardown(self, event: ops.EventBase) -> None:
"""Idempotently drain GARM resources for a local teardown event.

The operation intentionally re-observes GARM's current API state on every hook.
``GarmResourceCleanup`` disables resources before deleting them, polls asynchronous
runner removal, and treats already-missing resources as success, so repeated calls
remain safe after retries or a hook process restart. No relation-backed data is read.
"""
if not self.unit.is_leader():
logger.info("Skipping GARM teardown on a non-leader unit")
return
if not self._is_tearing_down():
logger.info("Skipping GARM teardown while the application still has planned units")
return
if self._teardown_claimed:
logger.info("GARM teardown already claimed in this hook process")
return
self._teardown_claimed = True

strict = isinstance(event, ops.RelationDepartedEvent)
logger.info("Handling GARM teardown for %s", event.handle.kind)
try:
initialized = GarmApiClient(GARM_LOCAL_API_BASE_URL).is_initialized()
except GarmApiError as exc:
self._teardown_failure(
f"Cannot determine whether GARM was initialized: {exc}. "
"Operator action: restore GARM/API availability, then retry teardown.",
strict=strict,
cause=exc,
)
return

if not initialized:
logger.info("GARM has not completed first-run; teardown is already complete")
return

credentials = self._teardown_credentials(strict=strict)
if credentials is None:
return
username, password = credentials

base_url = GARM_LOCAL_API_BASE_URL
try:
auth_client = GarmAuthenticatedClient.from_login(base_url, username, password)
auth_client = GarmAuthenticatedClient.from_login(
GARM_LOCAL_API_BASE_URL, username, password
)
GarmResourceCleanup(auth_client).run()
except GarmCleanupError as exc:
self._teardown_failure(
f"GARM teardown did not complete: {exc}",
strict=strict,
cause=exc,
)
except GarmApiError as exc:
logger.error(
"GARM removal cleanup failed: %s. Operator action: resolve the "
"reported GARM/API/runner issue, then retry Juju application removal.",
exc,
self._teardown_failure(
f"GARM teardown failed: {exc}. "
"Operator action: restore GARM/API/runner availability, then retry teardown.",
strict=strict,
cause=exc,
)
raise

def _on_get_credentials_action(self, event: ops.ActionEvent) -> None:
"""Return the GARM admin credentials to the operator.
Expand Down
Loading
Loading