Skip to content

Commit

Permalink
Recover from hook errors when creating/deleting MySQL users (#165)
Browse files Browse the repository at this point in the history
Fixes #157

Previously, the charm assumed that if a user was created or deleted that
the databag would be updated accordingly. However, if a user is created
or deleted and the event/hook fails, the user creation/deletion will go
through for MySQL but the databag will not be updated (since the event
failed). When the event is retried, the charm tried to create a user
that already exists or delete a user that doesn't exist
  • Loading branch information
carlcsaposs-canonical authored Nov 28, 2023
1 parent fdde410 commit fad69b1
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 38 deletions.
20 changes: 0 additions & 20 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 8 additions & 4 deletions src/mysql_shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,15 @@ def remove_router_from_cluster_metadata(self, router_id: str) -> None:
)
logger.debug(f"Removed {router_id=} from cluster metadata")

def delete_user(self, username: str) -> None:
def delete_user(self, username: str, *, must_exist=True) -> None:
"""Delete user."""
logger.debug(f"Deleting {username=}")
self._run_sql([f"DROP USER `{username}`"])
logger.debug(f"Deleted {username=}")
logger.debug(f"Deleting {username=} {must_exist=}")
if must_exist:
statement = f"DROP USER `{username}`"
else:
statement = f"DROP USER IF EXISTS `{username}`"
self._run_sql([statement])
logger.debug(f"Deleted {username=} {must_exist=}")

def is_router_in_cluster_set(self, router_id: str) -> bool:
"""Check if MySQL Router is part of InnoDB ClusterSet."""
Expand Down
39 changes: 25 additions & 14 deletions src/relations/database_provides.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,14 @@ def create_database_and_user(
) -> None:
"""Create database & user and update databag."""
username = self._get_username(shell.username)
# Delete user if exists
# (If the user was previously created by this charm—but the hook failed—the user will
# persist in MySQL but will not persist in the databag. Therefore, we lose the user's
# password and need to re-create the user.)
logger.debug("Deleting user if exists before creating user")
shell.delete_user(username, must_exist=False)
logger.debug("Deleted user if exists before creating user")

password = shell.create_application_database_and_user(
username=username, database=self._database
)
Expand All @@ -115,11 +123,11 @@ def create_database_and_user(
)


class _UserNotCreated(Exception):
class _UserNotShared(Exception):
"""Database & user has not been provided to related application charm"""


class _RelationWithCreatedUser(_Relation):
class _RelationWithSharedUser(_Relation):
"""Related application charm that has been provided with a database & user"""

def __init__(
Expand All @@ -130,7 +138,7 @@ def __init__(
self._local_databag = self._interface.fetch_my_relation_data([relation.id])[relation.id]
for key in ("database", "username", "password", "endpoints", "read-only-endpoints"):
if key not in self._local_databag:
raise _UserNotCreated
raise _UserNotShared

def delete_databag(self) -> None:
"""Remove connection information from databag."""
Expand All @@ -141,7 +149,10 @@ def delete_databag(self) -> None:
def delete_user(self, *, shell: mysql_shell.Shell) -> None:
"""Delete user and update databag."""
self.delete_databag()
shell.delete_user(self._get_username(shell.username))
# Delete user if exists
# (If the user was previously deleted by this charm—but the hook failed—the user will be
# deleted in MySQL but will persist in the databag.)
shell.delete_user(self._get_username(shell.username), must_exist=False)


class RelationEndpoint:
Expand All @@ -157,16 +168,16 @@ def __init__(self, charm_: "abstract_charm.MySQLRouterCharm") -> None:

@property
# TODO python3.10 min version: Use `list` instead of `typing.List`
def _created_users(self) -> typing.List[_RelationWithCreatedUser]:
created_users = []
def _shared_users(self) -> typing.List[_RelationWithSharedUser]:
shared_users = []
for relation in self._interface.relations:
try:
created_users.append(
_RelationWithCreatedUser(relation=relation, interface=self._interface)
shared_users.append(
_RelationWithSharedUser(relation=relation, interface=self._interface)
)
except _UserNotCreated:
except _UserNotShared:
pass
return created_users
return shared_users

def reconcile_users(
self,
Expand Down Expand Up @@ -199,15 +210,15 @@ def reconcile_users(
_UnsupportedExtraUserRole,
):
pass
logger.debug(f"State of reconcile users {requested_users=}, {self._created_users=}")
logger.debug(f"State of reconcile users {requested_users=}, {self._shared_users=}")
for relation in requested_users:
if relation not in self._created_users:
if relation not in self._shared_users:
relation.create_database_and_user(
router_read_write_endpoint=router_read_write_endpoint,
router_read_only_endpoint=router_read_only_endpoint,
shell=shell,
)
for relation in self._created_users:
for relation in self._shared_users:
if relation not in requested_users:
relation.delete_user(shell=shell)
logger.debug(
Expand All @@ -223,7 +234,7 @@ def delete_all_databags(self) -> None:
will need to be created.
"""
logger.debug("Deleting all application databags")
for relation in self._created_users:
for relation in self._shared_users:
# MySQL charm will delete user; just delete databag
relation.delete_databag()
logger.debug("Deleted all application databags")
Expand Down

0 comments on commit fad69b1

Please sign in to comment.