[DPE-5600] Add pgBackRest logrotate configuration (#722)

* Add pgBackRest logrotate configuration Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Update how logrotate starts Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Fix unit tests Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Add unit tests Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> --------- Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
canonical · Oct 18, 2024 · 9a7e4e6 · 9a7e4e6
1 parent 2a53fa3
commit 9a7e4e6
Show file tree

Hide file tree

Showing 8 changed files with 134 additions and 40 deletions.
diff --git a/src/backups.py b/src/backups.py
@@ -25,7 +25,13 @@
 from ops.pebble import ChangeError, ExecError
 from tenacity import RetryError, Retrying, stop_after_attempt, wait_fixed
 
-from constants import BACKUP_TYPE_OVERRIDES, BACKUP_USER, WORKLOAD_OS_GROUP, WORKLOAD_OS_USER
+from constants import (
+    BACKUP_TYPE_OVERRIDES,
+    BACKUP_USER,
+    PGBACKREST_LOGROTATE_FILE,
+    WORKLOAD_OS_GROUP,
+    WORKLOAD_OS_USER,
+)
 from relations.async_replication import REPLICATION_CONSUMER_RELATION, REPLICATION_OFFER_RELATION
 
 logger = logging.getLogger(__name__)
@@ -825,6 +831,7 @@ def _on_create_backup_action(self, event) -> None:  # noqa: C901
         self.charm.unit.status = ActiveStatus()
 
     def _on_s3_credential_gone(self, _) -> None:
+        self.container.stop(self.charm.rotate_logs_service)
         if self.charm.unit.is_leader():
             self.charm.app_peer_data.update({
                 "stanza": "",
@@ -1130,6 +1137,16 @@ def _render_pgbackrest_conf_file(self) -> bool:
             group=WORKLOAD_OS_GROUP,
         )
 
+        # Render the logrotate configuration file.
+        with open("templates/pgbackrest.logrotate.j2", "r") as file:
+            template = Template(file.read())
+        self.container.push(PGBACKREST_LOGROTATE_FILE, template.render())
+        self.container.push(
+            "/home/postgres/rotate_logs.py",
+            open("src/rotate_logs.py", "r").read(),
+        )
+        self.container.start(self.charm.rotate_logs_service)
+
         return True
 
     def _restart_database(self) -> None:

diff --git a/src/charm.py b/src/charm.py
@@ -190,6 +190,7 @@ def __init__(self, *args):
         )
 
         self._postgresql_service = "postgresql"
+        self.rotate_logs_service = "rotate-logs"
         self.pgbackrest_server_service = "pgbackrest server"
         self._metrics_service = "metrics_server"
         self._unit = self.model.unit.name
@@ -1674,6 +1675,12 @@ def _postgresql_layer(self) -> Layer:
                     "group": WORKLOAD_OS_GROUP,
                 },
                 self._metrics_service: self._generate_metrics_service(),
