Skip to content

Commit

Permalink
[DPE-2613] fix rel tests (#263)
Browse files Browse the repository at this point in the history
## 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 <[email protected]>
  • Loading branch information
MiaAltieri and delgod authored Oct 11, 2023
1 parent 15339e3 commit 41a2c13
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 3 deletions.
30 changes: 27 additions & 3 deletions lib/charms/mongodb/v0/mongodb_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -62,17 +62,20 @@ 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
)
self.framework.observe(
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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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", "{}"))
Expand Down
35 changes: 35 additions & 0 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down

0 comments on commit 41a2c13

Please sign in to comment.