From fff78dad58c38dd843c5928a05bc22aeb1e9536c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Luis=20Cano=20Rodr=C3=ADguez?= Date: Mon, 27 Oct 2025 15:13:32 +0100 Subject: [PATCH 1/2] fix: use event.fail() instead of set_results(success=False) Backport DPE-7649 from mysql-operator. --- lib/charms/mysql/v0/mysql.py | 27 +++--- tests/unit/test_action_get_cluster_status.py | 86 ++++++++++++++++++++ 2 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 tests/unit/test_action_get_cluster_status.py diff --git a/lib/charms/mysql/v0/mysql.py b/lib/charms/mysql/v0/mysql.py index 688390dcf7..1bc4fac69c 100644 --- a/lib/charms/mysql/v0/mysql.py +++ b/lib/charms/mysql/v0/mysql.py @@ -597,23 +597,26 @@ def _on_set_password(self, event: ActionEvent) -> None: def _get_cluster_status(self, event: ActionEvent) -> None: """Action used to retrieve the cluster status.""" - if event.params.get("cluster-set"): - logger.debug("Getting cluster set status") - status = self._mysql.get_cluster_set_status(extended=0) - else: - logger.debug("Getting cluster status") - status = self._mysql.get_cluster_status() + try: + if event.params.get("cluster-set"): + logger.debug("Getting cluster set status") + status = self._mysql.get_cluster_set_status(extended=0) + else: + logger.debug("Getting cluster status") + status = self._mysql.get_cluster_status() + + if not status: + event.fail("Failed to read cluster status. See logs for more information.") + return - if status: event.set_results({ "success": True, "status": status, }) - else: - event.set_results({ - "success": False, - "message": "Failed to read cluster status. See logs for more information.", - }) + + except Exception: + logger.exception("Error while reading cluster status") + event.fail("Error while reading cluster status. See logs for more information.") def _on_promote_to_primary(self, event: ActionEvent) -> None: """Action for setting this unit as the cluster primary.""" diff --git a/tests/unit/test_action_get_cluster_status.py b/tests/unit/test_action_get_cluster_status.py new file mode 100644 index 0000000000..288bda39a6 --- /dev/null +++ b/tests/unit/test_action_get_cluster_status.py @@ -0,0 +1,86 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +from unittest.mock import Mock, PropertyMock, patch + +import pytest +from ops.charm import ActionEvent +from ops.testing import Harness + +from charm import MySQLOperatorCharm + + +class FakeMySQLBackend: + """Simulates the real MySQL backend, either returning a dict or raising.""" + + def __init__(self, response=None, error=None): + self._response = response + self._error = error + + def get_cluster_status(self): + """Return the preset response or raise the preset error.""" + if self._error: + raise self._error + return self._response + + +@pytest.fixture +def harness(): + """Start the charm so harness.charm exists and peer databag works.""" + harness = Harness(MySQLOperatorCharm) + harness.begin() + return harness + + +def make_event(): + """Create a dummy ActionEvent with spies on set_results() and fail().""" + event = Mock(spec=ActionEvent) + event.set_results = Mock() + event.fail = Mock() + event.params = {} # ensure .params.get() won't AttributeError + return event + + +def test_get_cluster_status_action_success(harness): + """On success, the action wraps and forwards the status dict.""" + # Prepare peer-databag so handler finds a cluster-name + relation = harness.add_relation("database-peers", "database-peers") + harness.update_relation_data(relation, harness.charm.app.name, {"cluster-name": "my-cluster"}) + + # Patch out the MySQL backend to return a known dict + sample = {"clusterrole": "primary", "status": "ok"} + fake = FakeMySQLBackend(response=sample) + with patch.object(MySQLOperatorCharm, "_mysql", new_callable=PropertyMock, return_value=fake): + event = make_event() + + # Invoke the action + harness.charm._get_cluster_status(event) + + # Expect set_results called once with {'success': True, 'status': sample} + event.set_results.assert_called_once_with({"success": True, "status": sample}) + event.fail.assert_not_called() + + +@pytest.mark.parametrize( + "backend_result", + [ + {"error": RuntimeError("boom")}, + {"response": None}, # silent failure + ], +) +def test_get_cluster_status_action_failure(backend_result, harness): + """On backend error, the action calls event.fail() and does not set_results().""" + # Seed peer-databag for cluster-name lookup + relation = harness.add_relation("database-peers", "database-peers") + harness.update_relation_data(relation, harness.charm.app.name, {"cluster-name": "my-cluster"}) + + fake = FakeMySQLBackend(**backend_result) + with patch.object(MySQLOperatorCharm, "_mysql", new_callable=PropertyMock, return_value=fake): + event = make_event() + + # Invoke the action + harness.charm._get_cluster_status(event) + + # It should report failure and never set_results + event.fail.assert_called_once() + event.set_results.assert_not_called() From 02d8fb72ec97b9f37f87c41d9222814186aadc44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Luis=20Cano=20Rodr=C3=ADguez?= Date: Mon, 27 Oct 2025 15:31:09 +0100 Subject: [PATCH 2/2] test: remove unwanted tests Charm libs must be tested upstream, as is already the case. --- tests/unit/test_action_get_cluster_status.py | 86 -------------------- 1 file changed, 86 deletions(-) delete mode 100644 tests/unit/test_action_get_cluster_status.py diff --git a/tests/unit/test_action_get_cluster_status.py b/tests/unit/test_action_get_cluster_status.py deleted file mode 100644 index 288bda39a6..0000000000 --- a/tests/unit/test_action_get_cluster_status.py +++ /dev/null @@ -1,86 +0,0 @@ -# Copyright 2025 Canonical Ltd. -# See LICENSE file for licensing details. - -from unittest.mock import Mock, PropertyMock, patch - -import pytest -from ops.charm import ActionEvent -from ops.testing import Harness - -from charm import MySQLOperatorCharm - - -class FakeMySQLBackend: - """Simulates the real MySQL backend, either returning a dict or raising.""" - - def __init__(self, response=None, error=None): - self._response = response - self._error = error - - def get_cluster_status(self): - """Return the preset response or raise the preset error.""" - if self._error: - raise self._error - return self._response - - -@pytest.fixture -def harness(): - """Start the charm so harness.charm exists and peer databag works.""" - harness = Harness(MySQLOperatorCharm) - harness.begin() - return harness - - -def make_event(): - """Create a dummy ActionEvent with spies on set_results() and fail().""" - event = Mock(spec=ActionEvent) - event.set_results = Mock() - event.fail = Mock() - event.params = {} # ensure .params.get() won't AttributeError - return event - - -def test_get_cluster_status_action_success(harness): - """On success, the action wraps and forwards the status dict.""" - # Prepare peer-databag so handler finds a cluster-name - relation = harness.add_relation("database-peers", "database-peers") - harness.update_relation_data(relation, harness.charm.app.name, {"cluster-name": "my-cluster"}) - - # Patch out the MySQL backend to return a known dict - sample = {"clusterrole": "primary", "status": "ok"} - fake = FakeMySQLBackend(response=sample) - with patch.object(MySQLOperatorCharm, "_mysql", new_callable=PropertyMock, return_value=fake): - event = make_event() - - # Invoke the action - harness.charm._get_cluster_status(event) - - # Expect set_results called once with {'success': True, 'status': sample} - event.set_results.assert_called_once_with({"success": True, "status": sample}) - event.fail.assert_not_called() - - -@pytest.mark.parametrize( - "backend_result", - [ - {"error": RuntimeError("boom")}, - {"response": None}, # silent failure - ], -) -def test_get_cluster_status_action_failure(backend_result, harness): - """On backend error, the action calls event.fail() and does not set_results().""" - # Seed peer-databag for cluster-name lookup - relation = harness.add_relation("database-peers", "database-peers") - harness.update_relation_data(relation, harness.charm.app.name, {"cluster-name": "my-cluster"}) - - fake = FakeMySQLBackend(**backend_result) - with patch.object(MySQLOperatorCharm, "_mysql", new_callable=PropertyMock, return_value=fake): - event = make_event() - - # Invoke the action - harness.charm._get_cluster_status(event) - - # It should report failure and never set_results - event.fail.assert_called_once() - event.set_results.assert_not_called()