[DPE-10450] Fix crash restoring non-settable unit status in PG14 (#1791)

* [DPE-10450] Fix crash restoring non-settable unit status in _check_detached_storage

The unit.status getter can return statuses the setter rejects (e.g. an
"error" status left over from a previously failed hook). When storage is
already attached, _check_detached_storage restored the cached status
verbatim, raising InvalidStatusError ("status must be in active, blocked,
maintenance, waiting, not 'error'") and deadlocking the unit in error.

Only restore the cached status when it is a settable status type.

Assisted-by: Claude:claude-4.8-opus

* [DPE-10450] Guard restore of non-settable status in enable_disable_extensions

enable_disable_extensions caches the current unit status and restores it
verbatim at the end, exactly like _check_detached_storage did. The status
getter can return statuses the setter rejects (e.g. an "error" status left
over from a previously failed hook), so this restore hit the same
InvalidStatusError crash.

Extract the settable-status check into a shared _restore_unit_status helper
and route both restore paths through it. Also cover the "unknown" status
(not just "error") in both tests.

Backport of 66ffc20de7e6bb5ff964de24dcfbc9f29c6ced8a; adapted to this branch,
which sets self.unit.status directly (no set_unit_status helper).

Assisted-by: Claude:claude-4.8-opus

Author: taurus-forever

src/charm.py

@@ -53,6 +53,7 @@
     MaintenanceStatus,
     ModelError,
     Relation,
+    StatusBase,
     Unit,
     WaitingStatus,
 )
@@ -1293,7 +1294,7 @@ def enable_disable_extensions(self, database: str | None = None) -> None:
         if original_status.message == EXTENSION_OBJECT_MESSAGE:
             self.unit.status = ActiveStatus()
             return
-        self.unit.status = original_status
+        self._restore_unit_status(original_status)
 
     def _check_extension_dependencies(self, extension: str, enable: bool) -> bool:
         skip = False
@@ -1963,6 +1964,17 @@ def clean_ca_file_from_workload(self, secret_name: str) -> bool:
             logger.exception("CA file failed to clean. Error in config update")
             return False
 
+    def _restore_unit_status(self, status: StatusBase) -> None:
+        """Restore a previously cached unit status.
+
+        The status getter can return statuses that the setter rejects (e.g. an
+        "error" status left over from a previously failed hook). Skip restoring
+        any status juju does not allow us to set, to avoid an InvalidStatusError
+        that would deadlock the unit.
+        """
+        if isinstance(status, ActiveStatus | BlockedStatus | MaintenanceStatus | WaitingStatus):
+            self.unit.status = status
+
     def _check_detached_storage(self) -> None:
         """Wait for storage to become available.
 
@@ -1978,7 +1990,7 @@ def _check_detached_storage(self) -> None:
                     logger.error("Data directory not attached.")
                     self.unit.status = WaitingStatus("Data directory not attached")
                     raise StorageUnavailableError()
-        self.unit.status = cached_status
+        self._restore_unit_status(cached_status)
 
     def _restart(self, event: RunWithLock) -> None:
         """Restart PostgreSQL."""

tests/unit/test_charm.py

@@ -20,9 +20,11 @@
 from ops.model import (
     ActiveStatus,
     BlockedStatus,
+    ErrorStatus,
     MaintenanceStatus,
     ModelError,
     RelationDataTypeError,
+    UnknownStatus,
     WaitingStatus,
 )
 from ops.testing import Harness
@@ -402,6 +404,33 @@ def test_enable_disable_extensions(harness, caplog):
             assert isinstance(harness.charm.unit.status, ActiveStatus)
 
 
+@pytest.mark.parametrize("unsettable_status", [ErrorStatus(), UnknownStatus()])
+def test_enable_disable_extensions_does_not_restore_unsettable_status(harness, unsettable_status):
+    # enable_disable_extensions caches the current unit status and restores it
+    # afterwards. The getter can return statuses the backend rejects on set
+    # (e.g. "error" or "unknown") left over from a previously failed hook;
+    # restoring such a cached status must not be attempted, otherwise the backend
+    # raises InvalidStatusError/ModelError and deadlocks the unit.
+    with (
+        patch("charm.Patroni.get_primary") as _get_primary,
+        patch("charm.PostgresqlOperatorCharm._unit_ip"),
+        patch("charm.PostgresqlOperatorCharm._patroni"),
+        patch("subprocess.check_output", return_value=b"C"),
+        patch.object(PostgresqlOperatorCharm, "postgresql", Mock()) as postgresql_mock,
+    ):
+        _get_primary.return_value = harness.charm.unit
+        postgresql_mock.enable_disable_extensions.side_effect = None
+
+        # Cache an unsettable status that would otherwise be restored verbatim.
+        harness.charm.unit._status = unsettable_status
+
+        # Must not raise when the cached status is restored at the end.
+        harness.charm.enable_disable_extensions()
+
+        # The unsettable cached status was skipped, not restored.
+        assert not isinstance(harness.charm.unit.status, ErrorStatus | UnknownStatus)
+
+
 def test_on_start(harness):
     with (
         patch(
@@ -1023,6 +1052,18 @@ def test_check_detached_storage(harness):
         assert isinstance(harness.charm.unit.status, WaitingStatus)
 
 
+@pytest.mark.parametrize("unsettable_status", [ErrorStatus(), UnknownStatus()])
+def test_check_detached_storage_does_not_restore_unsettable_status(harness, unsettable_status):
+    # The unit.status getter can return statuses that the juju backend rejects
+    # on set (e.g. "error" or "unknown"). Restoring such a cached status must not
+    # be attempted, otherwise the backend raises InvalidStatusError/ModelError.
+    harness.charm.unit._status = unsettable_status
+    with patch("charm.PostgresqlOperatorCharm._is_storage_attached", return_value=True):
+        # Storage is attached, so the cached status would be restored verbatim;
+        # this must not raise.
+        harness.charm._check_detached_storage()
+
+
 def test_restart(harness):
     with (
         patch("charm.Patroni.restart_postgresql") as _restart_postgresql,