From 8e2883490b32213b6928c3e217572247f935a807 Mon Sep 17 00:00:00 2001 From: Dmitry Ratushnyy <132273757+dmitry-ratushnyy@users.noreply.github.com> Date: Thu, 22 Jun 2023 06:58:01 +0200 Subject: [PATCH] [DPE-2146] Refactoring: extract charm configuration (#165) * Add more logging to tests * Refactoring: extracted configuration to file --- src/charm.py | 116 +++++++++++++++-------------------- src/config.py | 66 ++++++++++++++++++++ tests/integration/helpers.py | 1 + 3 files changed, 118 insertions(+), 65 deletions(-) create mode 100644 src/config.py diff --git a/src/charm.py b/src/charm.py index a905e2a39..7941726b2 100755 --- a/src/charm.py +++ b/src/charm.py @@ -16,14 +16,6 @@ from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider from charms.loki_k8s.v0.loki_push_api import LogProxyConsumer from charms.mongodb.v0.helpers import ( - CONF_DIR, - DATA_DIR, - KEY_FILE, - MONGODB_LOG_FILENAME, - TLS_EXT_CA_FILE, - TLS_EXT_PEM_FILE, - TLS_INT_CA_FILE, - TLS_INT_PEM_FILE, generate_keyfile, generate_password, get_create_user_cmd, @@ -45,18 +37,10 @@ from pymongo.errors import PyMongoError from tenacity import before_log, retry, stop_after_attempt, wait_fixed +from config import Config from exceptions import AdminUserCreationError logger = logging.getLogger(__name__) -PEER = "database-peers" -MONITOR_PRIVILEGES = { - "resource": {"db": "", "collection": ""}, - "actions": ["listIndexes", "listCollections", "dbStats", "dbHash", "collStats", "find"], -} -UNIX_USER = "mongodb" -UNIX_GROUP = "mongodb" -MONGODB_EXPORTER_PORT = 9216 -REL_NAME = "database" class MongoDBCharm(CharmBase): @@ -68,25 +52,25 @@ def __init__(self, *args): self.framework.observe(self.on.mongod_pebble_ready, self.on_mongod_pebble_ready) self.framework.observe(self.on.start, self._on_start) self.framework.observe(self.on.leader_elected, self._reconfigure) - self.framework.observe(self.on[PEER].relation_changed, self._reconfigure) - self.framework.observe(self.on[PEER].relation_departed, self._reconfigure) + self.framework.observe(self.on[Config.Relations.PEERS].relation_changed, self._reconfigure) + self.framework.observe( + self.on[Config.Relations.PEERS].relation_departed, self._reconfigure + ) self.framework.observe(self.on.get_password_action, self._on_get_password) self.framework.observe(self.on.set_password_action, self._on_set_password) self.client_relations = MongoDBProvider(self) - self.tls = MongoDBTLS(self, PEER) + self.tls = MongoDBTLS(self, Config.Relations.PEERS) self.metrics_endpoint = MetricsEndpointProvider( - self, - refresh_event=self.on.start, - jobs=[{"static_configs": [{"targets": [f"*:{MONGODB_EXPORTER_PORT}"]}]}], + self, refresh_event=self.on.start, jobs=Config.Monitoring.JOBS ) self.grafana_dashboards = GrafanaDashboardProvider(self) self.loki_push = LogProxyConsumer( self, - log_files=[f"{DATA_DIR}/{MONGODB_LOG_FILENAME}"], - relation_name="logging", - container_name="mongod", + log_files=Config.LOG_FILES, + relation_name=Config.Relations.LOGGING, + container_name=Config.CONTAINER_NAME, ) # BEGIN: properties @@ -99,7 +83,7 @@ def mongodb_config(self) -> MongoDBConfiguration: Returns: A MongoDBConfiguration object """ - peers = self.model.get_relation(PEER) + peers = self.model.get_relation(Config.Relations.PEERS) hosts = [self.get_hostname_by_unit(self.unit.name)] + [ self.get_hostname_by_unit(unit.name) for unit in peers.units ] @@ -124,8 +108,8 @@ def _monitor_layer(self) -> Layer: "summary": "mongodb_exporter", "command": "mongodb_exporter --collector.diagnosticdata --compatible-mode", "startup": "enabled", - "user": UNIX_USER, - "group": UNIX_GROUP, + "user": Config.UNIX_USER, + "group": Config.UNIX_GROUP, "environment": {"MONGODB_URI": self.monitor_config.uri}, } }, @@ -144,8 +128,8 @@ def _mongod_layer(self) -> Layer: "summary": "mongod", "command": "mongod " + get_mongod_args(self.mongodb_config), "startup": "enabled", - "user": UNIX_USER, - "group": UNIX_GROUP, + "user": Config.UNIX_USER, + "group": Config.UNIX_GROUP, } }, } @@ -154,7 +138,7 @@ def _mongod_layer(self) -> Layer: @property def unit_peer_data(self) -> Dict: """Peer relation data object.""" - relation = self.model.get_relation(PEER) + relation = self.model.get_relation(Config.Relations.PEERS) if relation is None: return {} @@ -163,7 +147,7 @@ def unit_peer_data(self) -> Dict: @property def app_peer_data(self) -> Dict: """Peer relation data object.""" - relation = self.model.get_relation(PEER) + relation = self.model.get_relation(Config.Relations.PEERS) if relation is None: return {} @@ -188,7 +172,7 @@ def _db_initialised(self, value): def on_mongod_pebble_ready(self, event) -> None: """Configure MongoDB pebble layer specification.""" # Get a reference the container attribute - container = self.unit.get_container("mongod") + container = self.unit.get_container(Config.CONTAINER_NAME) # mongod needs keyFile and TLS certificates on filesystem if not container.can_connect(): @@ -212,13 +196,13 @@ def on_mongod_pebble_ready(self, event) -> None: # mongod cmd line arguments (returned from get_mongod_args). # In the second case, we should restart mongod # service only if arguments changed. - services = container.get_services("mongod") - if services and services["mongod"].is_running(): + services = container.get_services(Config.SERVICE_NAME) + if services and services[Config.SERVICE_NAME].is_running(): new_command = "mongod " + get_mongod_args(self.mongodb_config) - cur_command = container.get_plan().services["mongod"].command + cur_command = container.get_plan().services[Config.SERVICE_NAME].command if new_command != cur_command: logger.debug("restart MongoDB due to arguments change: %s", new_command) - container.stop("mongod") + container.stop(Config.SERVICE_NAME) # Add initial Pebble config layer using the Pebble API container.add_layer("mongod", self._mongod_layer, combine=True) @@ -252,13 +236,13 @@ def _on_start(self, event) -> None: if not self.unit.is_leader(): return - container = self.unit.get_container("mongod") + container = self.unit.get_container(Config.CONTAINER_NAME) if not container.can_connect(): logger.debug("mongod container is not ready yet.") event.defer() return - if not container.exists("/tmp/mongodb-27017.sock"): + if not container.exists(Config.SOCKET_PATH): logger.debug("The mongod socket is not ready yet.") event.defer() return @@ -359,7 +343,7 @@ def _on_get_password(self, event: ActionEvent) -> None: if not username: return key_name = MongoDBUser.get_password_key_name_for_user(username) - event.set_results({"password": self.get_secret("app", key_name)}) + event.set_results({Config.Actions.PASSWORD_PARAM_NAME: self.get_secret("app", key_name)}) def _on_set_password(self, event: ActionEvent) -> None: """Set the password for the specified user.""" @@ -374,13 +358,13 @@ def _on_set_password(self, event: ActionEvent) -> None: if not username: return - new_password = event.params.get("password", generate_password()) + new_password = event.params.get(Config.Actions.PASSWORD_PARAM_NAME, generate_password()) if new_password == self.get_secret( "app", MonitorUser.get_password_key_name_for_user(username) ): event.log("The old and new passwords are equal.") - event.set_results({"password": new_password}) + event.set_results({Config.Actions.PASSWORD_PARAM_NAME: new_password}) return with MongoDBConnection(self.mongodb_config) as mongo: @@ -400,7 +384,7 @@ def _on_set_password(self, event: ActionEvent) -> None: if username == MonitorUser.get_username(): self._connect_mongodb_exporter() - event.set_results({"password": new_password}) + event.set_results({Config.Actions.PASSWORD_PARAM_NAME: new_password}) # END: actions @@ -493,7 +477,7 @@ def _get_mongodb_config_for_user( def _get_user_or_fail_event(self, event: ActionEvent, default_username: str) -> Optional[str]: """Returns MongoDBUser object or raises ActionFail if user doesn't exist.""" - username = event.params.get("username", default_username) + username = event.params.get(Config.Actions.USERNAME_PARAM_NAME, default_username) if username not in CHARM_USERS: event.fail( f"The action can be run only for users used by the charm:" @@ -522,7 +506,7 @@ def _generate_secrets(self) -> None: def _update_app_relation_data(self, database_users): """Helper function to update application relation data.""" - for relation in self.model.relations[REL_NAME]: + for relation in self.model.relations[Config.Relations.NAME]: username = self.client_relations._get_username_from_relation_id(relation.id) password = relation.data[self.app]["password"] if username in database_users: @@ -561,12 +545,12 @@ def set_secret(self, scope: str, key: str, value: Optional[str]) -> None: def _push_keyfile_to_workload(self, container: Container) -> None: """Upload the keyFile to a workload container.""" container.push( - CONF_DIR + "/" + KEY_FILE, + Config.CONF_DIR + "/" + Config.TLS.KEY_FILE_NAME, self.get_secret("app", "keyfile"), make_dirs=True, permissions=0o400, - user=UNIX_USER, - group=UNIX_GROUP, + user=Config.UNIX_USER, + group=Config.UNIX_GROUP, ) def _push_certificate_to_workload(self, container: Container) -> None: @@ -575,44 +559,44 @@ def _push_certificate_to_workload(self, container: Container) -> None: if external_ca is not None: logger.debug("Uploading external ca to workload container") container.push( - CONF_DIR + "/" + TLS_EXT_CA_FILE, + Config.CONF_DIR + "/" + Config.TLS.EXT_CA_FILE, external_ca, make_dirs=True, permissions=0o400, - user=UNIX_USER, - group=UNIX_GROUP, + user=Config.UNIX_USER, + group=Config.UNIX_GROUP, ) if external_pem is not None: logger.debug("Uploading external pem to workload container") container.push( - CONF_DIR + "/" + TLS_EXT_PEM_FILE, + Config.CONF_DIR + "/" + Config.TLS.EXT_PEM_FILE, external_pem, make_dirs=True, permissions=0o400, - user=UNIX_USER, - group=UNIX_GROUP, + user=Config.UNIX_USER, + group=Config.UNIX_GROUP, ) internal_ca, internal_pem = self.tls.get_tls_files("app") if internal_ca is not None: logger.debug("Uploading internal ca to workload container") container.push( - CONF_DIR + "/" + TLS_INT_CA_FILE, + Config.CONF_DIR + "/" + Config.TLS.INT_CA_FILE, internal_ca, make_dirs=True, permissions=0o400, - user=UNIX_USER, - group=UNIX_GROUP, + user=Config.UNIX_USER, + group=Config.UNIX_GROUP, ) if internal_pem is not None: logger.debug("Uploading internal pem to workload container") container.push( - CONF_DIR + "/" + TLS_INT_PEM_FILE, + Config.CONF_DIR + "/" + Config.TLS.INT_PEM_FILE, internal_pem, make_dirs=True, permissions=0o400, - user=UNIX_USER, - group=UNIX_GROUP, + user=Config.UNIX_USER, + group=Config.UNIX_GROUP, ) def get_hostname_by_unit(self, unit_name: str) -> str: @@ -660,7 +644,7 @@ def _pull_licenses(container: Container) -> None: for license_name in licenses: try: - license_file = container.pull(path=f"/licenses/LICENSE-{license_name}") + license_file = container.pull(path=Config.get_license_path(license_name)) f = open("LICENSE", "x") f.write(str(license_file.read())) f.close() @@ -674,11 +658,13 @@ def _fix_data_dir(container: Container) -> None: Until the ability to set fsGroup and fsGroupChangePolicy via Pod securityContext is available, we fix permissions incorrectly with chown. """ - paths = container.list_files(DATA_DIR, itself=True) + paths = container.list_files(Config.DATA_DIR, 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}") - if paths[0].user != UNIX_USER or paths[0].group != UNIX_GROUP: - container.exec(f"chown {UNIX_USER}:{UNIX_GROUP} -R {DATA_DIR}".split(" ")) + 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 {Config.DATA_DIR}".split(" ") + ) # END: static methods diff --git a/src/config.py b/src/config.py new file mode 100644 index 000000000..82989ce94 --- /dev/null +++ b/src/config.py @@ -0,0 +1,66 @@ +"""Configuration for MongoDB Charm.""" +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + + +class Config: + """Configuration for MongoDB Charm.""" + + MONGODB_PORT = 27017 + UNIX_USER = "mongodb" + UNIX_GROUP = "mongodb" + DATA_DIR = "/var/lib/mongodb" + CONF_DIR = "/etc/mongod" + MONGODB_LOG_FILENAME = "mongodb.log" + LOG_FILES = [f"{DATA_DIR}/{MONGODB_LOG_FILENAME}"] + LICENSE_PATH = "/licenses/LICENSE" + CONTAINER_NAME = "mongod" + SERVICE_NAME = "mongod" + SOCKET_PATH = "/tmp/mongodb-27017.sock" + + class Actions: + """Actions related config for MongoDB Charm.""" + + PASSWORD_PARAM_NAME = "password" + USERNAME_PARAM_NAME = "username" + + class Backup: + """Backup related config for MongoDB Charm.""" + + SERVICE_NAME = "pbm-agent" + URI_PARAM_NAME = "pbm-uri" + + class Monitoring: + """Monitoring related config for MongoDB Charm.""" + + MONGODB_EXPORTER_PORT = 9216 + METRICS_ENDPOINTS = [ + {"path": "/metrics", "port": f"{MONGODB_EXPORTER_PORT}"}, + ] + METRICS_RULES_DIR = "./src/alert_rules/prometheus" + LOGS_RULES_DIR = "./src/alert_rules/loki" + LOG_SLOTS = ["charmed-mongodb:logs"] + URI_PARAM_NAME = "monitor-uri" + SERVICE_NAME = "mongodb-exporter" + JOBS = [{"static_configs": [{"targets": [f"*:{MONGODB_EXPORTER_PORT}"]}]}] + + class Relations: + """Relations related config for MongoDB Charm.""" + + NAME = "database" + PEERS = "database-peers" + LOGGING = "logging" + + class TLS: + """TLS related config for MongoDB Charm.""" + + EXT_PEM_FILE = "external-cert.pem" + EXT_CA_FILE = "external-ca.crt" + INT_PEM_FILE = "internal-cert.pem" + INT_CA_FILE = "internal-ca.crt" + KEY_FILE_NAME = "keyFile" + + @staticmethod + def get_license_path(license_name: str) -> str: + """Return the path to the license file.""" + return f"{Config.LICENSE_PATH}-{license_name}" diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index caf096b9d..f64b67a91 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -153,6 +153,7 @@ async def run_mongo_op( f"\n\tSTDERR:\n\t {stderr}" if stderr else "", ) ) + logger.error(f"Failed to serialize output: {output}".format(output=stdout)) if not ignore_errors: raise else: