Skip to content
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.

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

Check if relation is breaking during relation-broken event #50

Merged
merged 9 commits into from
Jul 14, 2023
43 changes: 28 additions & 15 deletions src/abstract_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import tenacity

import container
import lifecycle
import relations.database_provides
import relations.database_requires
import workload
Expand All @@ -24,13 +25,26 @@ class MySQLRouterCharm(ops.CharmBase, abc.ABC):

def __init__(self, *args) -> None:
super().__init__(*args)
# Instantiate before registering other event observers
self._unit_lifecycle = lifecycle.Unit(
self, subordinated_relation_endpoint_names=self._subordinate_relation_endpoint_names
)

self._workload_type = workload.Workload
self._authenticated_workload_type = workload.AuthenticatedWorkload
self._database_requires = relations.database_requires.RelationEndpoint(self)
self._database_provides = relations.database_provides.RelationEndpoint(self)
self.framework.observe(self.on.start, self._on_start)
self.framework.observe(self.on.leader_elected, self._on_leader_elected)

@property
@abc.abstractmethod
def _subordinate_relation_endpoint_names(self) -> typing.Optional[typing.Iterable[str]]:
"""Subordinate relation endpoint names

Does NOT include relations where charm is principal
"""

@property
def _tls_certificate_saved(self) -> bool:
"""Whether a TLS certificate is available to use"""
Expand Down Expand Up @@ -99,7 +113,7 @@ def _determine_unit_status(self, *, event) -> ops.StatusBase:

def set_status(self, *, event) -> None:
"""Set charm status."""
if self.unit.is_leader():
if self._unit_lifecycle.authorized_leader:
self.app.status = self._determine_app_status(event=event)
logger.debug(f"Set app status to {self.app.status}")
self.unit.status = self._determine_unit_status(event=event)
Expand Down Expand Up @@ -137,24 +151,23 @@ def reconcile_database_relations(self, event=None) -> None:
workload_ = self.get_workload(event=event)
logger.debug(
"State of reconcile "
f"{self.unit.is_leader()=}, "
f"{self._unit_lifecycle.authorized_leader=}, "
f"{isinstance(workload_, workload.AuthenticatedWorkload)=}, "
f"{workload_.container_ready=}, "
f"{self._database_requires.is_relation_breaking(event)=}"
)
if self.unit.is_leader() and self._database_requires.is_relation_breaking(event):
self._database_provides.delete_all_databags()
elif (
self.unit.is_leader()
and isinstance(workload_, workload.AuthenticatedWorkload)
and workload_.container_ready
):
self._database_provides.reconcile_users(
event=event,
router_read_write_endpoint=self._read_write_endpoint,
router_read_only_endpoint=self._read_only_endpoint,
shell=workload_.shell,
)
if self._unit_lifecycle.authorized_leader:
if self._database_requires.is_relation_breaking(event):
self._database_provides.delete_all_databags()
elif (
isinstance(workload_, workload.AuthenticatedWorkload) and workload_.container_ready
):
self._database_provides.reconcile_users(
event=event,
router_read_write_endpoint=self._read_write_endpoint,
router_read_only_endpoint=self._read_only_endpoint,
shell=workload_.shell,
)
if isinstance(workload_, workload.AuthenticatedWorkload) and workload_.container_ready:
workload_.enable(tls=self._tls_certificate_saved, unit_name=self.unit.name)
elif workload_.container_ready:
Expand Down
149 changes: 149 additions & 0 deletions src/lifecycle.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

"""Charm lifecycle

https://juju.is/docs/sdk/a-charms-life
"""
import enum
import logging
import typing

import ops

logger = logging.getLogger(__name__)


class _UnitTearingDownAndAppActive(enum.IntEnum):
"""Unit is tearing down and 1+ other units are NOT tearing down"""

FALSE = 0
TRUE = 1
UNKNOWN = 2

def __bool__(self):
return self is self.TRUE


class Unit(ops.Object):
"""Unit lifecycle

NOTE: Instantiate this object before registering event observers.
"""

_stored = ops.StoredState()

def __init__(
self,
charm: ops.CharmBase,
subordinated_relation_endpoint_names: typing.Optional[typing.Iterable[str]],
):
"""Unit lifecycle

NOTE: Instantiate this object before registering event observers.

Args:
charm: Charm
subordinated_relation_endpoint_names: Endpoint names for relations between subordinate
and principal charms where charm is subordinate

Does NOT include relations where charm is principal
"""
super().__init__(charm, str(type(self)))
if subordinated_relation_endpoint_names is None:
subordinated_relation_endpoint_names = ()
self._charm = charm
for relation_endpoint in self.model.relations:
if relation_endpoint in subordinated_relation_endpoint_names:
self.framework.observe(
self._charm.on[relation_endpoint].relation_departed,
self._on_subordinate_relation_departed,
)
self.framework.observe(
self._charm.on[relation_endpoint].relation_broken,
self._on_subordinate_relation_broken,
)
else:
self.framework.observe(
self._charm.on[relation_endpoint].relation_departed, self._on_relation_departed
)

@property
def _unit_tearing_down_and_app_active(self) -> _UnitTearingDownAndAppActive:
"""Whether unit is tearing down and 1+ other units are NOT tearing down"""
try:
return _UnitTearingDownAndAppActive(self._stored.unit_tearing_down_and_app_active)
except AttributeError:
return _UnitTearingDownAndAppActive.FALSE

