From 41a167f2d2b066be13a42f8f276fa50d93de4965 Mon Sep 17 00:00:00 2001 From: Dmitry Ratushnyy <132273757+dmitry-ratushnyy@users.noreply.github.com> Date: Fri, 23 Jun 2023 20:15:19 +0200 Subject: [PATCH] [DPE-2145] Refactoring: refactor charm events handlers (#166) * Reafactoring: charm events * Refactoring charm event handlers --------- Co-authored-by: Ubuntu --- lib/charms/mongodb/v0/mongodb.py | 4 +- lib/charms/mongodb/v0/mongodb_tls.py | 34 +-- metadata.yaml | 4 +- src/charm.py | 337 +++++++++++++++++--------- src/config.py | 1 + tests/integration/ha_tests/helpers.py | 4 +- tests/integration/test_teardown.py | 26 +- tests/unit/test_charm.py | 8 +- 8 files changed, 251 insertions(+), 167 deletions(-) diff --git a/lib/charms/mongodb/v0/mongodb.py b/lib/charms/mongodb/v0/mongodb.py index 231e9a19e..12bb76e3a 100644 --- a/lib/charms/mongodb/v0/mongodb.py +++ b/lib/charms/mongodb/v0/mongodb.py @@ -5,7 +5,7 @@ import logging import re from dataclasses import dataclass -from typing import Dict, List, Optional, Set +from typing import Dict, List, Set from urllib.parse import quote_plus from bson.json_util import dumps @@ -49,7 +49,7 @@ class MongoDBConfiguration: """ replset: str - database: Optional[str] + database: str username: str password: str hosts: Set[str] diff --git a/lib/charms/mongodb/v0/mongodb_tls.py b/lib/charms/mongodb/v0/mongodb_tls.py index a609ddcc1..0a9967fa6 100644 --- a/lib/charms/mongodb/v0/mongodb_tls.py +++ b/lib/charms/mongodb/v0/mongodb_tls.py @@ -42,7 +42,7 @@ class MongoDBTLS(Object): """In this class we manage client database relations.""" - def __init__(self, charm, peer_relation, substrate="k8s"): + def __init__(self, charm, peer_relation, substrate): """Manager of MongoDB client relations.""" super().__init__(charm, "client-relations") self.charm = charm @@ -139,13 +139,11 @@ def _on_tls_relation_broken(self, event: RelationBrokenEvent) -> None: event.defer() return - logger.debug("Restarting mongod with TLS disabled.") - if self.substrate == "vm": - self.charm.unit.status = MaintenanceStatus("disabling TLS") - self.charm.restart_mongod_service() - self.charm.unit.status = ActiveStatus() - else: - self.charm.on_mongod_pebble_ready(event) + logger.info("Restarting mongod with TLS disabled.") + self.charm.unit.status = MaintenanceStatus("disabling TLS") + self.charm.delete_tls_certificate_from_workload() + self.charm.restart_mongod_service() + self.charm.unit.status = ActiveStatus() def _on_certificate_available(self, event: CertificateAvailableEvent) -> None: """Enable TLS when TLS certificate available.""" @@ -165,9 +163,6 @@ def _on_certificate_available(self, event: CertificateAvailableEvent) -> None: logger.error("An unknown certificate available.") return - old_cert = self.charm.get_secret(scope, "cert") - renewal = old_cert and old_cert != event.certificate - if scope == "unit" or (scope == "app" and self.charm.unit.is_leader()): self.charm.set_secret( scope, "chain", "\n".join(event.chain) if event.chain is not None else None @@ -182,17 +177,12 @@ def _on_certificate_available(self, event: CertificateAvailableEvent) -> None: event.defer() return - if renewal and self.substrate == "k8s": - self.charm.unit.get_container("mongod").stop("mongod") - logger.debug("Restarting mongod with TLS enabled.") - if self.substrate == "vm": - self.charm._push_tls_certificate_to_workload() - self.charm.unit.status = MaintenanceStatus("enabling TLS") - self.charm.restart_mongod_service() - self.charm.unit.status = ActiveStatus() - else: - self.charm.on_mongod_pebble_ready(event) + + self.charm.push_tls_certificate_to_workload() + self.charm.unit.status = MaintenanceStatus("enabling TLS") + self.charm.restart_mongod_service() + self.charm.unit.status = ActiveStatus() def _waiting_for_certs(self): """Returns a boolean indicating whether additional certs are needed.""" @@ -276,4 +266,4 @@ def get_host(self, unit: Unit): if self.substrate == "vm": return self.charm._unit_ip(unit) else: - return self.charm.get_hostname_by_unit(unit.name) + return self.charm.get_hostname_for_unit(unit) diff --git a/metadata.yaml b/metadata.yaml index c0d711419..fcf53f87d 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -42,7 +42,7 @@ containers: mongod: resource: mongodb-image mounts: - - storage: db + - storage: mongodb location: /var/lib/mongodb resources: mongodb-image: @@ -50,6 +50,6 @@ resources: description: OCI image for mongodb upstream-source: "ghcr.io/canonical/charmed-mongodb:5.0.14-22.04_edge" storage: - db: + mongodb: type: filesystem location: /var/lib/mongodb diff --git a/src/charm.py b/src/charm.py index 7941726b2..3e8bdc6bf 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1,17 +1,10 @@ #!/usr/bin/env python3 +"""Charm code for MongoDB service on Kubernetes.""" # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. -"""Charm for MongoDB on Kubernetes. - -Run the developer's favourite document database — MongoDB! Charm for MongoDB is a fully supported, -automated solution from Canonical for running production-grade MongoDB on Kubernetes. It offers -a simple, secure and highly available setup with automatic recovery on fail-over. The solution -includes scaling and other capabilities. -""" - import logging -from typing import Dict, Optional +from typing import List, Optional, Set from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider from charms.loki_k8s.v0.loki_push_api import LogProxyConsumer @@ -30,12 +23,25 @@ from charms.mongodb.v0.mongodb_tls import MongoDBTLS from charms.mongodb.v0.users import CHARM_USERS, MongoDBUser, MonitorUser, OperatorUser from charms.prometheus_k8s.v0.prometheus_scrape import MetricsEndpointProvider -from ops.charm import ActionEvent, CharmBase +from ops.charm import ( + ActionEvent, + CharmBase, + RelationDepartedEvent, + StartEvent, + StorageDetachingEvent, +) from ops.main import main -from ops.model import ActiveStatus, Container +from ops.model import ( + ActiveStatus, + Container, + Relation, + RelationDataContent, + Unit, + WaitingStatus, +) from ops.pebble import ExecError, Layer, PathError, ProtocolError from pymongo.errors import PyMongoError -from tenacity import before_log, retry, stop_after_attempt, wait_fixed +from tenacity import Retrying, before_log, retry, stop_after_attempt, wait_fixed from config import Config from exceptions import AdminUserCreationError @@ -49,18 +55,30 @@ class MongoDBCharm(CharmBase): def __init__(self, *args): super().__init__(*args) - self.framework.observe(self.on.mongod_pebble_ready, self.on_mongod_pebble_ready) + 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[Config.Relations.PEERS].relation_changed, self._reconfigure) + self.framework.observe( - self.on[Config.Relations.PEERS].relation_departed, self._reconfigure + self.on[Config.Relations.PEERS].relation_joined, self._relation_changes_handler ) + + self.framework.observe( + self.on[Config.Relations.PEERS].relation_changed, self._relation_changes_handler + ) + + self.framework.observe( + self.on[Config.Relations.PEERS].relation_departed, self._relation_changes_handler + ) + + # if a new leader has been elected update hosts of MongoDB + self.framework.observe(self.on.leader_elected, self._relation_changes_handler) + self.framework.observe(self.on.mongodb_storage_detaching, self._on_storage_detaching) + 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, Config.Relations.PEERS) + self.tls = MongoDBTLS(self, Config.Relations.PEERS, Config.SUBSTRATE) self.metrics_endpoint = MetricsEndpointProvider( self, refresh_event=self.on.start, jobs=Config.Monitoring.JOBS @@ -74,6 +92,30 @@ def __init__(self, *args): ) # BEGIN: properties + + @property + def _unit_hosts(self) -> List[str]: + """Retrieve IP addresses associated with MongoDB application. + + Returns: + a list of IP address associated with MongoDB application. + """ + self_unit = [self.get_hostname_for_unit(self.unit)] + + if not self._peers: + return self_unit + + return self_unit + [self.get_hostname_for_unit(unit) for unit in self._peers.units] + + @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 mongodb_config(self) -> MongoDBConfiguration: """Create a configuration object with settings. @@ -83,22 +125,18 @@ def mongodb_config(self) -> MongoDBConfiguration: Returns: A MongoDBConfiguration object """ - 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 - ] - return self._get_mongodb_config_for_user(OperatorUser, hosts) + return self._get_mongodb_config_for_user(OperatorUser, self._unit_hosts) @property def monitor_config(self) -> MongoDBConfiguration: """Generates a MongoDBConfiguration object for this deployment of MongoDB.""" return self._get_mongodb_config_for_user( - MonitorUser, [self.get_hostname_by_unit(self.unit.name)] + MonitorUser, [self.get_hostname_for_unit(self.unit)] ) @property def _monitor_layer(self) -> Layer: - """Returns a Pebble configuration layer for mongod.""" + """Returns a Pebble configuration layer for mongodb_exporter.""" layer_config = { "summary": "mongodb_exporter layer", "description": "Pebble config layer for mongodb_exporter", @@ -114,7 +152,7 @@ def _monitor_layer(self) -> Layer: } }, } - return Layer(layer_config) + return Layer(layer_config) # type: ignore @property def _mongod_layer(self) -> Layer: @@ -133,23 +171,23 @@ def _mongod_layer(self) -> Layer: } }, } - return Layer(layer_config) + return Layer(layer_config) # type: ignore @property - def unit_peer_data(self) -> Dict: + def unit_peer_data(self) -> RelationDataContent: """Peer relation data object.""" relation = self.model.get_relation(Config.Relations.PEERS) if relation is None: - return {} + return {} # type: ignore return relation.data[self.unit] @property - def app_peer_data(self) -> Dict: + def app_peer_data(self) -> RelationDataContent: """Peer relation data object.""" relation = self.model.get_relation(Config.Relations.PEERS) if relation is None: - return {} + return {} # type: ignore return relation.data[self.app] @@ -169,41 +207,35 @@ def _db_initialised(self, value): # END: properties # BEGIN: charm events - def on_mongod_pebble_ready(self, event) -> None: + def _on_mongod_pebble_ready(self, event) -> None: """Configure MongoDB pebble layer specification.""" # Get a reference the container attribute container = self.unit.get_container(Config.CONTAINER_NAME) - # mongod needs keyFile and TLS certificates on filesystem if not container.can_connect(): logger.debug("mongod container is not ready yet.") event.defer() return + + if not self.get_secret("app", "keyfile"): + if self.unit.is_leader(): + self._generate_secrets() + else: + logger.debug( + f"Defer on_mongod_pebble_ready for non-leader unit {self.unit.name}: keyfile not available yet." + ) + event.defer() + return try: - self._push_certificate_to_workload(container) self._push_keyfile_to_workload(container) self._pull_licenses(container) - self._fix_data_dir(container) + self._set_data_dir_permissions(container) except (PathError, ProtocolError) as e: logger.error("Cannot put keyFile: %r", e) event.defer() return - # This function can be run in two cases: - # 1) during regular charm start. - # 2) if we forcefully want to apply new - # 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(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[Config.SERVICE_NAME].command - if new_command != cur_command: - logger.debug("restart MongoDB due to arguments change: %s", new_command) - container.stop(Config.SERVICE_NAME) - # Add initial Pebble config layer using the Pebble API container.add_layer("mongod", self._mongod_layer, combine=True) # Restart changed services and start startup-enabled services. @@ -212,9 +244,6 @@ def on_mongod_pebble_ready(self, event) -> None: # when a network cuts and the pod restarts - reconnect to the exporter self._connect_mongodb_exporter() - # TODO: rework status - self.unit.status = ActiveStatus() - def _on_start(self, event) -> None: """Initialise MongoDB. @@ -233,9 +262,6 @@ def _on_start(self, event) -> None: It is needed to install mongodb-clients inside the charm container to make this function work correctly. """ - if not self.unit.is_leader(): - return - container = self.unit.get_container(Config.CONTAINER_NAME) if not container.can_connect(): logger.debug("mongod container is not ready yet.") @@ -247,44 +273,20 @@ def _on_start(self, event) -> None: event.defer() return - if self._db_initialised: - # The replica set should be initialised only once. Check should be - # external (e.g., check initialisation inside peer relation). We - # shouldn't rely on MongoDB response because the data directory - # can be corrupted. - return - with MongoDBConnection(self.mongodb_config, "localhost", direct=True) as direct_mongo: if not direct_mongo.is_ready: logger.debug("mongodb service is not ready yet.") event.defer() return - try: - logger.info("Replica Set initialization") - direct_mongo.init_replset() - logger.info("User initialization") - self._init_operator_user(container) - self._init_monitor_user() - logger.info("Reconcile relations") - self.client_relations.oversee_users(None, event) - except ExecError as e: - logger.error( - "Deferring on_start: exit code: %i, stderr: %s", e.exit_code, e.stderr - ) - event.defer() - return - except PyMongoError as e: - logger.error("Deferring on_start since: error=%r", e) - event.defer() - return - self._db_initialised = True + # mongod is now active + self.unit.status = ActiveStatus() + self._connect_mongodb_exporter() - def _reconfigure(self, event) -> None: - """Reconfigure replica set. + self._initialise_replica_set(event) - The amount replicas in the MongoDB replica set is updated. - """ + def _relation_changes_handler(self, event) -> None: + """Handles different relation events and updates MongoDB replica set.""" self._connect_mongodb_exporter() if not self.unit.is_leader(): @@ -294,35 +296,31 @@ def _reconfigure(self, event) -> None: # This code runs on leader_elected event before mongod_pebble_ready self._generate_secrets() - # reconfiguration can be successful only if a replica set is initialised. if not self._db_initialised: return with MongoDBConnection(self.mongodb_config) as mongo: try: replset_members = mongo.get_replset_members() - + mongodb_hosts = self.mongodb_config.hosts # compare sets of mongod replica set members and juju hosts # to avoid unnecessary reconfiguration. - if replset_members == self.mongodb_config.hosts: + if replset_members == mongodb_hosts: + self._set_leader_unit_active_if_needed() return # remove members first, it is faster logger.info("Reconfigure replica set") - for member in replset_members - self.mongodb_config.hosts: + + for member in replset_members - mongodb_hosts: logger.debug("Removing %s from the replica set", member) mongo.remove_replset_member(member) - for member in self.mongodb_config.hosts - replset_members: - logger.debug("Adding %s to the replica set", member) - with MongoDBConnection( - self.mongodb_config, member, direct=True - ) as direct_mongo: - if not direct_mongo.is_ready: - logger.debug("Deferring reconfigure: %s is not ready yet.", member) - event.defer() - return - mongo.add_replset_member(member) + # to avoid potential race conditions - + # remove unit before adding new replica set members + if type(event) == RelationDepartedEvent and event.unit: + mongodb_hosts = mongodb_hosts - set([self.get_hostname_for_unit(event.unit)]) + self._remove_unit_from_replica_set(event, mongo, mongodb_hosts - replset_members) # app relations should be made aware of the new set of hosts self._update_app_relation_data(mongo.get_users()) except NotReadyError: @@ -332,6 +330,38 @@ def _reconfigure(self, event) -> None: logger.info("Deferring reconfigure: error=%r", e) event.defer() + def _on_storage_detaching(self, event: StorageDetachingEvent) -> None: + """Before storage detaches, allow removing unit to remove itself from the set. + + If the removing unit is primary also allow it to step down and elect another unit as + primary while it still has access to its storage. + """ + # if we are removing the last replica it will not be able to step down as primary and we + # cannot reconfigure the replica set to have 0 members. To prevent retrying for 10 minutes + # set this flag to True. please note that planned_units will always be >=1. When planned + # units is 1 that means there are no other peers expected. + + if self.app.planned_units() == 1 and (not self._peers or len(self._peers.units)) == 0: + return + + try: + logger.debug("Removing %s from replica set", self.get_hostname_for_unit(self.unit)) + # retries over a period of 10 minutes in an attempt to resolve race conditions it is + # not possible to defer in storage detached. + retries = Retrying(stop=stop_after_attempt(10), wait=wait_fixed(1), reraise=True) + for attempt in retries: + with attempt: + # remove_replset_member retries for 60 seconds + with MongoDBConnection(self.mongodb_config) as mongo: + hostname = self.get_hostname_for_unit(self.unit) + mongo.remove_replset_member(hostname) + except NotReadyError: + logger.info( + "Failed to remove %s from replica set, another member is syncing", self.unit.name + ) + except PyMongoError as e: + logger.error("Failed to remove %s from replica set, error=%r", self.unit.name, e) + # END: charm events # BEGIN: actions @@ -395,7 +425,7 @@ def _on_set_password(self, event: ActionEvent) -> None: reraise=True, before=before_log(logger, logging.DEBUG), ) - def _init_operator_user(self, container: Container) -> None: + def _init_operator_user(self) -> None: """Creates initial operator user for MongoDB. Initial operator user can be created only through localhost connection. @@ -409,6 +439,8 @@ def _init_operator_user(self, container: Container) -> None: if self._is_user_created(OperatorUser): return + container = self.unit.get_container(Config.CONTAINER_NAME) + mongo_cmd = ( "/usr/bin/mongosh" if container.exists("/usr/bin/mongosh") else "/usr/bin/mongo" ) @@ -463,12 +495,13 @@ def _get_mongodb_config_for_user( ) -> MongoDBConfiguration: external_ca, _ = self.tls.get_tls_files("unit") internal_ca, _ = self.tls.get_tls_files("app") + password = self.get_secret("app", user.get_password_key_name()) return MongoDBConfiguration( replset=self.app.name, database=user.get_database_name(), username=user.get_username(), - password=self.get_secret("app", user.get_password_key_name()), + password=password, # type: ignore hosts=set(hosts), roles=set(user.get_roles()), tls_external=external_ca is not None, @@ -491,6 +524,13 @@ def _check_or_set_user_password(self, user: MongoDBUser) -> None: if not self.get_secret("app", key): self.set_secret("app", key, generate_password()) + def _check_or_set_keyfile(self) -> None: + if not self.get_secret("app", "keyfile"): + self._generate_keyfile() + + def _generate_keyfile(self) -> None: + self.set_secret("app", "keyfile", generate_keyfile()) + def _generate_secrets(self) -> None: """Generate passwords and put them into peer relation. @@ -501,10 +541,9 @@ def _generate_secrets(self) -> None: self._check_or_set_user_password(OperatorUser) self._check_or_set_user_password(MonitorUser) - if not self.get_secret("app", "keyfile"): - self.set_secret("app", "keyfile", generate_keyfile()) + self._check_or_set_keyfile() - def _update_app_relation_data(self, database_users): + def _update_app_relation_data(self, database_users: Set[str]) -> None: """Helper function to update application relation data.""" for relation in self.model.relations[Config.Relations.NAME]: username = self.client_relations._get_username_from_relation_id(relation.id) @@ -518,6 +557,61 @@ def _update_app_relation_data(self, database_users): } ) + def _initialise_replica_set(self, event: StartEvent) -> None: + """Initialise replica set and create users.""" + if self._db_initialised: + # The replica set should be initialised only once. Check should be + # external (e.g., check initialisation inside peer relation). We + # shouldn't rely on MongoDB response because the data directory + # can be corrupted. + return + + # only leader should initialise the replica set + if not self.unit.is_leader(): + return + + with MongoDBConnection(self.mongodb_config, "localhost", direct=True) as direct_mongo: + try: + logger.info("Replica Set initialization") + direct_mongo.init_replset() + logger.info("User initialization") + self._init_operator_user() + self._init_monitor_user() + logger.info("Reconcile relations") + self.client_relations.oversee_users(None, event) + except ExecError as e: + logger.error( + "Deferring on_start: exit code: %i, stderr: %s", e.exit_code, e.stderr + ) + event.defer() + return + except PyMongoError as e: + logger.error("Deferring on_start since: error=%r", e) + event.defer() + return + + self._db_initialised = True + + def _remove_unit_from_replica_set( + self, event, mongo: MongoDBConnection, units_to_remove: Set[str] + ) -> None: + for member in units_to_remove: + logger.debug("Adding %s to the replica set", member) + with MongoDBConnection(self.mongodb_config, member, direct=True) as direct_mongo: + if not direct_mongo.is_ready: + logger.debug("Deferring reconfigure: %s is not ready yet.", member) + event.defer() + return + mongo.add_replset_member(member) + + def _set_leader_unit_active_if_needed(self): + # This can happen after restart mongod when enable \ disable TLS + if ( + isinstance(self.unit.status, WaitingStatus) + and self.unit.status.message == "waiting to reconfigure replica set" + ): + self.unit.status = ActiveStatus() + def get_secret(self, scope: str, key: str) -> Optional[str]: """Get TLS secret from the secret storage.""" if scope == "unit": @@ -528,7 +622,7 @@ def get_secret(self, scope: str, key: str) -> Optional[str]: raise RuntimeError("Unknown secret scope.") def set_secret(self, scope: str, key: str, value: Optional[str]) -> None: - """Get TLS secret from the secret storage.""" + """Set TLS secret in the secret storage.""" if scope == "unit": if not value: del self.unit_peer_data[key] @@ -542,19 +636,32 @@ def set_secret(self, scope: str, key: str, value: Optional[str]) -> None: else: raise RuntimeError("Unknown secret scope.") + def restart_mongod_service(self): + """Restart mongod service.""" + container = self.unit.get_container(Config.CONTAINER_NAME) + container.stop(Config.SERVICE_NAME) + + container.add_layer("mongod", self._mongod_layer, combine=True) + container.replan() + + self._connect_mongodb_exporter() + def _push_keyfile_to_workload(self, container: Container) -> None: """Upload the keyFile to a workload container.""" + keyfile = self.get_secret("app", "keyfile") + container.push( Config.CONF_DIR + "/" + Config.TLS.KEY_FILE_NAME, - self.get_secret("app", "keyfile"), + keyfile, # type: ignore make_dirs=True, permissions=0o400, user=Config.UNIX_USER, group=Config.UNIX_GROUP, ) - def _push_certificate_to_workload(self, container: Container) -> None: + def push_tls_certificate_to_workload(self) -> None: """Uploads certificate to the workload container.""" + container = self.unit.get_container(Config.CONTAINER_NAME) external_ca, external_pem = self.tls.get_tls_files("unit") if external_ca is not None: logger.debug("Uploading external ca to workload container") @@ -599,7 +706,16 @@ def _push_certificate_to_workload(self, container: Container) -> None: group=Config.UNIX_GROUP, ) - def get_hostname_by_unit(self, unit_name: str) -> str: + def delete_tls_certificate_from_workload(self) -> None: + """Deletes certificate from the workload container.""" + logger.error("Deleting TLS certificate from workload container") + container = self.unit.get_container(Config.CONTAINER_NAME) + container.remove_path(Config.CONF_DIR + "/" + Config.TLS.EXT_CA_FILE) + container.remove_path(Config.CONF_DIR + "/" + Config.TLS.EXT_PEM_FILE) + container.remove_path(Config.CONF_DIR + "/" + Config.TLS.INT_CA_FILE) + container.remove_path(Config.CONF_DIR + "/" + Config.TLS.INT_PEM_FILE) + + def get_hostname_for_unit(self, unit: Unit) -> str: """Create a DNS name for a MongoDB unit. Args: @@ -608,12 +724,12 @@ def get_hostname_by_unit(self, unit_name: str) -> str: Returns: A string representing the hostname of the MongoDB unit. """ - unit_id = unit_name.split("/")[1] + unit_id = unit.name.split("/")[1] return f"{self.app.name}-{unit_id}.{self.app.name}-endpoints" def _connect_mongodb_exporter(self) -> None: """Exposes the endpoint to mongodb_exporter.""" - container = self.unit.get_container("mongod") + container = self.unit.get_container(Config.CONTAINER_NAME) if not container.can_connect(): return @@ -621,7 +737,6 @@ def _connect_mongodb_exporter(self) -> None: # must wait for leader to set URI before connecting if not self.get_secret("app", MonitorUser.get_password_key_name()): return - # Add initial Pebble config layer using the Pebble API # mongodb_exporter --mongodb.uri= container.add_layer("mongodb_exporter", self._monitor_layer, combine=True) @@ -652,7 +767,7 @@ def _pull_licenses(container: Container) -> None: pass @staticmethod - def _fix_data_dir(container: Container) -> 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 diff --git a/src/config.py b/src/config.py index 82989ce94..b72d7d35e 100644 --- a/src/config.py +++ b/src/config.py @@ -6,6 +6,7 @@ class Config: """Configuration for MongoDB Charm.""" + SUBSTRATE = "k8s" MONGODB_PORT = 27017 UNIX_USER = "mongodb" UNIX_GROUP = "mongodb" diff --git a/tests/integration/ha_tests/helpers.py b/tests/integration/ha_tests/helpers.py index 36dea81ed..bc0d591e0 100644 --- a/tests/integration/ha_tests/helpers.py +++ b/tests/integration/ha_tests/helpers.py @@ -659,7 +659,9 @@ async def wait_until_unit_in_status( for member in data["members"]: if unit_to_check.name == host_to_unit(member["name"].split(":")[0]): - assert member["stateStr"] == status, f"{unit_to_check.name} status is not {status}" + assert ( + member["stateStr"] == status + ), f"{unit_to_check.name} status is not {status}. Actual status: {member['stateStr']}" return assert False, f"{unit_to_check.name} not found" diff --git a/tests/integration/test_teardown.py b/tests/integration/test_teardown.py index f7eae5f2a..983734117 100644 --- a/tests/integration/test_teardown.py +++ b/tests/integration/test_teardown.py @@ -49,31 +49,9 @@ async def test_build_and_deploy(ops_test: OpsTest): @pytest.mark.abort_on_fail -async def test_long_scale_up_scale_down_small_units(ops_test: OpsTest): +async def test_long_scale_up_scale_down_units(ops_test: OpsTest): """Scale up and down the application and verify the replica set is healthy.""" - scales = [2, -1, -1, 2, -2, 3, -3, 4, -4] - for iteration in range(len(scales)): - logger.info(f"Running scale up/down test. Iteration: {iteration}") - for count in scales: - await scale_and_verify(ops_test, count=count) - - -@pytest.mark.skip("Issues with juju teardown") -@pytest.mark.abort_on_fail -async def test_long_scale_up_scale_down_medium(ops_test: OpsTest): - """Scale up and down the application and verify the replica set is healthy.""" - scales = [5, -5] - for iteration in range(4): - logger.info(f"Running scale up/down test. Iteration: {iteration}") - for count in scales: - await scale_and_verify(ops_test, count=count) - - -@pytest.mark.skip("Issues with juju teardown") -@pytest.mark.abort_on_fail -async def test_scale_up_scale_down_large(ops_test: OpsTest): - """Scale up and down the application and verify the replica set is healthy.""" - scales = [6, -6, 7, -7] + scales = [2, -1, -1, 2, -2, 3, -3, 4, -4, 5, -5, 6, -6, 7, -7] for count in scales: await scale_and_verify(ops_test, count=count) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 56078dada..84789e51b 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -6,7 +6,7 @@ from unittest.mock import patch from charms.mongodb.v0.helpers import CONF_DIR, DATA_DIR, KEY_FILE -from ops.model import ActiveStatus, ModelError +from ops.model import ModelError from ops.pebble import APIError, ExecError, PathError, ProtocolError from ops.testing import Harness from pymongo.errors import ( @@ -46,7 +46,7 @@ def setUp(self): @patch("charm.MongoDBCharm._pull_licenses") @patch("ops.framework.EventBase.defer") - @patch("charm.MongoDBCharm._fix_data_dir") + @patch("charm.MongoDBCharm._set_data_dir_permissions") @patch("charm.MongoDBCharm._connect_mongodb_exporter") def test_mongod_pebble_ready(self, connect_exporter, fix_data_dir, defer, pull_licenses): # Expected plan after Pebble ready with default config @@ -81,8 +81,6 @@ def test_mongod_pebble_ready(self, connect_exporter, fix_data_dir, defer, pull_l # Check the service was started service = self.harness.model.unit.get_container("mongod").get_service("mongod") assert service.is_running() - # Ensure we set an ActiveStatus with no message - assert self.harness.model.unit.status == ActiveStatus() defer.assert_not_called() # Ensure that _connect_mongodb_exporter was called connect_exporter.assert_called_once() @@ -636,7 +634,7 @@ def test_connect_to_mongo_exporter_on_set_password(self, connect_exporter, conne @patch("charm.MongoDBCharm._pull_licenses") @patch("ops.framework.EventBase.defer") - @patch("charm.MongoDBCharm._fix_data_dir") + @patch("charm.MongoDBCharm._set_data_dir_permissions") @patch("charm.MongoDBConnection") def test__connect_mongodb_exporter_success( self, connection, fix_data_dir, defer, pull_licenses