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 so a genuinely in-progress backup
is not un-hidden mid-run.

Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>

Author: marceloneppel

src/backups.py

@@ -881,19 +881,21 @@ def _on_create_backup_action(self, event) -> None:  # noqa: C901
             event.fail(error_message)
             return
 
-        disabled_connectivity = False
-        if not self.charm.is_primary:
-            # Create a rule to mark the cluster as in a creating backup state and update
-            # the Patroni configuration.
-            self._change_connectivity_to_database(connectivity=False)
-            disabled_connectivity = True
-
-        self.charm.unit.status = MaintenanceStatus("creating backup")
-        # Set flag due to missing in progress backups on JSON output
-        # (reference: https://github.com/pgbackrest/pgbackrest/issues/2007)
-        self.charm.update_config(is_creating_backup=True)
-
+        # Decide before any side effect whether this unit disables connectivity, so the
+        # reset in the finally below runs even if disabling or the re-render raises partway
+        # — a stale "off" leaves pg_hba rejecting peer/replica connections across restarts.
+        disabled_connectivity = not self.charm.is_primary
         try:
+            if disabled_connectivity:
+                # Create a rule to mark the cluster as in a creating backup state and update
+                # the Patroni configuration.
+                self._change_connectivity_to_database(connectivity=False)
+
+            self.charm.unit.status = MaintenanceStatus("creating backup")
+            # Set flag due to missing in progress backups on JSON output
+            # (reference: https://github.com/pgbackrest/pgbackrest/issues/2007)
+            self.charm.update_config(is_creating_backup=True)
+
             command = [
                 "pgbackrest",
                 f"--stanza={self.stanza_name}",
@@ -955,11 +957,13 @@ def _on_create_backup_action(self, event) -> None:  # noqa: C901
             else:
                 logger.info(f"Backup succeeded: with backup-id {datetime_backup_requested}")
                 event.set_results({"backup-status": "backup created"})
-
-        if disabled_connectivity:
-            # Remove the rule that marks the cluster as in a creating backup state
-            # and update the Patroni configuration.
-            self._change_connectivity_to_database(connectivity=True)
+        finally:
+            # Reset connectivity even if the backup raised or was interrupted — a stale
+            # "off" leaves pg_hba rejecting peer/replica connections across restarts.
+            if disabled_connectivity:
+                # Remove the rule that marks the cluster as in a creating backup state
+                # and update the Patroni configuration.
+                self._change_connectivity_to_database(connectivity=True)
 
         self.charm.update_config(is_creating_backup=False)
         self.charm.unit.status = ActiveStatus()

src/charm.py

@@ -673,7 +673,9 @@ def _on_config_changed(self, event) -> None:
             logger.debug("Defer on_config_changed: upgrade in progress")
             event.defer()
             return
-
+        # Recover from a stale "connectivity: off" left by an interrupted backup before
+        # re-rendering the Patroni configuration below.
+        self.reset_stale_connectivity_flag()
         try:
             self._validate_config_options()
             # update config on every run
@@ -964,6 +966,10 @@ def _on_postgresql_pebble_ready(self, event: WorkloadEvent) -> None:
             event.defer()
             return
 
+        # Recover from a stale "connectivity: off" left by an interrupted backup before
+        # the workload is configured/started.
+        self.reset_stale_connectivity_flag()
+
         # Create the PostgreSQL data directory. This is needed on cloud environments
         # where the volume is mounted with more restrictive permissions.
         self._create_pgdata(container)
@@ -1664,6 +1670,21 @@ def is_connectivity_enabled(self) -> bool:
         """Return whether this unit can be connected externally."""
         return self.unit_peer_data.get("connectivity", "on") == "on"
 
+    def reset_stale_connectivity_flag(self) -> None:
+        """Reset a stale "connectivity: off" left by an interrupted backup.
+
+        The flag is written to the unit peer databag during replica backups and survives
+        restarts/upgrades; without this recovery a unit stuck "off" stays externally
+        disconnected (pg_hba rejects peer/replica connections). Skipped while any backup
+        is in progress cluster-wide to avoid un-hiding a unit mid-backup.
+        """
+        if self.unit_peer_data.get("connectivity") != "off":
+            return
+        if self._patroni.is_creating_backup:
+            return
+        logger.info("Resetting stale connectivity flag left by an interrupted backup")
+        self.unit_peer_data["connectivity"] = "on"
+
     @property
     def is_ldap_charm_related(self) -> bool:
         """Return whether this unit has an LDAP charm related."""

tests/unit/test_backups.py

@@ -452,6 +452,132 @@ def test_change_connectivity_to_database(harness):
         _update_config.assert_called_once()
 
 
+def test_create_backup_resets_connectivity_when_backup_raises(harness):
+    # Regression: if the backup run is interrupted (raises an unexpected exception) on a
+    # replica, the connectivity flag must still be reset to "on" via the finally block —
+    # otherwise pg_hba keeps rejecting peer/replica connections across restarts/upgrades.
+    with (
+        patch("charm.PostgresqlOperatorCharm.update_config"),
+        patch(
+            "charm.PostgreSQLBackups._change_connectivity_to_database"
+        ) as _change_connectivity_to_database,
+        patch(
+            "charm.PostgreSQLBackups._execute_command", side_effect=RuntimeError("boom")
+        ) as _execute_command,
+        patch(
+            "charm.PostgresqlOperatorCharm.is_primary", new_callable=PropertyMock
+        ) as _is_primary,
+        patch("charm.PostgreSQLBackups._upload_content_to_s3", return_value=True),
+        patch("backups.datetime"),
+        patch("ops.JujuVersion.from_environ"),
+        patch("charm.PostgreSQLBackups._retrieve_s3_parameters") as _retrieve_s3_parameters,
+        patch("charm.PostgreSQLBackups._can_unit_perform_backup", return_value=(True, None)),
+    ):
+        # This unit is a replica, so connectivity is disabled before the backup runs.
+        _is_primary.return_value = False
+        _retrieve_s3_parameters.return_value = (
+            {
+                "bucket": "test-bucket",
+                "access-key": "test-access-key",
+                "secret-key": "test-secret-key",
+                "endpoint": "test-endpoint",
+                "path": "test-path",
+                "region": "test-region",
+            },
+            [],
+        )
+        mock_event = MagicMock()
+        mock_event.params = {"type": "full"}
+
+        # The backup run raises; the reset must still run.
+        with pytest.raises(RuntimeError):
+            harness.charm.backup._on_create_backup_action(mock_event)
+
+        # Connectivity disabled then re-enabled despite the exception.
+        _change_connectivity_to_database.assert_has_calls([
+            call(connectivity=False),
+            call(connectivity=True),
+        ])
+        _execute_command.assert_called_once()
+
+
+def test_create_backup_resets_connectivity_when_disabling_raises(harness):
+    # Regression: if disabling connectivity itself raises (after the databag was written),
+    # the reset must still run — the latch is decided before any side effect, so the finally
+    # fires even though disabled_connectivity was never set inside the try body.
+    with (
+        patch("charm.PostgresqlOperatorCharm.update_config"),
+        patch(
+            "charm.PostgreSQLBackups._change_connectivity_to_database",
+            side_effect=[RuntimeError("boom"), None],
+        ) as _change_connectivity_to_database,
+        patch(
+            "charm.PostgresqlOperatorCharm.is_primary", new_callable=PropertyMock
+        ) as _is_primary,
+        patch("charm.PostgreSQLBackups._upload_content_to_s3", return_value=True),
+        patch("backups.datetime"),
+        patch("ops.JujuVersion.from_environ"),
+        patch("charm.PostgreSQLBackups._retrieve_s3_parameters") as _retrieve_s3_parameters,
+        patch("charm.PostgreSQLBackups._can_unit_perform_backup", return_value=(True, None)),
+    ):
+        _is_primary.return_value = False
+        _retrieve_s3_parameters.return_value = (
+            {
+                "bucket": "test-bucket",
+                "access-key": "test-access-key",
+                "secret-key": "test-secret-key",
+                "endpoint": "test-endpoint",
+                "path": "test-path",
+                "region": "test-region",
+            },
+            [],
+        )
+        mock_event = MagicMock()
+        mock_event.params = {"type": "full"}
+
+        # Disabling connectivity raises; the reset must still run.
+        with pytest.raises(RuntimeError):
+            harness.charm.backup._on_create_backup_action(mock_event)
+
+        # Disable (which raised) then re-enable despite the exception.
+        _change_connectivity_to_database.assert_has_calls([
+            call(connectivity=False),
+            call(connectivity=True),
+        ])
+
+
+def test_reset_stale_connectivity_flag(harness):
+    with patch("charm.PostgresqlOperatorCharm._patroni", new_callable=PropertyMock) as _patroni:
+        peer_rel_id = harness.model.get_relation(PEER).id
+
+        # Stale "off" with no backup in progress: reset to "on".
+        _patroni.return_value.is_creating_backup = False
+        with harness.hooks_disabled():
+            harness.update_relation_data(
+                peer_rel_id, harness.charm.unit.name, {"connectivity": "off"}
+            )
+        harness.charm.reset_stale_connectivity_flag()
+        assert harness.get_relation_data(peer_rel_id, harness.charm.unit)["connectivity"] == "on"
+
+        # While a backup is in progress cluster-wide: do not reset (stay "off").
+        with harness.hooks_disabled():
+            harness.update_relation_data(
+                peer_rel_id, harness.charm.unit.name, {"connectivity": "off"}
+            )
+        _patroni.return_value.is_creating_backup = True
+        harness.charm.reset_stale_connectivity_flag()
+        assert harness.get_relation_data(peer_rel_id, harness.charm.unit)["connectivity"] == "off"
+
+        # Already "on": no-op.
+        _patroni.return_value.is_creating_backup = False
+        with harness.hooks_disabled():
+            harness.update_relation_data(
+                peer_rel_id, harness.charm.unit.name, {"connectivity": "on"}
+            )
+        harness.charm.reset_stale_connectivity_flag()
+        assert harness.get_relation_data(peer_rel_id, harness.charm.unit)["connectivity"] == "on"
+
+
 def test_execute_command(harness):
     with patch("ops.model.Container.exec") as _exec:
         # Test when the command fails.