[LDAP] Early exit when snap not refreshed

Author: sinclert-canonical

src/charm.py

@@ -112,7 +112,7 @@
 from relations.postgresql_provider import PostgreSQLProvider
 from rotate_logs import RotateLogs
 from upgrade import PostgreSQLUpgrade, get_postgresql_dependencies_model
-from utils import new_password
+from utils import new_password, snap_refreshed
 
 logger = logging.getLogger(__name__)
 
@@ -753,7 +753,9 @@ def _start_stop_pgbackrest_service(self, event: HookEvent) -> None:
         self.backup.coordinate_stanza_fields()
 
         if "exporter-started" not in self.unit_peer_data:
-            self._setup_exporter()
+            cache = snap.SnapCache()
+            postgres_snap = cache[POSTGRESQL_SNAP_NAME]
+            self._setup_exporter(postgres_snap)
 
     def _update_new_unit_status(self) -> None:
         """Update the status of a new unit that recently joined the cluster."""
@@ -1330,7 +1332,7 @@ def _restart_metrics_service(self) -> None:
             return None
 
         if snap_password != self.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY):
-            self._setup_exporter()
+            self._setup_exporter(postgres_snap)
 
     def _restart_ldap_sync_service(self) -> None:
         """Restart the LDAP sync service in case any configuration changed."""
@@ -1352,37 +1354,26 @@ def _restart_ldap_sync_service(self) -> None:
             return
 
         if self.is_primary and self.is_ldap_enabled:
-            self._setup_ldap_sync()
+            self._setup_ldap_sync(postgres_snap)
 
-    def _setup_exporter(self) -> None:
+    def _setup_exporter(self, postgres_snap: snap.Snap) -> None:
         """Set up postgresql_exporter options."""
-        cache = snap.SnapCache()
-        postgres_snap = cache[POSTGRESQL_SNAP_NAME]
-
-        if postgres_snap.revision != next(
-            filter(lambda snap_package: snap_package[0] == POSTGRESQL_SNAP_NAME, SNAP_PACKAGES)
-        )[1]["revision"].get(platform.machine()):
-            logger.debug(
-                "Early exit _setup_exporter: snap was not refreshed to the right version yet"
-            )
-            return
-
         postgres_snap.set({
             "exporter.user": MONITORING_USER,
             "exporter.password": self.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY),
         })
+
         if postgres_snap.services[MONITORING_SNAP_SERVICE]["active"] is False:
             postgres_snap.start(services=[MONITORING_SNAP_SERVICE], enable=True)
         else:
             postgres_snap.restart(services=[MONITORING_SNAP_SERVICE])
+
         self.unit_peer_data.update({"exporter-started": "True"})
 
-    def _setup_ldap_sync(self) -> None:
+    def _setup_ldap_sync(self, postgres_snap: snap.Snap) -> None:
         """Set up postgresql_ldap_sync options."""
-        cache = snap.SnapCache()
-        postgres_snap = cache[POSTGRESQL_SNAP_NAME]
-
         ldap_params = self.get_ldap_parameters()
+
         ldap_url = urlparse(ldap_params["ldapurl"])
         ldap_host = ldap_url.hostname
         ldap_port = ldap_url.port
@@ -2053,6 +2044,14 @@ def update_config(self, is_creating_backup: bool = False, no_peers: bool = False
         })
 
         self._handle_postgresql_restart_need(enable_tls)
+
+        cache = snap.SnapCache()
+        postgres_snap = cache[POSTGRESQL_SNAP_NAME]
+
+        if not snap_refreshed(postgres_snap.revision):
+            logger.debug("Early exit: snap was not refreshed to the right version yet")
+            return True
+
         self._restart_metrics_service()
         self._restart_ldap_sync_service()
 

src/utils.py

@@ -3,9 +3,15 @@
 
 """A collection of utility functions that are used in the charm."""
 
+import platform
 import secrets
 import string
 
+from constants import (
+    POSTGRESQL_SNAP_NAME,
+    SNAP_PACKAGES,
+)
+
 
 def new_password() -> str:
     """Generate a random password string.
@@ -16,3 +22,16 @@ def new_password() -> str:
     choices = string.ascii_letters + string.digits
     password = "".join([secrets.choice(choices) for i in range(16)])
     return password
+
+
+def snap_refreshed(target_rev: str) -> bool:
+    """Whether the snap was refreshed to the target version."""
+    arch = platform.machine()
+
+    for snap_package in SNAP_PACKAGES:
+        snap_name = snap_package[0]
+        snap_revs = snap_package[1]["revision"]
+        if snap_name == POSTGRESQL_SNAP_NAME and target_rev != snap_revs.get(arch):
+            return False
+
+    return True

tests/unit/test_charm.py

@@ -1270,6 +1270,7 @@ def test_restart(harness):
 def test_update_config(harness):
     with (
         patch("subprocess.check_output", return_value=b"C"),
+        patch("charm.snap_refreshed", return_value=True),
         patch("charm.snap.SnapCache"),
         patch(
             "charm.PostgresqlOperatorCharm._handle_postgresql_restart_need"

tests/unit/test_utils.py

@@ -2,8 +2,10 @@
 # See LICENSE file for licensing details.
 
 import re
+from unittest.mock import patch
 
-from utils import new_password
+from constants import POSTGRESQL_SNAP_NAME
+from utils import new_password, snap_refreshed
 
 
 def test_new_password():
@@ -16,3 +18,19 @@ def test_new_password():
     second_password = new_password()
     assert re.fullmatch("[a-zA-Z0-9\b]{16}$", second_password) is not None
     assert second_password != first_password
+
+
+def test_snap_refreshed():
+    with patch(
+        "utils.SNAP_PACKAGES",
+        [(POSTGRESQL_SNAP_NAME, {"revision": {"aarch64": "100", "x86_64": "100"}})],
+    ):
+        assert snap_refreshed("100") is True
+        assert snap_refreshed("200") is False
+
+    with patch(
+        "utils.SNAP_PACKAGES",
+        [(POSTGRESQL_SNAP_NAME, {"revision": {}})],
+    ):
+        assert snap_refreshed("100") is False
+        assert snap_refreshed("200") is False