From 0a432eae9832c796f10501af91ea3f45f0c22b1d Mon Sep 17 00:00:00 2001 From: Mia Altieri Date: Fri, 22 Sep 2023 11:21:20 +0000 Subject: [PATCH] PR comments --- lib/charms/mongodb/v0/shards_interface.py | 55 +++++++++++++---------- metadata.yaml | 8 ++-- src/charm.py | 42 +++++++++++++++-- 3 files changed, 73 insertions(+), 32 deletions(-) diff --git a/lib/charms/mongodb/v0/shards_interface.py b/lib/charms/mongodb/v0/shards_interface.py index 23886fcaa..19375b2b4 100644 --- a/lib/charms/mongodb/v0/shards_interface.py +++ b/lib/charms/mongodb/v0/shards_interface.py @@ -14,6 +14,7 @@ from ops.charm import CharmBase from ops.framework import Object from ops.model import BlockedStatus, MaintenanceStatus +from tenacity import RetryError, Retrying, stop_after_delay, wait_fixed from config import Config @@ -29,9 +30,11 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version LIBPATCH = 1 +KEYFILE_KEY = "key-file" +OPERATOR_PASSWORD_KEY = "operator-password" -class ShardingRequirer(Object): +class ShardingProvider(Object): """Manage relations between the config server and the shard, on the config-server's side.""" def __init__( @@ -63,18 +66,18 @@ def _on_relation_joined(self, event): if not self.charm.unit.is_leader(): return - if "db_initialised" not in self.charm.app_peer_data: + if not self.charm.db_initialised: event.defer() # TODO Future PR, sync tls secrets and PBM password self._update_relation_data( event.relation.id, { - "operator-password": self.charm.get_secret( + OPERATOR_PASSWORD_KEY: self.charm.get_secret( Config.Relations.APP_SCOPE, MongoDBUser.get_password_key_name_for_user("operator"), ), - "key-file": self.charm.get_secret( + KEYFILE_KEY: self.charm.get_secret( Config.Relations.APP_SCOPE, Config.Secrets.SECRET_KEYFILE_NAME ), }, @@ -100,7 +103,7 @@ def _update_relation_data(self, relation_id: int, data: dict) -> None: relation.data[self.charm.model.app].update(data) -class ShardingProvider(Object): +class ConfigServerRequirer(Object): """Manage relations between the config server and the shard, on the shard's side.""" def __init__( @@ -130,19 +133,19 @@ def _on_relation_changed(self, event): ) return - if "db_initialised" not in self.charm.app_peer_data: + if not self.charm.db_initialised: event.defer() # shards rely on the config server for secrets relation_data = event.relation.data[event.app] + self.update_keyfile(key_file_contents=relation_data.get(KEYFILE_KEY)) + try: - self.update_operator_password(new_password=relation_data.get("operator-password")) - except (PyMongoError, NotReadyError): + self.update_operator_password(new_password=relation_data.get(OPERATOR_PASSWORD_KEY)) + except RetryError: self.charm.unit.status = BlockedStatus("Shard not added to config-server") return - self.update_keyfile(key_file_contents=relation_data.get("key-file")) - self.charm.unit.status = MaintenanceStatus("Adding shard to config-server") # TODO future PR, leader unit verifies shard was added to cluster @@ -153,7 +156,7 @@ def update_operator_password(self, new_password: str) -> None: """Updates the password for the operator user. Raises: - PyMongoError, NotReadyError + RetryError """ current_password = ( self.charm.get_secret( @@ -164,19 +167,23 @@ def update_operator_password(self, new_password: str) -> None: if not new_password or new_password == current_password or not self.charm.unit.is_leader(): return - # TODO, in the future use set_password from src/charm.py - this will require adding a - # library, for exceptions used in both charm code and lib code. - with MongoDBConnection(self.charm.mongodb_config) as mongo: - try: - mongo.set_user_password(OperatorUser.get_username(), new_password) - except NotReadyError: - logger.error( - "Failed changing the password: Not all members healthy or finished initial sync." - ) - raise - except PyMongoError as e: - logger.error(f"Failed changing the password: {e}") - raise + # updating operator password, usually comes after keyfile was updated, hence, the mongodb + # service was restarted. Sometimes this requires units getting insync again. + for attempt in Retrying(stop=stop_after_delay(60), wait=wait_fixed(3)): + with attempt: + # TODO, in the future use set_password from src/charm.py - this will require adding + # a library, for exceptions used in both charm code and lib code. + with MongoDBConnection(self.charm.mongodb_config) as mongo: + try: + mongo.set_user_password(OperatorUser.get_username(), new_password) + except NotReadyError: + logger.error( + "Failed changing the password: Not all members healthy or finished initial sync." + ) + raise + except PyMongoError as e: + logger.error(f"Failed changing the password: {e}") + raise self.charm.set_secret( Config.Relations.APP_SCOPE, diff --git a/metadata.yaml b/metadata.yaml index 8f8e91bcf..0cf7618da 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -25,10 +25,8 @@ provides: interface: mongodb cos-agent: interface: cos_agent - sharding: + config-server: interface: shards - # shards can only relate to one replica set - limit: 1 storage: mongodb: @@ -46,5 +44,7 @@ requires: s3-credentials: interface: s3 limit: 1 - config-server: + sharding: interface: shards + # shards can only relate to one config-server + limit: 1 diff --git a/src/charm.py b/src/charm.py index 763e711b3..6719d8d6e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -35,7 +35,7 @@ from charms.mongodb.v0.mongodb_provider import MongoDBProvider from charms.mongodb.v0.mongodb_tls import MongoDBTLS from charms.mongodb.v0.mongodb_vm_legacy_provider import MongoDBLegacyProvider -from charms.mongodb.v0.shards_interface import ShardingProvider, ShardingRequirer +from charms.mongodb.v0.shards_interface import ConfigServerRequirer, ShardingProvider from charms.mongodb.v0.users import ( CHARM_USERS, BackupUser, @@ -48,6 +48,7 @@ from ops.charm import ( ActionEvent, CharmBase, + ConfigChangedEvent, InstallEvent, LeaderElectedEvent, RelationDepartedEvent, @@ -92,6 +93,9 @@ class MongodbOperatorCharm(CharmBase): def __init__(self, *args): super().__init__(*args) self._port = Config.MONGODB_PORT + + # lifecycle events + self.framework.observe(self.on.config_changed, self._on_config_changed) self.framework.observe(self.on.install, self._on_install) self.framework.observe(self.on.start, self._on_start) self.framework.observe(self.on.update_status, self._on_update_status) @@ -123,8 +127,8 @@ def __init__(self, *args): self.legacy_client_relations = MongoDBLegacyProvider(self) self.tls = MongoDBTLS(self, Config.Relations.PEERS, substrate=Config.SUBSTRATE) self.backups = MongoDBBackups(self) - self.shard_relations = ShardingRequirer(self) - self.config_server_relations = ShardingProvider(self) + self.shard_relations = ShardingProvider(self) + self.config_server_relations = ConfigServerRequirer(self) # relation events for Prometheus metrics are handled in the MetricsEndpointProvider self._grafana_agent = COSAgentProvider( @@ -238,7 +242,17 @@ def db_initialised(self) -> bool: @property def role(self) -> str: """Returns role of MongoDB deployment.""" - return self.model.config["role"] + if "role" not in self.app_peer_data and self.unit.is_leader(): + self.app_peer_data["role"] = self.model.config["role"] + else: + # if leader hasn't set the role yet, use the one set by model + self.model.config["role"] + + return self.app_peer_data.get("role") + + def is_role_changed(self) -> bool: + """Checks if application is running in provided role.""" + return self.role != self.model.config["role"] def is_role(self, role_name: str) -> bool: """Checks if application is running in provided role.""" @@ -297,6 +311,22 @@ def _on_install(self, event: InstallEvent) -> None: # add licenses copy_licenses_to_unit() + def _on_config_changed(self, event: ConfigChangedEvent) -> None: + """Listen to changes in application configuration. + + To prevent a user from migrating a cluster, and causing the component to become + unresponsive therefore causing a cluster failure, error the component. This prevents it + from executing other hooks with a new role. + """ + # TODO in the future (24.04) support migration of components + if self.is_role_changed(): + logger.error( + f"cluster migration currently not supported, cannot change from { self.model.config['role']} to {self.role}" + ) + raise ShardingMigrationError( + f"Migration of sharding components not permitted, revert config role to {self.role}" + ) + def _on_start(self, event: StartEvent) -> None: """Enables MongoDB service and initialises replica set. @@ -1325,6 +1355,10 @@ def _juju_secret_remove(self, scope: Scopes, key: str) -> None: # END: helper functions +class ShardingMigrationError(Exception): + """Raised when there is an attempt to change the role of a sharding component.""" + + class SetPasswordError(Exception): """Raised on failure to set password for MongoDB user."""