Skip to content

Commit bf74780

Browse files
committed
fix(backups): always reset the connectivity flag after a replica backup
A replica backup disables external connectivity by writing the unit peer databag "connectivity" flag to "off". The flag was only reset to "on" when `not self.charm.is_primary` still held after the long-running backup run, so a replica→primary failover mid-backup — or any exception or interruption in the backup run — left the flag stuck "off". Because the flag lives in the unit peer databag it survives restarts and upgrades, leaving pg_hba rejecting peer/replica connections indefinitely (the cluster can no longer talk to itself). Latch the disabled-connectivity decision before the backup run and reset it in a finally block, so the reset runs regardless of role changes or exceptions. Also recover deployments already stuck with a stale "off" by resetting it in _on_start and _on_config_changed, guarded by the cluster-wide is_creating_backup signal so a genuinely in-progress backup is not un-hidden mid-run. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
1 parent 9b17fd3 commit bf74780

3 files changed

Lines changed: 117 additions & 6 deletions

File tree

src/backups.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -985,22 +985,28 @@ def _on_create_backup_action(self, event) -> None:
985985
event.fail(error_message)
986986
return
987987

988+
disabled_connectivity = False
988989
if not self.charm.is_primary:
989990
# Create a rule to mark the cluster as in a creating backup state and update
990991
# the Patroni configuration.
991992
self._change_connectivity_to_database(connectivity=False)
993+
disabled_connectivity = True
992994

993995
self.charm.unit.status = MaintenanceStatus("creating backup")
994996
# Set flag due to missing in progress backups on JSON output
995997
# (reference: https://github.com/pgbackrest/pgbackrest/issues/2007)
996998
self.charm.update_config(is_creating_backup=True)
997999

998-
self._run_backup(event, s3_parameters, datetime_backup_requested, backup_type)
999-
1000-
if not self.charm.is_primary:
1001-
# Remove the rule that marks the cluster as in a creating backup state
1002-
# and update the Patroni configuration.
1003-
self._change_connectivity_to_database(connectivity=True)
1000+
# Latch the disabled-connectivity decision before the long-running backup so the
1001+
# reset runs even if this unit is promoted or the action is interrupted — a stale
1002+
# "off" flag leaves pg_hba rejecting peer/replica connections across restarts.
1003+
try:
1004+
self._run_backup(event, s3_parameters, datetime_backup_requested, backup_type)
1005+
finally:
1006+
if disabled_connectivity:
1007+
# Remove the rule that marks the cluster as in a creating backup state
1008+
# and update the Patroni configuration.
1009+
self._change_connectivity_to_database(connectivity=True)
10041010

10051011
self.charm.update_config(is_creating_backup=False)
10061012
self.charm.unit.status = ActiveStatus()

src/charm.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,6 +1008,20 @@ def is_connectivity_enabled(self) -> bool:
10081008
"""Return whether this unit can be connected externally."""
10091009
return self.unit_peer_data.get("connectivity", "on") == "on"
10101010

1011+
def reset_stale_connectivity_flag(self) -> None:
1012+
"""Reset a stale "connectivity: off" left by an interrupted backup.
1013+
1014+
The flag is written to the unit peer databag during replica backups and survives
1015+
restarts/upgrades; without this recovery a unit stuck "off" stays externally
1016+
disconnected (pg_hba rejects peer/replica connections). Skipped while any backup
1017+
is in progress cluster-wide to avoid un-hiding a unit mid-backup.
1018+
"""
1019+
if self._patroni.is_creating_backup:
1020+
return
1021+
if self.unit_peer_data.get("connectivity") == "off":
1022+
logger.info("Resetting stale connectivity flag left by an interrupted backup")
1023+
self.unit_peer_data["connectivity"] = "on"
1024+
10111025
@property
10121026
def is_ldap_charm_related(self) -> bool:
10131027
"""Return whether this unit has an LDAP charm related."""
@@ -1233,6 +1247,9 @@ def _on_config_changed(self, event) -> None:
12331247
logger.debug("Defer on_config_changed: upgrade in progress")
12341248
event.defer()
12351249
return
1250+
# Recover from a stale "connectivity: off" left by an interrupted backup before
1251+
# re-rendering the Patroni configuration below.
1252+
self.reset_stale_connectivity_flag()
12361253
try:
12371254
self._validate_config_options()
12381255
# update config on every run
@@ -1363,6 +1380,10 @@ def _on_start(self, event: StartEvent) -> None:
13631380
if not self._can_start(event):
13641381
return
13651382

