[DPE-5373] Implement list-backups action (#157)

actions.yaml

@@ -33,3 +33,6 @@ set-tls-private-key:
 
 create-backup:
   description: Create a database backup and send it to an object storage. S3 credentials are retrieved from a relation with the S3 integrator charm.
+
+list-backups:
+  description: List database backups. S3 credentials are retrieved from a relation with the S3 integrator charm.

requirements.txt

@@ -5,7 +5,7 @@ botocore-stubs==1.35.19 ; python_version >= "3.10" and python_version < "4.0"
 botocore==1.35.19 ; python_version >= "3.10" and python_version < "4.0"
 certifi==2024.8.30 ; python_version >= "3.10" and python_version < "4.0"
 cffi==1.17.1 ; python_version >= "3.10" and python_version < "4.0" and platform_python_implementation != "PyPy"
-cosl==0.0.33 ; python_version >= "3.10" and python_version < "4.0"
+cosl==0.0.34 ; python_version >= "3.10" and python_version < "4.0"
 cryptography==43.0.1 ; python_version >= "3.10" and python_version < "4.0"
 exceptiongroup==1.2.2 ; python_version >= "3.10" and python_version < "3.11"
 h11==0.14.0 ; python_version >= "3.10" and python_version < "4.0"

src/events/backup.py

@@ -40,7 +40,7 @@ def __init__(self, charm):
         self.framework.observe(self.s3_requirer.on.credentials_gone, self._on_s3_credentials_gone)
 
         self.framework.observe(self.charm.on.create_backup_action, self._on_create_backup_action)
-        # self.framework.observe(self.charm.on.list_backups_action, self._on_list_backups_action)
+        self.framework.observe(self.charm.on.list_backups_action, self._on_list_backups_action)
         # self.framework.observe(self.charm.on.restore_action, self._on_restore_action)
 
     def _on_s3_credentials_changed(self, event: CredentialsChangedEvent):
@@ -118,9 +118,26 @@ def _on_create_backup_action(self, event: ActionEvent):
         )
         event.log(output)
 
-    def _on_list_backups_action(self, _):
-        # TODO
-        pass
+    def _on_list_backups_action(self, event: ActionEvent):
+        failure_conditions = [
+            (not self.charm.unit.is_leader(), "Action must be ran on the application leader"),
+            (
+                not self.charm.state.cluster.s3_credentials,
+                "Cluster needs an access to an object storage to make a backup",
+            ),
+        ]
+
+        for check, msg in failure_conditions:
+            if check:
+                logging.error(msg)
+                event.set_results({"error": msg})
+                event.fail(msg)
+                return
+
+        backups_metadata = self.backup_manager.list_backups()
+        output = self.backup_manager.format_backups_table(backups_metadata)
+        event.log(output)
+        event.set_results({"backups": json.dumps(backups_metadata)})
 
     def _on_restore_action(self, _):
         # TODO

src/literals.py

@@ -37,6 +37,7 @@
 
 S3_REL_NAME = "s3-credentials"
 S3_BACKUPS_PATH = "zookeeper_backups"
