From 58d917773ab02af6c81314294619b5b1950b6221 Mon Sep 17 00:00:00 2001 From: Phoevos Kalemkeris Date: Thu, 24 Aug 2023 12:02:39 +0300 Subject: [PATCH 1/2] fix: Handle relation-broken events (#272) * api: Handle relation-broken events In the kpf-api charm, '*-relation-broken' events trigger the same workflow as every other event, forcing the charm to recalculate its configuration. In order to do that, it attempts to retrieve info from its required relations. When executing in response to a relation-broken event for a removed relational-db relation, however, the library call to fetch the data raises an error. We need to catch this error and properly propagate it so that the charm lands in the desired state (i.e. Blocked, waiting for the required relation to be added). Closes #222 Signed-off-by: Phoevos Kalemkeris --- charms/kfp-api/src/charm.py | 10 ++++- .../kfp-api/tests/integration/test_charm.py | 8 ++++ charms/kfp-api/tests/unit/test_operator.py | 38 ++++++++++++++++++- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/charms/kfp-api/src/charm.py b/charms/kfp-api/src/charm.py index 22d840dd..32d549d8 100755 --- a/charms/kfp-api/src/charm.py +++ b/charms/kfp-api/src/charm.py @@ -483,7 +483,15 @@ def _get_relational_db_data(self) -> dict: self._get_db_relation("relational-db") # retrieve database data from library - relation_data = self.database.fetch_relation_data() + try: + # if called in response to a '*-relation-broken' event, this will raise an exception + relation_data = self.database.fetch_relation_data() + except KeyError: + self.logger.error("Failed to retrieve relation data from library") + raise GenericCharmRuntimeError( + "Failed to retrieve relational-db data. This is to be expected if executed in" + " response to a '*-relation-broken' event" + ) # parse data in relation # this also validates expected data by means of KeyError exception for val in relation_data.values(): diff --git a/charms/kfp-api/tests/integration/test_charm.py b/charms/kfp-api/tests/integration/test_charm.py index a71095d2..f01d224b 100644 --- a/charms/kfp-api/tests/integration/test_charm.py +++ b/charms/kfp-api/tests/integration/test_charm.py @@ -174,6 +174,9 @@ async def test_msql_relation_with_relational_db_relation(self, ops_test: OpsTest ) assert ops_test.model.applications[APP_NAME].units[0].workload_status == "blocked" + # remove redundant relation + await ops_test.juju("remove-relation", f"{APP_NAME}:mysql", "kfp-db:mysql") + async def test_prometheus_grafana_integration(self, ops_test: OpsTest): """Deploy prometheus, grafana and required relations, then test the metrics.""" prometheus = "prometheus-k8s" @@ -240,3 +243,8 @@ async def test_prometheus_grafana_integration(self, ops_test: OpsTest): wait=wait_exponential(multiplier=1, min=1, max=10), reraise=True, ) + + async def test_remove_application(self, ops_test: OpsTest): + """Test that the application can be removed successfully.""" + await ops_test.model.remove_application(app_name=APP_NAME, block_until_done=True) + assert APP_NAME not in ops_test.model.applications diff --git a/charms/kfp-api/tests/unit/test_operator.py b/charms/kfp-api/tests/unit/test_operator.py index 9fdc67d0..b35f7073 100644 --- a/charms/kfp-api/tests/unit/test_operator.py +++ b/charms/kfp-api/tests/unit/test_operator.py @@ -6,7 +6,7 @@ import pytest import yaml -from ops.model import ActiveStatus, BlockedStatus, WaitingStatus +from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus from ops.testing import Harness from charm import ErrorWithStatus, KfpApiOperator @@ -570,3 +570,39 @@ def test_relational_db_relation_with_data( "db_host": "host", "db_port": "1234", } + + def test_relational_db_relation_broken( + self, + mocked_resource_handler, + mocked_lightkube_client, + mocked_kubernetes_service_patcher, + harness: Harness, + ): + """Test that a relation broken event is properly handled.""" + database = MagicMock() + fetch_relation_data = MagicMock(side_effect=KeyError()) + database.fetch_relation_data = fetch_relation_data + harness.model.get_relation = MagicMock( + side_effect=self._get_relation_db_only_side_effect_func + ) + + rel_name = "relational-db" + rel_id = harness.add_relation(rel_name, "relational-db-provider") + + harness.begin() + harness.set_leader(True) + harness.container_pebble_ready(KFP_API_CONTAINER_NAME) + + assert harness.model.unit.status == WaitingStatus("Waiting for relational-db data") + + harness.charm.database = database + del harness.model.get_relation + + harness._emit_relation_broken(rel_name, rel_id, "kfp-api") + + assert harness.model.unit.status == BlockedStatus( + "Please add required database relation: eg. relational-db" + ) + + harness.charm.on.remove.emit() + assert harness.model.unit.status == MaintenanceStatus("K8S resources removed") From 61da1dface21ef94a320c9358478e38dd427d20c Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Wed, 27 Sep 2023 11:35:16 +0300 Subject: [PATCH 2/2] fix: Add KFP_API_CONTAINER_NAME in unit tests --- charms/kfp-api/tests/unit/test_operator.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/charms/kfp-api/tests/unit/test_operator.py b/charms/kfp-api/tests/unit/test_operator.py index b35f7073..57b1b622 100644 --- a/charms/kfp-api/tests/unit/test_operator.py +++ b/charms/kfp-api/tests/unit/test_operator.py @@ -11,6 +11,8 @@ from charm import ErrorWithStatus, KfpApiOperator +KFP_API_CONTAINER_NAME = "apiserver" + @pytest.fixture() def mocked_resource_handler(mocker):