Abstracted out create and delete operations into SnapMechanism class

Issue #32

src/code/configs.py

@@ -69,7 +69,7 @@ class Config:
     post_transaction_scripts: list[str] = dataclasses.field(default_factory=list)
 
     # If empty, btrfs is assumed.
-    snapshot_type: snap_mechanisms.SnapType = snap_mechanisms.SnapType.BTRFS
+    snap_type: snap_mechanisms.SnapType = snap_mechanisms.SnapType.BTRFS
 
     def is_schedule_enabled(self) -> bool:
         return (
@@ -97,7 +97,7 @@ def from_configfile(cls, config_file: str) -> "Config":
             if key == "snapshot_type":
                 if not value:
                     value = "btrfs"
-                result.snapshot_type = snap_mechanisms.SnapType[value.upper()]
+                result.snap_type = snap_mechanisms.SnapType[value.upper()]
                 continue
             if not hasattr(result, key):
                 logging.warning(f"Invalid field {key=} found in {config_file=}")
@@ -126,9 +126,7 @@ def call_post_hooks(self) -> None:
             os_utils.run_user_script(script, [self.config_file])
 
     def is_compatible_volume(self) -> bool:
-        if self.snapshot_type == snap_mechanisms.SnapType.BTRFS:
-            return os_utils.is_btrfs_volume(self.source)
-        raise RuntimeError(f"Unclear how to check volume type for {self.snapshot_type}")
+        return snap_mechanisms.get(self.snap_type).verify_volume(self.source)
 
 
 def iterate_configs(source: Optional[str]) -> Iterator[Config]:

src/code/main.py

@@ -93,7 +93,7 @@ def _delete_snap(configs_iter: Iterable[configs.Config], path_suffix: str, sync:
         snap = snap_operator.find_target(config, path_suffix)
         if snap:
             snap.delete()
-            if config.snapshot_type == snap_mechanisms.SnapType.BTRFS:
+            if config.snap_type == snap_mechanisms.SnapType.BTRFS:
                 mount_paths.add(config.mount_path)
 
         config.call_post_hooks()
@@ -129,7 +129,7 @@ def _config_operation(command: str, source: str, comment: str, sync: bool):
             raise ValueError(f"Command not implemented: {command}")
 
         if snapper.snaps_deleted:
-            if config.snapshot_type == snap_mechanisms.SnapType.BTRFS:
+            if config.snap_type == snap_mechanisms.SnapType.BTRFS:
                 mount_paths_to_sync.add(config.mount_path)
         if snapper.snaps_created or snapper.snaps_deleted:
             config.call_post_hooks()

src/code/os_utils.py

@@ -54,28 +54,6 @@ def run_user_script(script_name: str, args: list[str]) -> bool:
     return True
 
 
-def is_btrfs_volume(mount_point: str) -> bool:
-    """Test if directory is a btrfs volume."""
-    # Based on https://stackoverflow.com/a/32865333/196462
-    fstype = execute_sh("stat -f --format=%T " + mount_point, error_ok=True)
-    if not fstype:
-        logging.warning(f"Not btrfs (cannot determine filesystem): {mount_point}")
-        return False
-    if fstype.strip() != "btrfs":
-        logging.warning(f"Not btrfs (filesystem not btrfs): {mount_point}")
-        return False
-    inodenum = execute_sh("stat --format=%i " + mount_point, error_ok=True)
-    if not inodenum:
-        logging.warning(f"Not btrfs (cannot determine inode): {mount_point}")
-        return False
-    if inodenum.strip() != "256":
-        logging.warning(
-            f"Not btrfs (inode not 256, possibly a subdirectory of a btrfs mount): {mount_point}"
-        )
-        return False
-    return True
-
-
 def _get_pacman_log_path() -> str:
     logfile = execute_sh("pacman-conf LogFile", error_ok=True)
     if logfile is None:

src/code/rollbacker.py

@@ -19,6 +19,7 @@
 from . import configs
 from . import os_utils
 from . import snap_holder
+from . import snap_mechanisms
 from . import snap_operator
 
 from typing import Iterable, Optional
@@ -124,7 +125,9 @@ def drop_root_slash(s: str) -> str:
     backup_paths: list[str] = []
     current_dir: Optional[str] = None
     for snap in snapshots:
-        if not os_utils.is_btrfs_volume(snap.metadata.source):
+        if not snap_mechanisms.get(snap_mechanisms.SnapType.BTRFS).verify_volume(
+            snap.metadata.source
+        ):
             raise ValueError(
                 f"Mount point may no longer be a btrfs volume: {snap.metadata.source}"
             )

src/code/rollbacker_test.py

@@ -18,6 +18,7 @@
 from . import rollbacker
 from . import os_utils
 from . import snap_holder
+from . import snap_mechanisms
 
 # For testing, we can access private methods.
 # pyright: reportPrivateUsage=false
@@ -69,7 +70,7 @@ def test_rollback_for_two_snaps(self):
         snaps_list[1].metadata.source = "/root"
 
         with mock.patch.object(
-            os_utils, "is_btrfs_volume", return_value=True
+            snap_mechanisms._BtrfsSnapMechanism, "verify_volume", return_value=True
         ) as mock_is_btrfs_volume, mock.patch.object(
             rollbacker, "_get_now_str", return_value="20220202220000"
         ), mock.patch.object(

src/code/snap_holder.py

@@ -24,21 +24,17 @@
 import os
 
 from . import global_flags
+from . import snap_mechanisms
 from . import os_utils
 
 TIME_FORMAT = r"%Y%m%d%H%M%S"
 TIME_FORMAT_LEN = 14
 
 
-def _execute_sh(cmd: str):
-    if global_flags.FLAGS.dryrun:
-        os_utils.eprint("Would run " + cmd)
-    else:
-        os_utils.execute_sh(cmd)
-
-
 @dataclasses.dataclass
 class _Metadata:
+    # Snapshot type. If empty, assumed btrfs.
+    snap_type: str = snap_mechanisms.SnapType.UNKNOWN.value
     # Name of the subvolume from whcih this snap was taken.
     source: str = ""
     # Can be one of -
@@ -62,8 +58,12 @@ def save_file(self, fname: str) -> None:
     def load_file(cls, fname: str) -> "_Metadata":
         if os.path.isfile(fname):
             with open(fname) as f:
+                all_args = json.load(f)
+                if "snap_type" not in all_args:
+                    # For back compatibility. Older snaps will not have snap_type.
+                    all_args["snap_type"] = "BTRFS"
                 try:
-                    return cls(**json.load(f))
+                    return cls(**all_args)
                 except json.JSONDecodeError:
                     logging.warning(f"Unable to parse metadata file: {fname}")
         return cls()
@@ -88,24 +88,26 @@ def target(self) -> str:
     def snaptime(self) -> datetime.datetime:
         return self._snaptime
 
-    def create_from(self, parent: str) -> None:
-        if not os_utils.is_btrfs_volume(parent):
-            logging.error("Unable to validate source as btrfs - aborting snapshot!")
+    @property
+    def _snap_type(self) -> snap_mechanisms.SnapType:
+        return snap_mechanisms.SnapType[self.metadata.snap_type]
+
+    def create_from(self, snap_type: snap_mechanisms.SnapType, parent: str) -> None:
+        if not snap_mechanisms.get(snap_type).verify_volume(parent):
+            logging.error("Unable to validate source volume - aborting snapshot!")
             return
+        # Create the metadata before the snapshot.
+        # Thus we leave trace even if snapshotting fails.
+        self.metadata.snap_type = snap_type.value
         self.metadata.source = parent
         self.metadata.save_file(self._metadata_fname)
-        try:
-            _execute_sh("btrfs subvolume snapshot -r " f"{parent} {self._target}")
-        except os_utils.CommandError:
-            logging.error("Unable to create; are you running as root?")
-            raise
+        # Create the snap.
+        snap_mechanisms.get(snap_type).create(parent, self._target)
 
     def delete(self) -> None:
-        try:
-            _execute_sh(f"btrfs subvolume delete {self._target}")
-        except os_utils.CommandError:
-            logging.error("Unable to delete; are you running as root?")
-            raise
+        # First delete the snapshot.
+        snap_mechanisms.get(self._snap_type).delete(self._target)
+        # Then delete the metadata.
         if not global_flags.FLAGS.dryrun:
             if os.path.exists(self._metadata_fname):
                 os.remove(self._metadata_fname)

src/code/snap_holder_test.py

@@ -0,0 +1,71 @@
+# Copyright 2022 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import json
+import os
+import tempfile
+import unittest
+from unittest import mock
+
+from . import snap_holder
+from . import snap_mechanisms
+
+# For testing, we can access private methods.
+# pyright: reportPrivateUsage=false
+
+
+class SnapHolderTest(unittest.TestCase):
+    def test_create_and_delete(self):
+        with tempfile.TemporaryDirectory() as dir:
+            snap_destination = os.path.join(dir, "root-20231122193630")
+            snap = snap_holder.Snapshot(snap_destination)
+            self.assertEqual(snap._snap_type, snap_mechanisms.SnapType.UNKNOWN)
+
+            with mock.patch.object(
+                snap_mechanisms._BtrfsSnapMechanism, "verify_volume", return_value=True
+            ) as mock_verify_volume, mock.patch.object(
+                snap_mechanisms._BtrfsSnapMechanism, "create", return_value=None
+            ) as mock_create:
+                snap.create_from(snap_mechanisms.SnapType.BTRFS, "parent")
+            mock_verify_volume.assert_called_once_with("parent")
+            mock_create.assert_called_once_with("parent", snap_destination)
+
+            snap2 = snap_holder.Snapshot(snap_destination)
+            self.assertEqual(snap2._snap_type, snap_mechanisms.SnapType.BTRFS)
+
+            with open(f"{snap_destination}-meta.json") as f:
+                self.assertEqual(
+                    json.load(f), {"snap_type": "BTRFS", "source": "parent"}
+                )
+
+            with mock.patch.object(
+                snap_mechanisms._BtrfsSnapMechanism, "delete", return_value=None
+            ) as mock_delete:
+                snap2.delete()
+            mock_delete.assert_called_once_with(snap_destination)
+            self.assertFalse(os.path.exists(f"{snap_destination}-meta.json"))
+
+    def test_backcompat(self):
+        with tempfile.TemporaryDirectory() as dir:
+            snap_destination = os.path.join(dir, "root-20231122193630")
+            with open(f"{snap_destination}-meta.json", "w") as f:
+                json.dump({"source": "parent"}, f)
+            snap = snap_holder.Snapshot(snap_destination)
+
+            # Without any snap_type, defaults to BTRFS to continue working with old snaps.
+            self.assertEqual(snap._snap_type, snap_mechanisms.SnapType.BTRFS)
+
+
+if __name__ == "__main__":
+    unittest.main()

src/code/snap_mechanisms.py

@@ -1,29 +1,97 @@
+import abc
 import enum
 import functools
+import logging
 
 from . import global_flags
 from . import os_utils
 
+# TODO: Move btrfs based rollback to this abstraction.
+# TODO: Move btrfs sync to this abstraction.
 
 # The type of snapshot is maintained in two places -
 # 1. Config
 # 2. Snapshot
 class SnapType(enum.Enum):
+    UNKNOWN = "UNKNOWN"
     BTRFS = "BTRFS"
 
 
-class _BtrfsSnapshotter:
-    @staticmethod
-    def sync(mount_paths: set[str]) -> None:
-        for mount_path in sorted(mount_paths):
-            if global_flags.FLAGS.dryrun:
-                os_utils.eprint(f"Would sync {mount_path}")
-                continue
-            os_utils.eprint("Syncing ...", flush=True)
-            os_utils.execute_sh(f"btrfs subvolume sync {mount_path}")
+class SnapMechanism(abc.ABC):
+    """Interface with necessary methods to implement snapshotting system.
+
+    Example implementations may be based on btrfs, rsync, bcachefs, etc.
+    """
+    @abc.abstractmethod
+    def verify_volume(self, mount_point: str) -> bool:
+        pass
+
+    @abc.abstractmethod
+    def create(self, source: str, destination: str):
+        pass
+
+    @abc.abstractmethod
+    def delete(self, destination: str):
+        pass
+
+
+def _execute_sh(cmd: str):
+    if global_flags.FLAGS.dryrun:
+        os_utils.eprint("Would run " + cmd)
+    else:
+        os_utils.execute_sh(cmd)
+
+
+class _BtrfsSnapMechanism(SnapMechanism):
+    # @staticmethod
+    # def sync(mount_paths: set[str]) -> None:
+    #     for mount_path in sorted(mount_paths):
+    #         if global_flags.FLAGS.dryrun:
+    #             os_utils.eprint(f"Would sync {mount_path}")
+    #             continue
+    #         os_utils.eprint("Syncing ...", flush=True)
+    #         os_utils.execute_sh(f"btrfs subvolume sync {mount_path}")
+
+    def verify_volume(self, mount_point: str) -> bool:
+        # Based on https://stackoverflow.com/a/32865333/196462
+        fstype = os_utils.execute_sh(
+            "stat -f --format=%T " + mount_point, error_ok=True
+        )
+        if not fstype:
+            logging.warning(f"Not btrfs (cannot determine filesystem): {mount_point}")
+            return False
+        if fstype.strip() != "btrfs":
+            logging.warning(f"Not btrfs (filesystem not btrfs): {mount_point}")
+            return False
+        inodenum = os_utils.execute_sh("stat --format=%i " + mount_point, error_ok=True)
+        if not inodenum:
+            logging.warning(f"Not btrfs (cannot determine inode): {mount_point}")
+            return False
+        if inodenum.strip() != "256":
+            logging.warning(
+                f"Not btrfs (inode not 256, possibly a subdirectory of a btrfs mount): {mount_point}"
+            )
+            return False
+        return True
+
+    def create(self, source: str, destination: str):
+        try:
+            _execute_sh("btrfs subvolume snapshot -r " f"{source} {destination}")
+        except os_utils.CommandError:
+            logging.error("Unable to create; are you running as root?")
+            raise
+
+    def delete(self, destination: str):
+        try:
+            _execute_sh(f"btrfs subvolume delete {destination}")
+        except os_utils.CommandError:
+            logging.error("Unable to delete; are you running as root?")
+            raise
 
 
 @functools.cache
-def get() -> _BtrfsSnapshotter:
+def get(snap_type: SnapType) -> SnapMechanism:
     """Singleton implementation."""
-    return _BtrfsSnapshotter()
+    if snap_type == SnapType.BTRFS:
+        return _BtrfsSnapMechanism()
+    raise RuntimeError(f"Unknown snap_type {snap_type}")

src/code/snap_operator.py

@@ -22,6 +22,7 @@
 from . import human_interval
 from . import os_utils
 from . import snap_holder
+from . import snap_mechanisms
 
 from typing import Any, Iterable, Iterator, Optional, TypeVar
 
@@ -122,7 +123,7 @@ def _create_and_maintain_n_backups(
             snapshot.metadata.trigger = trigger
             if comment:
                 snapshot.metadata.comment = comment
-            snapshot.create_from(self._config.source)
+            snapshot.create_from(self._config.snap_type, self._config.source)
             self.snaps_created = True
         else:
             # From existing snaps, delete all.
@@ -203,7 +204,7 @@ def scheduled(self):
         if need_new:
             snapshot = snap_holder.Snapshot(self._config.dest_prefix + self._now_str)
             snapshot.metadata.trigger = "S"
-            snapshot.create_from(self._config.source)
+            snapshot.create_from(self._config.snap_type, self._config.source)
             self.snaps_created = True
 
     def list_snaps(self):