Skip to content

Commit f2575e7

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" after the backup run via a fall-through `if disabled_connectivity:`, reachable only on the success and ExecError paths. A different exception in the backup run — or an action interruption/restart — 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). Move the reset into a `finally` block so it runs regardless of which exception (or none) the backup run raises. Also recover deployments already stuck with a stale "off" by resetting it in _on_postgresql_pebble_ready and _on_config_changed, guarded by the cluster-wide is_creating_backup signal (via patroni_manager) so a genuinely in-progress backup is not un-hidden mid-run. The guard short-circuits on the cheap databag check first, so it adds no Patroni call on a healthy cluster. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
1 parent 750a342 commit f2575e7

3 files changed

Lines changed: 113 additions & 5 deletions

File tree

src/backups.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -906,11 +906,13 @@ def _on_create_backup_action(self, event) -> None:
906906
else:
907907
logger.info(f"Backup succeeded: with backup-id {datetime_backup_requested}")
908908
event.set_results({"backup-status": "backup created"})
909-
910-
if disabled_connectivity:
911-
# Remove the rule that marks the cluster as in a creating backup state
912-
# and update the Patroni configuration.
913-
self._change_connectivity_to_database(connectivity=True)
909+
finally:
910+
# Reset connectivity even if the backup raised or was interrupted — a stale
911+
# "off" leaves pg_hba rejecting peer/replica connections across restarts.
912+
if disabled_connectivity:
913+
# Remove the rule that marks the cluster as in a creating backup state
914+
# and update the Patroni configuration.
915+
self._change_connectivity_to_database(connectivity=True)
914916

915917
self.charm.update_config(is_creating_backup=False)
916918
self.charm.set_unit_status(ActiveStatus())

src/charm.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,9 @@ def _on_config_changed(self, event) -> None:
893893
event.defer()
894894
return
895895

896+
# Recover from a stale "connectivity: off" left by an interrupted backup before
897+
# re-rendering the Patroni configuration below.
898+
self.reset_stale_connectivity_flag()
896899
try:
897900
self._validate_config_options()
898901
# update config on every run
@@ -1416,6 +1419,10 @@ def _on_postgresql_pebble_ready(self, event: WorkloadEvent) -> None:
14161419
event.defer()
14171420
return
14181421

1422+
# Recover from a stale "connectivity: off" left by an interrupted backup before
1423+
# the workload is configured/started.
1424+
self.reset_stale_connectivity_flag()
1425+
14191426
# Create the PostgreSQL data directory. This is needed on cloud environments
14201427
# where the volume is mounted with more restrictive permissions.
14211428
self._create_pgdata(container)
@@ -2152,6 +2159,21 @@ def is_connectivity_enabled(self) -> bool:
21522159
"""Return whether this unit can be connected externally."""
21532160
return self.unit_peer_data.get("connectivity", "on") == "on"
21542161

2162+
def reset_stale_connectivity_flag(self) -> None:
2163+
"""Reset a stale "connectivity: off" left by an interrupted backup.
2164+
2165+
The flag is written to the unit peer databag during replica backups and survives
2166+
restarts/upgrades; without this recovery a unit stuck "off" stays externally
2167+
disconnected (pg_hba rejects peer/replica connections). Skipped while any backup
2168+
is in progress cluster-wide to avoid un-hiding a unit mid-backup.
2169+
"""
2170+
if self.unit_peer_data.get("connectivity") != "off":
2171+
return
2172+
if self.patroni_manager.is_creating_backup:
2173+
return
2174+
logger.info("Resetting stale connectivity flag left by an interrupted backup")
2175+
self.unit_peer_data["connectivity"] = "on"
2176+
21552177
@property
21562178
def is_ldap_charm_related(self) -> bool:
21572179
"""Return whether this unit has an LDAP charm related."""

tests/unit/test_backups.py

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

466466

