diff --git a/src/backups.py b/src/backups.py index 7c1700ab0d..09e8d9fe38 100644 --- 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 index 9cf89caf76..25f23296d0 100755 --- 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 index 7fb157e507..cc3615c073 100644 --- 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 new file mode 100644 index 0000000000..65223a4113 --- /dev/null +++ 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 new file mode 100644 index 0000000000..ae46883156 --- /dev/null +++ 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 index 42b533073e..c84af074bf 100644 --- 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 index 2437958e31..9b10277bc4 100644 --- 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 new file mode 100644 index 0000000000..32c9ee9d66 --- /dev/null +++ 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])