[DPE-3562] Don't block on missing Postgresql version (#626)

* Don't block on missing Postgresql version

* Defer on failure to create db resources

* Fix lint

* Bump libs

lib/charms/postgresql_k8s/v0/postgresql.py

@@ -36,7 +36,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 33
+LIBPATCH = 34
 
 INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles"
 
@@ -425,14 +425,20 @@ def get_postgresql_timezones(self) -> Set[str]:
             timezones = cursor.fetchall()
             return {timezone[0] for timezone in timezones}
 
-    def get_postgresql_version(self) -> str:
+    def get_postgresql_version(self, current_host=True) -> str:
         """Returns the PostgreSQL version.
 
         Returns:
             PostgreSQL version number.
         """
+        if current_host:
+            host = self.current_host
+        else:
+            host = None
         try:
-            with self._connect_to_database() as connection, connection.cursor() as cursor:
+            with self._connect_to_database(
+                database_host=host
+            ) as connection, connection.cursor() as cursor:
                 cursor.execute("SELECT version();")
                 # Split to get only the version number.
                 return cursor.fetchone()[0].split(" ")[1]

lib/charms/tempo_k8s/v1/charm_tracing.py

@@ -224,7 +224,6 @@ def _remove_stale_otel_sdk_packages():
 
 _remove_stale_otel_sdk_packages()
 
-
 import functools
 import inspect
 import logging
@@ -271,7 +270,7 @@ def _remove_stale_otel_sdk_packages():
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
 
-LIBPATCH = 14
+LIBPATCH = 15
 
 PYDEPS = ["opentelemetry-exporter-otlp-proto-http==1.21.0"]
 
@@ -281,7 +280,6 @@ def _remove_stale_otel_sdk_packages():
 # set this to 0 if you are debugging/developing this library source
 dev_logger.setLevel(logging.CRITICAL)
 
-
 _CharmType = Type[CharmBase]  # the type CharmBase and any subclass thereof
 _C = TypeVar("_C", bound=_CharmType)
 _T = TypeVar("_T", bound=type)
@@ -333,9 +331,22 @@ def _get_tracer() -> Optional[Tracer]:
     try:
         return tracer.get()
     except LookupError:
+        # fallback: this course-corrects for a user error where charm_tracing symbols are imported
+        # from different paths (typically charms.tempo_k8s... and lib.charms.tempo_k8s...)
         try:
             ctx: Context = copy_context()
             if context_tracer := _get_tracer_from_context(ctx):
+                logger.warning(
+                    "Tracer not found in `tracer` context var. "
+                    "Verify that you're importing all `charm_tracing` symbols from the same module path. \n"
+                    "For example, DO"
+                    ": `from charms.lib...charm_tracing import foo, bar`. \n"
+                    "DONT: \n"
+                    " \t - `from charms.lib...charm_tracing import foo` \n"
+                    " \t - `from lib...charm_tracing import bar` \n"
+                    "For more info: https://python-notes.curiousefficiency.org/en/latest/python"
+                    "_concepts/import_traps.html#the-double-import-trap"
+                )
                 return context_tracer.get()
             else:
                 return None

src/relations/db.py

@@ -96,7 +96,13 @@ def _on_relation_changed(self, event: RelationChangedEvent) -> None:
 
         logger.warning(f"DEPRECATION WARNING - `{self.relation_name}` is a legacy interface")
 
-        self.set_up_relation(event.relation)
+        if (
+            not self.set_up_relation(event.relation)
+            and self.charm.unit.status.message
+            == f"Failed to initialize {self.relation_name} relation"
+        ):
+            event.defer()
+            return
 
     def _check_exist_current_relation(self) -> bool:
         for r in self.charm.client_relations:
@@ -152,12 +158,9 @@ def set_up_relation(self, relation: Relation) -> bool:
             self.charm.unit.status = BlockedStatus(ROLES_BLOCKING_MESSAGE)
             return False
 
-        database = relation.data.get(relation.app, {}).get("database")
-        if not database:
+        if not (database := relation.data.get(relation.app, {}).get("database")):
             for unit in relation.units:
-                unit_database = relation.data.get(unit, {}).get("database")
-                if unit_database:
-                    database = unit_database
+                if database := relation.data.get(unit, {}).get("database"):
                     break
 
         if not database:
@@ -207,31 +210,40 @@ def set_up_relation(self, relation: Relation) -> bool:
                 )
             )
 
+            postgresql_version = None
+            try:
+                postgresql_version = self.charm.postgresql.get_postgresql_version()
+            except PostgreSQLGetPostgreSQLVersionError:
+                logger.exception(
+                    "Failed to retrieve the PostgreSQL version to initialise/update %s relation"
+                    % self.relation_name
+                )
+
             # Set the data in both application and unit data bag.
             # It's needed to run this logic on every relation changed event
             # setting the data again in the databag, otherwise the application charm that
             # is connecting to this database will receive a "database gone" event from the
             # old PostgreSQL library (ops-lib-pgsql) and the connection between the
             # application and this charm will not work.
