[DPE-2415] generalise lib code so easily repurposable by VM (#195)

* generalise code so easily repurposable by VM

* resolve bug in status check

lib/charms/mongodb/v0/helpers.py

@@ -1,9 +1,9 @@
 """Simple functions, which can be used in both K8s and VM charms."""
 # Copyright 2023 Canonical Ltd.
 # See LICENSE file for licensing details.
+import json
 import logging
 import os
-import re
 import secrets
 import string
 import subprocess
@@ -27,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 = 7
+LIBPATCH = 8
 
 
 # path to store mongodb ketFile
@@ -223,43 +223,26 @@ def process_pbm_error(error_string: Optional[_StrOrBytes]) -> str:
 
 def current_pbm_op(pbm_status: str) -> str:
     """Parses pbm status for the operation that pbm is running."""
-    pbm_status_lines = pbm_status.splitlines()
-    for i in range(0, len(pbm_status_lines)):
-        line = pbm_status_lines[i]
-
-        # operation is two lines after the line "Currently running:"
-        if line == "Currently running:":
-            return pbm_status_lines[i + 2]
-
-    return ""
+    pbm_status = json.loads(pbm_status)
+    return pbm_status["running"] if "running" in pbm_status else ""
 
 
 def process_pbm_status(pbm_status: str) -> StatusBase:
     """Parses current pbm operation and returns unit status."""
-    if type(pbm_status) == bytes:
-        pbm_status = pbm_status.decode("utf-8")
-
-    # pbm is running resync operation
-    if "Resync" in current_pbm_op(pbm_status):
-        return WaitingStatus("waiting to sync s3 configurations.")
-
+    current_op = current_pbm_op(pbm_status)
     # no operations are currently running with pbm
-    if "(none)" in current_pbm_op(pbm_status):
+    if current_op == {}:
         return ActiveStatus("")
 
-    # Example of backup id: 2023-08-21T13:08:22Z
-    backup_match = re.search(
-        r'Snapshot backup "(?P<backup_id>\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z)"', pbm_status
-    )
-    if backup_match:
-        backup_id = backup_match.group("backup_id")
+    if current_op["type"] == "backup":
+        backup_id = current_op["name"]
         return MaintenanceStatus(f"backup started/running, backup id:'{backup_id}'")
 
-    restore_match = re.search(
-        r'Snapshot restore "(?P<backup_id>\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z)"', pbm_status
-    )
-    if restore_match:
-        backup_id = restore_match.group("backup_id")
+    if current_op["type"] == "restore":
+        backup_id = current_op["name"]
         return MaintenanceStatus(f"restore started/running, backup id:'{backup_id}'")
 
+    if current_op["type"] == "resync":
+        return WaitingStatus("waiting to sync s3 configurations.")
+
     return ActiveStatus()

lib/charms/mongodb/v0/mongodb_backups.py

@@ -23,13 +23,7 @@
 )
 from charms.operator_libs_linux.v1 import snap
 from ops.framework import Object
-from ops.model import (
-    BlockedStatus,
-    MaintenanceStatus,
-    ModelError,
-    StatusBase,
-    WaitingStatus,
-)
+from ops.model import BlockedStatus, MaintenanceStatus, StatusBase, WaitingStatus
 from ops.pebble import ExecError
 from tenacity import (
     Retrying,
@@ -48,7 +42,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
 
 logger = logging.getLogger(__name__)
 
@@ -110,11 +104,10 @@ def _restore_retry_stop_condition(retry_state) -> bool:
 class MongoDBBackups(Object):
     """Manages MongoDB backups."""
 
-    def __init__(self, charm, substrate):
+    def __init__(self, charm):
         """Manager of MongoDB client relations."""
         super().__init__(charm, "client-relations")
         self.charm = charm
-        self.substrate = substrate
 
         # s3 relation handles the config options for s3 backups
         self.s3_client = S3Requirer(self.charm, S3_RELATION)
@@ -136,10 +129,7 @@ def _on_s3_credential_changed(self, event: CredentialsChangedEvent):
             )
             return
 