@_unit_tearing_down_and_app_active.setter
def _unit_tearing_down_and_app_active(self, enum_member: _UnitTearingDownAndAppActive) -> None:
self._stored.unit_tearing_down_and_app_active = enum_member.value

def _on_relation_departed(self, event: ops.RelationDepartedEvent) -> None:
if event.departing_unit == self._charm.unit:
self._unit_tearing_down_and_app_active = _UnitTearingDownAndAppActive.TRUE

def _on_subordinate_relation_departed(self, _) -> None:
if self._unit_tearing_down_and_app_active:
return
# We cannot use the output of `goal-state` until we get the *-relation-broken event.
# During *-relation-departed, it is not guaranteed that all units that are tearing down
# report "dying" status. It is guaranteed during the *-relation-broken event.
self._unit_tearing_down_and_app_active = _UnitTearingDownAndAppActive.UNKNOWN

def _on_subordinate_relation_broken(self, event: ops.RelationBrokenEvent) -> None:
if self._unit_tearing_down_and_app_active:
return
# Workaround for subordinate charms: https://bugs.launchpad.net/juju/+bug/2025676
# A subordinate unit will get a *-relation-departed event where
# `event.departing_unit == self._charm.unit` is `True` in any of these situations:
# 1. `juju remove-relation` with principal charm or `juju remove-application` for
# subordinate charm
# 2. `juju remove-application` for principal charm
# 3. App has 1 unit, then `juju remove-unit` for principal charm
# 4. App has 2+ units, then `juju remove-unit` for principal charm

# In situations #1-3, all units of the subordinate charm are tearing down.
# In situation #4, there will be 1+ units that are not tearing down.
# In situations #1-3, the current leader will not be replaced by another leader after
# it tears down, so it should act as a leader (e.g. handle user cleanup on
# *-relation-broken) now.
output = self._charm.model._backend._run("goal-state", return_output=True, use_json=True)
principal_unit_statuses = set()
for unit_or_app, info in output["relations"].items():
unit_or_app: str
info: dict
# Filter out app info & units for other apps
if unit_or_app.startswith(f"{event.relation.app}/"):
principal_unit_statuses.add(info["status"])

# In situation #1, NO `principal_unit_statuses` will be "dying"
# In situation #2 and #3, all `principal_unit_statuses` will be "dying"
if "dying" in principal_unit_statuses and principal_unit_statuses != {"dying"}:
# Situation #4
self._unit_tearing_down_and_app_active = _UnitTearingDownAndAppActive.TRUE

@property
def authorized_leader(self) -> bool:
"""Whether unit is authorized to act as leader

Returns `False` if unit is tearing down and will be replaced by another leader

For subordinate charms, this should not be accessed during *-relation-departed.

Teardown event sequence:
*-relation-departed -> *-relation-broken
stop
remove

Workaround for https://bugs.launchpad.net/juju/+bug/1979811
(Unit receives *-relation-broken event when relation still exists [for other units])
"""
if not self._charm.unit.is_leader():
return False
if self._unit_tearing_down_and_app_active is _UnitTearingDownAndAppActive.UNKNOWN:
logger.warning(
f"{type(self)}.authorized_leader should not be accessed during *-relation-departed for subordinate relations"
)
return self._unit_tearing_down_and_app_active is _UnitTearingDownAndAppActive.FALSE
8 changes: 8 additions & 0 deletions src/machine_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"""MySQL Router machine charm"""

import logging
import typing

import ops

Expand All @@ -32,6 +33,13 @@ def __init__(self, *args) -> None:
self.framework.observe(self.on.install, self._on_install)
self.framework.observe(self.on.remove, self._on_remove)

@property
def _subordinate_relation_endpoint_names(self) -> typing.Optional[typing.Iterable[str]]:
return (
"database",
"shared-db", # DEPRECATED shared-db
)

@property
def _container(self) -> snap.Snap:
return snap.Snap()
Expand Down
5 changes: 4 additions & 1 deletion src/relations/database_provides.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@


class _RelationBreaking(Exception):
"""Relation will be broken after the current event is handled"""
"""Relation will be broken for this unit after the current event is handled

If this unit is tearing down, the relation could still exist for other units.
"""


class _UnsupportedExtraUserRole(status_exception.StatusException):
Expand Down
9 changes: 7 additions & 2 deletions src/relations/database_requires.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,21 @@


class _MissingRelation(status_exception.StatusException):
"""Relation to MySQL charm does (or will) not exist"""
"""Relation to MySQL charm does (or will) not exist for this unit

If this unit is tearing down, the relation could still exist for other units.
"""

def __init__(self, *, endpoint_name: str) -> None:
super().__init__(ops.BlockedStatus(f"Missing relation: {endpoint_name}"))


class _RelationBreaking(_MissingRelation):
"""Relation to MySQL charm will be broken after the current event is handled
"""Relation to MySQL charm will be broken for this unit after the current event is handled

Relation currently exists

If this unit is tearing down, the relation could still exist for other units.
"""


Expand Down
5 changes: 4 additions & 1 deletion src/relations/deprecated_shared_db_database_provides.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ def __init__(self, relation: ops.Relation) -> None:


class _RelationBreaking(Exception):
"""Relation will be broken after the current event is handled"""
"""Relation will be broken for this unit after the current event is handled

If this unit is tearing down, the relation could still exist for other units.
"""


class _Relation:
Expand Down
Loading