From 0cf2152849555a1ede1d24a01d42366e03606420 Mon Sep 17 00:00:00 2001 From: Neha Oudin Date: Wed, 21 Aug 2024 14:32:30 +0200 Subject: [PATCH 1/5] feat: Check data permissions before restarting mongod --- src/charm.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/charm.py b/src/charm.py index e1388d411..46d2b16c8 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1205,6 +1205,14 @@ def restart_charm_services(self): container = self.unit.get_container(Config.CONTAINER_NAME) container.stop(Config.SERVICE_NAME) + # Ensure the permissions are right before restarting the service. + try: + self._set_data_dir_permissions(container, all_files=True) + + except (PathError, ProtocolError, MissingSecretError) as e: + logger.error("Cannot initialize workload: %r", e) + raise + container.add_layer("mongod", self._mongod_layer, combine=True) if self.is_role(Config.Role.CONFIG_SERVER): container.add_layer("mongos", self._mongos_layer, combine=True) @@ -1525,21 +1533,33 @@ def _pull_licenses(container: Container) -> None: pass @staticmethod - def _set_data_dir_permissions(container: Container) -> None: + def _set_data_dir_permissions(container: Container, all_files: bool = False) -> None: """Ensure the data directory for mongodb is writable for the "mongodb" user. Until the ability to set fsGroup and fsGroupChangePolicy via Pod securityContext is available, we fix permissions incorrectly with chown. + We want to check the directory and the files inside in case it got messed up. """ for path in [Config.DATA_DIR, Config.LOG_DIR, Config.LogRotate.LOG_STATUS_DIR]: paths = container.list_files(path, itself=True) - assert len(paths) == 1, "list_files doesn't return only the directory itself" logger.debug(f"Data directory ownership: {paths[0].user}:{paths[0].group}") + assert len(paths) == 1, "list_files doesn't return only the directory itself" + if paths[0].user != Config.UNIX_USER or paths[0].group != Config.UNIX_GROUP: container.exec( f"chown {Config.UNIX_USER}:{Config.UNIX_GROUP} -R {path}".split(" ") ) + if all_files: + files = container.list_files(path) + if any( + file.user != Config.UNIX_USER or file.group != Config.UNIX_GROUP + for file in files + ): + container.exec( + f"chown {Config.UNIX_USER}:{Config.UNIX_GROUP} -R {path}".split(" ") + ) + # END: static methods From 677c72d52c9f851154c03d62e6221a26f218581c Mon Sep 17 00:00:00 2001 From: Mia Altieri <32723809+MiaAltieri@users.noreply.github.com> Date: Fri, 23 Aug 2024 11:30:01 +0200 Subject: [PATCH 2/5] CHORE: charmcraft fetch lib (#321) * udpate libs * make updates for changes in lib * make necessary changes in tests and src --- .../mongodb/v0/config_server_interface.py | 34 ++- lib/charms/mongodb/v0/mongo.py | 282 ++++++++++++++++++ lib/charms/mongodb/v0/set_status.py | 22 +- lib/charms/mongodb/v1/helpers.py | 8 +- lib/charms/mongodb/v1/mongodb.py | 218 +------------- lib/charms/mongodb/v1/mongodb_provider.py | 10 +- lib/charms/mongodb/v1/mongos.py | 80 +---- lib/charms/mongodb/v1/shards_interface.py | 11 +- src/charm.py | 35 ++- src/config.py | 6 + tests/unit/test_charm.py | 4 +- tests/unit/test_mongodb_lib.py | 42 +-- 12 files changed, 400 insertions(+), 352 deletions(-) create mode 100644 lib/charms/mongodb/v0/mongo.py diff --git a/lib/charms/mongodb/v0/config_server_interface.py b/lib/charms/mongodb/v0/config_server_interface.py index e3209a090..ba515f3e0 100644 --- a/lib/charms/mongodb/v0/config_server_interface.py +++ b/lib/charms/mongodb/v0/config_server_interface.py @@ -42,8 +42,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version - -LIBPATCH = 8 +LIBPATCH = 9 class ClusterProvider(Object): @@ -206,9 +205,13 @@ class ClusterRequirer(Object): """Manage relations between the config server and mongos router on the mongos side.""" def __init__( - self, charm: CharmBase, relation_name: str = Config.Relations.CLUSTER_RELATIONS_NAME + self, + charm: CharmBase, + relation_name: str = Config.Relations.CLUSTER_RELATIONS_NAME, + substrate: str = Config.Substrate.VM, ) -> None: """Constructor for ShardingProvider object.""" + self.substrate = substrate self.relation_name = relation_name self.charm = charm self.database_requires = DatabaseRequires( @@ -254,7 +257,10 @@ def _on_database_created(self, event) -> None: logger.info("Database and user created for mongos application") self.charm.set_secret(Config.Relations.APP_SCOPE, Config.Secrets.USERNAME, event.username) self.charm.set_secret(Config.Relations.APP_SCOPE, Config.Secrets.PASSWORD, event.password) - self.charm.share_connection_info() + + # K8s charm have a 1:Many client scheme and share connection info in a different manner. + if self.substrate == Config.Substrate.VM: + self.charm.share_connection_info() def _on_relation_changed(self, event) -> None: """Starts/restarts monogs with config server information.""" @@ -318,7 +324,10 @@ def _on_relation_broken(self, event: RelationBrokenEvent) -> None: logger.info("Database and user removed for mongos application") self.charm.remove_secret(Config.Relations.APP_SCOPE, Config.Secrets.USERNAME) self.charm.remove_secret(Config.Relations.APP_SCOPE, Config.Secrets.PASSWORD) - self.charm.remove_connection_info() + + # K8s charm have a 1:Many client scheme and share connection info in a different manner. + if self.substrate == Config.Substrate.VM: + self.charm.remove_connection_info() # BEGIN: helper functions def pass_hook_checks(self, event): @@ -361,8 +370,8 @@ def is_mongos_running(self) -> bool: """Returns true if mongos service is running.""" connection_uri = f"mongodb://{self.charm.get_mongos_host()}" - # when running internally, connections through Unix Domain sockets do not need port. - if self.charm.is_external_client: + # use the mongos port for k8s charms and external connections on VM + if self.charm.is_external_client or self.substrate == Config.K8S_SUBSTRATE: connection_uri = connection_uri + f":{Config.MONGOS_PORT}" with MongosConnection(None, connection_uri) as mongo: @@ -373,7 +382,9 @@ def update_config_server_db(self, config_server_db) -> bool: if self.charm.config_server_db == config_server_db: return False - self.charm.update_mongos_args(config_server_db) + if self.substrate == Config.Substrate.VM: + self.charm.update_mongos_args(config_server_db) + return True def update_keyfile(self, key_file_contents: str) -> bool: @@ -420,6 +431,13 @@ def get_config_server_name(self) -> Optional[str]: # metadata.yaml prevents having multiple config servers return self.model.get_relation(self.relation_name).app.name + def get_config_server_uri(self) -> str: + """Returns the short form URI of the config server.""" + return self.database_requires.fetch_relation_field( + self.model.get_relation(Config.Relations.CLUSTER_RELATIONS_NAME).id, + CONFIG_SERVER_DB_KEY, + ) + def is_ca_compatible(self) -> bool: """Returns true if both the mongos and the config server use the same CA.""" config_server_relation = self.charm.model.get_relation(self.relation_name) diff --git a/lib/charms/mongodb/v0/mongo.py b/lib/charms/mongodb/v0/mongo.py new file mode 100644 index 000000000..f8ef0e44f --- /dev/null +++ b/lib/charms/mongodb/v0/mongo.py @@ -0,0 +1,282 @@ +"""Code for interactions with MongoDB.""" + +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +import logging +import re +from dataclasses import dataclass +from itertools import chain +from typing import List, Set +from urllib.parse import quote_plus + +from pymongo import MongoClient +from pymongo.errors import OperationFailure, PyMongoError +from tenacity import RetryError, Retrying, stop_after_delay, wait_fixed + +from config import Config + +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + + +class NotReadyError(PyMongoError): + """Raised when not all replica set members healthy or finished initial sync.""" + + +# The unique Charmhub library identifier, never change it +LIBID = "3037662a76cc4bf1876d4659c88e77e5" + +# Increment this major API version when introducing breaking changes +LIBAPI = 0 + +# Increment this PATCH version before using `charmcraft publish-lib` or reset +# to 0 if you are raising the major API version +LIBPATCH = 1 + +ADMIN_AUTH_SOURCE = "authSource=admin" +SYSTEM_DBS = ("admin", "local", "config") +REGULAR_ROLES = { + "admin": [ + {"role": "userAdminAnyDatabase", "db": "admin"}, + {"role": "readWriteAnyDatabase", "db": "admin"}, + {"role": "userAdmin", "db": "admin"}, + ], + "monitor": [ + {"role": "explainRole", "db": "admin"}, + {"role": "clusterMonitor", "db": "admin"}, + {"role": "read", "db": "local"}, + ], + "backup": [ + {"db": "admin", "role": "readWrite", "collection": ""}, + {"db": "admin", "role": "backup"}, + {"db": "admin", "role": "clusterMonitor"}, + {"db": "admin", "role": "restore"}, + {"db": "admin", "role": "pbmAnyAction"}, + ], +} + +# path to store mongodb ketFile +logger = logging.getLogger(__name__) + + +class AmbiguousConfigError(Exception): + """Raised when the config could correspond to a mongod config or mongos config.""" + + +@dataclass +class MongoConfiguration: + """Class for Mongo configurations usable my mongos and mongodb. + + — replset: name of replica set + — database: database name. + — username: username. + — password: password. + — hosts: full list of hosts to connect to, needed for the URI. + — tls_external: indicator for use of internal TLS connection. + — tls_internal: indicator for use of external TLS connection. + """ + + database: str | None + username: str + password: str + hosts: Set[str] + roles: Set[str] + tls_external: bool + tls_internal: bool + port: int = Config.MONGODB_PORT + replset: str | None = None + standalone: bool = False + + @property + def uri(self): + """Return URI concatenated from fields.""" + if self.port == Config.MONGOS_PORT and self.replset: + raise AmbiguousConfigError("Mongos cannot support replica set") + + if self.standalone: + return ( + f"mongodb://{quote_plus(self.username)}:" + f"{quote_plus(self.password)}@" + f"localhost:{self.port}/?authSource=admin" + ) + + self.complete_hosts = self.hosts + + # mongos using Unix Domain Socket to communicate do not use port + if self.port: + self.complete_hosts = [f"{host}:{self.port}" for host in self.hosts] + + complete_hosts = ",".join(self.complete_hosts) + + replset_str = f"replicaSet={quote_plus(self.replset)}" if self.replset else "" + + # Auth DB should be specified while user connects to application DB. + auth_source = "" + if self.database != "admin": + # "&"" is needed to concatenate multiple values in URI + auth_source = f"&{ADMIN_AUTH_SOURCE}" if self.replset else ADMIN_AUTH_SOURCE + + return ( + f"mongodb://{quote_plus(self.username)}:" + f"{quote_plus(self.password)}@" + f"{complete_hosts}/{quote_plus(self.database)}?" + f"{replset_str}" + f"{auth_source}" + ) + + +def supported_roles(config: MongoConfiguration): + """Return the supported roles for the given configuration.""" + return REGULAR_ROLES | {"default": [{"db": config.database, "role": "readWrite"}]} + + +class MongoConnection: + """In this class we create connection object to Mongo[s/db]. + + This class is meant for agnositc functions in mongos and mongodb. + + Real connection is created on the first call to Mongo[s/db]. + Delayed connectivity allows to firstly check database readiness + and reuse the same connection for an actual query later in the code. + + Connection is automatically closed when object destroyed. + Automatic close allows to have more clean code. + + Note that connection when used may lead to the following pymongo errors: ConfigurationError, + ConfigurationError, OperationFailure. It is suggested that the following pattern be adopted + when using MongoDBConnection: + + with MongoMongos(MongoConfig) as mongo: + try: + mongo. + except ConfigurationError, OperationFailure: + + """ + + def __init__(self, config: MongoConfiguration, uri=None, direct=False): + """A MongoDB client interface. + + Args: + config: MongoDB Configuration object. + uri: allow using custom MongoDB URI, needed for replSet init. + direct: force a direct connection to a specific host, avoiding + reading replica set configuration and reconnection. + """ + self.config = config + + if uri is None: + uri = config.uri + + self.client = MongoClient( + uri, + directConnection=direct, + connect=False, + serverSelectionTimeoutMS=1000, + connectTimeoutMS=2000, + ) + return + + def __enter__(self): + """Return a reference to the new connection.""" + return self + + def __exit__(self, object_type, value, traceback): + """Disconnect from MongoDB client.""" + self.client.close() + self.client = None + + @property + def is_ready(self) -> bool: + """Is the MongoDB server ready for services requests. + + Returns: + True if services is ready False otherwise. Retries over a period of 60 seconds times to + allow server time to start up. + + """ + try: + for attempt in Retrying(stop=stop_after_delay(60), wait=wait_fixed(3)): + with attempt: + # The ping command is cheap and does not require auth. + self.client.admin.command("ping") + except RetryError: + return False + + return True + + def create_user(self, config: MongoConfiguration): + """Create user. + + Grant read and write privileges for specified database. + """ + self.client.admin.command( + "createUser", + config.username, + pwd=config.password, + roles=self._get_roles(config), + mechanisms=["SCRAM-SHA-256"], + ) + + def update_user(self, config: MongoConfiguration): + """Update grants on database.""" + self.client.admin.command( + "updateUser", + config.username, + roles=self._get_roles(config), + ) + + def set_user_password(self, username, password: str): + """Update the password.""" + self.client.admin.command( + "updateUser", + username, + pwd=password, + ) + + def create_role(self, role_name: str, privileges: dict, roles: dict = []): + """Creates a new role. + + Args: + role_name: name of the role to be added. + privileges: privileges to be associated with the role. + roles: List of roles from which this role inherits privileges. + """ + try: + self.client.admin.command( + "createRole", role_name, privileges=[privileges], roles=roles + ) + except OperationFailure as e: + if not e.code == 51002: # Role already exists + logger.error("Cannot add role. error=%r", e) + raise e + + @staticmethod + def _get_roles(config: MongoConfiguration) -> List[dict]: + all_roles = supported_roles(config) + return list(chain.from_iterable(all_roles[role] for role in config.roles)) + + def drop_user(self, username: str): + """Drop user.""" + self.client.admin.command("dropUser", username) + + def get_users(self) -> Set[str]: + """Add a new member to replica set config inside MongoDB.""" + users_info = self.client.admin.command("usersInfo") + return set( + [ + user_obj["user"] + for user_obj in users_info["users"] + if re.match(r"^relation-\d+$", user_obj["user"]) + ] + ) + + def get_databases(self) -> Set[str]: + """Return list of all non-default databases.""" + databases = self.client.list_database_names() + return {db for db in databases if db not in SYSTEM_DBS} + + def drop_database(self, database: str): + """Drop a non-default database.""" + if database in SYSTEM_DBS: + return + self.client.drop_database(database) diff --git a/lib/charms/mongodb/v0/set_status.py b/lib/charms/mongodb/v0/set_status.py index 22c2f25b2..98d1dabac 100644 --- a/lib/charms/mongodb/v0/set_status.py +++ b/lib/charms/mongodb/v0/set_status.py @@ -4,9 +4,9 @@ # See LICENSE file for licensing details. import json import logging -from typing import Tuple +from typing import Optional, Tuple -from charms.mongodb.v1.mongodb import MongoDBConfiguration, MongoDBConnection +from charms.mongodb.v1.mongodb import MongoConfiguration, MongoDBConnection from ops.charm import CharmBase from ops.framework import Object from ops.model import ActiveStatus, BlockedStatus, StatusBase, WaitingStatus @@ -22,7 +22,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 3 AUTH_FAILED_CODE = 18 UNAUTHORISED_CODE = 13 @@ -223,8 +223,22 @@ def prioritize_statuses(self, statuses: Tuple) -> StatusBase: # if all statuses are active report mongodb status over sharding status return mongodb_status + def get_invalid_integration_status(self) -> Optional[StatusBase]: + """Returns a status if an invalid integration is present.""" + if not self.charm.cluster.is_valid_mongos_integration(): + return BlockedStatus( + "Relation to mongos not supported, config role must be config-server" + ) -def build_unit_status(mongodb_config: MongoDBConfiguration, unit_host: str) -> StatusBase: + if not self.charm.backups.is_valid_s3_integration(): + return BlockedStatus( + "Relation to s3-integrator is not supported, config role must be config-server" + ) + + return self.charm.get_cluster_mismatched_revision_status() + + +def build_unit_status(mongodb_config: MongoConfiguration, unit_host: str) -> StatusBase: """Generates the status of a unit based on its status reported by mongod.""" try: with MongoDBConnection(mongodb_config) as mongo: diff --git a/lib/charms/mongodb/v1/helpers.py b/lib/charms/mongodb/v1/helpers.py index 24eb70ddf..937786b89 100644 --- a/lib/charms/mongodb/v1/helpers.py +++ b/lib/charms/mongodb/v1/helpers.py @@ -10,7 +10,7 @@ import subprocess from typing import List -from charms.mongodb.v1.mongodb import MongoDBConfiguration +from charms.mongodb.v1.mongodb import MongoConfiguration from ops.model import ActiveStatus, MaintenanceStatus, StatusBase, WaitingStatus from config import Config @@ -23,7 +23,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 7 +LIBPATCH = 8 # path to store mongodb ketFile KEY_FILE = "keyFile" @@ -73,7 +73,7 @@ def _get_audit_log_settings(snap_install: bool) -> List[str]: # noinspection GrazieInspection -def get_create_user_cmd(config: MongoDBConfiguration, mongo_path=MONGO_SHELL) -> List[str]: +def get_create_user_cmd(config: MongoConfiguration, mongo_path=MONGO_SHELL) -> List[str]: """Creates initial admin user for MongoDB. Initial admin user can be created only through localhost connection. @@ -172,7 +172,7 @@ def get_mongos_args( def get_mongod_args( - config: MongoDBConfiguration, + config: MongoConfiguration, auth: bool = True, snap_install: bool = False, role: str = "replication", diff --git a/lib/charms/mongodb/v1/mongodb.py b/lib/charms/mongodb/v1/mongodb.py index 39e219122..e06fd517c 100644 --- a/lib/charms/mongodb/v1/mongodb.py +++ b/lib/charms/mongodb/v1/mongodb.py @@ -4,14 +4,11 @@ # See LICENSE file for licensing details. import logging -import re -from dataclasses import dataclass -from typing import Dict, List, Optional, Set -from urllib.parse import quote_plus +from typing import Dict, Set from bson.json_util import dumps -from pymongo import MongoClient -from pymongo.errors import OperationFailure, PyMongoError +from charms.mongodb.v0.mongo import MongoConfiguration, MongoConnection, NotReadyError +from pymongo.errors import OperationFailure from tenacity import ( RetryError, Retrying, @@ -22,8 +19,6 @@ wait_fixed, ) -from config import Config - # The unique Charmhub library identifier, never change it LIBID = "49c69d9977574dd7942eb7b54f43355b" @@ -32,7 +27,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 = 3 # path to store mongodb ketFile logger = logging.getLogger(__name__) @@ -42,59 +37,7 @@ class FailedToMovePrimaryError(Exception): """Raised when attempt to move a primary fails.""" -@dataclass -class MongoDBConfiguration: - """Class for MongoDB configuration. - - — replset: name of replica set, needed for connection URI. - — database: database name. - — username: username. - — password: password. - — hosts: full list of hosts to connect to, needed for the URI. - - tls_external: indicator for use of internal TLS connection. - - tls_internal: indicator for use of external TLS connection. - """ - - replset: str - database: Optional[str] - username: str - password: str - hosts: Set[str] - roles: Set[str] - tls_external: bool - tls_internal: bool - standalone: bool = False - - @property - def uri(self): - """Return URI concatenated from fields.""" - hosts = ",".join(self.hosts) - # Auth DB should be specified while user connects to application DB. - auth_source = "" - if self.database != "admin": - auth_source = "&authSource=admin" - - if self.standalone: - return ( - f"mongodb://{quote_plus(self.username)}:" - f"{quote_plus(self.password)}@" - f"localhost:{Config.MONGODB_PORT}/?authSource=admin" - ) - - return ( - f"mongodb://{quote_plus(self.username)}:" - f"{quote_plus(self.password)}@" - f"{hosts}/{quote_plus(self.database)}?" - f"replicaSet={quote_plus(self.replset)}" - f"{auth_source}" - ) - - -class NotReadyError(PyMongoError): - """Raised when not all replica set members healthy or finished initial sync.""" - - -class MongoDBConnection: +class MongoDBConnection(MongoConnection): """In this class we create connection object to MongoDB. Real connection is created on the first call to MongoDB. @@ -115,7 +58,7 @@ class MongoDBConnection: """ - def __init__(self, config: MongoDBConfiguration, uri=None, direct=False): + def __init__(self, config: MongoConfiguration, uri=None, direct=False): """A MongoDB client interface. Args: @@ -124,49 +67,7 @@ def __init__(self, config: MongoDBConfiguration, uri=None, direct=False): direct: force a direct connection to a specific host, avoiding reading replica set configuration and reconnection. """ - self.mongodb_config = config - - if uri is None: - uri = config.uri - - self.client = MongoClient( - uri, - directConnection=direct, - connect=False, - serverSelectionTimeoutMS=1000, - connectTimeoutMS=2000, - ) - return - - def __enter__(self): - """Return a reference to the new connection.""" - return self - - def __exit__(self, object_type, value, traceback): - """Disconnect from MongoDB client.""" - self.client.close() - self.client = None - - @property - def is_ready(self) -> bool: - """Is the MongoDB server ready for services requests. - - Returns: - True if services is ready False otherwise. Retries over a period of 60 seconds times to - allow server time to start up. - - Raises: - ConfigurationError, ConfigurationError, OperationFailure - """ - try: - for attempt in Retrying(stop=stop_after_delay(60), wait=wait_fixed(3)): - with attempt: - # The ping command is cheap and does not require auth. - self.client.admin.command("ping") - except RetryError: - return False - - return True + super().__init__(config, uri, direct) @retry( stop=stop_after_attempt(3), @@ -181,8 +82,8 @@ def init_replset(self) -> None: ConfigurationError, ConfigurationError, OperationFailure """ config = { - "_id": self.mongodb_config.replset, - "members": [{"_id": i, "host": h} for i, h in enumerate(self.mongodb_config.hosts)], + "_id": self.config.replset, + "members": [{"_id": i, "host": h} for i, h in enumerate(self.config.hosts)], } try: self.client.admin.command("replSetInitiate", config) @@ -348,107 +249,6 @@ def set_replicaset_election_priority(self, priority: int, ignore_member: str = N logger.debug("rs_config: %r", rs_config) self.client.admin.command("replSetReconfig", rs_config) - def create_user(self, config: MongoDBConfiguration): - """Create user. - - Grant read and write privileges for specified database. - """ - self.client.admin.command( - "createUser", - config.username, - pwd=config.password, - roles=self._get_roles(config), - mechanisms=["SCRAM-SHA-256"], - ) - - def update_user(self, config: MongoDBConfiguration): - """Update grants on database.""" - self.client.admin.command( - "updateUser", - config.username, - roles=self._get_roles(config), - ) - - def set_user_password(self, username, password: str): - """Update the password.""" - self.client.admin.command( - "updateUser", - username, - pwd=password, - ) - - def create_role(self, role_name: str, privileges: dict, roles: dict = []): - """Creates a new role. - - Args: - role_name: name of the role to be added. - privileges: privileges to be associated with the role. - roles: List of roles from which this role inherits privileges. - """ - try: - self.client.admin.command( - "createRole", role_name, privileges=[privileges], roles=roles - ) - except OperationFailure as e: - if not e.code == 51002: # Role already exists - logger.error("Cannot add role. error=%r", e) - raise e - - @staticmethod - def _get_roles(config: MongoDBConfiguration) -> List[dict]: - """Generate roles List.""" - supported_roles = { - "admin": [ - {"role": "userAdminAnyDatabase", "db": "admin"}, - {"role": "readWriteAnyDatabase", "db": "admin"}, - {"role": "userAdmin", "db": "admin"}, - ], - "monitor": [ - {"role": "explainRole", "db": "admin"}, - {"role": "clusterMonitor", "db": "admin"}, - {"role": "read", "db": "local"}, - ], - "backup": [ - {"db": "admin", "role": "readWrite", "collection": ""}, - {"db": "admin", "role": "backup"}, - {"db": "admin", "role": "clusterMonitor"}, - {"db": "admin", "role": "restore"}, - {"db": "admin", "role": "pbmAnyAction"}, - ], - "default": [ - {"role": "readWrite", "db": config.database}, - ], - } - return [role_dict for role in config.roles for role_dict in supported_roles[role]] - - def drop_user(self, username: str): - """Drop user.""" - self.client.admin.command("dropUser", username) - - def get_users(self) -> Set[str]: - """Add a new member to replica set config inside MongoDB.""" - users_info = self.client.admin.command("usersInfo") - return set( - [ - user_obj["user"] - for user_obj in users_info["users"] - if re.match(r"^relation-\d+$", user_obj["user"]) - ] - ) - - def get_databases(self) -> Set[str]: - """Return list of all non-default databases.""" - system_dbs = ("admin", "local", "config") - databases = self.client.list_database_names() - return set([db for db in databases if db not in system_dbs]) - - def drop_database(self, database: str): - """Drop a non-default database.""" - system_dbs = ("admin", "local", "config") - if database in system_dbs: - return - self.client.drop_database(database) - def _is_primary(self, rs_status: Dict, hostname: str) -> bool: """Returns True if passed host is the replica set primary. diff --git a/lib/charms/mongodb/v1/mongodb_provider.py b/lib/charms/mongodb/v1/mongodb_provider.py index 170716f5c..c9bd68d47 100644 --- a/lib/charms/mongodb/v1/mongodb_provider.py +++ b/lib/charms/mongodb/v1/mongodb_provider.py @@ -15,7 +15,7 @@ from charms.data_platform_libs.v0.data_interfaces import DatabaseProvides from charms.mongodb.v1.helpers import generate_password -from charms.mongodb.v1.mongodb import MongoDBConfiguration, MongoDBConnection +from charms.mongodb.v1.mongodb import MongoConfiguration, MongoDBConnection from ops.charm import CharmBase, EventBase, RelationBrokenEvent, RelationChangedEvent from ops.framework import Object from ops.model import Relation @@ -31,7 +31,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 9 +LIBPATCH = 10 logger = logging.getLogger(__name__) REL_NAME = "database" @@ -282,7 +282,7 @@ def _get_or_set_password(self, relation: Relation) -> str: self.database_provides.update_relation_data(relation.id, {"password": password}) return password - def _get_config(self, username: str, password: Optional[str]) -> MongoDBConfiguration: + def _get_config(self, username: str, password: Optional[str]) -> MongoConfiguration: """Construct the config object for future user creation.""" relation = self._get_relation_from_username(username) if not password: @@ -290,7 +290,7 @@ def _get_config(self, username: str, password: Optional[str]) -> MongoDBConfigur database_name = self._get_database_from_relation(relation) - return MongoDBConfiguration( + return MongoConfiguration( replset=self.charm.app.name, database=database_name, username=username, @@ -301,7 +301,7 @@ def _get_config(self, username: str, password: Optional[str]) -> MongoDBConfigur tls_internal=False, ) - def _set_relation(self, config: MongoDBConfiguration): + def _set_relation(self, config: MongoConfiguration): """Save all output fields into application relation.""" relation = self._get_relation_from_username(config.username) if relation is None: diff --git a/lib/charms/mongodb/v1/mongos.py b/lib/charms/mongodb/v1/mongos.py index cb3d8fb64..fc0e0bacd 100644 --- a/lib/charms/mongodb/v1/mongos.py +++ b/lib/charms/mongodb/v1/mongos.py @@ -1,15 +1,13 @@ -"""Code for interactions with MongoS.""" +"""Code for interactions with MongoDB.""" # Copyright 2024 Canonical Ltd. # See LICENSE file for licensing details. import logging -from dataclasses import dataclass from typing import List, Optional, Set, Tuple -from urllib.parse import quote_plus -from charms.mongodb.v1.mongodb import NotReadyError -from pymongo import MongoClient, collection +from charms.mongodb.v0.mongo import MongoConfiguration, MongoConnection, NotReadyError +from pymongo import collection from tenacity import RetryError, Retrying, stop_after_delay, wait_fixed from config import Config @@ -22,7 +20,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 6 +LIBPATCH = 7 # path to store mongodb ketFile logger = logging.getLogger(__name__) @@ -30,51 +28,6 @@ SHARD_AWARE_STATE = 1 -@dataclass -class MongosConfiguration: - """Class for mongos configuration. - - — database: database name. - — username: username. - — password: password. - — hosts: full list of hosts to connect to, needed for the URI. - - port: integer for the port to connect to connect to mongodb. - - tls_external: indicator for use of internal TLS connection. - - tls_internal: indicator for use of external TLS connection. - """ - - database: Optional[str] - username: str - password: str - hosts: Set[str] - port: int - roles: Set[str] - tls_external: bool - tls_internal: bool - - @property - def uri(self): - """Return URI concatenated from fields.""" - self.complete_hosts = self.hosts - - # mongos using Unix Domain Socket to communicate do not use port - if self.port: - self.complete_hosts = [f"{host}:{self.port}" for host in self.hosts] - - complete_hosts = ",".join(self.complete_hosts) - - # Auth DB should be specified while user connects to application DB. - auth_source = "" - if self.database != "admin": - auth_source = "authSource=admin" - return ( - f"mongodb://{quote_plus(self.username)}:" - f"{quote_plus(self.password)}@" - f"{complete_hosts}/{quote_plus(self.database)}?" - f"{auth_source}" - ) - - class NotEnoughSpaceError(Exception): """Raised when there isn't enough space to movePrimary.""" @@ -95,7 +48,7 @@ class BalancerNotEnabledError(Exception): """Raised when balancer process is not enabled.""" -class MongosConnection: +class MongosConnection(MongoConnection): """In this class we create connection object to Mongos. Real connection is created on the first call to Mongos. @@ -116,7 +69,7 @@ class MongosConnection: """ - def __init__(self, config: MongosConfiguration, uri=None, direct=False): + def __init__(self, config: MongoConfiguration, uri=None, direct=False): """A MongoDB client interface. Args: @@ -125,26 +78,7 @@ def __init__(self, config: MongosConfiguration, uri=None, direct=False): direct: force a direct connection to a specific host, avoiding reading replica set configuration and reconnection. """ - if uri is None: - uri = config.uri - - self.client = MongoClient( - uri, - directConnection=direct, - connect=False, - serverSelectionTimeoutMS=1000, - connectTimeoutMS=2000, - ) - return - - def __enter__(self): - """Return a reference to the new connection.""" - return self - - def __exit__(self, object_type, value, traceback): - """Disconnect from MongoDB client.""" - self.client.close() - self.client = None + super().__init__(config, uri, direct) def get_shard_members(self) -> Set[str]: """Gets shard members. diff --git a/lib/charms/mongodb/v1/shards_interface.py b/lib/charms/mongodb/v1/shards_interface.py index 63e727029..a0e60b55e 100644 --- a/lib/charms/mongodb/v1/shards_interface.py +++ b/lib/charms/mongodb/v1/shards_interface.py @@ -16,12 +16,7 @@ DatabaseRequires, ) from charms.mongodb.v1.helpers import KEY_FILE -from charms.mongodb.v1.mongodb import ( - MongoDBConnection, - NotReadyError, - OperationFailure, - PyMongoError, -) +from charms.mongodb.v1.mongodb import MongoDBConnection, NotReadyError, OperationFailure from charms.mongodb.v1.mongodb_provider import REL_NAME from charms.mongodb.v1.mongos import ( BalancerNotEnabledError, @@ -47,7 +42,7 @@ StatusBase, WaitingStatus, ) -from pymongo.errors import ServerSelectionTimeoutError +from pymongo.errors import PyMongoError, ServerSelectionTimeoutError from tenacity import Retrying, stop_after_delay, wait_fixed from config import Config @@ -63,7 +58,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 8 +LIBPATCH = 9 KEYFILE_KEY = "key-file" HOSTS_KEY = "host" diff --git a/src/charm.py b/src/charm.py index 46d2b16c8..6b88cc2b9 100755 --- a/src/charm.py +++ b/src/charm.py @@ -22,14 +22,13 @@ get_mongos_args, ) from charms.mongodb.v1.mongodb import ( - MongoDBConfiguration, + MongoConfiguration, MongoDBConnection, NotReadyError, ) from charms.mongodb.v1.mongodb_backups import MongoDBBackups from charms.mongodb.v1.mongodb_provider import MongoDBProvider from charms.mongodb.v1.mongodb_tls import MongoDBTLS -from charms.mongodb.v1.mongos import MongosConfiguration from charms.mongodb.v1.shards_interface import ConfigServerRequirer, ShardingProvider from charms.mongodb.v1.users import ( CHARM_USERS, @@ -172,29 +171,29 @@ def peers_units(self) -> list[Unit]: return self._peers.units @property - def mongodb_config(self) -> MongoDBConfiguration: + def mongodb_config(self) -> MongoConfiguration: """Create a configuration object with settings. Needed for correct handling interactions with MongoDB. Returns: - A MongoDBConfiguration object + A MongoConfiguration object """ return self._get_mongodb_config_for_user(OperatorUser, self.app_hosts) @property - def mongos_config(self) -> MongoDBConfiguration: - """Generates a MongoDBConfiguration object for mongos in the deployment of MongoDB.""" + def mongos_config(self) -> MongoConfiguration: + """Generates a MongoConfiguration object for mongos in the deployment of MongoDB.""" return self._get_mongos_config_for_user(OperatorUser, set(self.app_hosts)) @property - def monitor_config(self) -> MongoDBConfiguration: - """Generates a MongoDBConfiguration object for this deployment of MongoDB.""" + def monitor_config(self) -> MongoConfiguration: + """Generates a MongoConfiguration object for this deployment of MongoDB.""" return self._get_mongodb_config_for_user(MonitorUser, [self.unit_host(self.unit)]) @property - def backup_config(self) -> MongoDBConfiguration: - """Generates a MongoDBConfiguration object for backup.""" + def backup_config(self) -> MongoConfiguration: + """Generates a MongoConfiguration object for backup.""" return self._get_mongodb_config_for_user(BackupUser, [self.unit_host(self.unit)]) @property @@ -466,14 +465,14 @@ def primary(self) -> str | None: # END: properties # BEGIN: generic helper methods - def remote_mongos_config(self, hosts) -> MongosConfiguration: - """Generates a MongosConfiguration object for mongos in the deployment of MongoDB.""" + def remote_mongos_config(self, hosts) -> MongoConfiguration: + """Generates a MongoConfiguration object for mongos in the deployment of MongoDB.""" # mongos that are part of the cluster have the same username and password, but different # hosts return self._get_mongos_config_for_user(OperatorUser, hosts) - def remote_mongodb_config(self, hosts, replset=None, standalone=None) -> MongoDBConfiguration: - """Generates a MongoDBConfiguration object for mongod in the deployment of MongoDB.""" + def remote_mongodb_config(self, hosts, replset=None, standalone=None) -> MongoConfiguration: + """Generates a MongoConfiguration object for mongod in the deployment of MongoDB.""" # mongos that are part of the cluster have the same username and password, but different # hosts return self._get_mongodb_config_for_user( @@ -987,7 +986,7 @@ def _set_user_created(self, user: MongoDBUser) -> None: def _get_mongodb_config_for_user( self, user: MongoDBUser, hosts: List[str] - ) -> MongoDBConfiguration: + ) -> MongoConfiguration: external_ca, _ = self.tls.get_tls_files(internal=False) internal_ca, _ = self.tls.get_tls_files(internal=True) password = self.get_secret(APP_SCOPE, user.get_password_key_name()) @@ -996,7 +995,7 @@ def _get_mongodb_config_for_user( f"Password for '{APP_SCOPE}', '{user.get_username()}' couldn't be retrieved" ) else: - return MongoDBConfiguration( + return MongoConfiguration( replset=self.app.name, database=user.get_database_name(), username=user.get_username(), @@ -1009,11 +1008,11 @@ def _get_mongodb_config_for_user( def _get_mongos_config_for_user( self, user: MongoDBUser, hosts: Set[str] - ) -> MongosConfiguration: + ) -> MongoConfiguration: external_ca, _ = self.tls.get_tls_files(internal=False) internal_ca, _ = self.tls.get_tls_files(internal=True) - return MongosConfiguration( + return MongoConfiguration( database=user.get_database_name(), username=user.get_username(), password=self.get_secret(APP_SCOPE, user.get_password_key_name()), diff --git a/src/config.py b/src/config.py index 3b4bef334..71ea485be 100644 --- a/src/config.py +++ b/src/config.py @@ -125,6 +125,12 @@ class Secrets: SECRET_KEYFILE_NAME = "keyfile" MAX_PASSWORD_LENGTH = 4096 + class Substrate: + """Substrate related constants.""" + + VM = "vm" + K8S = "k8s" + class Status: """Status related constants. diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 4db014275..951616a06 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -496,7 +496,7 @@ def test_reconfigure_not_already_initialised(self, connection, defer): @patch("ops.framework.EventBase.defer") @patch("charm.MongoDBConnection") - @patch("charms.mongodb.v1.mongodb.MongoClient") + @patch("charms.mongodb.v0.mongo.MongoClient") def test_reconfigure_get_members_failure(self, client, connection, defer): """Tests reconfigure does not execute when unable to get the replica set members. @@ -938,7 +938,7 @@ def test__connect_mongodb_exporter_success( password = self.harness.charm.get_secret("app", "monitor-password") - uri_template = "mongodb://monitor:{password}@mongodb-k8s-0.mongodb-k8s-endpoints/admin?replicaSet=mongodb-k8s" + uri_template = "mongodb://monitor:{password}@mongodb-k8s-0.mongodb-k8s-endpoints:27017/admin?replicaSet=mongodb-k8s" expected_config = { "override": "replace", diff --git a/tests/unit/test_mongodb_lib.py b/tests/unit/test_mongodb_lib.py index da50d4188..2e6b3970b 100644 --- a/tests/unit/test_mongodb_lib.py +++ b/tests/unit/test_mongodb_lib.py @@ -24,9 +24,9 @@ class TestMongoServer(unittest.TestCase): - @patch("charms.mongodb.v1.mongodb.Retrying") - @patch("charms.mongodb.v1.mongodb.MongoClient") - @patch("charms.mongodb.v1.mongodb.MongoDBConfiguration") + @patch("charms.mongodb.v0.mongo.Retrying") + @patch("charms.mongodb.v0.mongo.MongoClient") + @patch("charms.mongodb.v0.mongo.MongoConfiguration") def test_is_ready_error_handling(self, config, mock_client, retrying): """Test failure to check ready of replica returns False. @@ -42,8 +42,8 @@ def test_is_ready_error_handling(self, config, mock_client, retrying): # verify we close connection (mock_client.return_value.close).assert_called() - @patch("charms.mongodb.v1.mongodb.MongoClient") - @patch("charms.mongodb.v1.mongodb.MongoDBConfiguration") + @patch("charms.mongodb.v0.mongo.MongoClient") + @patch("charms.mongodb.v1.mongodb.MongoConfiguration") def test_init_replset_error_handling(self, config, mock_client): """Test failure to initialise replica set raises an error. @@ -60,8 +60,8 @@ def test_init_replset_error_handling(self, config, mock_client): # verify we close connection (mock_client.return_value.close).assert_called() - @patch("charms.mongodb.v1.mongodb.MongoClient") - @patch("charms.mongodb.v1.mongodb.MongoDBConfiguration") + @patch("charms.mongodb.v0.mongo.MongoClient") + @patch("charms.mongodb.v1.mongodb.MongoConfiguration") def test_get_replset_members_error_handling(self, config, mock_client): """Test failure to get replica set members raises an error. @@ -76,8 +76,8 @@ def test_get_replset_members_error_handling(self, config, mock_client): # verify we close connection (mock_client.return_value.close).assert_called() - @patch("charms.mongodb.v1.mongodb.MongoClient") - @patch("charms.mongodb.v1.mongodb.MongoDBConfiguration") + @patch("charms.mongodb.v0.mongo.MongoClient") + @patch("charms.mongodb.v1.mongodb.MongoConfiguration") def test_add_replset_members_pymongo_error_handling(self, config, mock_client): """Test failures related to PyMongo properly get handled in add_replset_member. @@ -94,8 +94,8 @@ def test_add_replset_members_pymongo_error_handling(self, config, mock_client): (mock_client.return_value.close).assert_called() @patch("charms.mongodb.v1.mongodb.MongoDBConnection.is_any_sync") - @patch("charms.mongodb.v1.mongodb.MongoClient") - @patch("charms.mongodb.v1.mongodb.MongoDBConfiguration") + @patch("charms.mongodb.v0.mongo.MongoClient") + @patch("charms.mongodb.v1.mongodb.MongoConfiguration") def test_add_replset_member_wait_to_sync(self, config, mock_client, any_sync): """Tests that adding replica set members raises NotReadyError if another member is syncing. @@ -114,8 +114,8 @@ def test_add_replset_member_wait_to_sync(self, config, mock_client, any_sync): no_reconfig = call("replSetReconfig") not in actual_calls self.assertEqual(no_reconfig, True) - @patch("charms.mongodb.v1.mongodb.MongoClient") - @patch("charms.mongodb.v1.mongodb.MongoDBConfiguration") + @patch("charms.mongodb.v0.mongo.MongoClient") + @patch("charms.mongodb.v1.mongodb.MongoConfiguration") def test_remove_replset_members_pymongo_error_handling(self, config, mock_client): """Test failures related to PyMongo properly get handled in remove_replset_member. @@ -133,8 +133,8 @@ def test_remove_replset_members_pymongo_error_handling(self, config, mock_client (mock_client.return_value.close).assert_called() @patch("charms.mongodb.v1.mongodb.MongoDBConnection._is_any_removing") - @patch("charms.mongodb.v1.mongodb.MongoClient") - @patch("charms.mongodb.v1.mongodb.MongoDBConfiguration") + @patch("charms.mongodb.v0.mongo.MongoClient") + @patch("charms.mongodb.v1.mongodb.MongoConfiguration") def test_remove_replset_member_wait_to_remove(self, config, mock_client, any_remove): """Tests removing replica set members raises NotReadyError if another member is removing. @@ -155,8 +155,8 @@ def test_remove_replset_member_wait_to_remove(self, config, mock_client, any_rem self.assertEqual(no_reconfig, True) @patch("charms.mongodb.v1.mongodb.MongoDBConnection._is_any_removing") - @patch("charms.mongodb.v1.mongodb.MongoClient") - @patch("charms.mongodb.v1.mongodb.MongoDBConfiguration") + @patch("charms.mongodb.v0.mongo.MongoClient") + @patch("charms.mongodb.v1.mongodb.MongoConfiguration") def test_create_user_error_handling(self, config, mock_client, any_remove): """Test failures related to PyMongo properly get handled when creating a user. @@ -171,8 +171,8 @@ def test_create_user_error_handling(self, config, mock_client, any_remove): # verify we close connection (mock_client.return_value.close).assert_called() - @patch("charms.mongodb.v1.mongodb.MongoClient") - @patch("charms.mongodb.v1.mongodb.MongoDBConfiguration") + @patch("charms.mongodb.v0.mongo.MongoClient") + @patch("charms.mongodb.v1.mongodb.MongoConfiguration") def test_update_user_error_handling(self, config, mock_client): """Test failures related to PyMongo properly get handled when updating a user. @@ -187,8 +187,8 @@ def test_update_user_error_handling(self, config, mock_client): # verify we close connection (mock_client.return_value.close).assert_called() - @patch("charms.mongodb.v1.mongodb.MongoClient") - @patch("charms.mongodb.v1.mongodb.MongoDBConfiguration") + @patch("charms.mongodb.v0.mongo.MongoClient") + @patch("charms.mongodb.v1.mongodb.MongoConfiguration") def test_drop_user_error_handling(self, config, mock_client): """Test failures related to PyMongo properly get handled when dropping a user. From ba4e8d4735eae233a0c6047e40beac6a8fd1ff3b Mon Sep 17 00:00:00 2001 From: Neha Oudin Date: Fri, 23 Aug 2024 13:19:28 +0200 Subject: [PATCH 3/5] Revert "feat: Check data permissions before restarting mongod" This reverts commit 0cf2152849555a1ede1d24a01d42366e03606420. --- src/charm.py | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/src/charm.py b/src/charm.py index 6b88cc2b9..1df241039 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1204,14 +1204,6 @@ def restart_charm_services(self): container = self.unit.get_container(Config.CONTAINER_NAME) container.stop(Config.SERVICE_NAME) - # Ensure the permissions are right before restarting the service. - try: - self._set_data_dir_permissions(container, all_files=True) - - except (PathError, ProtocolError, MissingSecretError) as e: - logger.error("Cannot initialize workload: %r", e) - raise - container.add_layer("mongod", self._mongod_layer, combine=True) if self.is_role(Config.Role.CONFIG_SERVER): container.add_layer("mongos", self._mongos_layer, combine=True) @@ -1532,33 +1524,21 @@ def _pull_licenses(container: Container) -> None: pass @staticmethod - def _set_data_dir_permissions(container: Container, all_files: bool = False) -> None: + def _set_data_dir_permissions(container: Container) -> None: """Ensure the data directory for mongodb is writable for the "mongodb" user. Until the ability to set fsGroup and fsGroupChangePolicy via Pod securityContext is available, we fix permissions incorrectly with chown. - We want to check the directory and the files inside in case it got messed up. """ for path in [Config.DATA_DIR, Config.LOG_DIR, Config.LogRotate.LOG_STATUS_DIR]: paths = container.list_files(path, itself=True) - logger.debug(f"Data directory ownership: {paths[0].user}:{paths[0].group}") assert len(paths) == 1, "list_files doesn't return only the directory itself" - + logger.debug(f"Data directory ownership: {paths[0].user}:{paths[0].group}") if paths[0].user != Config.UNIX_USER or paths[0].group != Config.UNIX_GROUP: container.exec( f"chown {Config.UNIX_USER}:{Config.UNIX_GROUP} -R {path}".split(" ") ) - if all_files: - files = container.list_files(path) - if any( - file.user != Config.UNIX_USER or file.group != Config.UNIX_GROUP - for file in files - ): - container.exec( - f"chown {Config.UNIX_USER}:{Config.UNIX_GROUP} -R {path}".split(" ") - ) - # END: static methods From 10c617391b15d7fe1156499cb492e5bb9ce63d1d Mon Sep 17 00:00:00 2001 From: Neha Oudin Date: Fri, 23 Aug 2024 15:13:32 +0200 Subject: [PATCH 4/5] feat: Defer ready event until the storage is ready Also update the mongodb tls library --- lib/charms/mongodb/v1/mongodb_tls.py | 11 ++++++++++- src/charm.py | 5 +++++ tests/unit/test_charm.py | 26 +++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/lib/charms/mongodb/v1/mongodb_tls.py b/lib/charms/mongodb/v1/mongodb_tls.py index f911e2358..d78df5d13 100644 --- a/lib/charms/mongodb/v1/mongodb_tls.py +++ b/lib/charms/mongodb/v1/mongodb_tls.py @@ -38,7 +38,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 = 3 logger = logging.getLogger(__name__) @@ -159,6 +159,10 @@ def _on_tls_relation_joined(self, event: RelationJoinedEvent) -> None: def _on_tls_relation_broken(self, event: RelationBrokenEvent) -> None: """Disable TLS when TLS relation broken.""" logger.debug("Disabling external and internal TLS for unit: %s", self.charm.unit.name) + if not self.charm.db_initialised: + logger.info("Deferring %s. db is not initialised.", str(type(event))) + event.defer() + return if self.charm.upgrade_in_progress: logger.warning( "Disabling TLS is not supported during an upgrade. The charm may be in a broken, unrecoverable state." @@ -188,6 +192,11 @@ def _on_certificate_available(self, event: CertificateAvailableEvent) -> None: event.defer() return + if not self.charm.db_initialised: + logger.info("Deferring %s. db is not initialised.", str(type(event))) + event.defer() + return + int_csr = self.get_tls_secret(internal=True, label_name=Config.TLS.SECRET_CSR_LABEL) ext_csr = self.get_tls_secret(internal=False, label_name=Config.TLS.SECRET_CSR_LABEL) diff --git a/src/charm.py b/src/charm.py index 1df241039..3c796fd2f 100755 --- a/src/charm.py +++ b/src/charm.py @@ -526,6 +526,11 @@ def _on_mongod_pebble_ready(self, event) -> None: event.defer() return + if any(not storage for storage in self.model.storages.values()): + logger.debug("Storages are not attached yet") + event.defer() + return + try: # mongod needs keyFile and TLS certificates on filesystem self.push_tls_certificate_to_workload() diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 951616a06..dc7e12257 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -98,6 +98,8 @@ def test_mongod_pebble_ready(self, connect_exporter, fix_data_dir, defer, pull_l }, }, } + # Mock storages + self.harness.charm.model._storages = {"mongodb": "valid", "mongodb-logs": "valid"} # Get the mongod container from the model container = self.harness.model.unit.get_container("mongod") self.harness.set_can_connect(container, True) @@ -183,6 +185,22 @@ def test_pebble_ready_push_keyfile_to_workload_failure(self, push_keyfile_to_wor mock_container.replan.assert_not_called() defer.assert_called() + @patch("ops.framework.EventBase.defer") + def test_pebble_ready_no_storage_yet(self, defer): + """Test to ensure that the pebble ready event is deferred until the storage is ready.""" + # presets + mock_container = mock.Mock() + mock_container.return_value.can_connect.return_value = True + self.harness.charm.unit.get_container = mock_container + + # Mock storages + self.harness.charm.model._storages = {"mongodb": None, "mongodb-logs": None} + # Emit the PebbleReadyEvent carrying the mock_container + self.harness.charm.on.mongod_pebble_ready.emit(mock_container) + mock_container.add_layer.assert_not_called() + mock_container.replan.assert_not_called() + defer.assert_called() + @patch("ops.framework.EventBase.defer") @patch("charm.MongoDBProvider") @patch("charm.MongoDBCharm._init_operator_user") @@ -814,8 +832,11 @@ def test_on_other_secret_changed(self, scope, connect_exporter): connect_exporter.assert_not_called() @patch("charm.MongoDBConnection") + @patch("charm.MongoDBCharm._pull_licenses") @patch("charm.MongoDBCharm._connect_mongodb_exporter") - def test_connect_to_mongo_exporter_on_set_password(self, connect_exporter, connection): + def test_connect_to_mongo_exporter_on_set_password( + self, connect_exporter, pull_licenses, connection + ): """Test _connect_mongodb_exporter is called when the password is set for 'montior' user.""" container = self.harness.model.unit.get_container("mongod") self.harness.set_can_connect(container, True) @@ -927,6 +948,9 @@ def test__connect_mongodb_exporter_success( self, connection, fix_data_dir, defer, pull_licenses ): """Tests the _connect_mongodb_exporter method has been called.""" + # Mock storages + self.harness.charm.model._storages = {"mongodb": "valid", "mongodb-logs": "valid"} + # Get container container = self.harness.model.unit.get_container("mongod") self.harness.set_can_connect(container, True) container.exec = mock.Mock() From 2536a52ba56ea271b69d8f6b714249a72b6e2d91 Mon Sep 17 00:00:00 2001 From: Neha Oudin Date: Mon, 26 Aug 2024 09:13:58 +0200 Subject: [PATCH 5/5] doc: Add comment --- src/charm.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/charm.py b/src/charm.py index 3c796fd2f..9113fd412 100755 --- a/src/charm.py +++ b/src/charm.py @@ -526,6 +526,9 @@ def _on_mongod_pebble_ready(self, event) -> None: event.defer() return + # We need to check that the storages are attached before starting the services. + # pebble-ready is not guaranteed to run after storage-attached so this check allows + # to ensure that the storages are attached before the pebble-ready hook is run. if any(not storage for storage in self.model.storages.values()): logger.debug("Storages are not attached yet") event.defer()