From b58cd711d19569968c2b38d6cc4bde657555575a Mon Sep 17 00:00:00 2001 From: Mia Altieri <32723809+MiaAltieri@users.noreply.github.com> Date: Tue, 5 Dec 2023 11:07:28 -0800 Subject: [PATCH] [DPE-2995] Create mongos user (#312) ## Issue related mongos suboridinate charm doesn't have its own user ## Solution Create a user for related monogs subordinate charm ## Future PRs 1. implement relation on mongos suboridinate charm 2. enable mongos charm to change its requested database / roles 3. share URI with hosting charm of mongos 4. add int tests once both charms are up to date on charmhub ## Testing ``` # deploy shards + config server juju add-model test-0 cd ~/mongodb-operator charmcraft pack juju deploy ./*charm --config role="config-server" config-server juju deploy ./*charm --config role="shard" shard cd ~/mongos-operator cp ~/mongodb-operator/lib/charms/mongodb/v0/config_server_interface.py lib/charms/mongodb/v0/config_server_interface.py cp ~/mongodb-operator/lib/charms/mongodb/v1/helpers.py lib/charms/mongodb/v1/helpers.py tox -e build juju deploy ./*charm cd ~/mongos-operator/tests/integration/application juju deploy ./*charm # relate juju integrate mongos application juju integrate config-server:config-server shard:sharding juju integrate config-server:cluster mongos:cluster juju ssh mongos/0 sudo charmed-mongodb.mongosh mongodb://relation-5:DOVn0liQ2taeIaLyL3yii8pYO4fsjoRW@%2Fvar%2Fsnap%2Fcharmed-mongodb%2Fcommon%2Fvar%2Fmongodb-27018.sock ``` --- .../mongodb/v0/config_server_interface.py | 69 ++++++++++++++++--- lib/charms/mongodb/v1/mongodb_provider.py | 42 ++++++++--- src/charm.py | 4 +- tests/unit/test_config_server_lib.py | 10 +-- 4 files changed, 98 insertions(+), 27 deletions(-) diff --git a/lib/charms/mongodb/v0/config_server_interface.py b/lib/charms/mongodb/v0/config_server_interface.py index 565df4819..ca4c210be 100644 --- a/lib/charms/mongodb/v0/config_server_interface.py +++ b/lib/charms/mongodb/v0/config_server_interface.py @@ -6,9 +6,12 @@ This class handles the sharing of secrets between sharded components, adding shards, and removing shards. """ -import json import logging +from charms.data_platform_libs.v0.data_interfaces import ( + DatabaseProvides, + DatabaseRequires, +) from charms.mongodb.v1.helpers import add_args_to_env, get_mongos_args from charms.mongodb.v1.mongos import MongosConnection from ops.charm import CharmBase, EventBase @@ -32,7 +35,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 2 +LIBPATCH = 4 class ClusterProvider(Object): @@ -44,10 +47,11 @@ def __init__( """Constructor for ShardingProvider object.""" self.relation_name = relation_name self.charm = charm + self.database_provides = DatabaseProvides(self.charm, relation_name=self.relation_name) super().__init__(charm, self.relation_name) self.framework.observe( - charm.on[self.relation_name].relation_joined, self._on_relation_joined + charm.on[self.relation_name].relation_changed, self._on_relation_changed ) # TODO Future PRs handle scale down @@ -71,7 +75,7 @@ def pass_hook_checks(self, event: EventBase) -> bool: return True - def _on_relation_joined(self, event) -> None: + def _on_relation_changed(self, event) -> None: """Handles providing mongos with KeyFile and hosts.""" if not self.pass_hook_checks(event): logger.info("Skipping relation joined event: hook checks did not pass") @@ -79,8 +83,11 @@ def _on_relation_joined(self, event) -> None: config_server_db = self.generate_config_server_db() + # create user and set secrets for mongos relation + self.charm.client_relations.oversee_users(None, None) + # TODO Future PR, use secrets - self._update_relation_data( + self.update_relation_data( event.relation.id, { KEYFILE_KEY: self.charm.get_secret( @@ -90,7 +97,7 @@ def _on_relation_joined(self, event) -> None: }, ) - def _update_relation_data(self, relation_id: int, data: dict) -> None: + def update_relation_data(self, relation_id: int, data: dict) -> None: """Updates a set of key-value pairs in the relation. This function writes in the application data bag, therefore, only the leader unit can call @@ -102,9 +109,7 @@ def _update_relation_data(self, relation_id: int, data: dict) -> None: that should be updated in the relation. """ if self.charm.unit.is_leader(): - relation = self.charm.model.get_relation(self.relation_name, relation_id) - if relation: - relation.data[self.charm.model.app].update(data) + self.database_provides.update_relation_data(relation_id, data) def generate_config_server_db(self) -> str: """Generates the config server database for mongos to connect to.""" @@ -126,13 +131,38 @@ def __init__( """Constructor for ShardingProvider object.""" self.relation_name = relation_name self.charm = charm + self.database_requires = DatabaseRequires( + self.charm, + relation_name=self.relation_name, + database_name=self.charm.database, + extra_user_roles=self.charm.extra_user_roles, + ) super().__init__(charm, self.relation_name) + self.framework.observe( + charm.on[self.relation_name].relation_created, self._on_relation_created_event + ) self.framework.observe( charm.on[self.relation_name].relation_changed, self._on_relation_changed ) # TODO Future PRs handle scale down + def _on_relation_created_event(self, event): + """Sets database and extra user roles in the relation.""" + if not self.charm.unit.is_leader(): + return + + if not self.charm.database: + logger.info("Waiting for database from application") + event.defer() + return + + rel_data = {"database": self.charm.database} + if self.charm.extra_user_roles: + rel_data["extra-user-roles"] = str(self.charm.extra_user_roles) + + self.update_relation_data(event.relation.id, rel_data) + def _on_relation_changed(self, event) -> None: """Starts/restarts monogs with config server information.""" relation_data = event.relation.data[event.app] @@ -162,9 +192,11 @@ def _on_relation_changed(self, event) -> None: event.defer() return - # TODO: Follow up PR. Add a user for mongos once it has been started + self.charm.share_uri() self.charm.unit.status = ActiveStatus() + # BEGIN: helper functions + def is_mongos_running(self) -> bool: """Returns true if mongos service is running.""" with MongosConnection(None, f"mongodb://{MONGOS_SOCKET_URI_FMT}") as mongo: @@ -180,7 +212,6 @@ def update_config_server_db(self, config_server_db) -> bool: mongos_config, snap_install=True, config_server_db=config_server_db ) add_args_to_env("MONGOS_ARGS", mongos_start_args) - self.charm.unit_peer_data["config_server_db"] = json.dumps(config_server_db) return True def update_keyfile(self, key_file_contents: str) -> bool: @@ -202,3 +233,19 @@ def update_keyfile(self, key_file_contents: str) -> bool: ) return True + + def update_relation_data(self, relation_id: int, data: dict) -> None: + """Updates a set of key-value pairs in the relation. + + This function writes in the application data bag, therefore, only the leader unit can call + it. + + Args: + relation_id: the identifier for a particular relation. + data: dict containing the key-value pairs + that should be updated in the relation. + """ + if self.charm.unit.is_leader(): + self.database_requires.update_relation_data(relation_id, data) + + # END: helper functions diff --git a/lib/charms/mongodb/v1/mongodb_provider.py b/lib/charms/mongodb/v1/mongodb_provider.py index 00e052b1d..aa634c650 100644 --- a/lib/charms/mongodb/v1/mongodb_provider.py +++ b/lib/charms/mongodb/v1/mongodb_provider.py @@ -11,7 +11,7 @@ import logging import re from collections import namedtuple -from typing import Optional, Set +from typing import List, Optional, Set from charms.data_platform_libs.v0.data_interfaces import DatabaseProvides from charms.mongodb.v0.mongodb import MongoDBConfiguration, MongoDBConnection @@ -21,6 +21,8 @@ from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, Relation from pymongo.errors import PyMongoError +from config import Config + # The unique Charmhub library identifier, never change it LIBID = "4067879ef7dd4261bf6c164bc29d94b1" @@ -35,6 +37,7 @@ REL_NAME = "database" LEGACY_REL_NAME = "obsolete" +MONGOS_RELATIONS = "cluster" # We expect the MongoDB container to use the default ports MONGODB_PORT = 27017 @@ -191,6 +194,7 @@ def oversee_users(self, departed_relation_id: Optional[int], event): # set the database name into the relation. continue logger.info("Create relation user: %s on %s", config.username, config.database) + mongo.create_user(config) self._set_relation(config) @@ -259,7 +263,7 @@ def update_app_relation_data(self) -> None: with MongoDBConnection(self.charm.mongodb_config) as mongo: database_users = mongo.get_users() - for relation in self.charm.model.relations[REL_NAME]: + for relation in self._get_relations(rel=REL_NAME): username = self._get_username_from_relation_id(relation.id) password = self._get_or_set_password(relation) config = self._get_config(username, password) @@ -295,9 +299,11 @@ def _get_config(self, username: str, password: Optional[str]) -> MongoDBConfigur if not password: password = self._get_or_set_password(relation) + database_name = self._get_database_from_relation(relation) + return MongoDBConfiguration( replset=self.charm.app.name, - database=self._get_database_from_relation(relation), + database=database_name, username=username, password=password, hosts=self.charm.mongodb_config.hosts, @@ -314,6 +320,11 @@ def _set_relation(self, config: MongoDBConfiguration): self.database_provides.set_credentials(relation.id, config.username, config.password) self.database_provides.set_database(relation.id, config.database) + + # relations with the mongos server should not connect though the config-server directly + if self.charm.is_role(Config.Role.CONFIG_SERVER): + return + self.database_provides.set_endpoints( relation.id, ",".join(config.hosts), @@ -334,7 +345,7 @@ def _get_username_from_relation_id(relation_id: int) -> str: def _get_users_from_relations(self, departed_relation_id: Optional[int], rel=REL_NAME): """Return usernames for all relations except departed relation.""" - relations = self.model.relations[rel] + relations = self._get_relations(rel) return set( [ self._get_username_from_relation_id(relation.id) @@ -351,7 +362,7 @@ def _get_databases_from_relations(self, departed_relation_id: Optional[int]) -> except for those databases that belong to the departing relation specified. """ - relations = self.model.relations[REL_NAME] + relations = self._get_relations(rel=REL_NAME) databases = set() for relation in relations: if relation.id == departed_relation_id: @@ -370,15 +381,28 @@ def _get_relation_from_username(self, username: str) -> Relation: assert match is not None, "No relation match" relation_id = int(match.group(1)) logger.debug("Relation ID: %s", relation_id) - return self.model.get_relation(REL_NAME, relation_id) + relation_name = ( + MONGOS_RELATIONS if self.charm.is_role(Config.Role.CONFIG_SERVER) else REL_NAME + ) + return self.model.get_relation(relation_name, relation_id) + + def _get_relations(self, rel=REL_NAME) -> List[Relation]: + """Return the set of relations for users. + + We create users for either direct relations to charm or for relations through the mongos + charm. + """ + return ( + self.model.relations[MONGOS_RELATIONS] + if self.charm.is_role(Config.Role.CONFIG_SERVER) and rel != LEGACY_REL_NAME + else self.model.relations[rel] + ) @staticmethod def _get_database_from_relation(relation: Relation) -> Optional[str]: """Return database name from relation.""" database = relation.data[relation.app].get("database", None) - if database is not None: - return database - return None + return database @staticmethod def _get_roles_from_relation(relation: Relation) -> Set[str]: diff --git a/src/charm.py b/src/charm.py index 8e056528a..3c1ee3ca2 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1119,8 +1119,8 @@ def _initialise_replica_set(self, event: StartEvent) -> None: self._init_backup_user() self._init_monitor_user() - # in sharding, user management is handled by mongos subordinate charm - if self.is_role(Config.Role.REPLICATION): + # Bare replicas can create users or config-servers for related mongos apps + if not self.is_role(Config.Role.SHARD): logger.info("Manage user") self.client_relations.oversee_users(None, None) diff --git a/tests/unit/test_config_server_lib.py b/tests/unit/test_config_server_lib.py index 23150aaba..2e2e83291 100644 --- a/tests/unit/test_config_server_lib.py +++ b/tests/unit/test_config_server_lib.py @@ -22,8 +22,8 @@ def setUp(self): self.charm = self.harness.charm self.addCleanup(self.harness.cleanup) - @patch("charm.ClusterProvider._update_relation_data") - def test_on_relation_joined_failed_hook_checks(self, _update_relation_data): + @patch("charm.ClusterProvider.update_relation_data") + def test_on_relation_joined_failed_hook_checks(self, update_relation_data): """Tests that no relation data is set when cluster joining conditions are not met.""" def is_not_config_mock_call(*args): @@ -36,7 +36,7 @@ def is_not_config_mock_call(*args): self.harness.charm.is_role = is_not_config_mock_call relation_id = self.harness.add_relation("cluster", "mongos") self.harness.add_relation_unit(relation_id, "mongos/0") - _update_relation_data.assert_not_called() + update_relation_data.assert_not_called() # fails because db has not been initialized del self.harness.charm.app_peer_data["db_initialised"] @@ -47,9 +47,9 @@ def is_config_mock_call(*args): self.harness.charm.is_role = is_config_mock_call self.harness.add_relation_unit(relation_id, "mongos/1") - _update_relation_data.assert_not_called() + update_relation_data.assert_not_called() # fails because not leader self.harness.set_leader(False) self.harness.add_relation_unit(relation_id, "mongos/2") - _update_relation_data.assert_not_called() + update_relation_data.assert_not_called()