From ea0d94281c401ca4ed20f328d607547630965bd7 Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Wed, 10 May 2023 19:08:30 +0000 Subject: [PATCH 1/9] Cleanup router metadata --- lib/charms/mysql/v0/mysql.py | 20 ++++++++++++++++++++ src/relations/mysql_provider.py | 20 ++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/lib/charms/mysql/v0/mysql.py b/lib/charms/mysql/v0/mysql.py index 60bdef837..6585f1f8d 100644 --- a/lib/charms/mysql/v0/mysql.py +++ b/lib/charms/mysql/v0/mysql.py @@ -137,6 +137,10 @@ class MySQLDeleteUsersForRelationError(Error): """Exception raised when there is an issue deleting users for a relation.""" +class MySQLRemoveRouterFromMetadataError(Error): + """Exception raised when there is an issue removing MySQL Router from cluster metadata.""" + + class MySQLConfigureInstanceError(Error): """Exception raised when there is an issue configuring a MySQL instance.""" @@ -581,6 +585,22 @@ def delete_users_for_relation(self, relation_id: int) -> None: logger.exception(f"Failed to delete users for relation {relation_id}", exc_info=e) raise MySQLDeleteUsersForRelationError(e.message) + def remove_router_from_cluster_metadata(self, router_id: str) -> None: + """Remove MySQL Router from InnoDB Cluster metadata.""" + primary_address = self.get_cluster_primary_address() + if not primary_address: + raise MySQLRemoveRouterFromMetadataError("Unable to query cluster primary address") + command = [ + f"shell.connect('{self.cluster_admin_user}:{self.cluster_admin_password}@{primary_address}')", + "cluster = dba.get_cluster()", + f'cluster.remove_router_metadata("{router_id}")', + ] + try: + self._run_mysqlsh_script("\n".join(command)) + except MySQLClientError as e: + logger.exception(f"Failed to remove router from metadata with ID {router_id}") + raise MySQLRemoveRouterFromMetadataError(e.message) + def configure_instance(self, create_cluster_admin: bool = True) -> None: """Configure the instance to be used in an InnoDB cluster. diff --git a/src/relations/mysql_provider.py b/src/relations/mysql_provider.py index 9c3a155cc..c3331d0d0 100644 --- a/src/relations/mysql_provider.py +++ b/src/relations/mysql_provider.py @@ -17,6 +17,7 @@ MySQLGetClusterMembersAddressesError, MySQLGetMySQLVersionError, MySQLGrantPrivilegesToUserError, + MySQLRemoveRouterFromMetadataError, ) from ops.charm import RelationBrokenEvent, RelationDepartedEvent, RelationJoinedEvent from ops.framework import Object @@ -42,6 +43,10 @@ def __init__(self, charm): self.framework.observe( self.charm.on[DB_RELATION_NAME].relation_broken, self._on_database_broken ) + self.framework.observe( + self.charm.on[DB_RELATION_NAME].relation_departed, + self._on_database_provides_relation_departed, + ) self.framework.observe(self.charm.on[PEER].relation_joined, self._on_relation_joined) self.framework.observe(self.charm.on[PEER].relation_departed, self._on_relation_departed) @@ -256,3 +261,18 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None: except (MySQLDeleteUsersForRelationError, KeyError): logger.error(f"Failed to delete user for relation {relation_id}") return + + def _on_database_provides_relation_departed(self, event: RelationDepartedEvent) -> None: + """Remove MySQL Router cluster metadata for departing unit.""" + if not self.charm.unit.is_leader(): + return + + if event.departing_unit.app.name == self.charm.app.name: + return + + if router_id := event.relation.data[event.departing_unit].get("router_id"): + try: + self.charm._mysql.remove_router_from_cluster_metadata(router_id) + logger.info(f"Removed router from metadata {router_id}") + except MySQLRemoveRouterFromMetadataError: + logger.error(f"Failed to remove router from metadata with ID {router_id}") From a54e277c31b51efac7ff0fa3b9b45d7ae95e489b Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Thu, 11 May 2023 15:52:37 +0000 Subject: [PATCH 2/9] bump lib patch --- lib/charms/mysql/v0/mysql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/charms/mysql/v0/mysql.py b/lib/charms/mysql/v0/mysql.py index 6585f1f8d..abbd9b647 100644 --- a/lib/charms/mysql/v0/mysql.py +++ b/lib/charms/mysql/v0/mysql.py @@ -90,7 +90,7 @@ def wait_until_mysql_connection(self) -> None: # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 26 +LIBPATCH = 27 UNIT_TEARDOWN_LOCKNAME = "unit-teardown" From f04674761730ba6e343361140522eb3dde458b1e Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Fri, 12 May 2023 18:11:01 +0000 Subject: [PATCH 3/9] sync --- lib/charms/mysql/v0/mysql.py | 62 +++++++++++++++++++++++++++++++-- src/relations/mysql_provider.py | 34 +++++++++++++----- 2 files changed, 85 insertions(+), 11 deletions(-) diff --git a/lib/charms/mysql/v0/mysql.py b/lib/charms/mysql/v0/mysql.py index abbd9b647..9f34664b7 100644 --- a/lib/charms/mysql/v0/mysql.py +++ b/lib/charms/mysql/v0/mysql.py @@ -65,6 +65,7 @@ def wait_until_mysql_connection(self) -> None: """ +import dataclasses import json import logging import re @@ -129,6 +130,10 @@ class MySQLCreateApplicationDatabaseAndScopedUserError(Error): """Exception raised when creating application database and scoped user.""" +class MySQLGetRouterUsersError(Error): + """Exception raised when there is an issue getting MySQL Router users.""" + + class MySQLDeleteUsersForUnitError(Error): """Exception raised when there is an issue deleting users for a unit.""" @@ -137,6 +142,10 @@ class MySQLDeleteUsersForRelationError(Error): """Exception raised when there is an issue deleting users for a relation.""" +class MySQLDeleteUserError(Error): + """Exception raised when there is an issue deleting a user.""" + + class MySQLRemoveRouterFromMetadataError(Error): """Exception raised when there is an issue removing MySQL Router from cluster metadata.""" @@ -281,6 +290,14 @@ class MySQLKillSessionError(Error): """Exception raised when there is an issue killing a connection.""" +@dataclasses.dataclass +class RouterUser: + """MySQL Router user""" + + username: str + router_id: str + + class MySQLBase(ABC): """Abstract class to encapsulate all operations related to the MySQL instance and cluster. @@ -532,6 +549,32 @@ def _get_statements_to_delete_users_with_attribute( 'session.run_sql("DEALLOCATE PREPARE stmt")', ] + def get_mysql_router_users_for_unit( + self, *, relation_id: int, mysql_router_unit_name: str + ) -> list[RouterUser]: + """Get users for related MySQL Router unit. + + For each user, get username & router ID attribute. + """ + primary_address = self.get_cluster_primary_address() + if not primary_address: + raise MySQLGetRouterUsersError("Unable to query cluster primary address") + relation_user = f"relation-{relation_id}" + command = [ + f"shell.connect('{self.server_config_user}:{self.server_config_password}@{primary_address}')", + f"result = session.run_sql(\"SELECT USER, ATTRIBUTE->>'$.router_id' FROM INFORMATION_SCHEMA.USER_ATTRIBUTES WHERE ATTRIBUTE->'$.created_by_user'='{relation_user}' AND ATTRIBUTE->'$.created_by_juju_unit'='{mysql_router_unit_name}'\")", + "print(result.fetch_all())", + ] + try: + output = self._run_mysqlsh_script("\n".join(command)) + except MySQLClientError as e: + logger.exception( + f"Failed to get MySQL Router users for relation {relation_id} and unit {mysql_router_unit_name}" + ) + raise MySQLGetRouterUsersError(e.message) + rows = json.loads(output) + return [RouterUser(username=row[0], router_id=row[1]) for row in rows] + def delete_users_for_unit(self, unit_name: str) -> None: """Delete users for a unit. @@ -554,7 +597,7 @@ def delete_users_for_unit(self, unit_name: str) -> None: try: self._run_mysqlsh_script("\n".join(drop_users_command)) except MySQLClientError as e: - logger.exception(f"Failed to query and delete users for unit {unit_name}", exc_info=e) + logger.exception(f"Failed to query and delete users for unit {unit_name}") raise MySQLDeleteUsersForUnitError(e.message) def delete_users_for_relation(self, relation_id: int) -> None: @@ -582,9 +625,24 @@ def delete_users_for_relation(self, relation_id: int) -> None: try: self._run_mysqlsh_script("\n".join(drop_users_command)) except MySQLClientError as e: - logger.exception(f"Failed to delete users for relation {relation_id}", exc_info=e) + logger.exception(f"Failed to delete users for relation {relation_id}") raise MySQLDeleteUsersForRelationError(e.message) + def delete_user(self, username: str) -> None: + """Delete user.""" + primary_address = self.get_cluster_primary_address() + if not primary_address: + raise MySQLDeleteUserError("Unable to query cluster primary address") + drop_user_command = [ + f"shell.connect('{self.server_config_user}:{self.server_config_password}@{primary_address}')", + f'session.run_sql("DROP USER `{username}`")', + ] + try: + self._run_mysqlsh_script("\n".join(drop_user_command)) + except MySQLClientError as e: + logger.exception(f"Failed to delete user {username}") + raise MySQLDeleteUserError(e.message) + def remove_router_from_cluster_metadata(self, router_id: str) -> None: """Remove MySQL Router from InnoDB Cluster metadata.""" primary_address = self.get_cluster_primary_address() diff --git a/src/relations/mysql_provider.py b/src/relations/mysql_provider.py index c3331d0d0..e75a986f0 100644 --- a/src/relations/mysql_provider.py +++ b/src/relations/mysql_provider.py @@ -17,7 +17,7 @@ MySQLGetClusterMembersAddressesError, MySQLGetMySQLVersionError, MySQLGrantPrivilegesToUserError, - MySQLRemoveRouterFromMetadataError, + MySQLRemoveRouterFromMetadataError, MySQLDeleteUserError, ) from ops.charm import RelationBrokenEvent, RelationDepartedEvent, RelationJoinedEvent from ops.framework import Object @@ -263,16 +263,32 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None: return def _on_database_provides_relation_departed(self, event: RelationDepartedEvent) -> None: - """Remove MySQL Router cluster metadata for departing unit.""" + """Remove MySQL Router cluster metadata & user for departing unit.""" if not self.charm.unit.is_leader(): return - if event.departing_unit.app.name == self.charm.app.name: return - if router_id := event.relation.data[event.departing_unit].get("router_id"): - try: - self.charm._mysql.remove_router_from_cluster_metadata(router_id) - logger.info(f"Removed router from metadata {router_id}") - except MySQLRemoveRouterFromMetadataError: - logger.error(f"Failed to remove router from metadata with ID {router_id}") + users = self.charm._mysql.get_mysql_router_users_for_unit( + relation_id=event.relation.id, mysql_router_unit_name=event.departing_unit.name + ) + if not users: + return + + if len(users) > 1: + logger.error( + f"More than one router user for departing unit {event.departing_unit.name}" + ) + return + + user = users[0] + try: + self.charm._mysql.delete_user(user.username) + logger.info(f"Deleted router user {user.username}") + except MySQLDeleteUserError: + logger.error(f"Failed to delete user {user.username}") + try: + self.charm._mysql.remove_router_from_cluster_metadata(user.router_id) + logger.info(f"Removed router from metadata {user.router_id}") + except MySQLRemoveRouterFromMetadataError: + logger.error(f"Failed to remove router from metadata with ID {user.router_id}") \ No newline at end of file From 275f812d3c0618272a08d8e77d25ac820bdc3139 Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Fri, 12 May 2023 18:12:02 +0000 Subject: [PATCH 4/9] format --- lib/charms/mysql/v0/mysql.py | 2 +- src/relations/mysql_provider.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/charms/mysql/v0/mysql.py b/lib/charms/mysql/v0/mysql.py index 9f34664b7..9149a19d4 100644 --- a/lib/charms/mysql/v0/mysql.py +++ b/lib/charms/mysql/v0/mysql.py @@ -292,7 +292,7 @@ class MySQLKillSessionError(Error): @dataclasses.dataclass class RouterUser: - """MySQL Router user""" + """MySQL Router user.""" username: str router_id: str diff --git a/src/relations/mysql_provider.py b/src/relations/mysql_provider.py index e75a986f0..c68f99311 100644 --- a/src/relations/mysql_provider.py +++ b/src/relations/mysql_provider.py @@ -12,12 +12,13 @@ from charms.mysql.v0.mysql import ( MySQLClientError, MySQLCreateApplicationDatabaseAndScopedUserError, + MySQLDeleteUserError, MySQLDeleteUsersForRelationError, MySQLGetClusterEndpointsError, MySQLGetClusterMembersAddressesError, MySQLGetMySQLVersionError, MySQLGrantPrivilegesToUserError, - MySQLRemoveRouterFromMetadataError, MySQLDeleteUserError, + MySQLRemoveRouterFromMetadataError, ) from ops.charm import RelationBrokenEvent, RelationDepartedEvent, RelationJoinedEvent from ops.framework import Object @@ -291,4 +292,4 @@ def _on_database_provides_relation_departed(self, event: RelationDepartedEvent) self.charm._mysql.remove_router_from_cluster_metadata(user.router_id) logger.info(f"Removed router from metadata {user.router_id}") except MySQLRemoveRouterFromMetadataError: - logger.error(f"Failed to remove router from metadata with ID {user.router_id}") \ No newline at end of file + logger.error(f"Failed to remove router from metadata with ID {user.router_id}") From b3ecd7a830772129e480cc3a8ee91e0ec87a16fe Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Fri, 12 May 2023 18:13:50 +0000 Subject: [PATCH 5/9] unit test --- tests/unit/test_database.py | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/tests/unit/test_database.py b/tests/unit/test_database.py index d1eb258fb..3f54cfb94 100644 --- a/tests/unit/test_database.py +++ b/tests/unit/test_database.py @@ -4,7 +4,6 @@ import unittest from unittest.mock import patch -from charms.mysql.v0.mysql import MySQLDeleteUsersForRelationError from ops.testing import Harness from charm import MySQLOperatorCharm @@ -83,27 +82,3 @@ def test_database_requested( _create_application_database_and_scoped_user.assert_called_once() _get_cluster_endpoints.assert_called_once() _get_mysql_version.assert_called_once() - - @patch_network_get(private_address="1.1.1.1") - @patch("mysql_vm_helpers.MySQL.delete_users_for_relation") - def test_database_broken(self, _delete_users_for_relation): - # run start-up events to enable usage of the helper class - self.harness.set_leader(True) - self.charm.on.config_changed.emit() - - self.harness.remove_relation(self.database_relation_id) - - _delete_users_for_relation.assert_called_once_with(self.database_relation_id) - - @patch_network_get(private_address="1.1.1.1") - @patch("mysql_vm_helpers.MySQL.delete_users_for_relation") - def test_database_broken_failure(self, _delete_users_for_relation): - # run start-up events to enable usage of the helper class - self.harness.set_leader(True) - self.charm.on.config_changed.emit() - - _delete_users_for_relation.side_effect = MySQLDeleteUsersForRelationError() - - self.harness.remove_relation(self.database_relation_id) - - _delete_users_for_relation.assert_called_once() From 0892c7720e658a6b542aaed6d24fdcd82562d962 Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Mon, 15 May 2023 18:44:09 +0000 Subject: [PATCH 6/9] update docstrings --- src/relations/mysql_provider.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/relations/mysql_provider.py b/src/relations/mysql_provider.py index c68f99311..d836ddb0b 100644 --- a/src/relations/mysql_provider.py +++ b/src/relations/mysql_provider.py @@ -243,7 +243,9 @@ def _on_database_requested(self, event: DatabaseRequestedEvent): def _on_database_broken(self, event: RelationBrokenEvent) -> None: """Handle the removal of database relation. - Remove user, keeping database intact. + Remove users, keeping database intact. + + Includes users created by MySQL Router for MySQL Router <-> application relation """ if not self.charm.unit.is_leader(): # run once by the leader @@ -264,7 +266,7 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None: return def _on_database_provides_relation_departed(self, event: RelationDepartedEvent) -> None: - """Remove MySQL Router cluster metadata & user for departing unit.""" + """Remove MySQL Router cluster metadata & router user for departing unit.""" if not self.charm.unit.is_leader(): return if event.departing_unit.app.name == self.charm.app.name: From 3fa08836efc9f1e4287db36d870b3dff716c950f Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Tue, 16 May 2023 12:25:04 +0000 Subject: [PATCH 7/9] remove return --- src/relations/mysql_provider.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/relations/mysql_provider.py b/src/relations/mysql_provider.py index d836ddb0b..cb4330e53 100644 --- a/src/relations/mysql_provider.py +++ b/src/relations/mysql_provider.py @@ -263,7 +263,6 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None: logger.info(f"Removed user for relation {relation_id}") except (MySQLDeleteUsersForRelationError, KeyError): logger.error(f"Failed to delete user for relation {relation_id}") - return def _on_database_provides_relation_departed(self, event: RelationDepartedEvent) -> None: """Remove MySQL Router cluster metadata & router user for departing unit.""" From 3060aa1d9748076c39c582463e70fd2180d6b24f Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Tue, 16 May 2023 18:54:19 +0000 Subject: [PATCH 8/9] update lib --- lib/charms/mysql/v0/mysql.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/charms/mysql/v0/mysql.py b/lib/charms/mysql/v0/mysql.py index 9149a19d4..a4352ba59 100644 --- a/lib/charms/mysql/v0/mysql.py +++ b/lib/charms/mysql/v0/mysql.py @@ -556,12 +556,9 @@ def get_mysql_router_users_for_unit( For each user, get username & router ID attribute. """ - primary_address = self.get_cluster_primary_address() - if not primary_address: - raise MySQLGetRouterUsersError("Unable to query cluster primary address") relation_user = f"relation-{relation_id}" command = [ - f"shell.connect('{self.server_config_user}:{self.server_config_password}@{primary_address}')", + f"shell.connect('{self.server_config_user}:{self.server_config_password}@{self.instance_address}')", f"result = session.run_sql(\"SELECT USER, ATTRIBUTE->>'$.router_id' FROM INFORMATION_SCHEMA.USER_ATTRIBUTES WHERE ATTRIBUTE->'$.created_by_user'='{relation_user}' AND ATTRIBUTE->'$.created_by_juju_unit'='{mysql_router_unit_name}'\")", "print(result.fetch_all())", ] From b5de7a50a75eac3f2d9b8136d124d03b815cb6f6 Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Tue, 16 May 2023 19:04:36 +0000 Subject: [PATCH 9/9] add @'%" --- lib/charms/mysql/v0/mysql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/charms/mysql/v0/mysql.py b/lib/charms/mysql/v0/mysql.py index a4352ba59..3a860cad8 100644 --- a/lib/charms/mysql/v0/mysql.py +++ b/lib/charms/mysql/v0/mysql.py @@ -632,7 +632,7 @@ def delete_user(self, username: str) -> None: raise MySQLDeleteUserError("Unable to query cluster primary address") drop_user_command = [ f"shell.connect('{self.server_config_user}:{self.server_config_password}@{primary_address}')", - f'session.run_sql("DROP USER `{username}`")', + f"session.run_sql(\"DROP USER `{username}`@'%'\")", ] try: self._run_mysqlsh_script("\n".join(drop_user_command))