From 433e166ac55adedf72b65af11b7c0bfff5ec88a3 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 30 Jun 2026 17:31:27 -0300 Subject: [PATCH 01/13] fix(async-replication): own offer secret under a distinct label After a dead-datacenter failover, re-establishing async replication to a new cluster failed on create-replication with "committing requested changes failed" / "secret with label async-replication-secret already exists". The offer/primary and consumer/standby sides shared one fixed Juju secret label. A cluster that had been a standby keeps that label reserved as a consumer alias; Juju does not release it when the dead primary's relation is torn down. When the cluster is later promoted and owns the shared secret, the owner-create reuses the same label and deadlocks: the label is simultaneously unreadable (the aliased remote secret is gone) and uncreatable (the alias still reserves it). Own the secret under a distinct label so an owner-create can never collide with the consumer alias; the consumer keeps reading by secret id, so the handoff is unchanged and a wedged cluster self-heals without a redeploy. Signed-off-by: Marcelo Henrique Neppel --- src/relations/async_replication.py | 14 ++++++- tests/unit/test_async_replication.py | 62 +++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 79d9089a63..072245a5e8 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -69,6 +69,14 @@ READ_ONLY_MODE_BLOCKING_MESSAGE = "Standalone read-only cluster" # Labels are not confidential SECRET_LABEL = "async-replication-secret" # noqa: S105 +# The offer/primary side owns the shared secret under its own label, kept distinct from the +# consumer-side alias (SECRET_LABEL, set when the standby reads the secret by id). A cluster that +# was a standby and later becomes the primary keeps a stale SECRET_LABEL alias that Juju leaves +# reserved even after the remote secret is gone; creating the owned secret under SECRET_LABEL then +# deadlocks ("secret with label async-replication-secret already exists" while the same label is +# unreadable). Owning under a separate label sidesteps the collision and self-heals such a cluster +# without a redeploy (DPE-10203). The consumer keeps SECRET_LABEL as its local alias. +OFFER_SECRET_LABEL = "async-replication-secret-offer" # noqa: S105 if typing.TYPE_CHECKING: from charm import PostgresqlOperatorCharm @@ -340,7 +348,7 @@ def _get_secret(self) -> Secret | None: try: # Avoid recreating the secret. - secret = self.charm.model.get_secret(label=SECRET_LABEL) + secret = self.charm.model.get_secret(label=OFFER_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]}" @@ -353,7 +361,9 @@ def _get_secret(self) -> Secret | None: pass if self.charm.unit.is_leader(): - return self.charm.model.app.add_secret(content=shared_content, label=SECRET_LABEL) + return self.charm.model.app.add_secret( + content=shared_content, label=OFFER_SECRET_LABEL + ) def get_standby_endpoints(self) -> list[str]: """Return the standby endpoints.""" diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index 4c23d7962c..7d17d43686 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -4,12 +4,14 @@ from unittest.mock import MagicMock, PropertyMock, patch import pytest -from ops import Application +from ops import Application, SecretNotFoundError from single_kernel_postgresql.config.literals import REPLICATION_CONSUMER_RELATION from tenacity import RetryError from src.relations.async_replication import ( + OFFER_SECRET_LABEL, READ_ONLY_MODE_BLOCKING_MESSAGE, + SECRET_LABEL, PostgreSQLAsyncReplication, ) @@ -584,3 +586,61 @@ def test_on_async_relation_broken(): relation._on_async_relation_broken(mock_event) assert mock_charm.update_config.called + + +def test_get_secret_creates_owned_secret_under_offer_label(): + # Regression for DPE-10203: the offer/primary side must own the shared secret under a label + # distinct from the consumer alias (SECRET_LABEL). A former standby keeps a stale SECRET_LABEL + # alias that Juju leaves reserved after the remote secret is gone, so owning under SECRET_LABEL + # would deadlock with "secret with label already exists" on the next create-replication. + 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", + } + + # First get_secret: the peer app secret (content source). Second: the offer-label lookup, + # which is absent on a former standby -> triggers creation. + mock_charm.model.get_secret.side_effect = [app_secret, SecretNotFoundError()] + mock_charm.unit.is_leader.return_value = True + + result = relation._get_secret() + + # Owned secret is created under the offer-specific label, never the bare consumer alias. + mock_charm.model.app.add_secret.assert_called_once() + _, kwargs = mock_charm.model.app.add_secret.call_args + assert kwargs["label"] == OFFER_SECRET_LABEL + assert kwargs["label"] != SECRET_LABEL + # Only password fields are shared between clusters. + assert kwargs["content"] == {"operator-password": "op", "replication-password": "rep"} + assert result is mock_charm.model.app.add_secret.return_value + + +def test_get_secret_reuses_existing_offer_secret(): + # When the owned secret already exists under the offer label, reuse it (look it up by the + # offer label) instead of creating a new one; 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] + + result = relation._get_secret() + + mock_charm.model.app.add_secret.assert_not_called() + existing.set_content.assert_not_called() + assert result is existing + assert any( + call.kwargs.get("label") == OFFER_SECRET_LABEL + for call in mock_charm.model.get_secret.call_args_list + ) From 253e3c7c57fe220bf48729f9a71dfd665c05a14b Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 30 Jun 2026 17:34:06 -0300 Subject: [PATCH 02/13] fix(async-replication): clear stale promotion counter after dead-DC teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a dead datacenter is force-removed, Juju may never deliver the relation-broken event for the cross-model relation, so the promoted-cluster -counter set during promotion is left behind. A later, newly-formed async relation then re-counts that stale value and create-replication wrongly fails with "There is already a replication set up", blocking recovery. Reconcile the orphaned counter from the update-status handler, deciding solely from peer data (no Patroni call) because Patroni is frequently unreachable right after a dead-DC promotion — exactly when this must still run. Also harden relation-broken: building the Patroni client can transiently fail during a force-removal (network-get, goal-state, the REST API), which crashed the hook and wedged both units in error forever, so update-status never ran and the cluster never reverted to standalone. Tolerate those failures so the counter is always cleared. Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 5 ++ src/relations/async_replication.py | 61 +++++++++++++++++++++-- tests/unit/test_async_replication.py | 72 ++++++++++++++++++++++++++++ tests/unit/test_charm.py | 3 ++ 4 files changed, 138 insertions(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index 850cfd8541..8b69e1979a 100755 --- a/src/charm.py +++ b/src/charm.py @@ -2375,6 +2375,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() diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 072245a5e8..02c4e0272c 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -557,19 +557,74 @@ def _on_async_relation_broken(self, _) -> None: "unit-promoted-cluster-counter": "", }) + # During a force-removal of a dead offerer the model/Patroni can transiently fail + # (network-get, goal-state, the Patroni REST API). That used to crash this hook and wedge + # BOTH units in error forever: update-status then never runs, so the cluster never reverts + # to standalone and create-replication stays blocked with "already a replication set up" + # (DPE-10203 / Issue B). Tolerate those failures so the counter is always cleared. A + # non-empty promoted-cluster-counter is only ever set on a promoted cluster, so if the + # standby check is unavailable, treating this as a primary (clearing the counter) is the + # safe default. + 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 Exception 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 Exception 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. + + `_on_async_relation_broken` normally clears this, but a force-removed dead offerer may + never deliver `relation-broken` (a Juju cross-model teardown limitation). With no async + relation the counter is ignored by `_get_primary_cluster`, yet it would wrongly re-mark + this app as the primary cluster once a *new* async relation is formed — blocking + `create-replication` with "There is already a replication set up." Run from the + update-status reconciler so the surviving primary converges back to standalone (DPE-10203). + """ + if not self.charm.unit.is_leader(): + return + # Only act when there is no async relation; otherwise the counter is managed normally. + if self._relation is not None: + 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 with no async relation + # means this cluster was promoted and the relation went away without relation-broken + # finishing the teardown (dead-DC force-removal) -> revert it to a standalone primary. + # Deciding this from 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. + # A non-empty, non-"0" counter is only ever set by a promotion, so it is unambiguous. + if not counter or counter == "0": + return + logger.info( + "Clearing stale promoted-cluster-counter %s (no async relation present)", 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.""" diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index 7d17d43686..1821fd22f3 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -587,6 +587,78 @@ def test_on_async_relation_broken(): 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"} + relation = PostgreSQLAsyncReplication(mock_charm) + with patch.object( + PostgreSQLAsyncReplication, "_relation", new_callable=PropertyMock, return_value=None + ): + relation.clear_stale_promotion() + assert mock_charm.app_peer_data.get("promoted-cluster-counter") == "" + mock_charm.update_config.assert_called_once() + + # An async relation exists -> no-op (the counter is managed by the relation lifecycle). + mock_charm = MagicMock() + mock_charm.unit.is_leader.return_value = True + mock_charm._patroni.get_standby_leader.return_value = None + mock_charm.app_peer_data = {"promoted-cluster-counter": "2"} + relation = PostgreSQLAsyncReplication(mock_charm) + with patch.object( + PostgreSQLAsyncReplication, + "_relation", + new_callable=PropertyMock, + return_value=MagicMock(), + ): + 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) + with patch.object( + PostgreSQLAsyncReplication, "_relation", new_callable=PropertyMock, return_value=None + ): + 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) + with patch.object( + PostgreSQLAsyncReplication, "_relation", new_callable=PropertyMock, return_value=None + ): + 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_owned_secret_under_offer_label(): # Regression for DPE-10203: the offer/primary side must own the shared secret under a label diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 1a15bdce8c..4058da850a 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() From 9860645d87c23e3ff43b49bb36088bef2cbbb4b3 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 1 Jul 2026 22:52:09 -0300 Subject: [PATCH 03/13] fix(async-replication): survive an unreadable dying async relation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a dead-DC teardown (the primary datacenter force-killed and its offer cleared with remove-saas --force), the local cross-model replication relation lingers in a dying state whose databags return "permission denied" on every relation-get — yet get_relation still returns it and its active flag can still read True. The charm read that relation's data unconditionally in _get_primary_cluster (the remote app databag) and in _relation (used by every write path), so the unhandled ModelError crashed the charm in __init__ on every hook. All hooks then failed before update-status could run clear_stale_promotion, so a promoted primary never recovered and DPE-10203's create-replication could not proceed. Guard both reads: _get_primary_cluster skips a peer whose databag raises ModelError, and _relation probes a cheap own-unit read and treats an unreadable relation as absent so a promoted primary reconciles as a standalone cluster. Unit tests cover both guards. Signed-off-by: Marcelo Henrique Neppel --- src/relations/async_replication.py | 35 ++++++++++++-- tests/unit/test_async_replication.py | 69 +++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 5 deletions(-) diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 02c4e0272c..bb182bb3e1 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -32,6 +32,7 @@ Application, BlockedStatus, MaintenanceStatus, + ModelError, Object, Relation, RelationChangedEvent, @@ -319,7 +320,18 @@ 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 dead-DC teardown leaves the remote app's databag on the dying + # cross-model async relation unreadable — `relation-get --app ` + # returns "permission denied" once the offering DC is gone. Skip that + # peer so status reconciliation (and the update-status + # clear_stale_promotion that unblocks recovery) still runs instead of + # crashing every hook (DPE-10203). + continue if int(relation_promoted_cluster_counter) > int(promoted_cluster_counter): promoted_cluster_counter = relation_promoted_cluster_counter primary_cluster = app @@ -815,13 +827,28 @@ 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. During a + dead-DC teardown the cross-model async relation lingers in a dying state + whose ``relation-get`` returns "permission denied" on every databag (yet + ``active`` can still read True and ``get_relation`` still returns it), so a + promoted primary must reconcile as a standalone cluster instead of + crashing every hook on the unreadable relation (DPE-10203). A cheap + own-unit read probes for that state. + """ 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.""" diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index 1821fd22f3..9db7fbcd59 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -4,7 +4,7 @@ from unittest.mock import MagicMock, PropertyMock, patch import pytest -from ops import Application, SecretNotFoundError +from ops import Application, ModelError, SecretNotFoundError from single_kernel_postgresql.config.literals import REPLICATION_CONSUMER_RELATION from tenacity import RetryError @@ -15,6 +15,12 @@ PostgreSQLAsyncReplication, ) +# Several tests (e.g. ``test_on_create_replication``) reassign ``_relation`` on the class +# via ``type(relation)._relation = PropertyMock(...)`` with no cleanup, leaking a mock over +# the real property for later tests. Capture the real property once, before any test runs, +# so a test that needs to exercise the real ``_relation`` can restore it for its own scope. +_REAL_RELATION_PROPERTY = PostgreSQLAsyncReplication.__dict__["_relation"] + def create_mock_unit(name="unit"): unit = MagicMock() @@ -716,3 +722,64 @@ def test_get_secret_reuses_existing_offer_secret(): call.kwargs.get("label") == OFFER_SECRET_LABEL for call in mock_charm.model.get_secret.call_args_list ) + + +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. + # Restore the real property for this test's scope (an earlier test may have + # leaked a class-level PropertyMock over it); monkeypatch reverts it afterwards. + monkeypatch.setattr(PostgreSQLAsyncReplication, "_relation", _REAL_RELATION_PROPERTY) + 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") From 857b492f3c8e63dca11277c556235ffbf30c6e69 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 21 Aug 2026 11:46:58 -0300 Subject: [PATCH 04/13] fix(async-replication): clear stale counter even after a new relation forms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dead-DC recovery sequence (DPE-10203) offers the promoted cluster to a fresh cluster BEFORE running create-replication. clear_stale_promotion refused to reconcile the orphaned promoted-cluster-counter whenever ANY async relation existed, so once that new offer relation formed — the relation the old, force-removed one never delivered relation-broken for — the counter stayed positive in the peers app databag. create-replication then failed with "There is already a replication set up." on every retry: the cluster's own counter makes _get_primary_cluster report itself as the primary. A promotion writes the counter to both the async relation it was promoted under and the peers databag, so "live" is precisely "some current async relation mirrors the counter". Reconcile on that instead of mere relation existence: a relation formed after the promotion carries no mirror and the counter is stale exactly then. Verified on a live 3-model deployment of the ticket scenario (first Testflinger run since the PS7 outage): phases 1-2 passed, phase 3 wedged on this counter; with the reconciler fixed the counter clears from update-status and create-replication proceeds. Signed-off-by: Marcelo Henrique Neppel --- src/relations/async_replication.py | 36 ++++++++++++++++------ tests/unit/test_async_replication.py | 46 +++++++++++++++------------- 2 files changed, 51 insertions(+), 31 deletions(-) diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index bb182bb3e1..e114e3a0e0 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -619,21 +619,37 @@ def clear_stale_promotion(self) -> None: """ if not self.charm.unit.is_leader(): return - # Only act when there is no async relation; otherwise the counter is managed normally. - if self._relation is not None: - 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 with no async relation - # means this cluster was promoted and the relation went away without relation-broken - # finishing the teardown (dead-DC force-removal) -> revert it to a standalone primary. - # Deciding this from 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. - # A non-empty, non-"0" counter is only ever set by a promotion, so it is unambiguous. + # (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 async relation present)", counter + "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() diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index 9db7fbcd59..8407ba1d76 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -617,27 +617,37 @@ def test_clear_stale_promotion(): 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) - with patch.object( - PostgreSQLAsyncReplication, "_relation", new_callable=PropertyMock, return_value=None - ): - relation.clear_stale_promotion() + relation.clear_stale_promotion() assert mock_charm.app_peer_data.get("promoted-cluster-counter") == "" mock_charm.update_config.assert_called_once() - # An async relation exists -> no-op (the counter is managed by the relation lifecycle). + # 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._patroni.get_standby_leader.return_value = None 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) - with patch.object( - PostgreSQLAsyncReplication, - "_relation", - new_callable=PropertyMock, - return_value=MagicMock(), - ): - relation.clear_stale_promotion() + 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() @@ -646,10 +656,7 @@ def test_clear_stale_promotion(): mock_charm.unit.is_leader.return_value = False mock_charm.app_peer_data = {"promoted-cluster-counter": "2"} relation = PostgreSQLAsyncReplication(mock_charm) - with patch.object( - PostgreSQLAsyncReplication, "_relation", new_callable=PropertyMock, return_value=None - ): - relation.clear_stale_promotion() + 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. @@ -657,10 +664,7 @@ def test_clear_stale_promotion(): mock_charm.unit.is_leader.return_value = True mock_charm.app_peer_data = {"promoted-cluster-counter": "0"} relation = PostgreSQLAsyncReplication(mock_charm) - with patch.object( - PostgreSQLAsyncReplication, "_relation", new_callable=PropertyMock, return_value=None - ): - relation.clear_stale_promotion() + 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() From 0c600996261079f9f967ba91ae6ce092615d674c Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 2 Jul 2026 12:19:41 -0300 Subject: [PATCH 05/13] fix(async-replication): reference offer secret by id, not label, on the consumer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The standby/consumer side read the shared offer secret with a fixed Juju label, which registers a consumer-side alias under that label. Juju keeps the alias reserved even after the remote secret is force-removed during a dead-DC teardown, so a former standby is left carrying a stale alias it can neither read nor release. The owner-side OFFER_SECRET_LABEL split already breaks the promotion deadlock, but the consumer alias remains an avoidable latent hazard whose safe reuse depends on unverified Juju relabel behaviour. Referencing the secret purely by the id already published in the relation databag — as the MySQL async-replication charm does — means no consumer alias is ever registered and nothing can go stale, closing the DPE-10203 failure class on the consumer side too. This is backward compatible: the id is already present in primary-cluster-data and no peer depends on the alias existing. Signed-off-by: Marcelo Henrique Neppel --- src/relations/async_replication.py | 58 +++++++---- tests/unit/test_async_replication.py | 140 +++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 18 deletions(-) diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index e114e3a0e0..54c0f5a292 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -68,17 +68,30 @@ READ_ONLY_MODE_BLOCKING_MESSAGE = "Standalone read-only cluster" -# Labels are not confidential +# Labels are not confidential. +# The offer/primary side owns the shared cluster-credentials secret under OFFER_SECRET_LABEL and +# publishes its id in the relation databag. The consumer/standby side references that secret purely +# by id (see ``_update_internal_secret``) and never attaches a label, so no consumer-side alias is +# registered. This matters after a dead-DC failover: Juju leaves a consumer alias reserved even +# once the remote secret is gone, so a former standby that is later promoted would then deadlock +# creating its own secret ("secret with label ... already exists" while the label is unreadable) — +# DPE-10203. SECRET_LABEL is the legacy shared label; it is no longer attached by this charm and is +# kept only to assert we never reintroduce it. SECRET_LABEL = "async-replication-secret" # noqa: S105 -# The offer/primary side owns the shared secret under its own label, kept distinct from the -# consumer-side alias (SECRET_LABEL, set when the standby reads the secret by id). A cluster that -# was a standby and later becomes the primary keeps a stale SECRET_LABEL alias that Juju leaves -# reserved even after the remote secret is gone; creating the owned secret under SECRET_LABEL then -# deadlocks ("secret with label async-replication-secret already exists" while the same label is -# unreadable). Owning under a separate label sidesteps the collision and self-heals such a cluster -# without a redeploy (DPE-10203). The consumer keeps SECRET_LABEL as its local alias. OFFER_SECRET_LABEL = "async-replication-secret-offer" # noqa: S105 + +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] + + if typing.TYPE_CHECKING: from charm import PostgresqlOperatorCharm @@ -787,7 +800,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") @@ -943,17 +958,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 8407ba1d76..685210ef76 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -1,6 +1,7 @@ # Copyright 2025 Canonical Ltd. # See LICENSE file for licensing details. +import json from unittest.mock import MagicMock, PropertyMock, patch import pytest @@ -13,6 +14,7 @@ READ_ONLY_MODE_BLOCKING_MESSAGE, SECRET_LABEL, PostgreSQLAsyncReplication, + _same_secret_id, ) # Several tests (e.g. ``test_on_create_replication``) reassign ``_relation`` on the class @@ -787,3 +789,141 @@ def test__relation_skips_unreadable_dying_relation(monkeypatch): # 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=SECRET_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 = SECRET_LABEL # a legacy label must NOT trigger the sync anymore + + 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() From 9356bf3dd6d56274fa44811f1b0bb00f835c73ed Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 2 Jul 2026 21:26:34 -0300 Subject: [PATCH 06/13] fix(async-replication): survive an unreadable dead peer in all relation reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a dead-DC teardown the remote app/unit databag on the dying cross-model async relation raises ModelError ("permission denied") on any read. Only _get_primary_cluster was guarded; the sibling reads were not, so they crashed the hooks that keep a promoted cluster reconciling — replication-offer-relation-joined and database-peers-relation-changed both died, leaving units wedged in error and blocking re-replication to a fresh cluster. Route every async-relation databag read through a shared _safe_databag_get helper (and a shared _remote_unit_addresses for the two identical endpoint list comprehensions) that treats an unreadable databag as key-absent, matching the existing _get_primary_cluster behaviour, so the teardown always reconciles instead of crashing. Signed-off-by: Marcelo Henrique Neppel --- src/relations/async_replication.py | 81 +++++++++++++++++++--------- tests/unit/test_async_replication.py | 77 ++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 25 deletions(-) diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 54c0f5a292..ecc2bcc768 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 @@ -92,6 +93,21 @@ def _same_secret_id(a: str | None, b: str | None) -> bool: 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. + + After a dead-DC teardown the remote app/unit databag on the dying cross-model async + relation raises ModelError ("permission denied") on any read; callers must behave as if + the key is unset rather than crash the hook (DPE-10203). + """ + try: + return databag.get(key, default) + except ModelError: + return default + + if typing.TYPE_CHECKING: from charm import PostgresqlOperatorCharm @@ -204,7 +220,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 ): @@ -248,7 +264,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" @@ -267,16 +283,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.""" @@ -291,7 +298,17 @@ 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 dead-DC teardown leaves the remote app's databag on the dying + # cross-model async relation unreadable — `relation-get --app ` + # returns "permission denied" once the offering DC is gone. Skip that + # peer instead of crashing replication-offer-relation-joined on the + # promoted cluster, which would block re-replication (DPE-10203). + continue if int(relation_promoted_cluster_counter) > int(promoted_cluster_counter): promoted_cluster_counter = relation_promoted_cluster_counter return promoted_cluster_counter @@ -356,7 +373,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") @@ -399,16 +420,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.""" @@ -538,7 +569,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." ) diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index 685210ef76..7ce5aec191 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -14,6 +14,7 @@ READ_ONLY_MODE_BLOCKING_MESSAGE, SECRET_LABEL, PostgreSQLAsyncReplication, + _safe_databag_get, _same_secret_id, ) @@ -927,3 +928,79 @@ def test_on_secret_changed_consumer_defers_when_secret_not_ready(): 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"] From 2994702a169576e0c3e89f0bf1626df26e995d95 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 2 Jul 2026 21:58:51 -0300 Subject: [PATCH 07/13] fix: survive a transient goal-state failure in planned-units reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ops implements Application.planned_units() via the goal-state hook command, which fails ("saas application ... not found") while a cross-model SAAS force-removed during a dead-DC teardown still lingers in goal-state. Every caller — the _patroni property construction, the degraded-status check, the synchronous-node count and the backups pre-checks — crashed its hook, cascading through the _patroni property to wedge the promoted cluster and block re-replication to a fresh cluster. Route them through a single guarded _planned_units property that falls back to the current unit count when goal-state is unavailable, so hooks reconcile instead of crashing (DPE-10203 Issue B). Signed-off-by: Marcelo Henrique Neppel --- src/backups.py | 4 ++-- src/charm.py | 20 ++++++++++++++++---- tests/unit/test_charm.py | 20 ++++++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) 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 8b69e1979a..e2ebbc6614 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.""" @@ -2545,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}")) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 4058da850a..6aed1ac928 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -2919,3 +2919,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) From 9efa9a22fc0ba33062ffd07c24f44fd43492dcf5 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 21 Aug 2026 17:21:30 -0300 Subject: [PATCH 08/13] fix(async-replication): own the shared secret labelless, by persisted id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DPE-10203 rework of the owner-side secret handling: neither side of the async-replication relation attaches any label to the shared cluster-credentials secret. The owner creates it labelless and re-finds it by the id persisted in app peer data; the consumer already references it purely by the id published in the relation databag. This replaces the distinct-offer-label workaround. Labels were the collision surface in both deadlock directions Juju can produce: a stale consumer alias blocks a later owner-create ('secret with label already exists' while the label is unreadable), and an owner label cannot survive the secret's own id churn — a refreshed owner that misses its label lookup mints a second secret, and Juju refuses to rebind a consumer label to the new id, wedging any consumer still running label-attaching code. A cluster refreshed from the legacy charm has no id in peer data but its own offer-relation data still publishes the last-known secret id; the owner adopts that secret instead of creating a second one, so the id never changes across the refresh and old consumers keep working. Also re-applies the stale DCS standby_cluster clearing that the 16/edge single-kernel update_config adoption (#1849) had displaced: update_config now patches standby_cluster to None when this cluster is the primary, so a force-promoted cluster converges out of read-only standby-leader state on the next reconciliation. Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 20 +++++- src/relations/async_replication.py | 85 ++++++++++++++++-------- tests/unit/test_async_replication.py | 96 ++++++++++++++++++++-------- tests/unit/test_charm.py | 27 ++++++++ 4 files changed, 172 insertions(+), 56 deletions(-) diff --git a/src/charm.py b/src/charm.py index e2ebbc6614..c55914b469 100755 --- a/src/charm.py +++ b/src/charm.py @@ -2788,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 @@ -2802,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 ecc2bcc768..9d6904e099 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -69,17 +69,16 @@ READ_ONLY_MODE_BLOCKING_MESSAGE = "Standalone read-only cluster" -# Labels are not confidential. -# The offer/primary side owns the shared cluster-credentials secret under OFFER_SECRET_LABEL and -# publishes its id in the relation databag. The consumer/standby side references that secret purely -# by id (see ``_update_internal_secret``) and never attaches a label, so no consumer-side alias is -# registered. This matters after a dead-DC failover: Juju leaves a consumer alias reserved even -# once the remote secret is gone, so a former standby that is later promoted would then deadlock -# creating its own secret ("secret with label ... already exists" while the label is unreadable) — -# DPE-10203. SECRET_LABEL is the legacy shared label; it is no longer attached by this charm and is -# kept only to assert we never reintroduce it. -SECRET_LABEL = "async-replication-secret" # noqa: S105 -OFFER_SECRET_LABEL = "async-replication-secret-offer" # noqa: S105 +# Neither side of the async-replication relation attaches any label to the shared +# cluster-credentials secret (DPE-10203): the owner creates it labelless and references it by the +# id persisted in app peer data (``_get_secret``); the consumer references it purely by the id +# published in the relation databag (``_update_internal_secret``). Juju reserves labels even +# after the secret they pointed at is gone — a stale consumer alias deadlocks a later +# owner-create ("secret with label already exists" while the label is unreadable), and a stale +# owner label makes a refreshed owner mint a second secret whose id switch wedges any consumer +# still running label-attaching code. The legacy labels "async-replication-secret" and +# "async-replication-secret-offer" are intentionally not defined anywhere: this charm must +# never attach either again. def _same_secret_id(a: str | None, b: str | None) -> bool: @@ -392,24 +391,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=OFFER_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-replication-secret-id") + 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-replication-secret-id": secret.id}) + return secret if self.charm.unit.is_leader(): - return self.charm.model.app.add_secret( - content=shared_content, label=OFFER_SECRET_LABEL - ) + secret = self.charm.model.app.add_secret(content=shared_content) + self.charm.app_peer_data.update({"async-replication-secret-id": 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.""" diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index 7ce5aec191..a6e9ae43cc 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -5,14 +5,15 @@ from unittest.mock import MagicMock, PropertyMock, patch import pytest -from ops import Application, ModelError, SecretNotFoundError -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 ( - OFFER_SECRET_LABEL, READ_ONLY_MODE_BLOCKING_MESSAGE, - SECRET_LABEL, PostgreSQLAsyncReplication, _safe_databag_get, _same_secret_id, @@ -673,11 +674,11 @@ def test_clear_stale_promotion(): mock_charm._patroni.get_standby_leader.assert_not_called() -def test_get_secret_creates_owned_secret_under_offer_label(): - # Regression for DPE-10203: the offer/primary side must own the shared secret under a label - # distinct from the consumer alias (SECRET_LABEL). A former standby keeps a stale SECRET_LABEL - # alias that Juju leaves reserved after the remote secret is gone, so owning under SECRET_LABEL - # would deadlock with "secret with label already exists" on the next create-replication. +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) @@ -687,27 +688,67 @@ def test_get_secret_creates_owned_secret_under_offer_label(): "replication-password": "rep", "system-id": "x", } - - # First get_secret: the peer app secret (content source). Second: the offer-label lookup, - # which is absent on a former standby -> triggers creation. - mock_charm.model.get_secret.side_effect = [app_secret, SecretNotFoundError()] + 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() - # Owned secret is created under the offer-specific label, never the bare consumer alias. + # 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 kwargs["label"] == OFFER_SECRET_LABEL - assert kwargs["label"] != SECRET_LABEL - # Only password fields are shared between clusters. + assert "label" not in kwargs assert kwargs["content"] == {"operator-password": "op", "replication-password": "rep"} - assert result is mock_charm.model.app.add_secret.return_value + # 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_reuses_existing_offer_secret(): - # When the owned secret already exists under the offer label, reuse it (look it up by the - # offer label) instead of creating a new one; only rewrite content when it drifts. +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) @@ -719,16 +760,17 @@ def test_get_secret_reuses_existing_offer_secret(): 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 - assert any( - call.kwargs.get("label") == OFFER_SECRET_LABEL - for call in mock_charm.model.get_secret.call_args_list - ) + # 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(): @@ -793,7 +835,7 @@ def test__relation_skips_unreadable_dying_relation(monkeypatch): # --- 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=SECRET_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. @@ -889,7 +931,7 @@ def test_on_secret_changed_consumer_ignores_unrelated_secret(): mock_event = MagicMock() mock_event.secret.id = "secret://uuid/DIFFERENT" - mock_event.secret.label = SECRET_LABEL # a legacy label must NOT trigger the sync anymore + mock_event.secret.label = "async-replication-secret" # legacy label must NOT trigger the sync with ( patch.object( diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 6aed1ac928..a4e13a050f 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1454,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( From bf91621bbc962681f06d3435ecdcd7831c1ea9b4 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Mon, 24 Aug 2026 17:24:33 -0300 Subject: [PATCH 09/13] refactor(async-replication): drop the labelless-design comment block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The no-label invariant and the failure history it explained belong to the commit messages and the PR description (DPE-10203); restating them at the module head duplicates that record where it goes stale as the code evolves. The code itself — no label constant defined, secret access strictly by id — already expresses the invariant. Signed-off-by: Marcelo Henrique Neppel --- src/relations/async_replication.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 9d6904e099..60a2f43d49 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -69,16 +69,6 @@ READ_ONLY_MODE_BLOCKING_MESSAGE = "Standalone read-only cluster" -# Neither side of the async-replication relation attaches any label to the shared -# cluster-credentials secret (DPE-10203): the owner creates it labelless and references it by the -# id persisted in app peer data (``_get_secret``); the consumer references it purely by the id -# published in the relation databag (``_update_internal_secret``). Juju reserves labels even -# after the secret they pointed at is gone — a stale consumer alias deadlocks a later -# owner-create ("secret with label already exists" while the label is unreadable), and a stale -# owner label makes a refreshed owner mint a second secret whose id switch wedges any consumer -# still running label-attaching code. The legacy labels "async-replication-secret" and -# "async-replication-secret-offer" are intentionally not defined anywhere: this charm must -# never attach either again. def _same_secret_id(a: str | None, b: str | None) -> bool: From 336a4d6373aedea2057bcfa8fc5a86ecb92ed5cc Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 26 Aug 2026 17:03:52 -0300 Subject: [PATCH 10/13] refactor(tests): scope _relation mocks to the tests that use them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three sites in test_on_create_replication wrote the PropertyMock onto the class (type(relation)._relation = ...) without cleanup, leaking it to every later test; two more tests then silently RELIED on that leak — their instance writes only worked through the leaked mock's __set__, so both fail with AttributeError when run standalone. Six further instance writes were silent no-ops against whichever mock happened to be leaked, their assertions passing by coincidence of branch equivalence. Convert every site to scoped patch.object contexts: cleanup is automatic, order-dependence is gone, previously-red solo runs pass, and _REAL_RELATION_PROPERTY capture/restore machinery is deleted. The leaked .app PropertyMocks in test_promote_to_primary covered a path production never reads and are dropped entirely. Verified: full file green, and solo runs of test__configure_standby_ cluster / test_handle_forceful_promotion / the dying-relation probe now pass in isolation. Signed-off-by: Marcelo Henrique Neppel --- tests/unit/test_async_replication.py | 128 +++++++++++++++++---------- 1 file changed, 81 insertions(+), 47 deletions(-) diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index a6e9ae43cc..7dcbabe42d 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -19,12 +19,6 @@ _same_secret_id, ) -# Several tests (e.g. ``test_on_create_replication``) reassign ``_relation`` on the class -# via ``type(relation)._relation = PropertyMock(...)`` with no cleanup, leaking a mock over -# the real property for later tests. Capture the real property once, before any test runs, -# so a test that needs to exercise the real ``_relation`` can restore it for its own scope. -_REAL_RELATION_PROPERTY = PostgreSQLAsyncReplication.__dict__["_relation"] - def create_mock_unit(name="unit"): unit = MagicMock() @@ -176,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( @@ -196,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 @@ -214,9 +216,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 @@ -232,7 +238,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 @@ -244,14 +249,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) @@ -264,11 +267,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 @@ -279,12 +288,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" @@ -294,17 +311,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 @@ -518,16 +543,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." @@ -540,16 +569,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. @@ -559,16 +592,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 @@ -813,9 +850,6 @@ def test__relation_skips_unreadable_dying_relation(monkeypatch): # 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. - # Restore the real property for this test's scope (an earlier test may have - # leaked a class-level PropertyMock over it); monkeypatch reverts it afterwards. - monkeypatch.setattr(PostgreSQLAsyncReplication, "_relation", _REAL_RELATION_PROPERTY) mock_charm = MagicMock() dying = MagicMock() From cb1c654efbafdb3b719298d8508a413db47b82f5 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 1 Sep 2026 17:05:55 -0300 Subject: [PATCH 11/13] fix(async-replication): clear stale promotion counter in the actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create-replication and promote-to-primary read _get_primary_cluster, which counts an orphaned promoted-cluster-counter left in peer data by a dead-DC teardown whose relation-broken never fired. With the only reconciler in update-status, the action fails with "There is already a replication set up." (or promote-to-primary wrongly sees a primary) until an update-status cycle happens to run — which the dead-DC recovery window cannot rely on. Both actions now clear the stale counter before their guard; the mirror check inside clear_stale_promotion keeps live replications untouched. The update- status reconciler stays (idempotent, cheap, and covers paths that don't go through the actions). The dead-DC integration test drops its 10-minute retry loop, whose only purpose was absorbing that update- status latency. Signed-off-by: Marcelo Henrique Neppel --- src/relations/async_replication.py | 8 +++++ tests/unit/test_async_replication.py | 53 ++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 60a2f43d49..c83b7f4b38 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -792,6 +792,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 @@ -811,6 +816,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 diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index 7dcbabe42d..8a9ccff4d9 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -226,6 +226,59 @@ def test_on_create_replication(): 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(): # 1. From 77277502a7f69d05c455ff292571b4db3766530e Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 1 Sep 2026 17:13:53 -0300 Subject: [PATCH 12/13] refactor(async-replication): extract the secret-id peer-data key to a constant The async-replication-secret-id peer-data key is read in one place and written in two; a module constant keeps the three references in sync with a single definition. Signed-off-by: Marcelo Henrique Neppel --- src/relations/async_replication.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index c83b7f4b38..f3f665fbf0 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -70,6 +70,10 @@ READ_ONLY_MODE_BLOCKING_MESSAGE = "Standalone read-only cluster" +# 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. @@ -385,7 +389,7 @@ def _get_secret(self) -> Secret | None: # 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-replication-secret-id") + 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 @@ -404,12 +408,12 @@ def _get_secret(self) -> Secret | None: 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-replication-secret-id": secret.id}) + self.charm.app_peer_data.update({ASYNC_SHARED_SECRET_ID_KEY: secret.id}) return secret if self.charm.unit.is_leader(): secret = self.charm.model.app.add_secret(content=shared_content) - self.charm.app_peer_data.update({"async-replication-secret-id": secret.id}) + self.charm.app_peer_data.update({ASYNC_SHARED_SECRET_ID_KEY: secret.id}) return secret def _own_published_secret_id(self) -> str | None: From 7b71cc8235e6aa6e2e228271c51ade53463111b2 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 1 Sep 2026 17:29:18 -0300 Subject: [PATCH 13/13] fix(async-replication): follow repo comment style and narrow exception types Condense the multi-paragraph dead-DC comments to the repo's short single-purpose comments, trim the regression test module docstring to the same shape, and narrow two of the teardown exception handlers to their concrete failure types (DeployedWithoutTrustError, RetryError, ModelError). The get_standby_leader handler keeps Exception: it probes across three layers whose failure modes cannot be enumerated here, and a crash would defeat the counter-clear reconciliation. Signed-off-by: Marcelo Henrique Neppel --- src/relations/async_replication.py | 55 ++++++++++-------------------- 1 file changed, 18 insertions(+), 37 deletions(-) diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index f3f665fbf0..cf111f91c4 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -45,6 +45,7 @@ ) from single_kernel_postgresql.config.exceptions import ( ClusterNotPromotedError, + DeployedWithoutTrustError, NotReadyError, StandbyClusterAlreadyPromotedError, ) @@ -91,9 +92,8 @@ def _safe_databag_get( ) -> str | None: """Read a relation databag key, treating an unreadable databag as key-absent. - After a dead-DC teardown the remote app/unit databag on the dying cross-model async - relation raises ModelError ("permission denied") on any read; callers must behave as if - the key is unset rather than crash the hook (DPE-10203). + 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) @@ -296,11 +296,8 @@ def _get_highest_promoted_cluster_counter_value(self) -> str: "promoted-cluster-counter", "0" ) except ModelError: - # A dead-DC teardown leaves the remote app's databag on the dying - # cross-model async relation unreadable — `relation-get --app ` - # returns "permission denied" once the offering DC is gone. Skip that - # peer instead of crashing replication-offer-relation-joined on the - # promoted cluster, which would block re-replication (DPE-10203). + # 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 @@ -348,12 +345,8 @@ def _get_primary_cluster(self) -> Application | None: "promoted-cluster-counter", "0" ) except ModelError: - # A dead-DC teardown leaves the remote app's databag on the dying - # cross-model async relation unreadable — `relation-get --app ` - # returns "permission denied" once the offering DC is gone. Skip that - # peer so status reconciliation (and the update-status - # clear_stale_promotion that unblocks recovery) still runs instead of - # crashing every hook (DPE-10203). + # 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 @@ -638,14 +631,9 @@ def _on_async_relation_broken(self, _) -> None: "unit-promoted-cluster-counter": "", }) - # During a force-removal of a dead offerer the model/Patroni can transiently fail - # (network-get, goal-state, the Patroni REST API). That used to crash this hook and wedge - # BOTH units in error forever: update-status then never runs, so the cluster never reverts - # to standalone and create-replication stays blocked with "already a replication set up" - # (DPE-10203 / Issue B). Tolerate those failures so the counter is always cleared. A - # non-empty promoted-cluster-counter is only ever set on a promoted cluster, so if the - # standby check is unavailable, treating this as a primary (clearing the counter) is the - # safe default. + # 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: @@ -665,13 +653,13 @@ def _on_async_relation_broken(self, _) -> None: self.charm.app_peer_data.update({"promoted-cluster-counter": ""}) try: self.charm.update_config() - except Exception as e: + except (DeployedWithoutTrustError, RetryError, ModelError) as e: logger.warning("update_config failed during teardown (continuing): %s", e) if self.charm.unit.is_leader(): try: self.charm.watcher_offer.update_endpoints() - except Exception as e: + except (ModelError, RetryError) as e: logger.warning( "watcher endpoint update failed during teardown (continuing): %s", e ) @@ -679,12 +667,9 @@ def _on_async_relation_broken(self, _) -> None: def clear_stale_promotion(self) -> None: """Clear a promoted-cluster-counter left over from a removed async relation. - `_on_async_relation_broken` normally clears this, but a force-removed dead offerer may - never deliver `relation-broken` (a Juju cross-model teardown limitation). With no async - relation the counter is ignored by `_get_primary_cluster`, yet it would wrongly re-mark - this app as the primary cluster once a *new* async relation is formed — blocking - `create-replication` with "There is already a replication set up." Run from the - update-status reconciler so the surviving primary converges back to standalone (DPE-10203). + 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 @@ -924,13 +909,9 @@ def _reinitialise_pgdata(self) -> None: def _relation(self) -> Relation | None: """Return the usable async-replication relation, or None. - A relation whose databags are unreadable is treated as absent. During a - dead-DC teardown the cross-model async relation lingers in a dying state - whose ``relation-get`` returns "permission denied" on every databag (yet - ``active`` can still read True and ``get_relation`` still returns it), so a - promoted primary must reconcile as a standalone cluster instead of - crashing every hook on the unreadable relation (DPE-10203). A cheap - own-unit read probes for that state. + 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),