diff --git a/src/backups.py b/src/backups.py index ce97f5c713..90d26edf1b 100644 --- a/src/backups.py +++ b/src/backups.py @@ -186,7 +186,7 @@ def _can_unit_perform_backup(self) -> tuple[bool, str | None]: return False, "Unit cannot perform backups as the database seems to be offline" # Only enable backups on primary if there are replicas but TLS is not enabled. - if is_primary and self.charm.app.planned_units() > 1: + if is_primary and self.charm._planned_units > 1: return False, "Unit cannot perform backups as it is the cluster primary" if not self.charm.patroni_manager.member_started: @@ -1313,7 +1313,7 @@ def _pre_restore_checks(self, event: ActionEvent) -> bool: return False logger.info("Checking that the cluster does not have more than one unit") - if self.charm.app.planned_units() > 1: + if self.charm._planned_units > 1: error_message = ( "Unit cannot restore backup as there are more than one unit in the cluster" ) diff --git a/src/charm.py b/src/charm.py index 850cfd8541..c55914b469 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1565,6 +1565,21 @@ def _hosts(self) -> set[str]: hosts.append(unit.name.replace("/", "-")) return set(hosts) + @property + def _planned_units(self) -> int: + """Number of planned units, resilient to a transient goal-state failure. + + ops implements ``Application.planned_units()`` via ``goal-state``, which fails + ("saas application ... not found") while a cross-model SAAS force-removed during a + dead-DC teardown still lingers in goal-state. Fall back to the count of currently known + units so the hook reconciles instead of crashing the ``_patroni`` property and every + hook that touches it (DPE-10203). + """ + try: + return self.app.planned_units() + except ModelError: + return len(self._hosts) + @cached_property def _patroni(self) -> Patroni: """Returns an instance of the Patroni object.""" @@ -2375,6 +2390,11 @@ def _on_update_status(self, _) -> None: # Update the sync-standby endpoint in the async replication data. self.async_replication.update_async_replication_data() + # Clear a promoted-cluster-counter orphaned by a dead-DC teardown whose relation-broken + # never fired (Juju CMR limitation); otherwise a newly-formed async relation re-counts it + # and create-replication wrongly reports "There is already a replication set up.". + self.async_replication.clear_stale_promotion() + self.backup.coordinate_stanza_fields() # self.logical_replication.retry_validations() @@ -2540,10 +2560,7 @@ def _set_primary_status_message(self) -> None: danger_state = "" if not self._patroni.has_raft_quorum(): danger_state = " (read-only)" - elif ( - len(self.patroni_manager.get_running_cluster_members()) - < self.app.planned_units() - ): + elif len(self.patroni_manager.get_running_cluster_members()) < self._planned_units: danger_state = " (degraded)" unit_status = "Standby" if self.is_standby_leader else "Primary" self.set_unit_status(ActiveStatus(f"{unit_status}{danger_state}")) @@ -2771,12 +2788,13 @@ def update_config( """Updates Patroni config file based on the existence of the TLS files.""" if refresh is None: refresh = self.refresh - return self.config_manager.update_config( + primary_cluster_endpoint = self.async_replication.get_primary_cluster_endpoint() + result = self.config_manager.update_config( self.postgresql, is_creating_backup=is_creating_backup, relations_user_databases_map=self.relations_user_databases_map, ldap_parameters=self.get_ldap_parameters(), - async_primary_cluster_endpoint=self.async_replication.get_primary_cluster_endpoint(), + async_primary_cluster_endpoint=primary_cluster_endpoint, async_partner_addresses=self.async_replication.get_partner_addresses(), async_standby_endpoints=self.async_replication.get_standby_endpoints(), watcher_raft_address=self.watcher_offer.watcher_raft_address @@ -2785,6 +2803,21 @@ def update_config( no_peers=no_peers, refresh=refresh, ) + # The lib's apply_api_config only SETS the DCS standby_cluster (when another + # cluster is primary) and never CLEARS it. A force-promote bumps the + # promoted-cluster-counter but — while the dead-DC relation still lingers — does + # not call promote_standby_cluster(), so without this the reconciler never clears + # the stale standby and the cluster stays a read-only standby leader (DPE-10203). + if ( + result + and not no_peers + and self.patroni_manager.member_started + and primary_cluster_endpoint is None + ): + self.patroni_manager.bulk_update_parameters_controller_by_patroni( + {}, {"standby_cluster": None} + ) + return result def _validate_config_options(self) -> None: """Validates specific config options that need access to the database or to the TLS status.""" diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 79d9089a63..cf111f91c4 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -22,6 +22,7 @@ import shutil import subprocess import typing +from collections.abc import Mapping from datetime import datetime from pathlib import Path from subprocess import run @@ -32,6 +33,7 @@ Application, BlockedStatus, MaintenanceStatus, + ModelError, Object, Relation, RelationChangedEvent, @@ -43,6 +45,7 @@ ) from single_kernel_postgresql.config.exceptions import ( ClusterNotPromotedError, + DeployedWithoutTrustError, NotReadyError, StandbyClusterAlreadyPromotedError, ) @@ -67,8 +70,36 @@ READ_ONLY_MODE_BLOCKING_MESSAGE = "Standalone read-only cluster" -# Labels are not confidential -SECRET_LABEL = "async-replication-secret" # noqa: S105 + +# Peer-data key holding the id of the labelless shared cluster-credentials secret the +# owner persists (DPE-10203): referenced by id everywhere, never by label. +ASYNC_SHARED_SECRET_ID_KEY = "async-replication-secret-id" # noqa: S105 — a databag key name, not a credential + + +def _same_secret_id(a: str | None, b: str | None) -> bool: + """Whether two Juju secret ids refer to the same secret. + + Juju/ops may render an id as ``secret:`` or ``secret:///``; compare on the + trailing key so a format difference doesn't mask a real match. + """ + if not a or not b: + return False + return a.rsplit("/", 1)[-1].split(":")[-1] == b.rsplit("/", 1)[-1].split(":")[-1] + + +def _safe_databag_get( + databag: Mapping[str, str], key: str, default: str | None = None +) -> str | None: + """Read a relation databag key, treating an unreadable databag as key-absent. + + A force-removed dead DC leaves the remote databag raising ModelError on read + (DPE-10203); callers must behave as if the key is unset. + """ + try: + return databag.get(key, default) + except ModelError: + return default + if typing.TYPE_CHECKING: from charm import PostgresqlOperatorCharm @@ -182,7 +213,7 @@ def _configure_primary_cluster( if self.charm.app == primary_cluster: counter = self._get_highest_promoted_cluster_counter_value() if not all( - event.relation.data[unit].get("stopped") == counter + _safe_databag_get(event.relation.data[unit], "stopped") == counter for unit in event.relation.units if unit.app == event.relation.app ): @@ -226,7 +257,7 @@ def _configure_standby_cluster(self, event: RelationChangedEvent) -> bool: system_identifier, error = self.get_system_identifier() if error is not None: raise Exception(error) - if system_identifier != relation.data[relation.app].get("system-id"): + if system_identifier != _safe_databag_get(relation.data[relation.app], "system-id"): # Store current data in a tar.gz file. logger.info("Creating backup of data folder") filename = f"{POSTGRESQL_DATA_PATH}-{str(datetime.now()).replace(' ', '-').replace(':', '-')}.tar.gz" @@ -245,16 +276,7 @@ def get_all_primary_cluster_endpoints(self) -> list[str]: # List the primary endpoints only for the standby cluster. if relation is None or primary_cluster is None or self.charm.app == primary_cluster: return [] - return [ - relation.data[unit]["unit-address"] - for relation in [ - self.model.get_relation(REPLICATION_OFFER_RELATION), - self.model.get_relation(REPLICATION_CONSUMER_RELATION), - ] - if relation is not None - for unit in relation.units - if relation.data[unit].get("unit-address") is not None - ] + return self._remote_unit_addresses() def _get_highest_promoted_cluster_counter_value(self) -> str: """Return the highest promoted cluster counter.""" @@ -269,7 +291,14 @@ def _get_highest_promoted_cluster_counter_value(self) -> str: async_relation.data[async_relation.app], self.charm.app_peer_data, ]: - relation_promoted_cluster_counter = databag.get("promoted-cluster-counter", "0") + try: + relation_promoted_cluster_counter = databag.get( + "promoted-cluster-counter", "0" + ) + except ModelError: + # A force-removed dead DC leaves its databag unreadable; skip the + # peer instead of crashing the hook (DPE-10203). + continue if int(relation_promoted_cluster_counter) > int(promoted_cluster_counter): promoted_cluster_counter = relation_promoted_cluster_counter return promoted_cluster_counter @@ -311,7 +340,14 @@ def _get_primary_cluster(self) -> Application | None: self.charm.app: self.charm.all_peer_data, }.items(): databag = relation_data[app] - relation_promoted_cluster_counter = databag.get("promoted-cluster-counter", "0") + try: + relation_promoted_cluster_counter = databag.get( + "promoted-cluster-counter", "0" + ) + except ModelError: + # A force-removed dead DC leaves its databag unreadable; skip the + # peer so reconciliation still runs (DPE-10203). + continue if int(relation_promoted_cluster_counter) > int(promoted_cluster_counter): promoted_cluster_counter = relation_promoted_cluster_counter primary_cluster = app @@ -323,7 +359,11 @@ def get_primary_cluster_endpoint(self) -> str | None: if primary_cluster is None or self.charm.app == primary_cluster: return None relation = self._relation - primary_cluster_data = relation.data[relation.app].get("primary-cluster-data") # type: ignore + if relation is None: + return None + primary_cluster_data = _safe_databag_get( + relation.data[relation.app], "primary-cluster-data" + ) if primary_cluster_data is None: return None return json.loads(primary_cluster_data).get("endpoint") @@ -338,22 +378,56 @@ def _get_secret(self) -> Secret | None: # Filter out unnecessary secrets. shared_content = dict(filter(lambda x: "password" in x[0], content.items())) - try: - # Avoid recreating the secret. - secret = self.charm.model.get_secret(label=SECRET_LABEL) - if not secret.id: - # Workaround for the secret id not being set with model uuid. - secret._id = f"secret://{self.model.uuid}/{secret.get_info().id.split(':')[1]}" - if secret.peek_content() != shared_content: - logger.info("Updating outdated secret content") - secret.set_content(shared_content) - return secret - except SecretNotFoundError: - logger.debug("Secret not found, creating a new one") - pass + # The owner references its secret purely by the id persisted in app peer data — + # no label. Owning under a label risks colliding with a stale consumer alias Juju + # keeps reserved after a dead-DC teardown ("secret with label already exists"), + # and a label lookup cannot survive the secret's own id churn (DPE-10203). + secret_id = self.charm.app_peer_data.get(ASYNC_SHARED_SECRET_ID_KEY) + if not secret_id: + # Migration from the legacy charm (which owned the secret under a label): + # this cluster's own relation data still publishes the last-known id. Adopt + # that secret instead of creating a second one — an id switch would wedge + # any consumer still running label-attaching code, since Juju refuses to + # rebind a consumer label to a new secret id. + secret_id = self._own_published_secret_id() + if secret_id: + try: + secret = self.charm.model.get_secret(id=secret_id) + except SecretNotFoundError: + logger.debug("Persisted async-replication secret is gone; recreating") + else: + if secret.peek_content() != shared_content: + logger.info("Updating outdated secret content") + secret.set_content(shared_content) + # Persist the id (covers the migration path, where the id came from + # this cluster's own relation data rather than peer data). + self.charm.app_peer_data.update({ASYNC_SHARED_SECRET_ID_KEY: secret.id}) + return secret if self.charm.unit.is_leader(): - return self.charm.model.app.add_secret(content=shared_content, label=SECRET_LABEL) + secret = self.charm.model.app.add_secret(content=shared_content) + self.charm.app_peer_data.update({ASYNC_SHARED_SECRET_ID_KEY: secret.id}) + return secret + + def _own_published_secret_id(self) -> str | None: + """Return the secret id this cluster last published, from its own relation data.""" + for relation in [ + self.model.get_relation(REPLICATION_OFFER_RELATION), + self.model.get_relation(REPLICATION_CONSUMER_RELATION), + ]: + if relation is None: + continue + try: + primary_cluster_data = _safe_databag_get( + relation.data[self.charm.app], "primary-cluster-data" + ) + except ModelError: + continue + if primary_cluster_data is None: + continue + if secret_id := json.loads(primary_cluster_data).get("secret-id"): + return secret_id + return None def get_standby_endpoints(self) -> list[str]: """Return the standby endpoints.""" @@ -364,16 +438,26 @@ def get_standby_endpoints(self) -> list[str]: # List the standby endpoints only for the primary cluster. if relation is None or primary_cluster is None or self.charm.app != primary_cluster: return [] - return [ - relation.data[unit]["unit-address"] - for relation in [ - self.model.get_relation(REPLICATION_OFFER_RELATION), - self.model.get_relation(REPLICATION_CONSUMER_RELATION), - ] - if relation is not None - for unit in relation.units - if relation.data[unit].get("unit-address") is not None - ] + return self._remote_unit_addresses() + + def _remote_unit_addresses(self) -> list[str]: + """Return unit addresses published across both async relations. + + Skips units whose databag is unreadable — a dead-DC teardown leaves the dying + cross-model relation's unit databags raising ModelError on read (DPE-10203). + """ + addresses = [] + for relation in [ + self.model.get_relation(REPLICATION_OFFER_RELATION), + self.model.get_relation(REPLICATION_CONSUMER_RELATION), + ]: + if relation is None: + continue + for unit in relation.units: + address = _safe_databag_get(relation.data[unit], "unit-address") + if address is not None: + addresses.append(address) + return addresses def get_system_identifier(self) -> tuple[str | None, str | None]: """Returns the PostgreSQL system identifier from this instance.""" @@ -503,7 +587,7 @@ def _handle_replication_change(self, event: ActionEvent) -> bool: # If not, fail the action telling that all units must publish their pod addresses in the # relation data. for unit in remote_units: - if "unit-address" not in relation.data[unit]: + if _safe_databag_get(relation.data[unit], "unit-address") is None: event.fail( "All units from the other cluster must publish their unit addresses in the relation data." ) @@ -547,19 +631,82 @@ def _on_async_relation_broken(self, _) -> None: "unit-promoted-cluster-counter": "", }) + # A force-removed dead offerer can make the standby check fail transiently; + # crashing here would wedge the unit before the counter is cleared, so treat + # the cluster as primary (DPE-10203 / Issue B). + try: + is_standby = self.charm.patroni_manager.get_standby_leader() is not None + except Exception as e: + logger.warning( + "get_standby_leader unavailable during teardown, assuming primary: %s", e + ) + is_standby = False + # If this is the standby cluster, set 0 in the "promoted-cluster-counter" field to set # the cluster in read-only mode message also in the other units. - if self.charm.patroni_manager.get_standby_leader() is not None: + if is_standby: if self.charm.unit.is_leader(): self.charm.app_peer_data.update({"promoted-cluster-counter": "0"}) self.set_app_status() else: if self.charm.unit.is_leader(): self.charm.app_peer_data.update({"promoted-cluster-counter": ""}) - self.charm.update_config() + try: + self.charm.update_config() + except (DeployedWithoutTrustError, RetryError, ModelError) as e: + logger.warning("update_config failed during teardown (continuing): %s", e) if self.charm.unit.is_leader(): - self.charm.watcher_offer.update_endpoints() + try: + self.charm.watcher_offer.update_endpoints() + except (ModelError, RetryError) as e: + logger.warning( + "watcher endpoint update failed during teardown (continuing): %s", e + ) + + def clear_stale_promotion(self) -> None: + """Clear a promoted-cluster-counter left over from a removed async relation. + + A force-removed dead offerer never delivers ``relation-broken``, leaving the + counter behind; on a new async relation it would wrongly mark this app as the + primary and block ``create-replication`` (DPE-10203). + """ + if not self.charm.unit.is_leader(): + return + counter = self.charm.app_peer_data.get("promoted-cluster-counter") + # Empty -> standby/clean (nothing promoted). "0" -> a standby already in read-only mode + # (set by _on_async_relation_broken); leave it. A positive counter means this cluster + # was promoted -> revert it to a standalone primary unless a live relation still + # records that promotion. Deciding this from relation/peer data alone (no Patroni call) + # is deliberate: after a dead-DC promote Patroni is frequently unreachable, which is + # exactly when this must still run. + if not counter or counter == "0": + return + # A promotion writes the counter to both the async relation it was promoted under and + # the peers databag, so a counter mirrored on a current relation is a live replication + # and is managed by the relation lifecycle. The recovery sequence forms a *new* offer + # relation before running create-replication, and that relation carries no mirror — + # the counter left by the dead relation is stale exactly then and must clear even + # though a relation now exists (DPE-10203). + for relation in [ + self.model.get_relation(REPLICATION_OFFER_RELATION), + self.model.get_relation(REPLICATION_CONSUMER_RELATION), + ]: + if relation is None: + continue + try: + if relation.data[self.charm.app].get("promoted-cluster-counter") == counter: + return + except ModelError: + # A dying relation whose databags are unreadable cannot vouch for the + # counter either: the promotion's relation is gone for all purposes. + continue + logger.info( + "Clearing stale promoted-cluster-counter %s (no live async relation records it)", + counter, + ) + self.charm.app_peer_data.update({"promoted-cluster-counter": ""}) + self.charm.update_config() def _on_async_relation_changed(self, event: RelationChangedEvent) -> None: """Update the Patroni configuration if one of the clusters was already promoted.""" @@ -634,6 +781,11 @@ def _on_async_relation_joined(self, _) -> None: def _on_create_replication(self, event: ActionEvent) -> None: """Set up asynchronous replication between two clusters.""" + # A dead-DC teardown whose relation-broken never fired leaves the promoted- + # cluster-counter orphaned in peer data; clear it before the guard reads it, + # or create-replication reports "There is already a replication set up." + # until an update-status cycle happens to reconcile (DPE-10203). + self.clear_stale_promotion() if self._get_primary_cluster() is not None: event.fail("There is already a replication set up.") return @@ -653,6 +805,9 @@ def _on_create_replication(self, event: ActionEvent) -> None: def promote_to_primary(self, event: ActionEvent) -> None: """Promote this cluster to the primary cluster.""" + # Same stale-counter exposure as create-replication: a counter orphaned by a + # teardown without events would mask the "no primary" condition below. + self.clear_stale_promotion() if ( self.charm.app.status.message != READ_ONLY_MODE_BLOCKING_MESSAGE and self._get_primary_cluster() is None @@ -694,7 +849,9 @@ def _on_secret_changed(self, event: SecretChangedEvent) -> None: ) return - if relation.name == REPLICATION_CONSUMER_RELATION and event.secret.label == SECRET_LABEL: + if relation.name == REPLICATION_CONSUMER_RELATION and _same_secret_id( + event.secret.id, self._remote_secret_id() + ): logger.info("Relation secret changed, updating internal secret") if not self._update_internal_secret(): logger.debug("Secret not found, deferring event") @@ -750,13 +907,24 @@ def _reinitialise_pgdata(self) -> None: @property def _relation(self) -> Relation | None: - """Return the relation object.""" + """Return the usable async-replication relation, or None. + + A relation whose databags are unreadable is treated as absent — the dying + cross-model relation left by a force-removed dead DC reads as "permission + denied" on every databag (DPE-10203). A cheap own-unit read probes for that. + """ for relation in [ self.model.get_relation(REPLICATION_OFFER_RELATION), self.model.get_relation(REPLICATION_CONSUMER_RELATION), ]: - if relation is not None: - return relation + if relation is None: + continue + try: + relation.data[self.charm.unit].get("unit-address") + except ModelError: + continue + return relation + return None def set_app_status(self) -> None: """Set the app status.""" @@ -835,17 +1003,24 @@ def update_async_replication_data(self) -> None: if self.is_primary_cluster() and self.charm.unit.is_leader(): self._update_primary_cluster_data() - def _update_internal_secret(self) -> bool: - # Update the secrets between the clusters. + def _remote_secret_id(self) -> str | None: + """Return the shared secret id published by the primary cluster, or None.""" relation = self._relation - primary_cluster_info = relation.data[relation.app].get("primary-cluster-data") # type: ignore - secret_id = ( - None - if primary_cluster_info is None - else json.loads(primary_cluster_info).get("secret-id") - ) + if relation is None: + return None + primary_cluster_info = relation.data[relation.app].get("primary-cluster-data") + if primary_cluster_info is None: + return None + return json.loads(primary_cluster_info).get("secret-id") + + def _update_internal_secret(self) -> bool: + # Update the secrets between the clusters. Reference the secret purely by the id published + # in relation data — never by label — so no consumer-side alias is registered (DPE-10203). + secret_id = self._remote_secret_id() + if secret_id is None: + return False try: - secret = self.charm.model.get_secret(id=secret_id, label=SECRET_LABEL) + secret = self.charm.model.get_secret(id=secret_id) except SecretNotFoundError: return False credentials = secret.peek_content() diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index 4c23d7962c..8a9ccff4d9 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -1,16 +1,22 @@ # Copyright 2025 Canonical Ltd. # See LICENSE file for licensing details. +import json from unittest.mock import MagicMock, PropertyMock, patch import pytest -from ops import Application -from single_kernel_postgresql.config.literals import REPLICATION_CONSUMER_RELATION +from ops import Application, ModelError +from single_kernel_postgresql.config.literals import ( + REPLICATION_CONSUMER_RELATION, + REPLICATION_OFFER_RELATION, +) from tenacity import RetryError from src.relations.async_replication import ( READ_ONLY_MODE_BLOCKING_MESSAGE, PostgreSQLAsyncReplication, + _safe_databag_get, + _same_secret_id, ) @@ -164,9 +170,13 @@ def test_on_create_replication(): mock_relation = MagicMock() mock_relation.name = REPLICATION_CONSUMER_RELATION - type(relation)._relation = PropertyMock(return_value=mock_relation) - - result = relation._on_create_replication(mock_event) + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=mock_relation, + ): + result = relation._on_create_replication(mock_event) assert result is None mock_event.fail.assert_called_once_with( @@ -184,9 +194,13 @@ def test_on_create_replication(): mock_relation = MagicMock() mock_relation.name = "Something" - type(relation)._relation = PropertyMock(return_value=mock_relation) - - result = relation._on_create_replication(mock_event) + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=mock_relation, + ): + result = relation._on_create_replication(mock_event) assert result is None @@ -202,11 +216,68 @@ def test_on_create_replication(): mock_relation = MagicMock() mock_relation.name = "Something" - type(relation)._relation = PropertyMock(return_value=mock_relation) + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=mock_relation, + ): + result = relation._on_create_replication(mock_event) - result = relation._on_create_replication(mock_event) + assert result is None + + # Stale orphaned counter (set by a dead-DC teardown whose relation-broken never + # fired) is cleared BEFORE the guard runs, so create-replication succeeds on the + # first call instead of failing with "There is already a replication set up." + # until an update-status cycle happens to run (DPE-10203 dead-DC live-run regression). + mock_charm = MagicMock() + mock_charm.unit.is_leader.return_value = True + mock_charm.app_peer_data = {"promoted-cluster-counter": "2"} + stale_relation = MagicMock() + stale_relation.data = {mock_charm.unit: {}, mock_charm.app: {}} + mock_charm.framework.model.get_relation.return_value = stale_relation + relation = PostgreSQLAsyncReplication(mock_charm) + relation._handle_replication_change = MagicMock(return_value=True) + relation._get_primary_cluster = MagicMock(return_value=None) + mock_relation = MagicMock() + mock_relation.name = "Something" + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=mock_relation, + ): + result = relation._on_create_replication(mock_event) + + assert result is None + assert mock_charm.app_peer_data.get("promoted-cluster-counter") == "" + mock_event.fail.assert_not_called() + relation._handle_replication_change.assert_called_once() + + # A counter mirrored on a live relation (an actual replication) survives the + # pre-guard clearing: the action still refuses with "already a replication set up." + mock_charm = MagicMock() + mock_charm.unit.is_leader.return_value = True + mock_charm.app_peer_data = {"promoted-cluster-counter": "2"} + mirror_relation = MagicMock() + mirror_relation.data = {mock_charm.unit: {}, mock_charm.app: {"promoted-cluster-counter": "2"}} + mock_charm.framework.model.get_relation.return_value = mirror_relation + relation = PostgreSQLAsyncReplication(mock_charm) + relation._handle_replication_change = MagicMock(return_value=True) + relation._get_primary_cluster = MagicMock(return_value=mock_charm.app) + mock_relation = MagicMock() + mock_relation.name = "Something" + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=mock_relation, + ): + result = relation._on_create_replication(mock_event) assert result is None + assert mock_charm.app_peer_data.get("promoted-cluster-counter") == "2" + mock_event.fail.assert_called_once_with("There is already a replication set up.") def test_promote_to_primary(): @@ -220,7 +291,6 @@ def test_promote_to_primary(): relation = PostgreSQLAsyncReplication(mock_charm) relation._get_primary_cluster = MagicMock(return_value=None) - type(relation).app = PropertyMock(return_value=mock_relation) result = relation.promote_to_primary(mock_event) assert result is None @@ -232,14 +302,12 @@ def test_promote_to_primary(): mock_charm = MagicMock() mock_event = MagicMock() mock_relation = MagicMock() - mock_app = MagicMock(spec=Application) mock_relation.status = MagicMock() mock_relation.status.message = READ_ONLY_MODE_BLOCKING_MESSAGE relation = PostgreSQLAsyncReplication(mock_charm) relation._get_primary_cluster = MagicMock(return_value=None) - type(relation).app = PropertyMock(return_value=mock_app) relation._handle_replication_change = MagicMock(return_value=False) result = relation.promote_to_primary(mock_event) @@ -252,11 +320,17 @@ def test__configure_standby_cluster(): mock_event = MagicMock() relation = PostgreSQLAsyncReplication(mock_charm) - relation._relation = MagicMock() - relation._relation.name = REPLICATION_CONSUMER_RELATION + mock_relation = MagicMock() + mock_relation.name = REPLICATION_CONSUMER_RELATION relation._update_internal_secret = MagicMock(return_value=False) - result = relation._configure_standby_cluster(mock_event) + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=mock_relation, + ): + result = relation._configure_standby_cluster(mock_event) assert result is False @@ -267,12 +341,20 @@ def test__configure_standby_cluster(): mock_event = MagicMock() relation = PostgreSQLAsyncReplication(mock_charm) - relation._relation = MagicMock() - relation._relation.name = "something_else" + mock_relation = MagicMock() + mock_relation.name = "something_else" relation._update_internal_secret = MagicMock(return_value=True) relation.get_system_identifier = MagicMock(return_value=(None, 2)) - with pytest.raises(Exception) as exc_info: + with ( + patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=mock_relation, + ), + pytest.raises(Exception) as exc_info, + ): relation._configure_standby_cluster(mock_event) assert str(exc_info.value) == "2" @@ -282,17 +364,25 @@ def test__configure_standby_cluster(): mock_event = MagicMock() relation = PostgreSQLAsyncReplication(mock_charm) - relation._relation = MagicMock() - relation._relation.name = "some_relation" - relation._relation.app = "remote-app" - relation._relation.data = {relation._relation.app: {"system-id": "123"}} + mock_relation = MagicMock() + mock_relation.name = "some_relation" + mock_relation.app = "remote-app" + mock_relation.data = {"remote-app": {"system-id": "123"}} relation._update_internal_secret = MagicMock(return_value=True) relation.get_system_identifier = MagicMock(return_value=("456", None)) relation.charm = MagicMock() relation.charm.app_peer_data = {} - with patch("subprocess.check_call") as mock_check_call: + with ( + patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=mock_relation, + ), + patch("subprocess.check_call") as mock_check_call, + ): result = relation._configure_standby_cluster(mock_event) assert result is True @@ -506,16 +596,20 @@ def test_handle_forceful_promotion(): mock_event.params.get.return_value = False relation = PostgreSQLAsyncReplication(mock_charm) - - relation._relation = MagicMock() - relation._relation.app = MagicMock() - relation._relation.app.name = "test-app" + mock_relation = MagicMock() + mock_relation.app.name = "test-app" relation.get_all_primary_cluster_endpoints = MagicMock(return_value=[1, 2, 3]) mock_charm.patroni_manager.get_primary.side_effect = RetryError("timeout") - result = relation._handle_forceful_promotion(mock_event) + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=mock_relation, + ): + result = relation._handle_forceful_promotion(mock_event) mock_event.fail.assert_called_once_with( "test-app isn't reachable. Pass `force=true` to promote anyway." @@ -528,16 +622,20 @@ def test_handle_forceful_promotion(): mock_event.params.get.return_value = False relation = PostgreSQLAsyncReplication(mock_charm) - - relation._relation = MagicMock() - relation._relation.app = MagicMock() - relation._relation.app.name = "test-app" + mock_relation = MagicMock() + mock_relation.app.name = "test-app" relation.get_all_primary_cluster_endpoints = MagicMock(return_value=[1, 2, 3]) mock_charm._patroni.get_primary.side_effect = None - result = relation._handle_forceful_promotion(mock_event) + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=mock_relation, + ): + result = relation._handle_forceful_promotion(mock_event) assert result is True # 4. @@ -547,16 +645,20 @@ def test_handle_forceful_promotion(): mock_event.params.get.return_value = False relation = PostgreSQLAsyncReplication(mock_charm) - - relation._relation = MagicMock() - relation._relation.app = MagicMock() - relation._relation.app.name = "test-app" + mock_relation = MagicMock() + mock_relation.app.name = "test-app" relation.get_all_primary_cluster_endpoints = MagicMock(return_value=[]) mock_charm._patroni.get_primary.side_effect = None - result = relation._handle_forceful_promotion(mock_event) + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=mock_relation, + ): + result = relation._handle_forceful_promotion(mock_event) assert result is True @@ -584,3 +686,450 @@ def test_on_async_relation_broken(): relation._on_async_relation_broken(mock_event) assert mock_charm.update_config.called + + # 3. get_standby_leader raises (transient teardown failure, e.g. network-get during a dead-DC + # force-removal): the hook must NOT crash and must still clear the counter, so the unit does + # not wedge in error (DPE-10203 / Issue B). + mock_charm = MagicMock() + mock_charm._peers = MagicMock() + mock_charm.is_unit_departing = False + mock_charm.patroni_manager.get_standby_leader.side_effect = Exception( + "network-get exited status 1" + ) + mock_charm.unit.is_leader.return_value = True + mock_charm.app_peer_data = {"promoted-cluster-counter": "2"} + mock_event = MagicMock() + + relation = PostgreSQLAsyncReplication(mock_charm) + relation._on_async_relation_broken(mock_event) # must not raise + + assert mock_charm.app_peer_data.get("promoted-cluster-counter") == "" + + +def test_clear_stale_promotion(): + # Leader, no async relation, positive counter -> cleared + config re-rendered. + mock_charm = MagicMock() + mock_charm.unit.is_leader.return_value = True + mock_charm.app_peer_data = {"promoted-cluster-counter": "2"} + mock_charm.framework.model.get_relation.return_value = None + relation = PostgreSQLAsyncReplication(mock_charm) + relation.clear_stale_promotion() + assert mock_charm.app_peer_data.get("promoted-cluster-counter") == "" + mock_charm.update_config.assert_called_once() + + # A relation formed AFTER the promotion (the recovery sequence offers to a fresh + # cluster before create-replication) carries no counter mirror -> the orphaned + # counter must still clear, or create-replication stays blocked with "There is + # already a replication set up." (DPE-10203 dead-DC live-run regression). + mock_charm = MagicMock() + mock_charm.unit.is_leader.return_value = True + mock_charm.app_peer_data = {"promoted-cluster-counter": "2"} + async_relation = MagicMock() + async_relation.data = {mock_charm.unit: {}, mock_charm.app: {}} + mock_charm.framework.model.get_relation.return_value = async_relation + relation = PostgreSQLAsyncReplication(mock_charm) + relation.clear_stale_promotion() + assert mock_charm.app_peer_data.get("promoted-cluster-counter") == "" + mock_charm.update_config.assert_called_once() + + # A relation that mirrors the counter (an active replication) -> no-op: the + # counter is managed by the relation lifecycle. + mock_charm = MagicMock() + mock_charm.unit.is_leader.return_value = True + mock_charm.app_peer_data = {"promoted-cluster-counter": "2"} + async_relation = MagicMock() + async_relation.data = {mock_charm.unit: {}, mock_charm.app: {"promoted-cluster-counter": "2"}} + mock_charm.framework.model.get_relation.return_value = async_relation + relation = PostgreSQLAsyncReplication(mock_charm) + relation.clear_stale_promotion() + assert mock_charm.app_peer_data.get("promoted-cluster-counter") == "2" + mock_charm.update_config.assert_not_called() + + # Non-leader -> no-op. + mock_charm = MagicMock() + mock_charm.unit.is_leader.return_value = False + mock_charm.app_peer_data = {"promoted-cluster-counter": "2"} + relation = PostgreSQLAsyncReplication(mock_charm) + relation.clear_stale_promotion() + assert mock_charm.app_peer_data.get("promoted-cluster-counter") == "2" + + # Counter "0" (a standby already in read-only mode) -> left untouched, no Patroni call needed. + mock_charm = MagicMock() + mock_charm.unit.is_leader.return_value = True + mock_charm.app_peer_data = {"promoted-cluster-counter": "0"} + relation = PostgreSQLAsyncReplication(mock_charm) + relation.clear_stale_promotion() + assert mock_charm.app_peer_data.get("promoted-cluster-counter") == "0" + mock_charm.update_config.assert_not_called() + mock_charm._patroni.get_standby_leader.assert_not_called() + + +def test_get_secret_creates_labelless_owned_secret_and_persists_id(): + # Regression for DPE-10203: the owner creates the shared secret with NO label and + # persists its id in app peer data. Owning under any label risks colliding with a + # stale consumer alias Juju keeps reserved after a dead-DC teardown ("secret with + # label already exists"); labelless + id-in-peer-data has no label to collide. + mock_charm = MagicMock() + relation = PostgreSQLAsyncReplication(mock_charm) + + app_secret = MagicMock() + app_secret.peek_content.return_value = { + "operator-password": "op", + "replication-password": "rep", + "system-id": "x", + } + mock_charm.model.get_secret.return_value = app_secret + mock_charm.unit.is_leader.return_value = True + mock_charm.app_peer_data = {} + mock_charm.framework.model.get_relation.return_value = None # no relation yet + + created = MagicMock() + created.id = "secret://uuid/new" + mock_charm.model.app.add_secret.return_value = created + + result = relation._get_secret() + + # Created with NO label; only password fields are shared between clusters. + mock_charm.model.app.add_secret.assert_called_once() + _, kwargs = mock_charm.model.app.add_secret.call_args + assert "label" not in kwargs + assert kwargs["content"] == {"operator-password": "op", "replication-password": "rep"} + # The id is persisted so later hooks re-find the secret without a label. + assert mock_charm.app_peer_data.get("async-replication-secret-id") == "secret://uuid/new" + assert result is created + + +def test_get_secret_adopts_secret_from_own_relation_data_on_migration(): + # A cluster refreshed from the legacy charm (which owned the secret under the old + # label) has no id in peer data, but its own offer-relation data still publishes the + # last-known secret id. Adopt that secret instead of creating a second one — an id + # switch would wedge any consumer still running label-attaching code (Juju refuses + # to rebind a consumer label to a new secret id; DPE-10203). + mock_charm = MagicMock() + relation = PostgreSQLAsyncReplication(mock_charm) + + app_secret = MagicMock() + app_secret.peek_content.return_value = {"operator-password": "op"} + existing = MagicMock() + existing.id = "secret://uuid/legacy" + existing.peek_content.return_value = {"operator-password": "op"} + # First get_secret: the peer app secret. Second: the adopted secret by id. + mock_charm.model.get_secret.side_effect = [app_secret, existing] + mock_charm.unit.is_leader.return_value = True + mock_charm.app_peer_data = {} + + offer_relation = MagicMock() + offer_relation.name = REPLICATION_OFFER_RELATION + offer_relation.data = { + mock_charm.app: {"primary-cluster-data": json.dumps({"secret-id": "secret://uuid/legacy"})} + } + mock_model = MagicMock() + mock_model.get_relation.return_value = offer_relation + with patch.object( + PostgreSQLAsyncReplication, "model", new_callable=PropertyMock, return_value=mock_model + ): + result = relation._get_secret() + + mock_charm.model.app.add_secret.assert_not_called() + assert result is existing + # Adoption persists the id for future hooks. + assert mock_charm.app_peer_data.get("async-replication-secret-id") == "secret://uuid/legacy" + + +def test_get_secret_reuses_secret_by_persisted_id(): + # Later hooks re-find the owned secret purely by the id persisted in app peer data — + # no label anywhere — and only rewrite content when it drifts. + mock_charm = MagicMock() + relation = PostgreSQLAsyncReplication(mock_charm) + + app_secret = MagicMock() + app_secret.peek_content.return_value = {"operator-password": "op"} + + existing = MagicMock() + existing.id = "secret://uuid/abc" + existing.peek_content.return_value = {"operator-password": "op"} + + mock_charm.model.get_secret.side_effect = [app_secret, existing] + mock_charm.app_peer_data = {"async-replication-secret-id": "secret://uuid/abc"} + + result = relation._get_secret() + + mock_charm.model.app.add_secret.assert_not_called() + existing.set_content.assert_not_called() + assert result is existing + # The second lookup is by the persisted id, not by any label. + second = mock_charm.model.get_secret.call_args_list[1] + assert second.kwargs.get("id") == "secret://uuid/abc" + assert "label" not in second.kwargs + + +def test__get_primary_cluster_skips_unreadable_dead_peer_databag(): + # DPE-10203: after a dead-DC teardown the remote app's databag on the dying + # cross-model async relation is unreadable — `relation-get --app ` + # returns "permission denied" (surfaced as ModelError) once the offering DC is + # gone. _get_primary_cluster must skip that peer instead of crashing every hook, + # so the readable local peer is still evaluated. + mock_charm = MagicMock() + local_app = MagicMock() + mock_charm.app = local_app + + remote_app = MagicMock() + dead_databag = MagicMock() + dead_databag.get.side_effect = ModelError("ERROR permission denied") + offer_relation = MagicMock() + offer_relation.app = remote_app + offer_relation.data = {remote_app: dead_databag} + + local_databag = MagicMock() + local_databag.get.return_value = "1" + mock_charm.all_peer_data = {local_app: local_databag} + + mock_model = MagicMock() + mock_model.get_relation.side_effect = [offer_relation, None] + + relation = PostgreSQLAsyncReplication(mock_charm) + with patch.object( + PostgreSQLAsyncReplication, "model", new_callable=PropertyMock, return_value=mock_model + ): + # Must not raise ModelError; the unreadable dead peer is skipped and the + # readable local peer (counter "1") is selected as the primary. + assert relation._get_primary_cluster() is local_app + dead_databag.get.assert_called_once_with("promoted-cluster-counter", "0") + + +def test__relation_skips_unreadable_dying_relation(monkeypatch): + # DPE-10203: a dead-DC teardown leaves the cross-model async relation in a + # dying state whose databags raise ModelError ("permission denied") on any + # read, even though get_relation still returns it. _relation must probe and + # treat such a relation as absent, so the promoted primary reconciles as a + # standalone cluster instead of crashing every hook that writes relation data. + mock_charm = MagicMock() + + dying = MagicMock() + dying_databag = MagicMock() + dying_databag.get.side_effect = ModelError("ERROR permission denied") + dying.data.__getitem__.return_value = dying_databag + + readable = MagicMock() # its databag read succeeds (default MagicMock, no raise) + + relation = PostgreSQLAsyncReplication(mock_charm) + # First candidate (offer) is the dying relation; second (consumer) is readable. + relation.model.get_relation.side_effect = [dying, readable] + + # The dying relation is skipped (probe raised); the readable one is returned. + assert relation._relation is readable + dying_databag.get.assert_called_once_with("unit-address") + + +# --- DPE-10203 follow-up: consumer reads the shared secret by id, never by label ------------- +# The consumer used to fetch the offer secret with ``get_secret(id=..., label=)``, +# registering a local consumer alias that Juju leaves reserved after a dead-DC teardown. Matching +# MySQL's async-replication design, the consumer now references the secret purely by the id +# published in relation data, so no alias can go stale. These tests pin that behaviour. + + +@pytest.mark.parametrize( + ("a", "b", "expected"), + [ + ("secret://uuid/abc123", "secret://uuid/abc123", True), + ("secret://uuid/abc123", "secret:abc123", True), # format-insensitive + ("secret:abc123", "secret://uuid/abc123", True), + ("secret://uuid/abc123", "secret://uuid/xyz789", False), + (None, "secret:abc123", False), + ("secret:abc123", None, False), + (None, None, False), + ], +) +def test_same_secret_id(a, b, expected): + assert _same_secret_id(a, b) is expected + + +def _consumer_relation(secret_id): + relation = MagicMock() + relation.name = REPLICATION_CONSUMER_RELATION + relation.app = "primary-app" + relation.data = {"primary-app": {"primary-cluster-data": json.dumps({"secret-id": secret_id})}} + return relation + + +def test_update_internal_secret_reads_by_id_without_label(): + mock_charm = MagicMock() + relation = PostgreSQLAsyncReplication(mock_charm) + + secret = MagicMock() + secret.peek_content.return_value = {"operator-password": "pw"} + mock_charm.model.get_secret.return_value = secret + + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=_consumer_relation("secret://uuid/abc123"), + ): + assert relation._update_internal_secret() is True + + # Fetched purely by id, with no ``label=`` alias registered. + mock_charm.model.get_secret.assert_called_once_with(id="secret://uuid/abc123") + + +def test_update_internal_secret_returns_false_without_secret_id(): + mock_charm = MagicMock() + relation = PostgreSQLAsyncReplication(mock_charm) + + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=_consumer_relation(None), + ): + assert relation._update_internal_secret() is False + + mock_charm.model.get_secret.assert_not_called() + + +def test_on_secret_changed_consumer_matches_by_id_not_label(): + mock_charm = MagicMock() + relation = PostgreSQLAsyncReplication(mock_charm) + + mock_event = MagicMock() + mock_event.secret.id = "secret://uuid/abc123" # same key, different URI format + mock_event.secret.label = None # no alias any more + + with ( + patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=_consumer_relation("secret:abc123"), + ), + patch.object( + PostgreSQLAsyncReplication, "_update_internal_secret", return_value=True + ) as mock_update, + ): + relation._on_secret_changed(mock_event) + + mock_update.assert_called_once() + mock_event.defer.assert_not_called() + + +def test_on_secret_changed_consumer_ignores_unrelated_secret(): + mock_charm = MagicMock() + relation = PostgreSQLAsyncReplication(mock_charm) + + mock_event = MagicMock() + mock_event.secret.id = "secret://uuid/DIFFERENT" + mock_event.secret.label = "async-replication-secret" # legacy label must NOT trigger the sync + + with ( + patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=_consumer_relation("secret:abc123"), + ), + patch.object( + PostgreSQLAsyncReplication, "_update_internal_secret", return_value=True + ) as mock_update, + ): + relation._on_secret_changed(mock_event) + + mock_update.assert_not_called() + mock_event.defer.assert_not_called() + + +def test_on_secret_changed_consumer_defers_when_secret_not_ready(): + mock_charm = MagicMock() + relation = PostgreSQLAsyncReplication(mock_charm) + + mock_event = MagicMock() + mock_event.secret.id = "secret://uuid/abc123" + mock_event.secret.label = None + + with ( + patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=_consumer_relation("secret:abc123"), + ), + patch.object(PostgreSQLAsyncReplication, "_update_internal_secret", return_value=False), + ): + relation._on_secret_changed(mock_event) + + mock_event.defer.assert_called_once() + + +def test__get_highest_promoted_cluster_counter_value_skips_unreadable_dead_peer(): + # DPE-10203: after a dead-DC teardown the remote app's databag on the dying + # cross-model async relation is unreadable — `relation-get --app ` + # raises ModelError ("permission denied") once the offering DC is gone. Like + # _get_primary_cluster, _get_highest_promoted_cluster_counter_value must skip + # that peer instead of crashing the hook (it crashed replication-offer-relation + # -joined on the promoted cluster, blocking re-replication), still honouring the + # readable local peer counter. + mock_charm = MagicMock() + + remote_app = MagicMock() + dead_databag = MagicMock() + dead_databag.get.side_effect = ModelError("ERROR permission denied") + offer_relation = MagicMock() + offer_relation.app = remote_app + offer_relation.data = {remote_app: dead_databag} + + # The local peer databag is readable and holds a higher counter. + mock_charm.app_peer_data = {"promoted-cluster-counter": "3"} + + mock_model = MagicMock() + mock_model.get_relation.side_effect = [offer_relation, None] + + relation = PostgreSQLAsyncReplication(mock_charm) + with patch.object( + PostgreSQLAsyncReplication, "model", new_callable=PropertyMock, return_value=mock_model + ): + # Must not raise; the unreadable dead peer is skipped and the local counter wins. + assert relation._get_highest_promoted_cluster_counter_value() == "3" + dead_databag.get.assert_called_once_with("promoted-cluster-counter", "0") + + +# --- DPE-10203 dead-DC hardening: async-relation reads must survive an unreadable peer --------- + + +def test_safe_databag_get_returns_value_when_readable(): + assert _safe_databag_get({"k": "v"}, "k") == "v" + assert _safe_databag_get({}, "k", "default") == "default" + + +def test_safe_databag_get_treats_unreadable_databag_as_absent(): + # A dead-DC teardown makes the remote databag raise ModelError on any read; callers + # must see the key as absent instead of crashing the hook (DPE-10203). + dead_databag = MagicMock() + dead_databag.get.side_effect = ModelError("ERROR permission denied") + assert _safe_databag_get(dead_databag, "k") is None + assert _safe_databag_get(dead_databag, "k", "default") == "default" + + +def test_remote_unit_addresses_skips_unreadable_dead_peer_units(): + # The dying cross-model relation's unit databags raise ModelError on read; + # _remote_unit_addresses must skip them and still return the readable addresses. + mock_charm = MagicMock() + + good_unit = MagicMock() + offer_relation = MagicMock() + offer_relation.units = [good_unit] + offer_relation.data = {good_unit: {"unit-address": "10.0.0.1"}} + + dead_unit = MagicMock() + dead_databag = MagicMock() + dead_databag.get.side_effect = ModelError("ERROR permission denied") + dead_relation = MagicMock() + dead_relation.units = [dead_unit] + dead_relation.data = {dead_unit: dead_databag} + + mock_model = MagicMock() + mock_model.get_relation.side_effect = [offer_relation, dead_relation] + + relation = PostgreSQLAsyncReplication(mock_charm) + with patch.object( + PostgreSQLAsyncReplication, "model", new_callable=PropertyMock, return_value=mock_model + ): + assert relation._remote_unit_addresses() == ["10.0.0.1"] diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 1a15bdce8c..a4e13a050f 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1046,6 +1046,7 @@ def test_on_update_status(harness): "charm.PostgresqlOperatorCharm._set_primary_status_message" ) as _set_primary_status_message, patch("charm.PatroniManager.restart_patroni") as _restart_patroni, + patch("charm.PostgreSQLAsyncReplication.clear_stale_promotion") as _clear_stale_promotion, patch("charm.PatroniManager.is_member_isolated") as _is_member_isolated, patch("charm.PatroniManager.member_started", new_callable=PropertyMock) as _member_started, patch( @@ -1116,6 +1117,8 @@ def test_on_update_status(harness): harness.charm.unit.status = ActiveStatus() harness.charm.on.update_status.emit() _set_primary_status_message.assert_called_once() + # A stale promoted-cluster-counter from a dead-DC teardown is reconciled here too. + _clear_stale_promotion.assert_called_once_with() # Test call to restart when the member is isolated from the cluster. _set_primary_status_message.reset_mock() @@ -1451,6 +1454,33 @@ def test_update_config_delegates_to_config_manager(harness): assert kwargs["refresh"] is harness.charm.refresh +def test_update_config_clears_stale_standby_when_primary(harness): + """Test update_config clears a stale DCS standby_cluster when this cluster is the primary. + + A force-promote bumps the promoted-cluster-counter but, while the dead-DC relation still + lingers, does not call promote_standby_cluster() — so the reconciler must clear the stale + standby_cluster itself on the next update-config, or the cluster stays a read-only standby + leader (DPE-10203). + """ + with ( + patch.object(harness.charm, "patroni_manager") as _patroni_manager, + patch.object(harness.charm, "config_manager") as _config_manager, + patch.object(harness.charm.async_replication, "get_primary_cluster_endpoint") as _endpoint, + ): + # This cluster is the primary -> no primary endpoint. + _endpoint.return_value = None + _config_manager.update_config.return_value = True + _patroni_manager.member_started = True + + assert harness.charm.update_config() is True + + base_patch = _patroni_manager.bulk_update_parameters_controller_by_patroni.call_args[0][1] + + # standby_cluster must be explicitly cleared (patched to None) so the DCS converges; + # merely omitting it would leave the stale standby from before the promotion in place. + assert base_patch["standby_cluster"] is None + + def test_on_cluster_topology_change(harness): with ( patch( @@ -2916,3 +2946,23 @@ def test_on_secret_remove(harness): event.secret.label = None harness.charm._on_secret_remove(event) assert not event.remove_revision.called + + +def test_planned_units_returns_app_value_normally(harness): + charm = harness.charm + with patch.object(charm.app, "planned_units", return_value=5): + assert charm._planned_units == 5 + + +def test_planned_units_survives_goal_state_failure(harness): + # DPE-10203 Issue B: after a cross-model SAAS is force-removed, the goal-state hook + # command fails ("saas application ... not found"), so app.planned_units() raises + # ModelError. _planned_units must fall back to the current unit count instead of + # crashing the _patroni property (and every hook that touches it). + charm = harness.charm + with patch.object( + charm.app, + "planned_units", + side_effect=ModelError('ERROR saas application "db1" not found'), + ): + assert charm._planned_units == len(charm._hosts)