Skip to content

Commit

Permalink
[DPE-2844] Split indico and extension blocking tests (#163)
Browse files Browse the repository at this point in the history
* Split indico and extension blocking tests

* Bump libs
  • Loading branch information
dragomirp authored Nov 24, 2023
1 parent 767790e commit 1c9704f
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 62 deletions.
59 changes: 49 additions & 10 deletions lib/charms/data_platform_libs/v0/data_interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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)]

Expand All @@ -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}.")

Expand All @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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("-", "_"), "")
Expand Down Expand Up @@ -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}
Expand Down
24 changes: 11 additions & 13 deletions lib/charms/postgresql_k8s/v0/postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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"
Expand Down Expand Up @@ -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.
Expand Down
47 changes: 13 additions & 34 deletions tests/integration/relations/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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"
Expand All @@ -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():
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 1c9704f

Please sign in to comment.