-            for databag in [application_relation_databag, unit_relation_databag]:
-                updates = {
-                    "allowed-subnets": self._get_allowed_subnets(relation),
-                    "allowed-units": self._get_allowed_units(relation),
-                    "host": self.charm.endpoint,
-                    "master": primary,
-                    "port": DATABASE_PORT,
-                    "standbys": standbys,
-                    "version": self.charm.postgresql.get_postgresql_version(),
-                    "user": user,
-                    "password": password,
-                    "database": database,
-                    "extensions": ",".join(required_extensions),
-                }
-                databag.update(updates)
+            updates = {
+                "allowed-subnets": self._get_allowed_subnets(relation),
+                "allowed-units": self._get_allowed_units(relation),
+                "host": self.charm.endpoint,
+                "master": primary,
+                "port": DATABASE_PORT,
+                "standbys": standbys,
+                "user": user,
+                "password": password,
+                "database": database,
+                "extensions": ",".join(required_extensions),
+            }
+            if postgresql_version:
+                updates["version"] = postgresql_version
+            application_relation_databag.update(updates)
+            unit_relation_databag.update(updates)
         except (
             PostgreSQLCreateDatabaseError,
             PostgreSQLCreateUserError,
-            PostgreSQLGetPostgreSQLVersionError,
         ):
             self.charm.unit.status = BlockedStatus(
                 f"Failed to initialize {self.relation_name} relation"

tests/unit/test_db.py

@@ -191,6 +191,7 @@ def test_set_up_relation(harness):
         patch("relations.db.DbProvides._update_unit_status") as _update_unit_status,
         patch("relations.db.new_password", return_value="test-password") as _new_password,
         patch("relations.db.DbProvides._get_extensions") as _get_extensions,
+        patch("relations.db.logger") as _logger,
     ):
         rel_id = harness.model.get_relation(RELATION_NAME).id
         # Define some mocks' side effects.
@@ -210,16 +211,7 @@ def test_set_up_relation(harness):
         postgresql_mock.create_database = PropertyMock(
             side_effect=[None, None, PostgreSQLCreateDatabaseError, None]
         )
-        postgresql_mock.get_postgresql_version = PropertyMock(
-            side_effect=[
-                POSTGRESQL_VERSION,
-                POSTGRESQL_VERSION,
-                POSTGRESQL_VERSION,
-                POSTGRESQL_VERSION,
-                POSTGRESQL_VERSION,
-                PostgreSQLGetPostgreSQLVersionError,
-            ]
-        )
+        postgresql_mock.get_postgresql_version = PropertyMock(return_value=POSTGRESQL_VERSION)
 
         # Assert no operation is done when at least one of the requested extensions
         # is disabled.
@@ -244,7 +236,7 @@ def test_set_up_relation(harness):
         postgresql_mock.create_database.assert_called_once_with(
             DATABASE, user, plugins=[], client_relations=[relation]
         )
-        assert postgresql_mock.get_postgresql_version.call_count == 2
+        assert postgresql_mock.get_postgresql_version.call_count == 1
         _update_unit_status.assert_called_once()
         expected_data = {
             "allowed-units": "application/0",
@@ -289,7 +281,7 @@ def test_set_up_relation(harness):
         postgresql_mock.create_database.assert_called_once_with(
             DATABASE, user, plugins=[], client_relations=[relation]
         )
-        assert postgresql_mock.get_postgresql_version.call_count == 2
+        assert postgresql_mock.get_postgresql_version.call_count == 1
         _update_unit_status.assert_called_once()
         assert harness.get_relation_data(rel_id, harness.charm.app.name) == expected_data
         assert harness.get_relation_data(rel_id, harness.charm.unit.name) == expected_data
@@ -343,11 +335,13 @@ def test_set_up_relation(harness):
         assert harness.get_relation_data(rel_id, harness.charm.app.name) == {}
         assert harness.get_relation_data(rel_id, harness.charm.unit.name) == {}
 
-        # BlockedStatus due to a PostgreSQLGetPostgreSQLVersionError.
+        # version is not updated due to a PostgreSQLGetPostgreSQLVersionError.
+        postgresql_mock.get_postgresql_version.side_effect = PostgreSQLGetPostgreSQLVersionError
         harness.charm.unit.status = ActiveStatus()
-        assert not harness.charm.legacy_db_relation.set_up_relation(relation)
-        _update_unit_status.assert_not_called()
-        assert isinstance(harness.model.unit.status, BlockedStatus)
+        assert harness.charm.legacy_db_relation.set_up_relation(relation)
+        _logger.exception.assert_called_once_with(
+            "Failed to retrieve the PostgreSQL version to initialise/update db relation"
+        )
 
 
 def test_update_unit_status(harness):