From 0141b7a76663d3f09ca89cd5712a699507837d50 Mon Sep 17 00:00:00 2001
From: Neha Oudin <17551419+Gu1nness@users.noreply.github.com>
Date: Tue, 27 Aug 2024 10:17:00 +0200
Subject: [PATCH] DPE-4904: Storage race condition + TLS update (#311)

Co-authored-by: Mia Altieri <32723809+MiaAltieri@users.noreply.github.com>
---
 lib/charms/mongodb/v1/mongodb_tls.py | 11 ++++++++++-
 src/charm.py                         |  8 ++++++++
 tests/unit/test_charm.py             | 26 +++++++++++++++++++++++++-
 3 files changed, 43 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..9113fd412 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -526,6 +526,14 @@ 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()
+            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()