From c309d12657da931f58b915c4a91a61f8516767e5 Mon Sep 17 00:00:00 2001 From: Mia Altieri Date: Wed, 29 Nov 2023 21:32:14 +0000 Subject: [PATCH 1/6] mongos daemon uses config db --- .../mongodb/v0/config_server_interface.py | 84 ++++++++++++--- lib/charms/mongodb/v1/helpers.py | 7 +- src/charm.py | 102 ++++++++++++++++-- 3 files changed, 169 insertions(+), 24 deletions(-) diff --git a/lib/charms/mongodb/v0/config_server_interface.py b/lib/charms/mongodb/v0/config_server_interface.py index cdaa8c97..f72e4360 100644 --- a/lib/charms/mongodb/v0/config_server_interface.py +++ b/lib/charms/mongodb/v0/config_server_interface.py @@ -7,10 +7,14 @@ shards. """ import logging +import json from ops.charm import CharmBase, EventBase from ops.framework import Object -from ops.model import WaitingStatus +from ops.model import WaitingStatus, MaintenanceStatus, ActiveStatus + +from charms.mongodb.v1.helpers import get_mongos_args, add_args_to_env +from charms.mongodb.v1.mongos import MongosConnection from config import Config @@ -18,7 +22,7 @@ KEYFILE_KEY = "key-file" KEY_FILE = "keyFile" HOSTS_KEY = "host" -CONFIG_SERVER_URI_KEY = "config-server-uri" +CONFIG_SERVER_DB_KEY = "config-server-db" # The unique Charmhub library identifier, never change it LIBID = "58ad1ccca4974932ba22b97781b9b2a0" @@ -47,6 +51,7 @@ def __init__( ) # TODO Future PRs handle scale down + # TODO Future PRs handle changing of units/passwords to be propogated to mongos def pass_hook_checks(self, event: EventBase) -> bool: """Runs the pre-hooks checks for ClusterProvider, returns True if all pass.""" @@ -72,7 +77,8 @@ def _on_relation_joined(self, event): logger.info("Skipping relation joined event: hook checks did not pass") return - # TODO Future PR, provide URI + config_server_db = self.generate_config_server_db() + # TODO Future PR, use secrets self._update_relation_data( event.relation.id, @@ -80,6 +86,7 @@ def _on_relation_joined(self, event): KEYFILE_KEY: self.charm.get_secret( Config.Relations.APP_SCOPE, Config.Secrets.SECRET_KEYFILE_NAME ), + CONFIG_SERVER_DB_KEY: config_server_db, }, ) @@ -99,6 +106,16 @@ def _update_relation_data(self, relation_id: int, data: dict) -> None: if relation: relation.data[self.charm.model.app].update(data) + def generate_config_server_db(self) -> str: + """Generates the config server database for mongos to connect to.""" + replica_set_name = self.charm.app.name + hosts = [] + for host in self.charm._unit_ips: + hosts.append(f"{host}:{Config.MONGODB_PORT}") + + hosts = ",".join(hosts) + return f"{replica_set_name}/{hosts}" + class ClusterRequirer(Object): """Manage relations between the config server and mongos router on the mongos side.""" @@ -119,32 +136,71 @@ def __init__( def _on_relation_changed(self, event): """Starts/restarts monogs with config server information.""" relation_data = event.relation.data[event.app] - if not relation_data.get(KEYFILE_KEY): + if not relation_data.get(KEYFILE_KEY) or not relation_data.get(CONFIG_SERVER_DB_KEY): event.defer() self.charm.unit.status = WaitingStatus("Waiting for secrets from config-server") return - self.update_keyfile(key_file_contents=relation_data.get(KEYFILE_KEY)) + updated_keyfile = self.update_keyfile(key_file_contents=relation_data.get(KEYFILE_KEY)) + updated_config = self.update_config_server_db( + config_server_db=relation_data.get(CONFIG_SERVER_DB_KEY) + ) + + # avoid restarting mongos when possible + if not updated_keyfile and not updated_config and self.charm.monogs_initialised: + return + + # mongos is not available until it is using new secrets + del self.charm.unit_peer_data["mongos_initialised"] + logger.info("Restarting mongos with new secrets") + self.unit.status = MaintenanceStatus("starting mongos") + self.charm.restart_mongos_service() + + # restart on high loaded databases can be very slow (e.g. up to 10-20 minutes). + if not self.is_mongos_running(): + logger.info("mongos has not started, deferfing") + self.charm.unit.status = WaitingStatus("Waiting for mongos to start") + event.defer() + return - # TODO: Follow up PR. Start mongos with the config-server URI # TODO: Follow up PR. Add a user for mongos once it has been started + self.charm.unit_peer_data["mongos_initialised"] = json.dumps(True) + self.unit.status = ActiveStatus() + + def is_mongos_running(self) -> bool: + """Returns true if mongos service is running.""" + with MongosConnection(None, "mongodb://localhost:27018") as mongo: + return mongo.is_ready - def update_keyfile(self, key_file_contents: str) -> None: - """Updates keyfile on all units.""" + def update_config_server_db(self, config_server_db) -> bool: + """Updates config server str when necessary.""" + if self.charm.config_server_db == config_server_db: + return False + + mongos_config = self.charm.mongos_config + mongos_start_args = get_mongos_args( + 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: + """Updates keyfile when necessary.""" # keyfile is set by leader in application data, application data does not necessarily # match what is on the machine. current_key_file = self.charm.get_keyfile_contents() if not key_file_contents or key_file_contents == current_key_file: - return + return False # put keyfile on the machine with appropriate permissions self.charm.push_file_to_unit( parent_dir=Config.MONGOD_CONF_DIR, file_name=KEY_FILE, file_contents=key_file_contents ) - if not self.charm.unit.is_leader(): - return + if self.charm.unit.is_leader(): + self.charm.set_secret( + Config.Relations.APP_SCOPE, Config.Secrets.SECRET_KEYFILE_NAME, key_file_contents + ) - self.charm.set_secret( - Config.Relations.APP_SCOPE, Config.Secrets.SECRET_KEYFILE_NAME, key_file_contents - ) + return True diff --git a/lib/charms/mongodb/v1/helpers.py b/lib/charms/mongodb/v1/helpers.py index f0ba1c90..6140a7a1 100644 --- a/lib/charms/mongodb/v1/helpers.py +++ b/lib/charms/mongodb/v1/helpers.py @@ -81,8 +81,9 @@ def get_create_user_cmd(config: MongoDBConfiguration, mongo_path=MONGO_SHELL) -> def get_mongos_args( - config: MongoDBConfiguration, + config, snap_install: bool = False, + config_server_db: str = None, ) -> str: """Returns the arguments used for starting mongos on a config-server side application. @@ -91,14 +92,14 @@ def get_mongos_args( """ # mongos running on the config server communicates through localhost # use constant for port - config_server_uri = f"{config.replset}/localhost:27017" + config_server_db = config_server_db or f"{config.replset}/localhost:27017" full_conf_dir = f"{MONGODB_SNAP_DATA_DIR}{CONF_DIR}" if snap_install else CONF_DIR cmd = [ # mongos on config server side should run on 0.0.0.0 so it can be accessed by other units # in the sharded cluster "--bind_ip_all", - f"--configdb {config_server_uri}", + f"--configdb {config_server_db}", # config server is already using 27017 f"--port {Config.MONGOS_PORT}", f"--keyFile={full_conf_dir}/{KEY_FILE}", diff --git a/src/charm.py b/src/charm.py index 59da690f..c84e5135 100755 --- a/src/charm.py +++ b/src/charm.py @@ -4,12 +4,14 @@ # See LICENSE file for licensing details. import os import pwd +import json +import subprocess from charms.mongodb.v1.helpers import copy_licenses_to_unit, KEY_FILE from charms.operator_libs_linux.v1 import snap from pathlib import Path from charms.mongodb.v0.mongodb_secrets import SecretCache -from typing import Set, List, Optional +from typing import Set, List, Optional, Dict from charms.mongodb.v0.mongodb_secrets import generate_secret_label from charms.mongodb.v1.mongos import MongosConfiguration from charms.mongodb.v0.mongodb import MongoDBConfiguration @@ -71,6 +73,12 @@ def _on_install(self, event: InstallEvent) -> None: def _on_start(self, event: StartEvent) -> None: """Handle the start event.""" + try: + self._open_ports_tcp([Config.MONGOS_PORT]) + except subprocess.CalledProcessError: + self.unit.status = BlockedStatus("failed to open TCP port for MongoDB") + return + # start hooks are fired before relation hooks and `mongos` requires a config-server in # order to start. Wait to receive config-server info from the relation event before # starting `mongos` daemon @@ -105,15 +113,14 @@ def _install_snap_packages(self, packages: List[str]) -> None: raise @property - def mongos_config(self) -> MongoDBConfiguration: + def mongos_config(self) -> MongosConfiguration: """Generates a MongoDBConfiguration object for mongos in the deployment of MongoDB.""" return self._get_mongos_config_for_user(OperatorUser, set("/tmp/mongos.sock")) def _get_mongos_config_for_user( - self, user: MongoDBUser, hosts: Set[str], config_server_uri: str + self, user: MongoDBUser, hosts: Set[str] ) -> MongosConfiguration: return MongosConfiguration( - config_server_uri=config_server_uri, database=user.get_database_name(), username=user.get_username(), password=self.get_secret(APP_SCOPE, user.get_password_key_name()), @@ -174,9 +181,7 @@ def remove_secret(self, scope, key) -> None: content = secret.get_content() if not content.get(key) or content[key] == Config.Secrets.SECRET_DELETED_LABEL: - logger.error( - f"Non-existing secret {scope}:{key} was attempted to be removed." - ) + logger.error(f"Non-existing secret {scope}:{key} was attempted to be removed.") return content[key] = Config.Secrets.SECRET_DELETED_LABEL @@ -216,6 +221,89 @@ def push_file_to_unit(self, parent_dir, file_name, file_contents) -> None: mongodb_user = pwd.getpwnam(MONGO_USER) os.chown(file_name, mongodb_user.pw_uid, ROOT_USER_GID) + def start_mongos_service(self) -> None: + """Starts the mongos service. + + Raises: + snap.SnapError + """ + snap_cache = snap.SnapCache() + mongodb_snap = snap_cache["charmed-mongodb"] + mongodb_snap.start(services=["mongos"], enable=True) + + def stop_mongos_service(self) -> None: + """Stops the mongos service. + + Raises: + snap.SnapError + """ + snap_cache = snap.SnapCache() + mongodb_snap = snap_cache["charmed-mongodb"] + mongodb_snap.stop(services=["mongos"]) + + def restart_mongos_service(self) -> None: + """Retarts the mongos service. + + Raises: + snap.SnapError + """ + self.stop_mongos_service() + self.start_mongos_service() + + @property + def _peers(self) -> Optional[Relation]: + """Fetch the peer relation. + + Returns: + An `ops.model.Relation` object representing the peer relation. + """ + return self.model.get_relation(Config.Relations.PEERS) + + @property + def unit_peer_data(self) -> Dict: + """Unit peer relation data object.""" + return self._peers.data[self.unit] + + @property + def config_server_db(self): + """Fetch current the config server database that this unit is connected to. + + Returns: + A list of hosts addresses (strings). + """ + if "config_server_db" not in self.unit_peer_data: + return "" + + return json.loads(self.unit_peer_data.get("config_server_db")) + + @property + def db_initialised(self) -> bool: + """Abstraction for client connection code. + + mongodb_provider waits for database to be initialised before creating new users. For this + charm it is only necessary to have mongos initialised. + """ + return self.mongos_initialised + + @property + def mongos_initialised(self) -> bool: + """Check if MongoDB is initialised.""" + return "mongos_initialised" in self.unit_peer_data + + def _open_ports_tcp(self, ports: int) -> None: + """Open the given port. + + Args: + ports: The ports to open. + """ + for port in ports: + try: + logger.debug("opening tcp port") + subprocess.check_call(["open-port", "{}/TCP".format(port)]) + except subprocess.CalledProcessError as e: + logger.exception("failed opening port: %s", str(e)) + raise + # END: helper functions From 78778253e6a0cf0198c9fe3ed55fecb93943d2f9 Mon Sep 17 00:00:00 2001 From: Mia Altieri Date: Thu, 30 Nov 2023 20:52:35 +0000 Subject: [PATCH 2/6] mongos starts with socket and config db --- .../mongodb/v0/config_server_interface.py | 30 +++++++------ lib/charms/mongodb/v1/helpers.py | 14 +++++-- src/charm.py | 42 ++----------------- src/config.py | 1 + 4 files changed, 28 insertions(+), 59 deletions(-) diff --git a/lib/charms/mongodb/v0/config_server_interface.py b/lib/charms/mongodb/v0/config_server_interface.py index f72e4360..565df481 100644 --- a/lib/charms/mongodb/v0/config_server_interface.py +++ b/lib/charms/mongodb/v0/config_server_interface.py @@ -6,15 +6,14 @@ This class handles the sharing of secrets between sharded components, adding shards, and removing shards. """ -import logging import json +import logging +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 from ops.framework import Object -from ops.model import WaitingStatus, MaintenanceStatus, ActiveStatus - -from charms.mongodb.v1.helpers import get_mongos_args, add_args_to_env -from charms.mongodb.v1.mongos import MongosConnection +from ops.model import ActiveStatus, MaintenanceStatus, WaitingStatus from config import Config @@ -23,6 +22,7 @@ KEY_FILE = "keyFile" HOSTS_KEY = "host" CONFIG_SERVER_DB_KEY = "config-server-db" +MONGOS_SOCKET_URI_FMT = "%2Fvar%2Fsnap%2Fcharmed-mongodb%2Fcommon%2Fvar%2Fmongodb-27018.sock" # The unique Charmhub library identifier, never change it LIBID = "58ad1ccca4974932ba22b97781b9b2a0" @@ -32,7 +32,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 = 2 class ClusterProvider(Object): @@ -51,7 +51,7 @@ def __init__( ) # TODO Future PRs handle scale down - # TODO Future PRs handle changing of units/passwords to be propogated to mongos + # TODO Future PRs handle changing of units/passwords to be propagated to mongos def pass_hook_checks(self, event: EventBase) -> bool: """Runs the pre-hooks checks for ClusterProvider, returns True if all pass.""" @@ -71,7 +71,7 @@ def pass_hook_checks(self, event: EventBase) -> bool: return True - def _on_relation_joined(self, event): + def _on_relation_joined(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") @@ -133,7 +133,7 @@ def __init__( ) # TODO Future PRs handle scale down - def _on_relation_changed(self, event): + def _on_relation_changed(self, event) -> None: """Starts/restarts monogs with config server information.""" relation_data = event.relation.data[event.app] if not relation_data.get(KEYFILE_KEY) or not relation_data.get(CONFIG_SERVER_DB_KEY): @@ -147,29 +147,27 @@ def _on_relation_changed(self, event): ) # avoid restarting mongos when possible - if not updated_keyfile and not updated_config and self.charm.monogs_initialised: + if not updated_keyfile and not updated_config and self.is_mongos_running(): return # mongos is not available until it is using new secrets - del self.charm.unit_peer_data["mongos_initialised"] logger.info("Restarting mongos with new secrets") - self.unit.status = MaintenanceStatus("starting mongos") + self.charm.unit.status = MaintenanceStatus("starting mongos") self.charm.restart_mongos_service() # restart on high loaded databases can be very slow (e.g. up to 10-20 minutes). if not self.is_mongos_running(): - logger.info("mongos has not started, deferfing") + logger.info("mongos has not started, deferring") self.charm.unit.status = WaitingStatus("Waiting for mongos to start") event.defer() return # TODO: Follow up PR. Add a user for mongos once it has been started - self.charm.unit_peer_data["mongos_initialised"] = json.dumps(True) - self.unit.status = ActiveStatus() + self.charm.unit.status = ActiveStatus() def is_mongos_running(self) -> bool: """Returns true if mongos service is running.""" - with MongosConnection(None, "mongodb://localhost:27018") as mongo: + with MongosConnection(None, f"mongodb://{MONGOS_SOCKET_URI_FMT}") as mongo: return mongo.is_ready def update_config_server_db(self, config_server_db) -> bool: diff --git a/lib/charms/mongodb/v1/helpers.py b/lib/charms/mongodb/v1/helpers.py index 6140a7a1..ea2dd736 100644 --- a/lib/charms/mongodb/v1/helpers.py +++ b/lib/charms/mongodb/v1/helpers.py @@ -29,7 +29,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 0 +LIBPATCH = 1 # path to store mongodb ketFile KEY_FILE = "keyFile" @@ -90,15 +90,21 @@ def get_mongos_args( Returns: A string representing the arguments to be passed to mongos. """ + # suborinate charm which provides its own config_server_db, should only use unix domain socket + binding_ips = ( + f"--bind_ip {MONGODB_COMMON_DIR}/var/mongodb-27018.sock" + if config_server_db + else "--bind_ip_all" + ) + # mongos running on the config server communicates through localhost - # use constant for port - config_server_db = config_server_db or f"{config.replset}/localhost:27017" + config_server_db = config_server_db or f"{config.replset}/localhost:{Config.MONGODB_PORT}" full_conf_dir = f"{MONGODB_SNAP_DATA_DIR}{CONF_DIR}" if snap_install else CONF_DIR cmd = [ # mongos on config server side should run on 0.0.0.0 so it can be accessed by other units # in the sharded cluster - "--bind_ip_all", + binding_ips, f"--configdb {config_server_db}", # config server is already using 27017 f"--port {Config.MONGOS_PORT}", diff --git a/src/charm.py b/src/charm.py index c84e5135..401371f9 100755 --- a/src/charm.py +++ b/src/charm.py @@ -51,10 +51,8 @@ def __init__(self, *args): self.cluster = ClusterRequirer(self) self.secrets = SecretCache(self) - # todo future PRs: - # 1. start daemon when relation to config server is made - # 2. add users for related application - # 3. update status indicates missing relations + # 1. add users for related application (to be done on config-server charm side) + # 2. update status indicates missing relations # BEGIN: hook functions def _on_install(self, event: InstallEvent) -> None: @@ -73,12 +71,6 @@ def _on_install(self, event: InstallEvent) -> None: def _on_start(self, event: StartEvent) -> None: """Handle the start event.""" - try: - self._open_ports_tcp([Config.MONGOS_PORT]) - except subprocess.CalledProcessError: - self.unit.status = BlockedStatus("failed to open TCP port for MongoDB") - return - # start hooks are fired before relation hooks and `mongos` requires a config-server in # order to start. Wait to receive config-server info from the relation event before # starting `mongos` daemon @@ -115,7 +107,7 @@ def _install_snap_packages(self, packages: List[str]) -> None: @property def mongos_config(self) -> MongosConfiguration: """Generates a MongoDBConfiguration object for mongos in the deployment of MongoDB.""" - return self._get_mongos_config_for_user(OperatorUser, set("/tmp/mongos.sock")) + return self._get_mongos_config_for_user(OperatorUser, set(Config.MONGOS_SOCKET)) def _get_mongos_config_for_user( self, user: MongoDBUser, hosts: Set[str] @@ -276,34 +268,6 @@ def config_server_db(self): return json.loads(self.unit_peer_data.get("config_server_db")) - @property - def db_initialised(self) -> bool: - """Abstraction for client connection code. - - mongodb_provider waits for database to be initialised before creating new users. For this - charm it is only necessary to have mongos initialised. - """ - return self.mongos_initialised - - @property - def mongos_initialised(self) -> bool: - """Check if MongoDB is initialised.""" - return "mongos_initialised" in self.unit_peer_data - - def _open_ports_tcp(self, ports: int) -> None: - """Open the given port. - - Args: - ports: The ports to open. - """ - for port in ports: - try: - logger.debug("opening tcp port") - subprocess.check_call(["open-port", "{}/TCP".format(port)]) - except subprocess.CalledProcessError as e: - logger.exception("failed opening port: %s", str(e)) - raise - # END: helper functions diff --git a/src/config.py b/src/config.py index 7edfb9cc..25df664c 100644 --- a/src/config.py +++ b/src/config.py @@ -10,6 +10,7 @@ class Config: """Configuration for MongoDB Charm.""" MONGOS_PORT = 27018 + MONGOS_SOCKET = "/var/snap/charmed-mongodb/common/var/mongodb-27018.sock" MONGODB_PORT = 27017 SUBSTRATE = "vm" ENV_VAR_PATH = "/etc/environment" From a5fcb7923d87fe87e4008f383c129cb76dafd1ab Mon Sep 17 00:00:00 2001 From: Mia Altieri Date: Thu, 30 Nov 2023 21:50:58 +0000 Subject: [PATCH 3/6] add unit tests --- src/charm.py | 6 +- tests/unit/test_config_server_lib.py | 92 ++++++++++++++++++++++++++-- 2 files changed, 89 insertions(+), 9 deletions(-) diff --git a/src/charm.py b/src/charm.py index 401371f9..f472ba3f 100755 --- a/src/charm.py +++ b/src/charm.py @@ -5,7 +5,6 @@ import os import pwd import json -import subprocess from charms.mongodb.v1.helpers import copy_licenses_to_unit, KEY_FILE from charms.operator_libs_linux.v1 import snap from pathlib import Path @@ -14,7 +13,6 @@ from typing import Set, List, Optional, Dict from charms.mongodb.v0.mongodb_secrets import generate_secret_label from charms.mongodb.v1.mongos import MongosConfiguration -from charms.mongodb.v0.mongodb import MongoDBConfiguration from charms.mongodb.v0.config_server_interface import ClusterRequirer from charms.mongodb.v1.users import ( MongoDBUser, @@ -173,7 +171,9 @@ def remove_secret(self, scope, key) -> None: content = secret.get_content() if not content.get(key) or content[key] == Config.Secrets.SECRET_DELETED_LABEL: - logger.error(f"Non-existing secret {scope}:{key} was attempted to be removed.") + logger.error( + f"Non-existing secret {scope}:{key} was attempted to be removed." + ) return content[key] = Config.Secrets.SECRET_DELETED_LABEL diff --git a/tests/unit/test_config_server_lib.py b/tests/unit/test_config_server_lib.py index 4efebb24..ddfc5f23 100644 --- a/tests/unit/test_config_server_lib.py +++ b/tests/unit/test_config_server_lib.py @@ -1,6 +1,8 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. import unittest +import json + from unittest.mock import patch from ops.testing import Harness @@ -11,7 +13,10 @@ PEER_ADDR = {"private-address": "127.4.5.6"} -KEYFILE = {"key-file": "key-file-contents"} +REL_DATA = { + "key-file": "key-file-contents", + "config-server-db": "config-server-db/host:port", +} class TestConfigServerInterface(unittest.TestCase): @@ -39,12 +44,23 @@ def test_on_relation_changed_waits_keyfile(self, update_keyfile, defer): @patch("charm.MongosOperatorCharm.push_file_to_unit") @patch("charm.MongosOperatorCharm.get_keyfile_contents") @patch("charm.MongosOperatorCharm.set_secret") - def test_same_keyfile(self, set_secret, get_keyfile_contents, push_file_to_unit): + @patch("charm.ClusterRequirer.update_config_server_db") + @patch("charm.ClusterRequirer.is_mongos_running") + @patch("charm.MongosOperatorCharm.restart_mongos_service") + def test_same_keyfile( + self, + restart_mongos_service, + is_mongos_running, + update_config_server_db, + set_secret, + get_keyfile_contents, + push_file_to_unit, + ): """Tests that charm doesn't update keyfile when they are the same.""" - get_keyfile_contents.return_value = KEYFILE["key-file"] + get_keyfile_contents.return_value = REL_DATA["key-file"] relation_id = self.harness.add_relation("cluster", "config-server") self.harness.add_relation_unit(relation_id, "config-server/0") - self.harness.update_relation_data(relation_id, "config-server", KEYFILE) + self.harness.update_relation_data(relation_id, "config-server", REL_DATA) push_file_to_unit.assert_not_called() set_secret.assert_not_called() @@ -52,15 +68,79 @@ def test_same_keyfile(self, set_secret, get_keyfile_contents, push_file_to_unit) @patch("charm.MongosOperatorCharm.push_file_to_unit") @patch("charm.MongosOperatorCharm.get_keyfile_contents") @patch("charm.MongosOperatorCharm.set_secret") + @patch("charm.ClusterRequirer.update_config_server_db", return_value=False) + @patch("charm.ClusterRequirer.is_mongos_running") + @patch("charm.MongosOperatorCharm.restart_mongos_service") def test_non_leader_doesnt_set_keyfile_secret( - self, set_secret, get_keyfile_contents, push_file_to_unit + self, + restart_mongos_service, + is_mongos_running, + update_config_server_db, + set_secret, + get_keyfile_contents, + push_file_to_unit, ): """Tests that non leaders dont set keyfile secret.""" self.harness.set_leader(False) relation_id = self.harness.add_relation("cluster", "config-server") self.harness.add_relation_unit(relation_id, "config-server/0") - self.harness.update_relation_data(relation_id, "config-server", KEYFILE) + self.harness.update_relation_data(relation_id, "config-server", REL_DATA) push_file_to_unit.assert_called() set_secret.assert_not_called() + + @patch("charm.MongosOperatorCharm.push_file_to_unit") + @patch("charm.MongosOperatorCharm.get_keyfile_contents") + @patch("charm.MongosOperatorCharm.set_secret") + @patch("charm.ClusterRequirer.is_mongos_running") + @patch("charm.MongosOperatorCharm.restart_mongos_service") + @patch("charms.mongodb.v0.config_server_interface.add_args_to_env") + def test_same_config_db( + self, + add_args_to_env, + restart_mongos_service, + is_mongos_running, + set_secret, + get_keyfile_contents, + push_file_to_unit, + ): + """Tests that charm doesn't update config-server when they are the same.""" + get_keyfile_contents.return_value = REL_DATA["key-file"] + self.harness.charm.unit_peer_data["config_server_db"] = json.dumps( + "config-server-db/host:port" + ) + relation_id = self.harness.add_relation("cluster", "config-server") + self.harness.add_relation_unit(relation_id, "config-server/0") + self.harness.update_relation_data(relation_id, "config-server", REL_DATA) + + add_args_to_env.assert_not_called() + + @patch("charm.MongosOperatorCharm.push_file_to_unit") + @patch("charm.MongosOperatorCharm.get_keyfile_contents") + @patch("charm.MongosOperatorCharm.set_secret") + @patch("charm.ClusterRequirer.is_mongos_running", return_value=False) + @patch("ops.framework.EventBase.defer") + @patch("charm.MongosOperatorCharm.restart_mongos_service") + def retry_restart_mongos( + self, + restart_mongos, + defer, + is_mongos_running, + set_secret, + get_keyfile_contents, + push_file_to_unit, + ): + """Tests that when the charm failed to start mongos it tries again.""" + relation_id = self.harness.add_relation("cluster", "config-server") + self.harness.add_relation_unit(relation_id, "config-server/0") + self.harness.update_relation_data(relation_id, "config-server", REL_DATA) + restart_mongos.assert_called() + defer.assert_called() + + relation_id = self.harness.add_relation("cluster", "config-server") + self.harness.add_relation_unit(relation_id, "config-server/0") + + # update with the same data + self.harness.update_relation_data(relation_id, "config-server", REL_DATA) + restart_mongos.assert_called() From c14490fee43e7981a4f60ab658068647fa9502b5 Mon Sep 17 00:00:00 2001 From: Mia Altieri Date: Fri, 1 Dec 2023 15:14:45 +0000 Subject: [PATCH 4/6] int tests + fmt --- tests/integration/helpers.py | 22 +++++++++++++++ tests/integration/test_charm.py | 48 ++++++++++++++++++++++++++++++--- 2 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 tests/integration/helpers.py diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py new file mode 100644 index 00000000..8b5a089b --- /dev/null +++ b/tests/integration/helpers.py @@ -0,0 +1,22 @@ +from charms.mongodb.v1.helpers import MONGO_SHELL +from pytest_operator.plugin import OpsTest +import ops + +MONGOS_URI = ( + "mongodb://%2Fvar%2Fsnap%2Fcharmed-mongodb%2Fcommon%2Fvar%2Fmongodb-27018.sock" +) +MONGOS_APP_NAME = "mongos" + + +async def mongs_command(ops_test: OpsTest) -> str: + """Generates a command which verifies TLS status.""" + return f"{MONGO_SHELL} '{MONGOS_URI}' --eval 'use test-db'" + + +async def check_mongos(ops_test: OpsTest, unit: ops.model.Unit) -> bool: + """Returns whether mongos is running on the provided unit.""" + mongos_check = await mongs_command(ops_test) + check_tls_cmd = f"exec --unit {unit.name} -- {mongos_check}" + return_code, _, _ = await ops_test.juju(*check_tls_cmd.split()) + mongos_running = return_code == 0 + return mongos_running diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index fe994dcd..bb7e475e 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -3,9 +3,16 @@ # See LICENSE file for licensing details. import pytest from pytest_operator.plugin import OpsTest +from .helpers import check_mongos APPLICATION_APP_NAME = "application" MONGOS_APP_NAME = "mongos" +CLUSTER_REL_NAME = "cluster" + +CONFIG_SERVER_APP_NAME = "config-server" +SHARD_APP_NAME = "shard" +SHARD_REL_NAME = "sharding" +CONFIG_SERVER_REL_NAME = "config-server" @pytest.mark.abort_on_fail @@ -45,8 +52,41 @@ async def test_waits_for_config_server(ops_test: OpsTest) -> None: idle_period=10, ), - config_server_unit = ops_test.model.applications[MONGOS_APP_NAME].units[0] - assert ( - config_server_unit.workload_status_message - == "Missing relation to config-server." + mongos_unit = ops_test.model.applications[MONGOS_APP_NAME].units[0] + assert mongos_unit.workload_status_message == "Missing relation to config-server." + + +@pytest.mark.abort_on_fail +# wait for 6/edge charm on mongodb to be updated before running this test on CI +@pytest.mark.skip() +async def test_mongos_starts_with_config_server(ops_test: OpsTest) -> None: + # prepare sharded cluster + await ops_test.model.wait_for_idle( + apps=[CONFIG_SERVER_APP_NAME, SHARD_APP_NAME], + idle_period=10, + raise_on_blocked=False, + ) + await ops_test.model.integrate( + f"{SHARD_APP_NAME}:{SHARD_REL_NAME}", + f"{CONFIG_SERVER_APP_NAME}:{CONFIG_SERVER_REL_NAME}", + ) + await ops_test.model.wait_for_idle( + apps=[CONFIG_SERVER_APP_NAME, SHARD_APP_NAME], + idle_period=10, + raise_on_blocked=False, + ) + + # connect sharded cluster to mongos + await ops_test.model.integrate( + f"{MONGOS_APP_NAME}:{CLUSTER_REL_NAME}", + f"{CONFIG_SERVER_APP_NAME}:{CLUSTER_REL_NAME}", ) + await ops_test.model.wait_for_idle( + apps=[CONFIG_SERVER_APP_NAME, SHARD_APP_NAME, MONGOS_APP_NAME], + idle_period=10, + status="active", + ) + + mongos_unit = ops_test.model.applications[MONGOS_APP_NAME].units[0] + mongos_running = await check_mongos(ops_test, mongos_unit) + assert mongos_running, "Mongos is not currently running." From 3e20ae9334a643c44be8d16f9b7240b4284cd113 Mon Sep 17 00:00:00 2001 From: Mia Altieri Date: Fri, 1 Dec 2023 17:50:33 +0000 Subject: [PATCH 5/6] PR comments --- tests/integration/helpers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 8b5a089b..cdf7ea79 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -8,14 +8,14 @@ MONGOS_APP_NAME = "mongos" -async def mongs_command(ops_test: OpsTest) -> str: +async def mongos_command(ops_test: OpsTest) -> str: """Generates a command which verifies TLS status.""" - return f"{MONGO_SHELL} '{MONGOS_URI}' --eval 'use test-db'" + return f"{MONGO_SHELL} '{MONGOS_URI}' --eval 'ping'" async def check_mongos(ops_test: OpsTest, unit: ops.model.Unit) -> bool: """Returns whether mongos is running on the provided unit.""" - mongos_check = await mongs_command(ops_test) + mongos_check = await mongos_command(ops_test) check_tls_cmd = f"exec --unit {unit.name} -- {mongos_check}" return_code, _, _ = await ops_test.juju(*check_tls_cmd.split()) mongos_running = return_code == 0 From 76ba878643c66e738f7a54331cc9b42ff36642c0 Mon Sep 17 00:00:00 2001 From: Mia Altieri Date: Fri, 1 Dec 2023 18:43:29 +0000 Subject: [PATCH 6/6] pr feedbaack round 2 --- tests/integration/helpers.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index cdf7ea79..8d2390ef 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -8,15 +8,14 @@ MONGOS_APP_NAME = "mongos" -async def mongos_command(ops_test: OpsTest) -> str: - """Generates a command which verifies TLS status.""" +async def generate_mongos_command(ops_test: OpsTest) -> str: + """Generates a command which verifies mongos is running.""" return f"{MONGO_SHELL} '{MONGOS_URI}' --eval 'ping'" async def check_mongos(ops_test: OpsTest, unit: ops.model.Unit) -> bool: """Returns whether mongos is running on the provided unit.""" - mongos_check = await mongos_command(ops_test) + mongos_check = await generate_mongos_command(ops_test) check_tls_cmd = f"exec --unit {unit.name} -- {mongos_check}" return_code, _, _ = await ops_test.juju(*check_tls_cmd.split()) - mongos_running = return_code == 0 - return mongos_running + return return_code == 0