[DPE-2844] Split indico and extension blocking tests (#163)

* Split indico and extension blocking tests

* Bump libs

Author: dragomirp

lib/charms/data_platform_libs/v0/data_interfaces.py

@@ -320,7 +320,7 @@ def _on_topic_requested(self, event: TopicRequestedEvent):
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 22
+LIBPATCH = 24
 
 PYDEPS = ["ops>=2.0.0"]
 
@@ -526,7 +526,16 @@ def get_content(self) -> Dict[str, str]:
         """Getting cached secret content."""
         if not self._secret_content:
             if self.meta:
-                self._secret_content = self.meta.get_content()
+                try:
+                    self._secret_content = self.meta.get_content(refresh=True)
+                except (ValueError, ModelError) as err:
+                    # https://bugs.launchpad.net/juju/+bug/2042596
+                    # Only triggered when 'refresh' is set
+                    msg = "ERROR either URI or label should be used for getting an owned secret but not both"
+                    if isinstance(err, ModelError) and msg not in str(err):
+                        raise
+                    # Due to: ValueError: Secret owner cannot use refresh=True
+                    self._secret_content = self.meta.get_content()
         return self._secret_content
 
     def set_content(self, content: Dict[str, str]) -> None:
@@ -807,6 +816,9 @@ def _fetch_relation_data_without_secrets(
         This is used typically when the Provides side wants to read the Requires side's data,
         or when the Requires side may want to read its own data.
         """
+        if app not in relation.data or not relation.data[app]:
+            return {}
+
         if fields:
             return {k: relation.data[app][k] for k in fields if k in relation.data[app]}
         else:
@@ -830,6 +842,9 @@ def _fetch_relation_data_with_secrets(
         normal_fields = []
 
         if not fields:
+            if app not in relation.data or not relation.data[app]:
+                return {}
+
             all_fields = list(relation.data[app].keys())
             normal_fields = [field for field in all_fields if not self._is_secret_field(field)]
 
@@ -853,8 +868,11 @@ def _fetch_relation_data_with_secrets(
 
     def _update_relation_data_without_secrets(
         self, app: Application, relation: Relation, data: Dict[str, str]
-    ):
+    ) -> None:
         """Updating databag contents when no secrets are involved."""
+        if app not in relation.data or relation.data[app] is None:
+            return
+
         if any(self._is_secret_field(key) for key in data.keys()):
             raise SecretsIllegalUpdateError("Can't update secret {key}.")
 
@@ -865,8 +883,19 @@ def _delete_relation_data_without_secrets(
         self, app: Application, relation: Relation, fields: List[str]
     ) -> None:
         """Remove databag fields 'fields' from Relation."""
+        if app not in relation.data or not relation.data[app]:
+            return
+
         for field in fields:
-            relation.data[app].pop(field)
+            try:
+                relation.data[app].pop(field)
+            except KeyError:
+                logger.debug(
+                    "Non-existing field was attempted to be removed from the databag %s, %s",
+                    str(relation.id),
+                    str(field),
+                )
+                pass
 
     # Public interface methods
     # Handling Relation Fields seamlessly, regardless if in databag or a Juju Secret
@@ -880,9 +909,6 @@ def get_relation(self, relation_name, relation_id) -> Relation:
                 "Relation %s %s couldn't be retrieved", relation_name, relation_id
             )
 
-        if not relation.app:
-            raise DataInterfacesError("Relation's application missing")
-
         return relation
 
     def fetch_relation_data(
@@ -1068,7 +1094,7 @@ def _delete_relation_secret(
         secret = self._get_relation_secret(relation.id, group)
 
         if not secret:
-            logging.error("Can't update secret for relation %s", str(relation.id))
+            logging.error("Can't delete secret for relation %s", str(relation.id))
             return False
 
         old_content = secret.get_content()
@@ -1089,7 +1115,10 @@ def _delete_relation_secret(
         # Remove secret from the relation if it's fully gone
         if not new_content:
             field = self._generate_secret_field_name(group)
-            relation.data[self.local_app].pop(field)
+            try:
+                relation.data[self.local_app].pop(field)
+            except KeyError:
+                pass
 
         # Return the content that was removed
         return True
@@ -1807,7 +1836,8 @@ def _assign_relation_alias(self, relation_id: int) -> None:
 
         # We need to set relation alias also on the application level so,
         # it will be accessible in show-unit juju command, executed for a consumer application unit
-        self.update_relation_data(relation_id, {"alias": available_aliases[0]})
+        if self.local_unit.is_leader():
+            self.update_relation_data(relation_id, {"alias": available_aliases[0]})
 
     def _emit_aliased_event(self, event: RelationChangedEvent, event_name: str) -> None:
         """Emit an aliased event to a particular relation if it has an alias.
@@ -1894,6 +1924,9 @@ def _on_relation_created_event(self, event: RelationCreatedEvent) -> None:
 
         # Sets both database and extra user roles in the relation
         # if the roles are provided. Otherwise, sets only the database.
+        if not self.local_unit.is_leader():
+            return
+
         if self.extra_user_roles:
             self.update_relation_data(
                 event.relation.id,
@@ -2153,6 +2186,9 @@ def _on_relation_created_event(self, event: RelationCreatedEvent) -> None:
         """Event emitted when the Kafka relation is created."""
         super()._on_relation_created_event(event)
 
+        if not self.local_unit.is_leader():
+            return
+
         # Sets topic, extra user roles, and "consumer-group-prefix" in the relation
         relation_data = {
             f: getattr(self, f.replace("-", "_"), "")
@@ -2325,6 +2361,9 @@ def _on_relation_created_event(self, event: RelationCreatedEvent) -> None:
         """Event emitted when the OpenSearch relation is created."""
         super()._on_relation_created_event(event)
 
+        if not self.local_unit.is_leader():
+            return
+
         # Sets both index and extra user roles in the relation if the roles are provided.
         # Otherwise, sets only the index.
         data = {"index": self.index}

lib/charms/postgresql_k8s/v0/postgresql.py

@@ -32,7 +32,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 18
+LIBPATCH = 20
 
 INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles"
 
@@ -172,8 +172,7 @@ def create_database(self, database: str, user: str, plugins: List[str] = []) ->
             raise PostgreSQLCreateDatabaseError()
 
         # Enable preset extensions
-        for plugin in plugins:
-            self.enable_disable_extension(plugin, True, database)
+        self.enable_disable_extensions({plugin: True for plugin in plugins}, database)
 
     def create_user(
         self, user: str, password: str = None, admin: bool = False, extra_user_roles: str = None
@@ -270,22 +269,16 @@ def delete_user(self, user: str) -> None:
             logger.error(f"Failed to delete user: {e}")
             raise PostgreSQLDeleteUserError()
 
-    def enable_disable_extension(self, extension: str, enable: bool, database: str = None) -> None:
+    def enable_disable_extensions(self, extensions: Dict[str, bool], database: str = None) -> None:
         """Enables or disables a PostgreSQL extension.
 
         Args:
-            extension: the name of the extensions.
-            enable: whether the extension should be enabled or disabled.
+            extensions: the name of the extensions.
             database: optional database where to enable/disable the extension.
 
         Raises:
             PostgreSQLEnableDisableExtensionError if the operation fails.
         """
-        statement = (
-            f"CREATE EXTENSION IF NOT EXISTS {extension};"
-            if enable
-            else f"DROP EXTENSION IF EXISTS {extension};"
-        )
         connection = None
         try:
             if database is not None:
@@ -301,7 +294,12 @@ def enable_disable_extension(self, extension: str, enable: bool, database: str =
                 with self._connect_to_database(
                     database=database
                 ) as connection, connection.cursor() as cursor:
-                    cursor.execute(statement)
+                    for extension, enable in extensions.items():
+                        cursor.execute(
+                            f"CREATE EXTENSION IF NOT EXISTS {extension};"
+                            if enable
+                            else f"DROP EXTENSION IF EXISTS {extension};"
+                        )
         except psycopg2.errors.UniqueViolation:
             pass
         except psycopg2.Error:
@@ -514,7 +512,7 @@ def build_postgresql_parameters(
             )
         if profile == "production":
             # Use 25% of the available memory for shared_buffers.
-            # and the remaind as cache memory.
+            # and the remaining as cache memory.
             shared_buffers = int(available_memory * 0.25)
             effective_cache_size = int(available_memory - shared_buffers)
             parameters.setdefault("shared_buffers", f"{int(shared_buffers/10**6)}MB")

tests/integration/relations/pgbouncer_provider/test_pgbouncer_provider.py

@@ -4,17 +4,18 @@
 import asyncio
 import json
 import logging
-from pathlib import Path
 
 import psycopg2
 import pytest
-import yaml
 from pytest_operator.plugin import OpsTest
 
 from constants import BACKEND_RELATION_NAME
 
 from ...helpers.helpers import (
     CHARM_SERIES,
+    PG,
+    PGB,
+    PGB_METADATA,
     get_app_relation_databag,
     get_backend_relation,
     get_backend_user_pass,
@@ -40,12 +41,9 @@
 CLIENT_APP_NAME = "postgresql-test-app"
 SECONDARY_CLIENT_APP_NAME = "secondary-application"
 DATA_INTEGRATOR_APP_NAME = "data-integrator"
-PG = "postgresql-k8s"
-PGB_METADATA = yaml.safe_load(Path("./metadata.yaml").read_text())
 PGB_RESOURCES = {
     "pgbouncer-image": PGB_METADATA["resources"]["pgbouncer-image"]["upstream-source"]
 }
-PGB = "pgbouncer-k8s"
 APP_NAMES = [CLIENT_APP_NAME, PG, PGB]
 FIRST_DATABASE_RELATION_NAME = "first-database"
 SECOND_DATABASE_RELATION_NAME = "second-database"
@@ -468,6 +466,42 @@ async def test_relation_with_data_integrator(ops_test: OpsTest):
         await ops_test.model.wait_for_idle(status="active")
 
 
+async def test_indico_datatabase(ops_test: OpsTest) -> None:
+    """Tests deploying and relating to the Indico charm."""
+    async with ops_test.fast_forward(fast_interval="30s"):
+        await ops_test.model.deploy(
+            "indico",
+            channel="stable",
+            application_name="indico",
+            num_units=1,
+        )
+        await ops_test.model.deploy("redis-k8s", channel="stable", application_name="redis-broker")
+        await ops_test.model.deploy("redis-k8s", channel="stable", application_name="redis-cache")
+        await asyncio.gather(
+            ops_test.model.relate("redis-broker", "indico:redis-broker"),
+            ops_test.model.relate("redis-cache", "indico:redis-cache"),
+        )
+
+        # Wait for model to stabilise
+        await ops_test.model.wait_for_idle(
+            apps=["indico"],
+            status="waiting",
+            timeout=1000,
+        )
+
+        # Verify that the charm doesn't block when the extensions are enabled.
+        logger.info("Verifying that the charm doesn't block when the extensions are enabled")
+        config = {"plugin_pg_trgm_enable": "True", "plugin_unaccent_enable": "True"}
+        await ops_test.model.applications[PG].set_config(config)
+        await ops_test.model.wait_for_idle(apps=[PG], status="active")
+        await ops_test.model.relate(PGB, "indico")
+        await ops_test.model.wait_for_idle(
+            apps=[PG, PGB, "indico"],
+            status="active",
+            timeout=2000,
+        )
+
+
 async def test_connection_is_possible_after_pod_deletion(ops_test: OpsTest) -> None:
     """Tests that the connection is possible after the pod is deleted."""
     # Delete the pod.

tests/integration/relations/test_db.py

@@ -3,16 +3,18 @@
 
 import asyncio
 import logging
-from pathlib import Path
 
 import pytest
-import yaml
 from pytest_operator.plugin import OpsTest
 
 from constants import EXTENSIONS_BLOCKING_MESSAGE
 
 from ..helpers.helpers import (
     CHARM_SERIES,
+    CLIENT_APP_NAME,
+    PG,
+    PGB,
+    PGB_METADATA,
     get_app_relation_databag,
     get_backend_user_pass,
     get_cfg,
@@ -25,9 +27,6 @@
     check_database_users_existence,
 )
 
-METADATA = yaml.safe_load(Path("./metadata.yaml").read_text())
-PGB = METADATA["name"]
-PG = "postgresql-k8s"
 FINOS_WALTZ = "finos-waltz"
 ANOTHER_FINOS_WALTZ = "another-finos-waltz"
 OPENLDAP = "openldap"
@@ -39,7 +38,7 @@ async def test_create_db_legacy_relation(ops_test: OpsTest, pgb_charm):
     """Test that the pgbouncer and postgres charms can relate to one another."""
     # Build, deploy, and relate charms.
     resources = {
-        "pgbouncer-image": METADATA["resources"]["pgbouncer-image"]["upstream-source"],
+        "pgbouncer-image": PGB_METADATA["resources"]["pgbouncer-image"]["upstream-source"],
     }
 
     async with ops_test.fast_forward():
@@ -170,48 +169,28 @@ async def test_create_db_legacy_relation(ops_test: OpsTest, pgb_charm):
         assert "waltz_standby" not in cfg["databases"].keys()
 
 
-@pytest.mark.skip(reason="Should be ported and moved to the new relation tests")
-async def test_relation_with_indico(ops_test: OpsTest):
-    """Test the relation with Indico charm."""
-    logger.info("Deploying indico")
-    await ops_test.model.deploy("indico", channel="stable")
-    await ops_test.model.deploy("redis-k8s", channel="edge", application_name="redis-broker")
-    await ops_test.model.deploy("redis-k8s", channel="edge", application_name="redis-cache")
-    await asyncio.gather(
-        ops_test.model.relate("redis-broker", "indico"),
-        ops_test.model.relate("redis-cache", "indico"),
-    )
-    await ops_test.model.add_relation(f"{PGB}:db", "indico:db")
-
-    # Wait for model to stabilise
-    await ops_test.model.wait_for_idle(
-        apps=["indico"],
-        status="waiting",
-        raise_on_blocked=False,
-        timeout=1000,
-    )
-    unit = ops_test.model.units.get("indico/0")
-    await ops_test.model.block_until(
-        lambda: unit.workload_status_message == "Waiting for database availability",
-        timeout=1000,
-    )
+async def test_extensions_blocking(ops_test: OpsTest) -> None:
+    """Test the relation blocks with extensions."""
+    logger.info("Deploying test app")
+    await ops_test.model.deploy(CLIENT_APP_NAME)
+    await ops_test.model.add_relation(f"{PGB}:db", f"{CLIENT_APP_NAME}:db")
 
     logger.info("Wait for PGB to block due to extensions")
     await ops_test.model.wait_for_idle(apps=[PGB], status="blocked", timeout=1000)
     assert (
         ops_test.model.applications[PGB].units[0].workload_status_message
         == EXTENSIONS_BLOCKING_MESSAGE
     )
-    await ops_test.model.applications[PGB].destroy_relation(f"{PGB}:db", "indico:db")
+    await ops_test.model.applications[PGB].destroy_relation(f"{PGB}:db", f"{CLIENT_APP_NAME}:db")
     await ops_test.model.wait_for_idle(apps=[PGB], status="active", idle_period=15)
 
     logger.info("Rerelate with extensions enabled")
     config = {"plugin_pg_trgm_enable": "True", "plugin_unaccent_enable": "True"}
     await ops_test.model.applications[PG].set_config(config)
     await ops_test.model.wait_for_idle(apps=[PG], status="active", idle_period=15)
-    await ops_test.model.relate(f"{PGB}:db", "indico:db")
+    await ops_test.model.relate(f"{PGB}:db", f"{CLIENT_APP_NAME}:db")
     await ops_test.model.wait_for_idle(
-        apps=[PG, PGB, "indico"],
+        apps=[PG, PGB, CLIENT_APP_NAME],
         status="active",
         raise_on_blocked=False,
         timeout=3000,