diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index f7d361b5a9..c3412d36df 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/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] diff --git a/lib/charms/tempo_k8s/v1/charm_tracing.py b/lib/charms/tempo_k8s/v1/charm_tracing.py index fa926539bc..2dbdddd684 100644 --- a/lib/charms/tempo_k8s/v1/charm_tracing.py +++ b/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 diff --git a/src/relations/db.py b/src/relations/db.py index 1bba5cec43..290528d251 100644 --- a/src/relations/db.py +++ b/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" diff --git a/tests/unit/test_db.py b/tests/unit/test_db.py index 2848f8f013..2e262428f1 100644 --- a/tests/unit/test_db.py +++ b/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):