NAS-130271 / 24.10 / Check SnapshotCountAlert on SMB shares only (#14161

)

* only alert on smb shares

* fix middleware call

* allow multiple alerts

* test with an smb share

* fix test

* address @yocalebo

* test

* Revert "test"

This reverts commit 67e4973.

* address @anodos325

src/middlewared/middlewared/alert/source/snapshot_count.py

@@ -1,5 +1,6 @@
 from middlewared.alert.base import AlertClass, AlertCategory, AlertLevel, Alert, AlertSource
 from middlewared.alert.schedule import CrontabSchedule
+from middlewared.utils.path import FSLocation, path_location
 
 
 class SnapshotTotalCountAlertClass(AlertClass):
@@ -17,37 +18,50 @@ class SnapshotCountAlertClass(AlertClass):
     level = AlertLevel.WARNING
     title = "Too Many Snapshots Exist For Dataset"
     text = (
-        "Dataset %(dataset)s has more snapshots (%(count)d) than recommended (%(max)d). Performance or functionality "
-        "might degrade."
+        "SMB share %(dataset)s has more snapshots (%(count)d) than recommended (%(max)d). File Explorer may not "
+        "display all snapshots in the Previous Versions tab."
     )
 
 
 class SnapshotCountAlertSource(AlertSource):
     schedule = CrontabSchedule(hour=1)
     run_on_backup_node = False
 
-    async def check(self):
-        max_ = await self.middleware.call("pool.snapshottask.max_count")
+    async def _check_total(self, snapshot_counts: dict[str, int]) -> list[Alert]:
+        """Return an `Alert` if the total number of snapshots exceeds the limit."""
         max_total = await self.middleware.call("pool.snapshottask.max_total_count")
-
-        total = 0
-        datasets = await self.middleware.call("zfs.snapshot.count")
-
-        for cnt in datasets.values():
-            total += cnt
+        total = sum(snapshot_counts.values())
 
         if total > max_total:
-            return Alert(
+            return [Alert(
                 SnapshotTotalCountAlertClass,
                 {"count": total, "max": max_total},
                 key=None,
-            )
+            )]
+
+        return []
 
-        for dataset in sorted(datasets.keys()):
-            count = datasets[dataset]
+    async def _check_smb(self, snapshot_counts: dict[str, int]) -> list[Alert]:
+        """Return an `Alert` for every dataset shared over smb whose number of snapshots exceeds the limit."""
+        max_ = await self.middleware.call("pool.snapshottask.max_count")
+        to_alert = list()
+
+        for share in await self.middleware.call("sharing.smb.query"):
+            if path_location(share["path"]) != FSLocation.LOCAL:
+                continue
+            path = share["path"].removeprefix("/mnt/")
+            count = snapshot_counts.get(path, 0)
             if count > max_:
-                return Alert(
+                to_alert.append(Alert(
                     SnapshotCountAlertClass,
-                    {"dataset": dataset, "count": count, "max": max_},
-                    key=None,
-                )
+                    {"dataset": path, "count": count, "max": max_},
+                    key=path,
+                ))
+
+        return to_alert
+
+    async def check(self):
+        snapshot_counts = await self.middleware.call("zfs.snapshot.count")
+        alerts = await self._check_smb(snapshot_counts)
+        alerts.extend(await self._check_total(snapshot_counts))
+        return alerts

tests/api2/test_snapshot_count_alert.py

@@ -1,44 +1,52 @@
 import pytest
 from pytest_dependency import depends
 from middlewared.test.integration.assets.pool import dataset
+from middlewared.test.integration.assets.smb import smb_share
 from middlewared.test.integration.utils import call, mock
 from time import sleep
 
 
+DATASET_NAME = "snapshot_count"
+NUM_SNAPSHOTS = 10
+
+
 def test_snapshot_total_count_alert(request):
-    with dataset("snapshot_count") as ds:
+    with dataset(DATASET_NAME) as ds:
         base = call("zfs.snapshot.query", [], {"count": True})
-        with mock("pool.snapshottask.max_total_count", return_value=base + 10):
-            for i in range(10):
+        with mock("pool.snapshottask.max_total_count", return_value=base + NUM_SNAPSHOTS):
+            for i in range(NUM_SNAPSHOTS):
                 call("zfs.snapshot.create", {"dataset": ds, "name": f"snap-{i}"})
 
             assert call("alert.run_source", "SnapshotCount") == []
             # snapshots_changed ZFS dataset property has 1 second resolution
             sleep(1)
 
-            call("zfs.snapshot.create", {"dataset": ds, "name": "snap-10"})
+            call("zfs.snapshot.create", {"dataset": ds, "name": f"snap-{NUM_SNAPSHOTS}"})
 
             alert = call("alert.run_source", "SnapshotCount")[0]
             assert alert["text"] % alert["args"] == (
-                f"Your system has more snapshots ({base + 11}) than recommended ({base + 10}). Performance or "
-                "functionality might degrade."
+                f"Your system has more snapshots ({base + NUM_SNAPSHOTS + 1}) than recommended ({base + NUM_SNAPSHOTS}"
+                "). Performance or functionality might degrade."
             )
 
 
 def test_snapshot_count_alert(request):
-    with dataset("snapshot_count") as ds:
-        with mock("pool.snapshottask.max_count", return_value=10):
-            for i in range(10):
+    with (
+        dataset(DATASET_NAME) as ds,
+        smb_share(f"/mnt/{ds}", DATASET_NAME),
+        mock("pool.snapshottask.max_count", return_value=NUM_SNAPSHOTS)
+    ):
+            for i in range(NUM_SNAPSHOTS):
                 call("zfs.snapshot.create", {"dataset": ds, "name": f"snap-{i}"})
 
             assert call("alert.run_source", "SnapshotCount") == []
             # snapshots_changed ZFS dataset property has 1 second resolution
             sleep(1)
 
-            call("zfs.snapshot.create", {"dataset": ds, "name": "snap-10"})
+            call("zfs.snapshot.create", {"dataset": ds, "name": f"snap-{NUM_SNAPSHOTS}"})
 
             alert = call("alert.run_source", "SnapshotCount")[0]
             assert alert["text"] % alert["args"] == (
-                "Dataset tank/snapshot_count has more snapshots (11) than recommended (10). Performance or "
-                "functionality might degrade."
+                f"SMB share {ds} has more snapshots ({NUM_SNAPSHOTS + 1}) than recommended ({NUM_SNAPSHOTS}). File "
+                "Explorer may not display all snapshots in the Previous Versions tab."
             )