From a870470d39d68ce61aae7d3e10ae1e3c2c7c8980 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Date: Thu, 1 Aug 2024 18:21:43 -0300 Subject: [PATCH] [DPE-4967] Block charm if dependent objects on disabled plugins (#560) * add block config feature * fix integration test --- lib/charms/postgresql_k8s/v0/postgresql.py | 4 +- src/charm.py | 14 ++++++ tests/integration/test_plugins.py | 51 ++++++++++++++++++++++ tests/unit/test_charm.py | 13 ++++++ 4 files changed, 81 insertions(+), 1 deletion(-) diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 071643841f..f7d361b5a9 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 = 30 +LIBPATCH = 33 INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles" @@ -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: diff --git a/src/charm.py b/src/charm.py index 1403ba37bd..752e3ec2d9 100755 --- a/src/charm.py +++ b/src/charm.py @@ -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] @@ -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__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: @@ -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 diff --git a/tests/integration/test_plugins.py b/tests/integration/test_plugins.py index 5d78dcd3aa..07e862403c 100644 --- a/tests/integration/test_plugins.py +++ b/tests/integration/test_plugins.py @@ -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") diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 95599a5b73..811ea7490b 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -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 ( @@ -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):