From 2f6f9cd3e3fd3df5c2a96bdc458f01f8add3ddf7 Mon Sep 17 00:00:00 2001 From: Mia Altieri <32723809+MiaAltieri@users.noreply.github.com> Date: Wed, 25 Oct 2023 16:25:33 +0200 Subject: [PATCH] [DPE-2613] correct rel tests (5/edge) (#272) ## Issue relation tests are failing ## Solution port over work from #263 --- lib/charms/mongodb/v0/mongodb_backups.py | 2 +- lib/charms/mongodb/v0/mongodb_provider.py | 28 ++++++++++++++++-- src/charm.py | 35 +++++++++++++++++++++++ tests/integration/ha_tests/test_ha.py | 2 +- 4 files changed, 63 insertions(+), 4 deletions(-) diff --git a/lib/charms/mongodb/v0/mongodb_backups.py b/lib/charms/mongodb/v0/mongodb_backups.py index 48f1be0eb..972e6884f 100644 --- a/lib/charms/mongodb/v0/mongodb_backups.py +++ b/lib/charms/mongodb/v0/mongodb_backups.py @@ -636,7 +636,7 @@ def _get_backup_restore_operation_result(self, current_pbm_status, previous_pbm_ to contain the operation type (backup/restore) and the backup id. """ if ( - type(current_pbm_status) is type(previous_pbm_status) + isinstance(current_pbm_status, type(previous_pbm_status)) and current_pbm_status.message == previous_pbm_status.message ): return f"Operation is still in progress: '{current_pbm_status.message}'" diff --git a/lib/charms/mongodb/v0/mongodb_provider.py b/lib/charms/mongodb/v0/mongodb_provider.py index b479b1926..3ca717b11 100644 --- a/lib/charms/mongodb/v0/mongodb_provider.py +++ b/lib/charms/mongodb/v0/mongodb_provider.py @@ -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 cc349f48e..352f67ff8 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1119,6 +1119,41 @@ def _peer_data(self, scope: Scopes): scope_obj = self._scope_obj(scope) return self._peers.data[scope_obj] + 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 diff --git a/tests/integration/ha_tests/test_ha.py b/tests/integration/ha_tests/test_ha.py index 600f5bafc..36c2d937b 100644 --- a/tests/integration/ha_tests/test_ha.py +++ b/tests/integration/ha_tests/test_ha.py @@ -117,7 +117,7 @@ async def test_storage_re_use(ops_test, continuous_writes): assert await helpers.reused_storage( ops_test, new_unit.public_address, removal_time - ), "attached storage not properly re-used by MongoDB." + ), "attached storage not properly reused by MongoDB." # verify that the no writes were skipped total_expected_writes = await helpers.stop_continous_writes(ops_test)