Skip to content

Commit

Permalink
[DPE-4967] Block charm if dependent objects on disabled plugins (#560)
Browse files Browse the repository at this point in the history
* add block config feature

* fix integration test
  • Loading branch information
lucasgameiroborges authored Aug 1, 2024
1 parent 57eb505 commit a870470
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/charms/postgresql_k8s/v0/postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 30
LIBPATCH = 33

INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles"

Expand Down Expand Up @@ -327,6 +327,8 @@ def enable_disable_extensions(self, extensions: Dict[str, bool], database: str =
)
except psycopg2.errors.UniqueViolation:
pass
except psycopg2.errors.DependentObjectsStillExist:
raise
except psycopg2.Error:
raise PostgreSQLEnableDisableExtensionError()
finally:
Expand Down
14 changes: 14 additions & 0 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@

PRIMARY_NOT_REACHABLE_MESSAGE = "waiting for primary to be reachable from this unit"
EXTENSIONS_DEPENDENCY_MESSAGE = "Unsatisfied plugin dependencies. Please check the logs"
EXTENSION_OBJECT_MESSAGE = "Cannot disable plugins: Existing objects depend on it. See logs"

Scopes = Literal[APP_SCOPE, UNIT_SCOPE]

Expand Down Expand Up @@ -1013,8 +1014,18 @@ def enable_disable_extensions(self, database: str = None) -> None:
self.unit.status = WaitingStatus("Updating extensions")
try:
self.postgresql.enable_disable_extensions(extensions, database)
except psycopg2.errors.DependentObjectsStillExist as e:
logger.error(
"Failed to disable plugin: %s\nWas the plugin enabled manually? If so, update charm config with `juju config postgresql-k8s plugin_<plugin_name>_enable=True`",
str(e),
)
self.unit.status = BlockedStatus(EXTENSION_OBJECT_MESSAGE)
return
except PostgreSQLEnableDisableExtensionError as e:
logger.exception("failed to change plugins: %s", str(e))
if original_status.message == EXTENSION_OBJECT_MESSAGE:
self.unit.status = ActiveStatus()
return
self.unit.status = original_status

def _check_extension_dependencies(self, extension: str, enable: bool) -> bool:
Expand Down Expand Up @@ -1358,6 +1369,9 @@ def _can_run_on_update_status(self) -> bool:
return False

if self.is_blocked:
# If charm was failing to disable plugin, try again (user may have removed the objects)
if self.unit.status.message == EXTENSION_OBJECT_MESSAGE:
self.enable_disable_extensions()
logger.debug("on_update_status early exit: Unit is in Blocked status")
return False

Expand Down
51 changes: 51 additions & 0 deletions tests/integration/test_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,3 +208,54 @@ def enable_disable_config(enabled: False):
else:
connection.cursor().execute(query)
connection.close()


@pytest.mark.group(1)
async def test_plugin_objects(ops_test: OpsTest) -> None:
"""Checks if charm gets blocked when trying to disable a plugin in use."""
primary = await get_primary(ops_test, f"{DATABASE_APP_NAME}/0")
password = await get_password(ops_test, primary)
address = get_unit_address(ops_test, primary)

logger.info("Creating an index object which depends on the pg_trgm config")
with db_connect(host=address, password=password) as connection:
connection.autocommit = True
connection.cursor().execute("CREATE TABLE example (value VARCHAR(10));")
connection.cursor().execute(
"CREATE INDEX example_idx ON example USING gin(value gin_trgm_ops);"
)
connection.close()

logger.info("Disabling the plugin on charm config, waiting for blocked status")
await ops_test.model.applications[DATABASE_APP_NAME].set_config({
"plugin_pg_trgm_enable": "False"
})
await ops_test.model.block_until(
lambda: ops_test.model.units[primary].workload_status == "blocked",
timeout=100,
)

logger.info("Enabling the plugin back on charm config, status should resolve")
await ops_test.model.applications[DATABASE_APP_NAME].set_config({
"plugin_pg_trgm_enable": "True"
})
await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active")

logger.info("Re-disabling plugin, waiting for blocked status to come back")
await ops_test.model.applications[DATABASE_APP_NAME].set_config({
"plugin_pg_trgm_enable": "False"
})
await ops_test.model.block_until(
lambda: ops_test.model.units[primary].workload_status == "blocked",
timeout=100,
)

logger.info("Delete the object which depends on the plugin")
with db_connect(host=address, password=password) as connection:
connection.autocommit = True
connection.cursor().execute("DROP INDEX example_idx;")
connection.close()

logger.info("Waiting for status to resolve again")
async with ops_test.fast_forward(fast_interval="60s"):
await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active")
13 changes: 13 additions & 0 deletions tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from unittest import TestCase
from unittest.mock import MagicMock, Mock, PropertyMock, call, mock_open, patch, sentinel

import psycopg2
import pytest
from charms.operator_libs_linux.v2 import snap
from charms.postgresql_k8s.v0.postgresql import (
Expand Down Expand Up @@ -556,6 +557,18 @@ def test_enable_disable_extensions(harness, caplog):
new_harness.charm.enable_disable_extensions()
assert postgresql_mock.enable_disable_extensions.call_count == 1

# Block if extension-dependent object error is raised
postgresql_mock.reset_mock()
postgresql_mock.enable_disable_extensions.side_effect = [
psycopg2.errors.DependentObjectsStillExist,
None,
]
harness.charm.enable_disable_extensions()
assert isinstance(harness.charm.unit.status, BlockedStatus)
# Should resolve afterwards
harness.charm.enable_disable_extensions()
assert isinstance(harness.charm.unit.status, ActiveStatus)


@patch_network_get(private_address="1.1.1.1")
def test_on_start(harness):
Expand Down

0 comments on commit a870470

Please sign in to comment.