467+
def test_create_backup_resets_connectivity_when_backup_raises(harness):
468+
# Regression: if the backup run is interrupted (raises an unexpected exception) on a
469+
# replica, the connectivity flag must still be reset to "on" via the finally block —
470+
# otherwise pg_hba keeps rejecting peer/replica connections across restarts/upgrades.
471+
with (
472+
patch("charm.PostgresqlOperatorCharm.update_config"),
473+
patch(
474+
"charm.PostgreSQLBackups._change_connectivity_to_database"
475+
) as _change_connectivity_to_database,
476+
patch(
477+
"charm.PostgreSQLBackups._execute_command", side_effect=RuntimeError("boom")
478+
) as _execute_command,
479+
patch("charm.PostgresqlOperatorCharm.is_primary", new_callable=PropertyMock) as _is_primary,
480+
patch("charm.PostgreSQLBackups._upload_content_to_s3", return_value=True),
481+
patch("backups.datetime"),
482+
patch("ops.JujuVersion.from_environ"),
483+
patch("charm.PostgreSQLBackups._retrieve_s3_parameters") as _retrieve_s3_parameters,
484+
patch("charm.PostgreSQLBackups._can_unit_perform_backup", return_value=(True, None)),
485+
):
486+
# This unit is a replica, so connectivity is disabled before the backup runs.
487+
_is_primary.return_value = False
488+
_retrieve_s3_parameters.return_value = (
489+
{
490+
"bucket": "test-bucket",
491+
"access-key": "test-access-key",
492+
"secret-key": "test-secret-key",
493+
"endpoint": "test-endpoint",
494+
"path": "test-path",
495+
"region": "test-region",
496+
},
497+
[],
498+
)
499+
mock_event = MagicMock()
500+
mock_event.params = {"type": "full"}
501+
502+
# The backup run raises; the reset must still run.
503+
with pytest.raises(RuntimeError):
504+
harness.charm.backup._on_create_backup_action(mock_event)
505+
506+
# Connectivity disabled then re-enabled despite the exception.
507+
_change_connectivity_to_database.assert_has_calls(
508+
[call(connectivity=False), call(connectivity=True)]
509+
)
510+
_execute_command.assert_called_once()
511+
512+
513+
def test_reset_stale_connectivity_flag(harness):
514+
harness.charm.patroni_manager = MagicMock()
515+
peer_rel_id = harness.model.get_relation(PEER_RELATION).id
516+
517+
# Stale "off" with no backup in progress: reset to "on".
518+
harness.charm.patroni_manager.is_creating_backup = False
519+
with harness.hooks_disabled():
520+
harness.update_relation_data(
521+
peer_rel_id, harness.charm.unit.name, {"connectivity": "off"}
522+
)
523+
harness.charm.reset_stale_connectivity_flag()
524+
assert (
525+
harness.get_relation_data(peer_rel_id, harness.charm.unit)["connectivity"] == "on"
526+
)
527+
528+
# While a backup is in progress cluster-wide: do not reset (stay "off").
529+
with harness.hooks_disabled():
530+
harness.update_relation_data(
531+
peer_rel_id, harness.charm.unit.name, {"connectivity": "off"}
532+
)
533+
harness.charm.patroni_manager.is_creating_backup = True
534+
harness.charm.reset_stale_connectivity_flag()
535+
assert (
536+
harness.get_relation_data(peer_rel_id, harness.charm.unit)["connectivity"] == "off"
537+
)
538+
539+
# Already "on": no-op (does not even consult the backup flag).
540+
harness.charm.patroni_manager.is_creating_backup = False
541+
with harness.hooks_disabled():
542+
harness.update_relation_data(
543+
peer_rel_id, harness.charm.unit.name, {"connectivity": "on"}
544+
)
545+
harness.charm.reset_stale_connectivity_flag()
546+
assert (
547+
harness.get_relation_data(peer_rel_id, harness.charm.unit)["connectivity"] == "on"
548+
)
549+
550+
467551
def test_execute_command(harness):
468552
with patch("ops.model.Container.exec") as _exec:
469553
command = ["rm", "-r", "/var/lib/postgresql/16/main"]

0 commit comments

Comments
 (0)