From 2d1f85c8048ce8ff392332fec8401f6ca53b82c7 Mon Sep 17 00:00:00 2001 From: Alex Lutay <1928266+taurus-forever@users.noreply.github.com> Date: Sun, 14 Jun 2026 23:01:33 +0200 Subject: [PATCH] =?UTF-8?q?[DPE-10397]=20The=20watcher=20now=20handles=202?= =?UTF-8?q?,=204,=206,=20=E2=80=A6=20PostgreSQL=20units=20reliably?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The behavior you asked to keep is untouched (votes at 1 unit; odd ≥3 stands down with an Active+WARNING status; AZ blocking logic unchanged). The key finding from the audit: the vote rule itself (_should_watcher_vote) was already correct for even counts — what was broken was the lifecycle around it. The most serious bug was the 3→4 rejoin: it depended entirely on a single relation-changed event, with no recovery if that event was missed, and the update-status path never told PostgreSQL the watcher had stood down. Fixes in src/relations/watcher_requirer.py: - Added a self-heal path in update-status: if the watcher should be voting but its Raft service isn't running, it reconfigures and flips raft-status back to "connected" (the core 3→4 rejoin fix). - The update-status stand-down now publishes raft-status: "disabled" so PostgreSQL actually drops the watcher from its Patroni raft config, stops the service before removing the raft member (a running pysyncobj node could rejoin in between), and is idempotent instead of re-probing every 5 minutes. - Fixed two return-instead-of-continue bugs that let one relation starve all others (multi-cluster), and the stand-down now sets the warning status immediately instead of waiting for the next update-status. - Provider disable-watcher is now distinguished from the odd-count stand-down: it tears the watcher down even at even counts, is never self-healed, and doesn't emit the misleading "odd number units" warning. - Status while stood down now reads "Watcher standing down, monitoring N PostgreSQL endpoints" instead of falsely claiming "Raft connected". Tests (written first, watched fail, then made green): - 16 new unit tests: the vote rule parametrized over 0–7 units, plus stand-down/rejoin/idempotency/self-heal/multi-relation scenarios — tox run -e unit: 31 passed, tox run -e lint: clean. - New integration test tests/integration/ha_tests/test_watcher_scaling.py scaling 2→3→4→2 with continuous writes, asserting raft membership (3 → 3 without watcher → 5 → 3) via the shared verify_raft_cluster_health helper (moved to ha_tests/helpers.py with a new expect_watcher_absent flag), plus a spread task so CI picks it up. Not run locally — it needs a Juju+LXD model (tox run -e integration -- tests/integration/ha_tests/test_watcher_scaling.py --model testing), so the rejoin path should be considered proven only once that passes in CI. Assisted-by: Claude:claude-fable-5 (before-banned-by-trump) :-( --- src/raft_controller.py | 4 +- src/relations/watcher_requirer.py | 121 +++++--- tests/integration/ha_tests/helpers.py | 107 +++++++ .../integration/ha_tests/test_stereo_mode.py | 107 +------ .../ha_tests/test_watcher_scaling.py | 185 ++++++++++++ .../spread/test_watcher_scaling.py/task.yaml | 7 + tests/unit/test_watcher_requirer.py | 282 +++++++++++++++++- 7 files changed, 671 insertions(+), 142 deletions(-) create mode 100644 tests/integration/ha_tests/test_watcher_scaling.py create mode 100644 tests/spread/test_watcher_scaling.py/task.yaml diff --git a/src/raft_controller.py b/src/raft_controller.py index 1576af3..438912f 100644 --- a/src/raft_controller.py +++ b/src/raft_controller.py @@ -4,8 +4,8 @@ """Raft controller management for PostgreSQL watcher. This module manages a Patroni raft_controller node that participates in -consensus without running PostgreSQL, providing the necessary third vote -for quorum in 2-node PostgreSQL clusters. +consensus without running PostgreSQL, providing the extra vote that keeps +quorum odd in even-sized (2, 4, 6, ...) PostgreSQL clusters. Uses Patroni's own ``patroni_raft_controller`` from the charmed-postgresql snap, which is the same battle-tested Raft implementation used by the diff --git a/src/relations/watcher_requirer.py b/src/relations/watcher_requirer.py index 71307b9..13d5b5a 100644 --- a/src/relations/watcher_requirer.py +++ b/src/relations/watcher_requirer.py @@ -6,7 +6,8 @@ This module handles the watcher (requirer) side of the relation, used when the charm is deployed with role=watcher. It connects to one or more PostgreSQL applications (which provide the watcher-offer relation) and participates in -Raft consensus as a lightweight witness for stereo mode (2-node clusters). +Raft consensus as a lightweight witness whenever the PostgreSQL unit count is +even (2, 4, 6, ...), standing down at odd counts of three or more. Multi-cluster support: - Each watcher relation gets its own RaftController instance @@ -354,6 +355,51 @@ def _update_unit_address_if_changed(self) -> None: raft_controller.cleanup_raft_cluster(watcher_addr, raft_password, partner_addrs) self.charm.app_peer_data["unit-address"] = new_address + def _reconcile_relation_raft(self, relation: Relation) -> tuple[bool, bool, str | None]: + """Reconcile the Raft service state for a single relation. + + Stands the watcher down (stops the service and leaves the quorum) when + the provider disabled it or the PostgreSQL unit count is odd >= 3, and + self-heals the service and Raft membership when the watcher must vote. + + Returns: + Tuple of (connected, stood_down, warning) where warning carries the + odd-unit-count message when the watcher stands down because of it. + """ + port = self._get_port_for_relation(relation.id) + password = self._get_raft_password(relation) + raft_controller = RaftController(self.charm, instance_id=f"rel{relation.id}") + raft_status = raft_controller.get_status(port, password) + connected = bool(raft_status.get("connected")) + + partner_addrs = self._get_raft_partner_addrs(relation) + provider_disabled = self._is_disabled(relation) + stand_down = provider_disabled or not self._should_watcher_vote(partner_addrs) + + if password and stand_down: + # Stop the service before removing the raft member, otherwise the + # still-running pysyncobj node can rejoin before the service stops + if relation.data[self.charm.app].get("raft-status") != "disabled": + raft_controller.remove_service() + raft_controller.remove_raft_member( + f"{self.unit_ip}:{port}", password, partner_addrs + ) + relation.data[self.charm.app]["raft-status"] = "disabled" + warning = None if provider_disabled else self._odd_units_warning(relation) + return False, True, warning + + if password and partner_addrs: + # Self-heal: rejoin the quorum if the relation-changed event that + # should have re-enabled the watcher was missed + if not raft_status.get("running"): + raft_controller.configure( + port, self.unit_ip, partner_addrs, password, self._get_patroni_cas(relation) + ) + if service_running(raft_controller.service_name): + relation.data[self.charm.app]["raft-status"] = "connected" + connected = True + return connected, provider_disabled, None + def _on_update_status(self, event: UpdateStatusEvent) -> None: """Handle update status event in watcher mode.""" if not self.charm.unit.is_leader(): @@ -375,29 +421,12 @@ def _on_update_status(self, event: UpdateStatusEvent) -> None: info_warnings: list[str] = [] for relation in relations: - port = self._get_port_for_relation(relation.id) - password = self._get_raft_password(relation) - raft_controller = RaftController(self.charm, instance_id=f"rel{relation.id}") - raft_status = raft_controller.get_status(port, password) - disabled = disabled or self._is_disabled(relation) - connected_count += 1 if raft_status.get("connected") else 0 - - pg_endpoints = self._get_raft_partner_addrs(relation) - total_endpoints += len(pg_endpoints) - partner_addrs = self._get_raft_partner_addrs(relation) - - if password and not self._should_watcher_vote(partner_addrs): - cluster_name = self._get_cluster_name(relation) - raft_controller.remove_raft_member( - f"{self.unit_ip}:{port}", password, pg_endpoints - ) - info_warnings.append( - f"WARNING: cluster '{cluster_name}' has odd number units;" - " adding a watcher creates even Raft membership," - " which degrades partition tolerance" - ) - raft_controller.remove_service() - disabled = True + connected, stood_down, warning = self._reconcile_relation_raft(relation) + total_endpoints += len(self._get_raft_partner_addrs(relation)) + connected_count += 1 if connected else 0 + disabled = disabled or stood_down + if warning: + info_warnings.append(warning) az_warning = self._check_az_colocation(relation) if az_warning: @@ -407,25 +436,37 @@ def _on_update_status(self, event: UpdateStatusEvent) -> None: self.charm.unit.status = WaitingStatus("Connecting to Raft cluster") return - cluster_count = len(relations) - msg = ( - f"Raft connected, monitoring {total_endpoints} PostgreSQL endpoints" - if cluster_count == 1 - else ( + self.charm.unit.status = self._summary_status( + len(relations), connected_count, total_endpoints, az_warnings, info_warnings + ) + + def _summary_status( + self, + cluster_count: int, + connected_count: int, + total_endpoints: int, + az_warnings: list[str], + info_warnings: list[str], + ) -> ActiveStatus | BlockedStatus: + """Compose the unit status summarizing all watcher relations.""" + if connected_count == 0: + msg = f"Watcher standing down, monitoring {total_endpoints} PostgreSQL endpoints" + elif cluster_count == 1: + msg = f"Raft connected, monitoring {total_endpoints} PostgreSQL endpoints" + else: + msg = ( f"Raft connected to {connected_count}/{cluster_count} clusters, " f"monitoring {total_endpoints} PostgreSQL endpoints" ) - ) # AZ co-location blocks in production; odd-count warnings never block if az_warnings and self.charm.config.profile == "production": - self.charm.unit.status = BlockedStatus("AZ co-location: " + "; ".join(az_warnings)) - return + return BlockedStatus("AZ co-location: " + "; ".join(az_warnings)) if all_warnings := az_warnings + info_warnings: msg += "; " + "; ".join(all_warnings) - self.charm.unit.status = ActiveStatus(msg) + return ActiveStatus(msg) def _check_az_colocation(self, relation: Relation) -> str | None: """Check if the watcher is in the same AZ as any PostgreSQL unit. @@ -470,6 +511,14 @@ def _should_watcher_vote(self, partner_addrs: list[str]) -> bool: pg_num = len(partner_addrs) return pg_num < 3 or pg_num % 2 == 0 + def _odd_units_warning(self, relation: Relation) -> str: + """Warning shown while the watcher stands down for an odd-sized cluster.""" + return ( + f"WARNING: cluster '{self._get_cluster_name(relation)}' has odd number units;" + " adding a watcher creates even Raft membership," + " which degrades partition tolerance" + ) + def _on_watcher_relation_changed( self, event: RelationChangedEvent | SecretChangedEvent ) -> None: @@ -494,7 +543,7 @@ def _on_watcher_relation_changed( partner_addrs := self._get_raft_partner_addrs(relation) ): logger.debug("Raft details are not yet available") - return + continue # Get or assign a port for this relation port = self._get_port_for_relation(relation.id) @@ -509,7 +558,9 @@ def _on_watcher_relation_changed( f"{self.unit_ip}:{port}", raft_password, partner_addrs ) relation.data[self.charm.app]["raft-status"] = "disabled" - return + if not self._is_disabled(relation): + self.charm.unit.status = ActiveStatus(self._odd_units_warning(relation)) + continue raft_controller.configure( port, unit_ip, partner_addrs, raft_password, self._get_patroni_cas(relation) diff --git a/tests/integration/ha_tests/helpers.py b/tests/integration/ha_tests/helpers.py index 2cec213..526f353 100644 --- a/tests/integration/ha_tests/helpers.py +++ b/tests/integration/ha_tests/helpers.py @@ -9,6 +9,7 @@ import requests import yaml from juju.model import Model +from pysyncobj.utility import TcpUtility from pytest_operator.plugin import OpsTest from tenacity import ( RetryError, @@ -19,6 +20,8 @@ wait_fixed, ) +from constants import RAFT_PARTNER_PREFIX + from ..helpers import ( APPLICATION_NAME, get_password, @@ -419,3 +422,107 @@ async def wait_network_restore(ops_test: OpsTest, unit_name: str, old_ip: str) - # Juju status too quickly. if (await get_ip_from_inside_the_unit(ops_test, unit_name)) == old_ip: raise Exception + + +async def start_writes(ops_test: OpsTest) -> None: + """Start continuous writes to PostgreSQL (assumes relation already exists).""" + for attempt in Retrying(stop=stop_after_delay(60 * 5), wait=wait_fixed(3), reraise=True): + with attempt: + action = ( + await ops_test.model + .applications[APPLICATION_NAME] + .units[0] + .run_action("start-continuous-writes") + ) + await action.wait() + assert action.results["result"] == "True", "Unable to create continuous_writes table" + + +async def verify_raft_cluster_health( + ops_test: OpsTest, + db_app_name: str, + watcher_app_name: str, + expected_members: int = 3, + check_watcher_ip: bool = True, + expect_watcher_absent: bool = False, +) -> None: + """Verify that the Raft cluster has the expected number of members and quorum. + + This function checks that all PostgreSQL units see the expected number of + Raft members (including the watcher) and have quorum. This is critical + after watcher re-deployment to ensure the cluster is properly formed. + + Args: + ops_test: The OpsTest instance. + db_app_name: The PostgreSQL application name. + watcher_app_name: The watcher application name. + expected_members: Expected number of Raft members (default 3 for stereo mode). + check_watcher_ip: Whether to verify the watcher IP in Raft status (default True). + Set to False after network isolation tests where watcher may have been + redeployed with a new IP that isn't yet in the Raft configuration. + expect_watcher_absent: When True, assert that the watcher IP is NOT a Raft + member (the watcher stood down, e.g. with an odd number of PostgreSQL + units). Takes precedence over check_watcher_ip. + + Raises: + AssertionError: If the Raft cluster is not healthy. + """ + logger.info(f"Verifying Raft cluster health with {expected_members} expected members") + + # Get watcher address for verification using juju exec to avoid cached IPs + watcher_unit = ops_test.model.applications[watcher_app_name].units[0] + return_code, watcher_ip, _ = await ops_test.juju( + "exec", "--unit", watcher_unit.name, "--", "unit-get", "private-address" + ) + assert return_code == 0, f"Failed to get watcher address from {watcher_unit.name}" + watcher_ip = watcher_ip.strip() + + for attempt in Retrying(stop=stop_after_delay(180), wait=wait_fixed(5), reraise=True): + with attempt: + for unit in ops_test.model.applications[db_app_name].units: + # Get the Raft password from Patroni config using juju exec directly + # We need to avoid shell interpretation issues with run_command_on_unit + complete_command = [ + "exec", + "--unit", + unit.name, + "--", + "cat", + "/var/snap/charmed-postgresql/current/etc/patroni/patroni.yaml", + ] + return_code, stdout, _ = await ops_test.juju(*complete_command) + assert return_code == 0, f"Failed to read patroni.yaml on {unit.name}" + + conf = yaml.safe_load(stdout) + password = conf.get("raft", {}).get("password") + self_addr = conf.get("raft", {}).get("self_addr") + assert password, f"Could not find Raft password in patroni.yaml on {unit.name}" + + # Check Raft status using the password + syncobj_util = TcpUtility(password=password, timeout=3) + status = syncobj_util.executeCommand(self_addr, ["status"]) + logger.info(f"Raft status on {unit.name}: {status}") + + # Verify quorum + assert status["has_quorum"] is True, f"Unit {unit.name} does not have Raft quorum" + + assert status["partner_nodes_count"] + 1 == expected_members + + member_ips = [ + key.split(":")[0].split(RAFT_PARTNER_PREFIX)[-1] + for key in status + if key.startswith(RAFT_PARTNER_PREFIX) + ] + if expect_watcher_absent: + assert watcher_ip not in member_ips, ( + f"Watcher {watcher_ip} still in Raft cluster on {unit.name}" + ) + elif check_watcher_ip: + # Verify watcher is in the cluster (if requested) + # After network isolation tests, the watcher may have been redeployed + # with a new IP that isn't yet updated in the Raft configuration + assert watcher_ip in member_ips, ( + f"Watcher {watcher_ip} not found in Raft cluster on {unit.name}" + ) + + logger.info("Raft cluster health verified successfully") diff --git a/tests/integration/ha_tests/test_stereo_mode.py b/tests/integration/ha_tests/test_stereo_mode.py index c0aee5b..2cb2729 100644 --- a/tests/integration/ha_tests/test_stereo_mode.py +++ b/tests/integration/ha_tests/test_stereo_mode.py @@ -18,15 +18,10 @@ import logging import pytest -from pysyncobj.utility import TcpUtility from pytest_operator.plugin import OpsTest from tenacity import Retrying, stop_after_delay, wait_fixed -from yaml import safe_load - -from constants import RAFT_PARTNER_PREFIX from ..helpers import APPLICATION_NAME, DATABASE_APP_NAME, get_machine_from_unit, stop_machine -from .helpers import APPLICATION_NAME as TEST_APP_NAME from .helpers import ( are_writes_increasing, check_writes, @@ -36,108 +31,16 @@ get_primary, restore_network_for_unit, restore_network_for_unit_without_ip_change, + start_writes, + verify_raft_cluster_health, ) WATCHER_APP_NAME = "postgresql-watcher" SECOND_PG_APP_NAME = "postgresql-b" - -async def start_writes(ops_test: OpsTest) -> None: - """Start continuous writes to PostgreSQL (assumes relation already exists).""" - for attempt in Retrying(stop=stop_after_delay(60 * 5), wait=wait_fixed(3), reraise=True): - with attempt: - action = ( - await ops_test.model - .applications[TEST_APP_NAME] - .units[0] - .run_action("start-continuous-writes") - ) - await action.wait() - assert action.results["result"] == "True", "Unable to create continuous_writes table" - - logger = logging.getLogger(__name__) -async def verify_raft_cluster_health( - ops_test: OpsTest, - db_app_name: str, - watcher_app_name: str, - expected_members: int = 3, - check_watcher_ip: bool = True, -) -> None: - """Verify that the Raft cluster has the expected number of members and quorum. - - This function checks that all PostgreSQL units see the expected number of - Raft members (including the watcher) and have quorum. This is critical - after watcher re-deployment to ensure the cluster is properly formed. - - Args: - ops_test: The OpsTest instance. - db_app_name: The PostgreSQL application name. - watcher_app_name: The watcher application name. - expected_members: Expected number of Raft members (default 3 for stereo mode). - check_watcher_ip: Whether to verify the watcher IP in Raft status (default True). - Set to False after network isolation tests where watcher may have been - redeployed with a new IP that isn't yet in the Raft configuration. - - Raises: - AssertionError: If the Raft cluster is not healthy. - """ - logger.info(f"Verifying Raft cluster health with {expected_members} expected members") - - # Get watcher address for verification using juju exec to avoid cached IPs - watcher_unit = ops_test.model.applications[watcher_app_name].units[0] - return_code, watcher_ip, _ = await ops_test.juju( - "exec", "--unit", watcher_unit.name, "--", "unit-get", "private-address" - ) - assert return_code == 0, f"Failed to get watcher address from {watcher_unit.name}" - watcher_ip = watcher_ip.strip() - - for attempt in Retrying(stop=stop_after_delay(180), wait=wait_fixed(5), reraise=True): - with attempt: - for unit in ops_test.model.applications[db_app_name].units: - # Get the Raft password from Patroni config using juju exec directly - # We need to avoid shell interpretation issues with run_command_on_unit - complete_command = [ - "exec", - "--unit", - unit.name, - "--", - "cat", - "/var/snap/charmed-postgresql/current/etc/patroni/patroni.yaml", - ] - return_code, stdout, _ = await ops_test.juju(*complete_command) - assert return_code == 0, f"Failed to read patroni.yaml on {unit.name}" - - conf = safe_load(stdout) - password = conf.get("raft", {}).get("password") - self_addr = conf.get("raft", {}).get("self_addr") - assert password, f"Could not find Raft password in patroni.yaml on {unit.name}" - - # Check Raft status using the password - syncobj_util = TcpUtility(password=password, timeout=3) - status = syncobj_util.executeCommand(self_addr, ["status"]) - logger.info(f"Raft status on {unit.name}: {status}") - - # Verify quorum - assert status["has_quorum"] is True, f"Unit {unit.name} does not have Raft quorum" - - assert status["partner_nodes_count"] + 1 == expected_members - - # Verify watcher is in the cluster (if requested) - # After network isolation tests, the watcher may have been redeployed - # with a new IP that isn't yet updated in the Raft configuration - if check_watcher_ip: - assert watcher_ip in [ - key.split(":")[0].split(RAFT_PARTNER_PREFIX)[-1] - for key in status - if key.startswith(RAFT_PARTNER_PREFIX) - ], f"Watcher {watcher_ip} not found in Raft cluster on {unit.name}" - - logger.info("Raft cluster health verified successfully") - - @pytest.mark.abort_on_fail async def test_build_and_deploy_stereo_mode(ops_test: OpsTest, charm) -> None: """Build and deploy PostgreSQL in stereo mode with watcher. @@ -400,7 +303,7 @@ async def test_primary_shutdown_with_watcher(ops_test: OpsTest, continuous_write # First clear the old writes state action = ( await ops_test.model - .applications[TEST_APP_NAME] + .applications[APPLICATION_NAME] .units[0] .run_action("clear-continuous-writes") ) @@ -686,8 +589,8 @@ async def test_multi_cluster_watcher(ops_test: OpsTest, charm) -> None: f"{SECOND_PG_APP_NAME}:watcher-offer", f"{WATCHER_APP_NAME}:watcher" ) - # Use fast_forward to trigger update_status quickly, which runs - # ensure_watcher_in_raft to add the watcher to the second cluster's Raft + # Use fast_forward to trigger update_status quickly, which self-heals + # Raft membership and adds the watcher to the second cluster's Raft async with ops_test.fast_forward(): # Wait for the watcher to connect to both clusters await ops_test.model.wait_for_idle( diff --git a/tests/integration/ha_tests/test_watcher_scaling.py b/tests/integration/ha_tests/test_watcher_scaling.py new file mode 100644 index 0000000..60fab28 --- /dev/null +++ b/tests/integration/ha_tests/test_watcher_scaling.py @@ -0,0 +1,185 @@ +#!/usr/bin/env python3 +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Integration tests for watcher behavior across even/odd PostgreSQL unit counts. + +Per the stereo mode documentation, the watcher must vote whenever the +PostgreSQL unit count is even (2, 4, 6, ...) and stand down (stop its Raft +service and leave the quorum) at odd counts of three or more (3, 5, 7, ...), +rejoining automatically when the count becomes even again. + +Test scenario: +1. Deploy 2 PostgreSQL units + watcher (3 Raft voters) +2. Scale to 3 units: watcher stands down (3 Raft voters, all PostgreSQL) +3. Scale to 4 units: watcher rejoins (5 Raft voters) +4. Scale back to 2 units: watcher keeps voting (3 Raft voters) +Continuous writes must keep flowing through every transition. +""" + +import logging + +import pytest +from pytest_operator.plugin import OpsTest +from tenacity import Retrying, stop_after_delay, wait_fixed + +from ..helpers import APPLICATION_NAME, DATABASE_APP_NAME +from .helpers import ( + are_writes_increasing, + check_writes, + get_primary, + start_writes, + verify_raft_cluster_health, +) + +WATCHER_APP_NAME = "postgresql-watcher" + +logger = logging.getLogger(__name__) + + +async def assert_watcher_stood_down(ops_test: OpsTest) -> None: + """Assert the watcher stood down: warning status and no running Raft service.""" + watcher_unit = ops_test.model.applications[WATCHER_APP_NAME].units[0] + assert watcher_unit.workload_status == "active", ( + f"Watcher must stay active while standing down, got {watcher_unit.workload_status}" + ) + assert "odd number units" in watcher_unit.workload_status_message, ( + f"Expected stand-down warning in status, got: {watcher_unit.workload_status_message}" + ) + + return_code, stdout, _ = await ops_test.juju( + "exec", + "--unit", + watcher_unit.name, + "--", + "systemctl", + "list-units", + "--no-legend", + "--state=active", + "watcher-raft@*", + ) + assert return_code == 0, f"Failed to list watcher services on {watcher_unit.name}" + assert not stdout.strip(), f"Watcher Raft service still active: {stdout}" + + +async def assert_watcher_voting(ops_test: OpsTest) -> None: + """Assert the watcher is voting: active status without the stand-down warning.""" + watcher_unit = ops_test.model.applications[WATCHER_APP_NAME].units[0] + assert watcher_unit.workload_status == "active", ( + f"Watcher must be active, got {watcher_unit.workload_status}" + ) + assert "odd number units" not in watcher_unit.workload_status_message, ( + f"Stand-down warning must be cleared, got: {watcher_unit.workload_status_message}" + ) + + +@pytest.mark.abort_on_fail +async def test_build_and_deploy(ops_test: OpsTest, charm) -> None: + """Deploy 2 PostgreSQL units, the watcher and the test application.""" + async with ops_test.fast_forward(): + logger.info("Deploying PostgreSQL charm with 2 units...") + await ops_test.model.deploy( + DATABASE_APP_NAME, + application_name=DATABASE_APP_NAME, + num_units=2, + series="noble", + channel="16/edge", + config={"profile": "testing", "synchronous-mode-strict": False}, + ) + logger.info("Deploying watcher...") + await ops_test.model.deploy( + charm, + application_name=WATCHER_APP_NAME, + num_units=1, + series="noble", + config={"profile": "testing"}, + ) + logger.info("Deploying test application...") + await ops_test.model.deploy( + APPLICATION_NAME, + application_name=APPLICATION_NAME, + series="noble", + channel="edge", + ) + + logger.info("Relating PostgreSQL to watcher and test application") + await ops_test.model.integrate( + f"{DATABASE_APP_NAME}:watcher-offer", f"{WATCHER_APP_NAME}:watcher" + ) + await ops_test.model.integrate(DATABASE_APP_NAME, f"{APPLICATION_NAME}:database") + + await ops_test.model.wait_for_idle(status="active", timeout=1800) + + assert len(ops_test.model.applications[DATABASE_APP_NAME].units) == 2 + assert len(ops_test.model.applications[WATCHER_APP_NAME].units) == 1 + + +@pytest.mark.abort_on_fail +async def test_even_odd_scaling_transitions(ops_test: OpsTest, continuous_writes) -> None: + """Scale PostgreSQL 2->3->4->2 and verify the watcher stands down and rejoins.""" + await start_writes(ops_test) + + # Baseline: 2 PostgreSQL units + watcher = 3 Raft voters + await verify_raft_cluster_health( + ops_test, DATABASE_APP_NAME, WATCHER_APP_NAME, expected_members=3 + ) + + async with ops_test.fast_forward(): + logger.info("Scaling PostgreSQL to 3 units; the watcher must stand down") + await ops_test.model.applications[DATABASE_APP_NAME].add_unit(count=1) + await ops_test.model.wait_for_idle( + apps=[DATABASE_APP_NAME, WATCHER_APP_NAME], + status="active", + timeout=1800, + idle_period=30, + ) + for attempt in Retrying(stop=stop_after_delay(600), wait=wait_fixed(10), reraise=True): + with attempt: + await verify_raft_cluster_health( + ops_test, + DATABASE_APP_NAME, + WATCHER_APP_NAME, + expected_members=3, + expect_watcher_absent=True, + ) + await assert_watcher_stood_down(ops_test) + await are_writes_increasing(ops_test) + + logger.info("Scaling PostgreSQL to 4 units; the watcher must rejoin and vote") + await ops_test.model.applications[DATABASE_APP_NAME].add_unit(count=1) + await ops_test.model.wait_for_idle( + apps=[DATABASE_APP_NAME, WATCHER_APP_NAME], + status="active", + timeout=1800, + idle_period=30, + ) + for attempt in Retrying(stop=stop_after_delay(600), wait=wait_fixed(10), reraise=True): + with attempt: + await verify_raft_cluster_health( + ops_test, DATABASE_APP_NAME, WATCHER_APP_NAME, expected_members=5 + ) + await assert_watcher_voting(ops_test) + await are_writes_increasing(ops_test) + + logger.info("Scaling PostgreSQL back to 2 units; the watcher must keep voting") + primary = await get_primary(ops_test, DATABASE_APP_NAME) + units_to_remove = [ + unit.name + for unit in ops_test.model.applications[DATABASE_APP_NAME].units + if unit.name != primary + ][:2] + await ops_test.model.destroy_units(*units_to_remove) + await ops_test.model.wait_for_idle( + apps=[DATABASE_APP_NAME, WATCHER_APP_NAME], + status="active", + timeout=1800, + idle_period=30, + ) + for attempt in Retrying(stop=stop_after_delay(600), wait=wait_fixed(10), reraise=True): + with attempt: + await verify_raft_cluster_health( + ops_test, DATABASE_APP_NAME, WATCHER_APP_NAME, expected_members=3 + ) + await assert_watcher_voting(ops_test) + + await check_writes(ops_test) diff --git a/tests/spread/test_watcher_scaling.py/task.yaml b/tests/spread/test_watcher_scaling.py/task.yaml new file mode 100644 index 0000000..d622b1d --- /dev/null +++ b/tests/spread/test_watcher_scaling.py/task.yaml @@ -0,0 +1,7 @@ +summary: test_watcher_scaling.py +environment: + TEST_MODULE: ha_tests/test_watcher_scaling.py +execute: | + tox run -e integration -- "tests/integration/$TEST_MODULE" --model testing --alluredir="$SPREAD_TASK/allure-results" +artifacts: + - allure-results diff --git a/tests/unit/test_watcher_requirer.py b/tests/unit/test_watcher_requirer.py index 26d0d34..787a456 100644 --- a/tests/unit/test_watcher_requirer.py +++ b/tests/unit/test_watcher_requirer.py @@ -1,14 +1,18 @@ # Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. -"""Unit tests for the watcher requirer relation handler (AZ co-location logic).""" +"""Unit tests for the watcher requirer relation handler.""" +import json from unittest.mock import MagicMock, patch +import pytest from ops import ActiveStatus, BlockedStatus, WaitingStatus from src.relations.watcher_requirer import WatcherRequirerHandler +MODULE = "src.relations.watcher_requirer" + def create_mock_charm(profile="testing"): """Create a mock charm for watcher requirer testing.""" @@ -19,12 +23,16 @@ def create_mock_charm(profile="testing"): return mock_charm -def create_mock_relation(units_with_az=None): +def create_mock_relation(units_with_az=None, app_data=None, charm=None, local_app_data=None): """Create a mock relation with units that have AZ data. Args: units_with_az: Dict mapping unit names to their AZ values. Example: {"postgresql/0": "az1", "postgresql/1": "az2"} + app_data: Provider (PostgreSQL) application databag contents. + charm: When given, add real dict databags for the charm's own app and unit + so writes like relation.data[charm.app]["raft-status"] can be asserted. + local_app_data: Initial contents of the charm's own app databag. """ mock_relation = MagicMock() mock_relation.id = 42 @@ -46,11 +54,42 @@ def create_mock_relation(units_with_az=None): mock_relation.units = set(mock_units) mock_relation.app = MagicMock() mock_relation.app.name = "postgresql" - mock_data[mock_relation.app] = {} + mock_data[mock_relation.app] = dict(app_data) if app_data else {} + if charm is not None: + mock_data[charm.app] = dict(local_app_data) if local_app_data else {} + mock_data[charm.unit] = {} mock_relation.data = mock_data return mock_relation +def create_handler(charm, relations): + """Create a WatcherRequirerHandler wired to mocked relations.""" + with patch.object(WatcherRequirerHandler, "__init__", return_value=None): + handler = WatcherRequirerHandler.__new__(WatcherRequirerHandler) + handler.charm = charm + mock_framework = MagicMock() + mock_framework.model = charm.model + handler.framework = mock_framework + charm.model.relations.get.return_value = relations + handler._get_raft_password = MagicMock(return_value="raft-pwd") + handler._get_port_for_relation = MagicMock(return_value=2223) + handler._update_unit_address_if_changed = MagicMock() + return handler + + +def partner_addrs(count): + """Return a list of fake PostgreSQL unit IPs.""" + return [f"10.0.0.{i + 1}" for i in range(count)] + + +def relation_for_units(charm, count, local_app_data=None, provider_extra=None): + """Create a mock relation whose provider databag lists `count` PG units.""" + app_data = {"raft-partner-addrs": json.dumps(partner_addrs(count))} + if provider_extra: + app_data.update(provider_extra) + return create_mock_relation({}, app_data=app_data, charm=charm, local_app_data=local_app_data) + + class TestAZColocation: """Tests for AZ co-location detection and enforcement.""" @@ -308,3 +347,240 @@ def test_relation_broken_removes_port(self): _remove_service.assert_called_once_with() handler._release_port_for_relation.assert_called_once_with(42) + + +class TestShouldWatcherVote: + """The watcher votes at even unit counts (and a single unit), stands down at odd >= 3.""" + + @pytest.mark.parametrize( + ("count", "expected"), + [ + (0, True), + (1, True), + (2, True), + (3, False), + (4, True), + (5, False), + (6, True), + (7, False), + ], + ) + def test_should_watcher_vote(self, count, expected): + with patch.object(WatcherRequirerHandler, "__init__", return_value=None): + handler = WatcherRequirerHandler.__new__(WatcherRequirerHandler) + assert handler._should_watcher_vote(partner_addrs(count)) is expected + + +class TestVoteTransitions: + """Stand-down/rejoin transitions on relation-changed events.""" + + def test_relation_changed_stands_down_at_three_units(self): + """At 3 PG units the watcher stops voting, publishes 'disabled' and warns.""" + charm = create_mock_charm() + relation = relation_for_units(charm, 3, local_app_data={"raft-status": "connected"}) + handler = create_handler(charm, [relation]) + + with ( + patch(f"{MODULE}.RaftController.cleanup_raft_cluster"), + patch(f"{MODULE}.RaftController.configure") as configure, + patch(f"{MODULE}.RaftController.remove_service") as remove_service, + patch(f"{MODULE}.RaftController.remove_raft_member") as remove_member, + patch.dict("os.environ", {}, clear=True), + ): + handler._on_watcher_relation_changed(MagicMock()) + + remove_service.assert_called_once() + remove_member.assert_called_once() + configure.assert_not_called() + assert relation.data[charm.app]["raft-status"] == "disabled" + status = charm.unit.status + assert isinstance(status, ActiveStatus), f"Expected ActiveStatus, got {status}" + assert "odd number units" in status.message + + def test_relation_changed_reenables_at_four_units(self): + """At 4 PG units a previously stood-down watcher rejoins and votes.""" + charm = create_mock_charm() + relation = relation_for_units(charm, 4, local_app_data={"raft-status": "disabled"}) + handler = create_handler(charm, [relation]) + + with ( + patch(f"{MODULE}.RaftController.cleanup_raft_cluster"), + patch(f"{MODULE}.RaftController.configure") as configure, + patch(f"{MODULE}.RaftController.remove_service") as remove_service, + patch(f"{MODULE}.RaftController.remove_raft_member"), + patch(f"{MODULE}.service_running", return_value=True), + patch.dict("os.environ", {}, clear=True), + ): + handler._on_watcher_relation_changed(MagicMock()) + + configure.assert_called_once() + remove_service.assert_not_called() + assert relation.data[charm.app]["raft-status"] == "connected" + assert isinstance(charm.unit.status, ActiveStatus) + + def test_relation_changed_continues_past_relation_without_details(self): + """A relation without raft details must not starve other relations.""" + charm = create_mock_charm() + rel_no_details = create_mock_relation({}, app_data={}, charm=charm) + rel_no_details.id = 1 + rel_healthy = relation_for_units(charm, 2) + rel_healthy.id = 2 + handler = create_handler(charm, [rel_no_details, rel_healthy]) + + with ( + patch(f"{MODULE}.RaftController.cleanup_raft_cluster"), + patch(f"{MODULE}.RaftController.configure") as configure, + patch(f"{MODULE}.RaftController.remove_service"), + patch(f"{MODULE}.RaftController.remove_raft_member"), + patch(f"{MODULE}.service_running", return_value=True), + patch.dict("os.environ", {}, clear=True), + ): + handler._on_watcher_relation_changed(MagicMock()) + + configure.assert_called_once() + assert rel_healthy.data[charm.app]["raft-status"] == "connected" + + def test_relation_changed_continues_past_disabled_relation(self): + """Standing down for one cluster must not starve other relations.""" + charm = create_mock_charm() + rel_odd = relation_for_units(charm, 3) + rel_odd.id = 1 + rel_even = relation_for_units(charm, 2) + rel_even.id = 2 + handler = create_handler(charm, [rel_odd, rel_even]) + + with ( + patch(f"{MODULE}.RaftController.cleanup_raft_cluster"), + patch(f"{MODULE}.RaftController.configure") as configure, + patch(f"{MODULE}.RaftController.remove_service"), + patch(f"{MODULE}.RaftController.remove_raft_member"), + patch(f"{MODULE}.service_running", return_value=True), + patch.dict("os.environ", {}, clear=True), + ): + handler._on_watcher_relation_changed(MagicMock()) + + configure.assert_called_once() + assert rel_odd.data[charm.app]["raft-status"] == "disabled" + assert rel_even.data[charm.app]["raft-status"] == "connected" + + +class TestUpdateStatusStandDown: + """Stand-down, self-heal and status reporting on update-status.""" + + def test_update_status_stands_down_at_three_units(self): + """Update-status tears down (service first), publishes 'disabled' and warns.""" + charm = create_mock_charm() + relation = relation_for_units(charm, 3, local_app_data={"raft-status": "connected"}) + handler = create_handler(charm, [relation]) + manager = MagicMock() + + with ( + patch( + f"{MODULE}.RaftController.get_status", + return_value={"connected": True, "running": True}, + ), + patch(f"{MODULE}.RaftController.remove_service", manager.remove_service), + patch(f"{MODULE}.RaftController.remove_raft_member", manager.remove_raft_member), + patch(f"{MODULE}.RaftController.configure") as configure, + patch(f"{MODULE}.service_running", return_value=False), + patch.dict("os.environ", {}, clear=True), + ): + handler._on_update_status(MagicMock()) + + manager.remove_service.assert_called_once() + manager.remove_raft_member.assert_called_once() + call_names = [name for name, _, _ in manager.mock_calls] + assert call_names.index("remove_service") < call_names.index("remove_raft_member"), ( + "service must be stopped before removing the raft member" + ) + configure.assert_not_called() + assert relation.data[charm.app]["raft-status"] == "disabled" + status = charm.unit.status + assert isinstance(status, ActiveStatus), f"Expected ActiveStatus, got {status}" + assert status.message.startswith("Watcher standing down") + assert "odd number units" in status.message + assert "Raft connected" not in status.message + + def test_update_status_stand_down_is_idempotent(self): + """While already stood down, update-status must not re-run the teardown.""" + charm = create_mock_charm() + relation = relation_for_units(charm, 3, local_app_data={"raft-status": "disabled"}) + handler = create_handler(charm, [relation]) + + with ( + patch( + f"{MODULE}.RaftController.get_status", + return_value={"connected": False, "running": False}, + ), + patch(f"{MODULE}.RaftController.remove_service") as remove_service, + patch(f"{MODULE}.RaftController.remove_raft_member") as remove_member, + patch(f"{MODULE}.RaftController.configure") as configure, + patch(f"{MODULE}.service_running", return_value=False), + patch.dict("os.environ", {}, clear=True), + ): + handler._on_update_status(MagicMock()) + + remove_service.assert_not_called() + remove_member.assert_not_called() + configure.assert_not_called() + status = charm.unit.status + assert isinstance(status, ActiveStatus) + assert "odd number units" in status.message + + def test_update_status_self_heals_at_four_units(self): + """If the rejoin event was missed, update-status reconfigures and reconnects.""" + charm = create_mock_charm() + relation = relation_for_units(charm, 4, local_app_data={"raft-status": "disabled"}) + handler = create_handler(charm, [relation]) + + with ( + patch( + f"{MODULE}.RaftController.get_status", + return_value={"connected": False, "running": False}, + ), + patch(f"{MODULE}.RaftController.remove_service") as remove_service, + patch(f"{MODULE}.RaftController.remove_raft_member"), + patch(f"{MODULE}.RaftController.configure") as configure, + patch(f"{MODULE}.service_running", return_value=True), + patch.dict("os.environ", {}, clear=True), + ): + handler._on_update_status(MagicMock()) + + configure.assert_called_once() + remove_service.assert_not_called() + assert relation.data[charm.app]["raft-status"] == "connected" + status = charm.unit.status + assert isinstance(status, ActiveStatus) + assert "monitoring 4 PostgreSQL endpoints" in status.message + + def test_update_status_provider_disable_not_resurrected(self): + """A provider-disabled watcher is torn down and never self-healed.""" + charm = create_mock_charm() + relation = relation_for_units( + charm, + 2, + local_app_data={"raft-status": "connected"}, + provider_extra={"disable-watcher": "true"}, + ) + handler = create_handler(charm, [relation]) + + with ( + patch( + f"{MODULE}.RaftController.get_status", + return_value={"connected": False, "running": False}, + ), + patch(f"{MODULE}.RaftController.remove_service") as remove_service, + patch(f"{MODULE}.RaftController.remove_raft_member") as remove_member, + patch(f"{MODULE}.RaftController.configure") as configure, + patch(f"{MODULE}.service_running", return_value=True), + patch.dict("os.environ", {}, clear=True), + ): + handler._on_update_status(MagicMock()) + + configure.assert_not_called() + remove_service.assert_called_once() + remove_member.assert_called_once() + assert relation.data[charm.app]["raft-status"] == "disabled" + status = charm.unit.status + assert isinstance(status, ActiveStatus) + assert "odd number units" not in status.message