1383+
# Recover from a stale "connectivity: off" left by an interrupted backup before
1384+
# the cluster is rendered/started.
1385+
self.reset_stale_connectivity_flag()
1386+
13661387
try:
13671388
postgres_password = self._get_password()
13681389
except ModelError:

tests/unit/test_backups.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,90 @@ def test_change_connectivity_to_database(harness):
640640
_update_config.assert_called_once()
641641

642642

643+
def test_create_backup_resets_connectivity_when_run_backup_raises(harness):
644+
# Regression: if the backup run is interrupted (raises) on a replica, the connectivity
645+
# flag must still be reset to "on" via the finally block — otherwise pg_hba keeps
646+
# rejecting peer/replica connections across restarts/upgrades.
647+
with (
648+
patch("charm.PostgresqlOperatorCharm.update_config"),
649+
patch(
650+
"charm.PostgreSQLBackups._change_connectivity_to_database"
651+
) as _change_connectivity_to_database,
652+
patch(
653+
"charm.PostgreSQLBackups._run_backup", side_effect=RuntimeError("boom")
654+
) as _run_backup,
655+
patch("charm.PostgresqlOperatorCharm.is_primary", new_callable=PropertyMock) as _is_primary,
656+
patch("charm.PostgreSQLBackups._upload_content_to_s3", return_value=True),
657+
patch("backups.datetime"),
658+
patch("ops.JujuVersion.from_environ"),
659+
patch("charm.PostgreSQLBackups._retrieve_s3_parameters") as _retrieve_s3_parameters,
660+
patch("charm.PostgreSQLBackups._can_unit_perform_backup", return_value=(True, None)),
661+
):
662+
# This unit is a replica, so connectivity is disabled before the backup runs.
663+
_is_primary.return_value = False
664+
_retrieve_s3_parameters.return_value = (
665+
{
666+
"bucket": "test-bucket",
667+
"access-key": "test-access-key",
668+
"secret-key": "test-secret-key",
669+
"endpoint": "test-endpoint",
670+
"path": "test-path",
671+
"region": "test-region",
672+
},
673+
[],
674+
)
675+
mock_event = MagicMock()
676+
mock_event.params = {"type": "full"}
677+
678+
# The backup run raises; the reset must still run.
679+
with pytest.raises(RuntimeError):
680+
harness.charm.backup._on_create_backup_action(mock_event)
681+
682+
# Connectivity disabled then re-enabled despite the exception.
683+
_change_connectivity_to_database.assert_has_calls(
684+
[call(connectivity=False), call(connectivity=True)]
685+
)
686+
_run_backup.assert_called_once()
687+
688+
689+
def test_reset_stale_connectivity_flag(harness):
690+
with patch("charm.PostgresqlOperatorCharm._patroni", new_callable=PropertyMock) as _patroni:
691+
peer_rel_id = harness.model.get_relation(PEER).id
692+
693+
# Stale "off" with no backup in progress: reset to "on".
694+
_patroni.return_value.is_creating_backup = False
695+
with harness.hooks_disabled():
696+
harness.update_relation_data(
697+
peer_rel_id, harness.charm.unit.name, {"connectivity": "off"}
698+
)
699+
harness.charm.reset_stale_connectivity_flag()
700+
assert (
701+
harness.get_relation_data(peer_rel_id, harness.charm.unit)["connectivity"] == "on"
702+
)
703+
704+
# While a backup is in progress cluster-wide: do not reset (stay "off").
705+
with harness.hooks_disabled():
706+
harness.update_relation_data(
707+
peer_rel_id, harness.charm.unit.name, {"connectivity": "off"}
708+
)
709+
_patroni.return_value.is_creating_backup = True
710+
harness.charm.reset_stale_connectivity_flag()
711+
assert (
712+
harness.get_relation_data(peer_rel_id, harness.charm.unit)["connectivity"] == "off"
713+
)
714+
715+
# Already "on": no-op.
716+
_patroni.return_value.is_creating_backup = False
717+
with harness.hooks_disabled():
718+
harness.update_relation_data(
719+
peer_rel_id, harness.charm.unit.name, {"connectivity": "on"}
720+
)
721+
harness.charm.reset_stale_connectivity_flag()
722+
assert (
723+
harness.get_relation_data(peer_rel_id, harness.charm.unit)["connectivity"] == "on"
724+
)
725+
726+
643727
def test_execute_command(harness):
644728
with (
645729
patch("backups.run") as _run,

0 commit comments

Comments
 (0)