-        try:
-            # TODO VM charm should implement this  method
-            self.charm.get_backup_service()
-        except ModelError:
+        if not self.charm.has_backup_service():
             self._defer_action_with_info_log(
                 event, action, "Set PBM configurations, pbm-agent service not found."
             )
@@ -335,14 +325,14 @@ def _configure_pbm_options(self, event) -> None:
 
     def _set_config_options(self):
         """Applying given configurations with pbm."""
-        # TODO VM charm should implement this method
-        self.charm.set_pbm_config_file()
+        # clearing out configurations options before resetting them leads to a quicker reysnc
+        # process
+        self.charm.clear_pbm_config_file()
 
         # the pbm tool can only set one configuration at a time.
         for pbm_key, pbm_value in self._get_pbm_configs().items():
             try:
                 config_cmd = ["config", "--set", f"{pbm_key}={pbm_value}"]
-                # TODO VM charm should implement this method
                 self.charm.run_pbm_command(config_cmd)
             except (subprocess.CalledProcessError, ExecError):
                 logger.error(
@@ -364,7 +354,6 @@ def _get_pbm_configs(self) -> Dict:
 
     def _resync_config_options(self):
         """Attempts to sync pbm config options and sets status in case of failure."""
-        # TODO VM charm should implement this method
         self.charm.start_backup_service()
 
         # pbm has a flakely resync and it is necessary to wait for no actions to be running before
@@ -382,7 +371,6 @@ def _resync_config_options(self):
 
                 # if a resync is running restart the service
                 if isinstance(pbm_status, (WaitingStatus)):
-                    # TODO VM charm should implement this method
                     self.charm.restart_backup_service()
                     raise PBMBusyError
 
@@ -420,7 +408,6 @@ def _wait_pbm_status(self) -> None:
         ):
             with attempt:
                 try:
-                    # TODO VM charm should implement this method
                     pbm_status = self.charm.run_pbm_command(PBM_STATUS_CMD)
 
                     if "Resync" in current_pbm_op(pbm_status):
@@ -435,10 +422,7 @@ def _wait_pbm_status(self) -> None:
 
     def _get_pbm_status(self) -> StatusBase:
         """Retrieve pbm status."""
-        try:
-            # TODO VM charm should implement this method
-            self.charm.get_backup_service()
-        except ModelError:
+        if not self.charm.has_backup_service():
             return WaitingStatus("waiting for pbm to start")
 
         try:
@@ -534,7 +518,10 @@ def _try_to_restore(self, backup_id: str) -> None:
             with attempt:
                 try:
                     remapping_args = self._remap_replicaset(backup_id)
-                    self.charm.run_pbm_restore_command(backup_id, remapping_args)
+                    restore_cmd = ["restore", backup_id]
+                    if remapping_args:
+                        restore_cmd = restore_cmd + remapping_args.split(" ")
+                    self.charm.run_pbm_command(restore_cmd)
                 except (subprocess.CalledProcessError, ExecError) as e:
                     if type(e) == subprocess.CalledProcessError:
                         error_message = e.output.decode("utf-8")

src/charm.py

@@ -48,6 +48,7 @@
     BlockedStatus,
     Container,
     MaintenanceStatus,
+    ModelError,
     Relation,
     RelationDataContent,
     SecretNotFoundError,
@@ -110,7 +111,7 @@ def __init__(self, *args):
 
         self.client_relations = MongoDBProvider(self)
         self.tls = MongoDBTLS(self, Config.Relations.PEERS, Config.SUBSTRATE)
