From 41a2c13987f06d04f7af20e20a7ee7fca81e8754 Mon Sep 17 00:00:00 2001 From: Mia Altieri <32723809+MiaAltieri@users.noreply.github.com> Date: Wed, 11 Oct 2023 09:59:45 +0200 Subject: [PATCH] [DPE-2613] fix rel tests (#263) ## Issue After updating to Juju 3.1 the charm failed on its relation tests #247 This was due to when apps scale down in 3.1, they receive relation broken events. Thereby attempts to remove users were made when relations were actually not removed, but instead a unit was being removed. ## Solution Add a check in `relation-departed`. In this function we can tell if we are receiving the `relation-broken` event due to scale down or relation removal. ## more Once this PR is accepted, we will make another one for branch `5/edge` --------- Co-authored-by: Mykola Marzhan <303592+delgod@users.noreply.github.com> --- lib/charms/mongodb/v0/mongodb_provider.py | 30 +++++++++++++++++-- src/charm.py | 35 +++++++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/lib/charms/mongodb/v0/mongodb_provider.py b/lib/charms/mongodb/v0/mongodb_provider.py index 8ad03639a..972f4b6e5 100644 --- a/lib/charms/mongodb/v0/mongodb_provider.py +++ b/lib/charms/mongodb/v0/mongodb_provider.py @@ -29,7 +29,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 4 +LIBPATCH = 5 logger = logging.getLogger(__name__) REL_NAME = "database" @@ -62,8 +62,13 @@ def __init__(self, charm: CharmBase, substrate="k8s", relation_name: str = "data """ self.relation_name = relation_name self.substrate = substrate + self.charm = charm super().__init__(charm, self.relation_name) + self.framework.observe( + charm.on[self.relation_name].relation_departed, + self.charm.check_relation_broken_or_scale_down, + ) self.framework.observe( charm.on[self.relation_name].relation_broken, self._on_relation_event ) @@ -71,8 +76,6 @@ def __init__(self, charm: CharmBase, substrate="k8s", relation_name: str = "data charm.on[self.relation_name].relation_changed, self._on_relation_event ) - self.charm = charm - # Charm events defined in the database provides charm library. self.database_provides = DatabaseProvides(self.charm, relation_name=self.relation_name) self.framework.observe( @@ -110,7 +113,25 @@ def _on_relation_event(self, event): departed_relation_id = None if type(event) is RelationBrokenEvent: + # Only relation_deparated events can check if scaling down departed_relation_id = event.relation.id + if not self.charm.has_departed_run(departed_relation_id): + logger.info( + "Deferring, must wait for relation departed hook to decide if relation should be removed." + ) + event.defer() + return + + # check if were scaling down and add a log message + if self.charm.is_scaling_down(departed_relation_id): + logger.info( + "Relation broken event occurring due to scale down, do not proceed to remove users." + ) + return + + logger.info( + "Relation broken event occurring due to relation removal, proceed to remove user." + ) try: self.oversee_users(departed_relation_id, event) @@ -186,6 +207,9 @@ def _diff(self, event: RelationChangedEvent) -> Diff: a Diff instance containing the added, deleted and changed keys from the event relation databag. """ + if not isinstance(event, RelationChangedEvent): + logger.info("Cannot compute diff of event type: %s", type(event)) + return # TODO import marvelous unit tests in a future PR # Retrieve the old data from the data key in the application relation databag. old_data = json.loads(event.relation.data[self.charm.model.app].get("data", "{}")) diff --git a/src/charm.py b/src/charm.py index fba4b6c4b..4c2999a7e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1407,6 +1407,41 @@ def _juju_secret_remove(self, scope: Scopes, key: str) -> None: secret.set_content(secret_cache) logging.debug(f"Secret {scope}:{key}") + def check_relation_broken_or_scale_down(self, event: RelationDepartedEvent) -> None: + """Checks relation departed event is the result of removed relation or scale down. + + Relation departed and relation broken events occur during scaling down or during relation + removal, only relation departed events have access to metadata to determine which case. + """ + self.set_scaling_down(event) + + if self.is_scaling_down(event.relation.id): + logger.info( + "Scaling down the application, no need to process removed relation in broken hook." + ) + + def is_scaling_down(self, rel_id: int) -> bool: + """Returns True if the application is scaling down.""" + rel_departed_key = self._generate_relation_departed_key(rel_id) + return json.loads(self.unit_peer_data[rel_departed_key]) + + def has_departed_run(self, rel_id: int) -> bool: + """Returns True if the relation departed event has run.""" + rel_departed_key = self._generate_relation_departed_key(rel_id) + return rel_departed_key in self.unit_peer_data + + def set_scaling_down(self, event: RelationDepartedEvent) -> None: + """Sets whether or not the current unit is scaling down.""" + # check if relation departed is due to current unit being removed. (i.e. scaling down the + # application.) + rel_departed_key = self._generate_relation_departed_key(event.relation.id) + self.unit_peer_data[rel_departed_key] = json.dumps(event.departing_unit == self.unit) + + @staticmethod + def _generate_relation_departed_key(rel_id: int) -> str: + """Generates the relation departed key for a specified relation id.""" + return f"relation_{rel_id}_departed" + # END: helper functions