[MISC] Move pg wal (#988)

* Move pg_wal dir instead of deleting

* Check there's at least one dir

* Move pg_wal once

* PGB lib fix

Author: dragomirp

lib/charms/postgresql_k8s/v1/postgresql.py

@@ -386,7 +386,7 @@ def create_predefined_instance_roles(self) -> None:
                 connection.close()
 
     def grant_database_privileges_to_user(
-        self, user: str, database: str, privileges: list[str]
+        self, user: str, database: str, privileges: List[str]
     ) -> None:
         """Grant the specified privileges on the provided database for the user."""
         try:

src/charm.py

@@ -15,7 +15,7 @@
 import subprocess
 import sys
 import time
-from datetime import datetime
+from datetime import UTC, datetime
 from pathlib import Path
 from typing import Literal, Optional, get_args
 from urllib.parse import urlparse
@@ -2038,8 +2038,11 @@ def _handle_processes_failures(self) -> bool:
         if self._unit_ip in self.members_ips and self._patroni.member_inactive:
             data_directory_contents = os.listdir(POSTGRESQL_DATA_PATH)
             if len(data_directory_contents) == 1 and data_directory_contents[0] == "pg_wal":
-                os.remove(os.path.join(POSTGRESQL_DATA_PATH, "pg_wal"))
-                logger.info("PostgreSQL data directory was not empty. Removed pg_wal")
+                os.rename(
+                    os.path.join(POSTGRESQL_DATA_PATH, "pg_wal"),
+                    os.path.join(POSTGRESQL_DATA_PATH, f"pg_wal-{datetime.now(UTC).isoformat()}"),
+                )
+                logger.info("PostgreSQL data directory was not empty. Moved pg_wal")
                 return True
             try:
                 self._patroni.restart_patroni()

tests/unit/test_charm.py

@@ -3,9 +3,11 @@
 import itertools
 import json
 import logging
+import os
 import pathlib
 import platform
 import subprocess
+from datetime import UTC, datetime
 from unittest.mock import MagicMock, Mock, PropertyMock, call, mock_open, patch, sentinel
 
 import charm_refresh
@@ -43,7 +45,7 @@
     SwitchoverFailedError,
     SwitchoverNotSyncError,
 )
-from constants import PEER, SECRET_INTERNAL_LABEL, UPDATE_CERTS_BIN_PATH
+from constants import PEER, POSTGRESQL_DATA_PATH, SECRET_INTERNAL_LABEL, UPDATE_CERTS_BIN_PATH
 
 CREATE_CLUSTER_CONF_PATH = "/etc/postgresql-common/createcluster.d/pgcharm.conf"
 
@@ -2642,3 +2644,55 @@ def test_get_ldap_parameters(harness):
         harness.charm.get_ldap_parameters()
         _get_relation_data.assert_called_once()
         _get_relation_data.reset_mock()
+
+
+def test_handle_processes_failures(harness):
+    _now = datetime.now(UTC)
+    with (
+        patch(
+            "charm.Patroni.member_inactive",
+            new_callable=PropertyMock,
+            return_value=False,
+        ) as _member_inactive,
+        patch(
+            "charm.Patroni.restart_patroni",
+        ) as _restart_patroni,
+        patch("charm.os.listdir", return_value=["other_dirs", "pg_wal"]) as _listdir,
+        patch("charm.os.rename") as _rename,
+        patch("charm.datetime") as _datetime,
+    ):
+        _datetime.now.return_value = _now
+        rel_id = harness.model.get_relation(PEER).id
+        with harness.hooks_disabled():
+            harness.update_relation_data(
+                rel_id,
+                harness.charm.app.name,
+                {"cluster_initialised": "True", "members_ips": '["192.0.2.0"]'},
+            )
+
+        # Does nothing if member is inactive
+        assert not harness.charm._handle_processes_failures()
+        assert not _restart_patroni.called
+
+        # Will not remove pg_wal if dir look right
+        _member_inactive.return_value = True
+        assert harness.charm._handle_processes_failures()
+        _restart_patroni.assert_called_once_with()
+        _restart_patroni.reset_mock()
+
+        # Will move pg_wal if there's only a pg_wal dir
+        _listdir.return_value = ["pg_wal"]
+        assert harness.charm._handle_processes_failures()
+        assert not _restart_patroni.called
+        _rename.assert_called_once_with(
+            os.path.join(POSTGRESQL_DATA_PATH, "pg_wal"),
+            os.path.join(POSTGRESQL_DATA_PATH, f"pg_wal-{_now.isoformat()}"),
+        )
+        _rename.reset_mock()
+
+        # Will not move pg_wal if there's only a pg_wal dir or other moved dirs
+        _listdir.return_value = ["pg_wal", f"pg_wal-{_now.isoformat()}"]
+        assert harness.charm._handle_processes_failures()
+        _restart_patroni.assert_called_once_with()
+        assert not _rename.called
+        _restart_patroni.reset_mock()