-        self.backups = MongoDBBackups(self, substrate=Config.SUBSTRATE)
+        self.backups = MongoDBBackups(self)
 
         self.metrics_endpoint = MetricsEndpointProvider(
             self, refresh_event=self.on.start, jobs=Config.Monitoring.JOBS
@@ -1109,10 +1110,15 @@ def _connect_pbm_agent(self) -> None:
         container.add_layer(Config.Backup.SERVICE_NAME, self._backup_layer, combine=True)
         container.replan()
 
-    def get_backup_service(self) -> ServiceInfo:
+    def has_backup_service(self) -> ServiceInfo:
         """Returns the backup service."""
         container = self.unit.get_container(Config.CONTAINER_NAME)
-        return container.get_service(Config.Backup.SERVICE_NAME)
+        try:
+            container.get_service(Config.Backup.SERVICE_NAME)
+        except ModelError:
+            return False
+
+        return True
 
     def is_unit_in_replica_set(self) -> bool:
         """Check if the unit is in the replica set."""
@@ -1134,15 +1140,8 @@ def run_pbm_command(self, cmd: List[str]) -> str:
         stdout, _ = process.wait_output()
         return stdout
 
-    def run_pbm_restore_command(self, backup_id: str, remapping_args: str) -> str:
-        """Executes a restore command in the workload container."""
-        restore_cmd = ["restore", backup_id]
-        if remapping_args:
-            restore_cmd = restore_cmd + remapping_args.split(" ")
-        return self.run_pbm_command(restore_cmd)
-
-    def set_pbm_config_file(self) -> None:
-        """Sets the pbm config file."""
+    def clear_pbm_config_file(self) -> None:
+        """Overwrites existing config file with an empty file."""
         container = self.unit.get_container(Config.CONTAINER_NAME)
         container.push(
             Config.Backup.PBM_CONFIG_FILE_PATH,

tests/unit/test_charm.py

@@ -746,18 +746,18 @@ def test_connect_to_mongo_exporter_on_set_password(self, connect_exporter, conne
         connect_exporter.assert_called()
 
     @patch("charm.MongoDBBackups._get_pbm_status")
-    @patch("charm.MongoDBCharm.get_backup_service")
+    @patch("charm.MongoDBCharm.has_backup_service")
     @patch("charm.MongoDBConnection")
     @patch("charm.MongoDBCharm._connect_mongodb_exporter")
     def test_event_set_password_secrets(
-        self, connect_exporter, connection, get_backup_service, get_pbm_status
+        self, connect_exporter, connection, has_backup_service, get_pbm_status
     ):
         """Test _connect_mongodb_exporter is called when the password is set for 'montior' user.
 
         Furthermore: in Juju 3.x we want to use secrets
         """
         pw = "bla"
-        get_backup_service.return_value = "pbm"
+        has_backup_service.return_value = True
         get_pbm_status.return_value = ActiveStatus()
         self.harness.set_leader(True)
 
@@ -778,17 +778,17 @@ def test_event_set_password_secrets(
         assert args_pw["password"] == pw
 
     @patch("charm.MongoDBBackups._get_pbm_status")
-    @patch("charm.MongoDBCharm.get_backup_service")
+    @patch("charm.MongoDBCharm.has_backup_service")
     @patch("charm.MongoDBConnection")
     @patch("charm.MongoDBCharm._connect_mongodb_exporter")
     def test_event_auto_reset_password_secrets_when_no_pw_value_shipped(
-        self, connect_exporter, connection, get_backup_service, get_pbm_status
+        self, connect_exporter, connection, has_backup_service, get_pbm_status
     ):
         """Test _connect_mongodb_exporter is called when the password is set for 'montior' user.
 
         Furthermore: in Juju 3.x we want to use secrets
         """
-        get_backup_service.return_value = "pbm"
+        has_backup_service.return_value = True
         get_pbm_status.return_value = ActiveStatus()
         self._setup_secrets()
         self.harness.set_leader(True)
@@ -927,14 +927,14 @@ def test_set_password_provided(self, connection):
         self.assertEqual("canonical123", new_password)
 
     @patch_network_get(private_address="1.1.1.1")
-    @patch("charm.MongoDBCharm.get_backup_service")
+    @patch("charm.MongoDBCharm.has_backup_service")
     @patch("charm.MongoDBBackups._get_pbm_status")
-    def test_set_backup_password_pbm_busy(self, pbm_status, get_backup_service):
+    def test_set_backup_password_pbm_busy(self, pbm_status, has_backup_service):
         """Tests changes to passwords fail when pbm is restoring/backing up."""
         self.harness.set_leader(True)
         original_password = "pass123"
         action_event = mock.Mock()
-        get_backup_service.return_value = "pbm"
+        has_backup_service.return_value = True
 
         for username in ["backup", "monitor", "operator"]:
             self.harness.charm.app_peer_data[f"{username}-password"] = original_password