+S3_BACKUPS_LIMIT = 20
 
 DEPENDENCIES = {
     "service": {

src/managers/backup.py

@@ -7,7 +7,9 @@
 import logging
 import os
 from datetime import datetime
-from io import StringIO
+from io import BytesIO, StringIO
+from itertools import islice
+from operator import attrgetter
 
 import boto3
 import httpx
@@ -20,7 +22,7 @@
 
 from core.cluster import ClusterState
 from core.stubs import BackupMetadata, S3ConnectionInfo
-from literals import ADMIN_SERVER_PORT, S3_BACKUPS_PATH
+from literals import ADMIN_SERVER_PORT, S3_BACKUPS_LIMIT, S3_BACKUPS_PATH
 
 logger = logging.getLogger(__name__)
 
@@ -102,7 +104,6 @@ def create_backup(self) -> BackupMetadata:
         zk_pwd = self.state.cluster.internal_user_credentials.get("super", "")
         date = datetime.now()
         snapshot_name = f"{date:%Y-%m-%dT%H:%M:%SZ}"
-        snapshot_path = f"{snapshot_name}/snapshot"
 
         # It is very likely that the file is fully loaded in memory, because the file-like interface is
         # not seekable, and I have a strong suspicion that boto uses this to figure out if it can
@@ -121,7 +122,7 @@ def create_backup(self) -> BackupMetadata:
             metadata: BackupMetadata = {
                 "id": snapshot_name,
                 "log-sequence-number": quorum_leader_zxid,
-                "path": snapshot_path,
+                "path": os.path.join(self.backups_path, snapshot_name, "snapshot"),
             }
 
             self.bucket.put_object(
@@ -136,6 +137,26 @@ def create_backup(self) -> BackupMetadata:
 
         return metadata
 
+    def list_backups(self) -> list[BackupMetadata]:
+        """List snapshots present in object storage."""
+        backups_metadata: list[BackupMetadata] = []
+
+        # S3 API is limited so we end up with an N+1 query problem. Thus, we limit the
+        # number of backups and fetch the latest modified ones.
+        remote_files = self.bucket.objects.filter(Prefix=self.backups_path)
+        remote_metadata = filter(lambda rfile: rfile.key.endswith(".yaml"), remote_files)
+        sorted_remote_files = sorted(
+            remote_metadata, key=attrgetter("last_modified"), reverse=True
+        )
+
+        for remote_file in islice(sorted_remote_files, S3_BACKUPS_LIMIT):
+            buffer = BytesIO()
+            self.bucket.download_fileobj(remote_file.key, buffer)
+            metadata = yaml.safe_load(buffer.getvalue())
+            backups_metadata.append(metadata)
+
+        return backups_metadata
+
     def format_backups_table(
         self, backup_entries: list[BackupMetadata], title: str = "Backups"
     ) -> str:
@@ -144,7 +165,7 @@ def format_backups_table(
 
         table.add_column("Id", no_wrap=True)
         table.add_column("Log-sequence-number", justify="right")
-        table.add_column("path")
+        table.add_column("Path", overflow="fold")
 
         for meta in backup_entries:
             table.add_row(meta["id"], str(meta["log-sequence-number"]), meta["path"])

tests/integration/test_backup.py

@@ -3,9 +3,9 @@
 # See LICENSE file for licensing details.
 
 import asyncio
+import json
 import logging
 import socket
-from io import BytesIO
 
 import boto3
 import pytest
@@ -101,19 +101,17 @@ async def test_relate_active_bucket_created(ops_test: OpsTest, s3_bucket):
     assert s3_bucket.meta.client.head_bucket(Bucket=s3_bucket.name)
 
 
-# @pytest.mark.abort_on_fail
-async def write_content(ops_test: OpsTest, s3_bucket: Bucket):
-    # TODO (backup): Remove and replace with ZK snapshot write
-
+@pytest.mark.abort_on_fail
+async def test_create_backup(ops_test: OpsTest, s3_bucket: Bucket):
     for unit in ops_test.model.applications[APP_NAME].units:
         if await unit.is_leader_from_status():
             leader_unit = unit
 
-    write_action = await leader_unit.run_action(
-        "create-backup",
-    )
-    await write_action.wait()
+    create_action = await leader_unit.run_action("create-backup")
+    await create_action.wait()
+
+    list_action = await leader_unit.run_action("list-backups")
+    response = await list_action.wait()
 
-    f = BytesIO()
-    s3_bucket.download_fileobj("test_file.txt", f)
-    assert f.getvalue() == b"test string"
+    backups = json.loads(response.results.get("backups", "[]"))
+    assert len(backups) == 1

tests/unit/scenario/test_backup.py

@@ -202,3 +202,32 @@ def test_action_create_backup_no_creds(ctx: Context, base_state: State):
     assert (
         exc_info.value.message == "Cluster needs an access to an object storage to make a backup"
     )
+
+
+def test_action_list_backups_not_leader(ctx: Context, base_state: State):
+    # Given
+    state_in = dataclasses.replace(base_state, leader=False)
+
+    # When
+    # Then
+    with pytest.raises(ActionFailed) as exc_info:
+        _ = ctx.run(ctx.on.action("list-backups"), state_in)
+
+    assert exc_info.value.message == "Action must be ran on the application leader"
+
+
+def test_action_list_backups_no_creds(ctx: Context, base_state: State):
+    # Given
+    state_in = base_state
+
+    # When
+    # Then
+    with (
+        patch("core.cluster.ClusterState.stable", new_callable=PropertyMock, return_value=True),
+        pytest.raises(ActionFailed) as exc_info,
+    ):
+        _ = ctx.run(ctx.on.action("list-backups"), state_in)
+
+    assert (
+        exc_info.value.message == "Cluster needs an access to an object storage to make a backup"
+    )