+                self.rotate_logs_service: {
+                    "override": "replace",
+                    "summary": "rotate logs",
+                    "command": "python3 /home/postgres/rotate_logs.py",
+                    "startup": "disabled",
+                },
             },
             "checks": {
                 self._postgresql_service: {

diff --git a/src/constants.py b/src/constants.py
@@ -57,3 +57,5 @@
 ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE = (
     "Please choose one endpoint to use. No need to relate all of them simultaneously!"
 )
+
+PGBACKREST_LOGROTATE_FILE = "/etc/logrotate.d/pgbackrest.logrotate"
diff --git a/src/rotate_logs.py b/src/rotate_logs.py
@@ -0,0 +1,20 @@
+# Copyright 2024 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Service for rotating logs."""
+
+import subprocess
+import time
+
+
+def main():
+    """Main loop that calls logrotate."""
+    while True:
+        subprocess.run(["logrotate", "-f", "/etc/logrotate.d/pgbackrest.logrotate"])
+
+        # Wait 60 seconds before executing logrotate again.
+        time.sleep(60)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/templates/pgbackrest.logrotate.j2 b/templates/pgbackrest.logrotate.j2
@@ -0,0 +1,10 @@
+/var/log/pgbackrest/*.log {
+    rotate 10
+    missingok
+    notifempty
+    nocompress
+    daily
+    create 0600 postgres postgres
+    dateext
+    dateformat -%Y%m%d_%H:%M.log
+}
diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py
@@ -1206,47 +1206,48 @@ def test_on_s3_credential_changed(harness):
 
 
 def test_on_s3_credential_gone(harness):
-    peer_rel_id = harness.model.get_relation(PEER).id
-    # Test that unrelated blocks will remain
-    harness.charm.unit.status = BlockedStatus("test block")
-    harness.charm.backup._on_s3_credential_gone(None)
-    assert isinstance(harness.charm.unit.status, BlockedStatus)
+    with patch("ops.model.Container.stop") as _stop:
+        peer_rel_id = harness.model.get_relation(PEER).id
+        # Test that unrelated blocks will remain
+        harness.charm.unit.status = BlockedStatus("test block")
+        harness.charm.backup._on_s3_credential_gone(None)
+        assert isinstance(harness.charm.unit.status, BlockedStatus)
 
-    # Test that s3 related blocks will be cleared
-    harness.charm.unit.status = BlockedStatus(ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE)
-    harness.charm.backup._on_s3_credential_gone(None)
-    assert isinstance(harness.charm.unit.status, ActiveStatus)
+        # Test that s3 related blocks will be cleared
+        harness.charm.unit.status = BlockedStatus(ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE)
+        harness.charm.backup._on_s3_credential_gone(None)
+        assert isinstance(harness.charm.unit.status, ActiveStatus)
 
-    # Test removal of relation data when the unit is not the leader.
-    with harness.hooks_disabled():
-        harness.update_relation_data(
-            peer_rel_id,
-            harness.charm.app.name,
-            {"stanza": "test-stanza", "init-pgbackrest": "True"},
-        )
-        harness.update_relation_data(
-            peer_rel_id,
-            harness.charm.app.name,
-            {"stanza": "test-stanza", "init-pgbackrest": "True"},
-        )
-    harness.charm.backup._on_s3_credential_gone(None)
-    assert harness.get_relation_data(peer_rel_id, harness.charm.app) == {
-        "stanza": "test-stanza",
-        "init-pgbackrest": "True",
-    }
-    assert harness.get_relation_data(peer_rel_id, harness.charm.unit) == {}
+        # Test removal of relation data when the unit is not the leader.
+        with harness.hooks_disabled():
+            harness.update_relation_data(
+                peer_rel_id,
+                harness.charm.app.name,
+                {"stanza": "test-stanza", "init-pgbackrest": "True"},
+            )
+            harness.update_relation_data(
+                peer_rel_id,
+                harness.charm.app.name,
+                {"stanza": "test-stanza", "init-pgbackrest": "True"},
+            )
+        harness.charm.backup._on_s3_credential_gone(None)
+        assert harness.get_relation_data(peer_rel_id, harness.charm.app) == {
+            "stanza": "test-stanza",
+            "init-pgbackrest": "True",
+        }
+        assert harness.get_relation_data(peer_rel_id, harness.charm.unit) == {}
 
-    # Test removal of relation data when the unit is the leader.
-    with harness.hooks_disabled():
-        harness.set_leader()
-        harness.update_relation_data(
-            peer_rel_id,
-            harness.charm.unit.name,
-            {"stanza": "test-stanza", "init-pgbackrest": "True"},
-        )
-    harness.charm.backup._on_s3_credential_gone(None)
-    assert harness.get_relation_data(peer_rel_id, harness.charm.app) == {}
-    assert harness.get_relation_data(peer_rel_id, harness.charm.unit) == {}
+        # Test removal of relation data when the unit is the leader.
+        with harness.hooks_disabled():
+            harness.set_leader()
+            harness.update_relation_data(
+                peer_rel_id,
+                harness.charm.unit.name,
+                {"stanza": "test-stanza", "init-pgbackrest": "True"},
+            )
+        harness.charm.backup._on_s3_credential_gone(None)
+        assert harness.get_relation_data(peer_rel_id, harness.charm.app) == {}
+        assert harness.get_relation_data(peer_rel_id, harness.charm.unit) == {}
 
 
 def test_on_create_backup_action(harness):
@@ -1744,6 +1745,7 @@ def test_pre_restore_checks(harness):
 )
 def test_render_pgbackrest_conf_file(harness, tls_ca_chain_filename):
     with (
+        patch("ops.model.Container.start") as _start,
         patch("ops.model.Container.push") as _push,
         patch(
             "charm.PostgreSQLBackups._tls_ca_chain_filename",
@@ -1811,8 +1813,16 @@ def test_render_pgbackrest_conf_file(harness, tls_ca_chain_filename):
         # Check the template is opened read-only in the call to open.
         assert mock.call_args_list[0][0] == ("templates/pgbackrest.conf.j2", "r")
 
+        # Get the expected content from a file.
+        with open("templates/pgbackrest.conf.j2") as file:
+            template = Template(file.read())
+        log_rotation_expected_content = template.render()
+
         # Ensure the correct rendered template is sent to _render_file method.
-        calls = [call("/etc/pgbackrest.conf", expected_content, user="postgres", group="postgres")]
+        calls = [
+            call("/etc/pgbackrest.conf", expected_content, user="postgres", group="postgres"),
+            call("/etc/logrotate.d/pgbackrest.logrotate", log_rotation_expected_content),
+        ]
         if tls_ca_chain_filename != "":
             calls.insert(
                 0,

diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
@@ -33,6 +33,7 @@
 POSTGRESQL_SERVICE = "postgresql"
 METRICS_SERVICE = "metrics_server"
 PGBACKREST_SERVER_SERVICE = "pgbackrest server"
+ROTATE_LOGS_SERVICE = "rotate-logs"
 
 # used for assert functions
 tc = TestCase()
@@ -951,6 +952,12 @@ def test_postgresql_layer(harness):
                     "user": "postgres",
                     "group": "postgres",
                 },
+                ROTATE_LOGS_SERVICE: {
+                    "override": "replace",
+                    "summary": "rotate logs",
+                    "command": "python3 /home/postgres/rotate_logs.py",
+                    "startup": "disabled",
+                },
             },
             "checks": {
                 POSTGRESQL_SERVICE: {

diff --git a/tests/unit/test_rotate_logs.py b/tests/unit/test_rotate_logs.py
@@ -0,0 +1,21 @@
+# Copyright 2024 Canonical Ltd.
+# See LICENSE file for licensing details.
+from unittest.mock import call, patch
+
+from rotate_logs import main
+
+
+def test_main():
+    with patch("subprocess.run") as _run, patch(
+        "time.sleep", side_effect=[None, InterruptedError]
+    ) as _sleep:
+        try:
+            main()
+        except InterruptedError:
+            pass
+        assert _run.call_count == 2
+        run_call = call(["logrotate", "-f", "/etc/logrotate.d/pgbackrest.logrotate"])
+        _run.assert_has_calls([run_call, run_call])
+        assert _sleep.call_count == 2
+        sleep_call = call(60)
+        _sleep.assert_has_calls([sleep_call, sleep_call])