From 49b8b693af925f1fc5d754df71d3bf45dc600801 Mon Sep 17 00:00:00 2001
From: Dan Helfman <witten@torsion.org>
Date: Wed, 4 Dec 2024 15:39:04 -0800
Subject: [PATCH] Don't try to unmount a ZFS snapshot if it's already deleted
 (#80).

---
 borgmatic/hooks/data_source/btrfs.py       |  2 +-
 borgmatic/hooks/data_source/lvm.py         |  4 ++++
 borgmatic/hooks/data_source/snapshot.py    |  4 ++--
 borgmatic/hooks/data_source/zfs.py         |  4 ++++
 tests/unit/hooks/data_source/test_btrfs.py | 20 +++++++++++++-------
 5 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/borgmatic/hooks/data_source/btrfs.py b/borgmatic/hooks/data_source/btrfs.py
index 927e81cb..5d6aa134 100644
--- a/borgmatic/hooks/data_source/btrfs.py
+++ b/borgmatic/hooks/data_source/btrfs.py
@@ -60,7 +60,7 @@ def get_subvolumes_for_filesystem(btrfs_command, filesystem_mount_point):
     )
 
 
-Subvolume = collections.namedtuple('Subvolume', ('path', 'contained_source_directories'))
+Subvolume = collections.namedtuple('Subvolume', ('path', 'contained_source_directories'), defaults=(()))
 
 
 def get_subvolumes(btrfs_command, findmnt_command, source_directories=None):
diff --git a/borgmatic/hooks/data_source/lvm.py b/borgmatic/hooks/data_source/lvm.py
index a88fe053..27a83485 100644
--- a/borgmatic/hooks/data_source/lvm.py
+++ b/borgmatic/hooks/data_source/lvm.py
@@ -330,6 +330,10 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
             if not dry_run:
                 shutil.rmtree(snapshot_mount_path, ignore_errors=True)
 
+            # If the delete was successful, that means there's nothing to unmount.
+            if not os.path.isdir(snapshot_mount_path):
+                continue
+
             logger.debug(
                 f'{log_prefix}: Unmounting LVM snapshot at {snapshot_mount_path}{dry_run_label}'
             )
diff --git a/borgmatic/hooks/data_source/snapshot.py b/borgmatic/hooks/data_source/snapshot.py
index f5ef4575..9567d226 100644
--- a/borgmatic/hooks/data_source/snapshot.py
+++ b/borgmatic/hooks/data_source/snapshot.py
@@ -24,8 +24,8 @@ def get_contained_directories(parent_directory, candidate_contained_directories)
     contained = tuple(
         candidate
         for candidate in candidate_contained_directories
-        if parent_directory == candidate
-        or pathlib.PurePosixPath(parent_directory) in pathlib.PurePath(candidate).parents
+        if pathlib.PurePath(parent_directory) == pathlib.PurePath(candidate)
+        or pathlib.PurePath(parent_directory) in pathlib.PurePath(candidate).parents
     )
     candidate_contained_directories -= set(contained)
 
diff --git a/borgmatic/hooks/data_source/zfs.py b/borgmatic/hooks/data_source/zfs.py
index c472484d..794712ae 100644
--- a/borgmatic/hooks/data_source/zfs.py
+++ b/borgmatic/hooks/data_source/zfs.py
@@ -330,6 +330,10 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
             if not dry_run:
                 shutil.rmtree(snapshot_mount_path, ignore_errors=True)
 
+            # If the delete was successful, that means there's nothing to unmount.
+            if not os.path.isdir(snapshot_mount_path):
+                continue
+
             logger.debug(
                 f'{log_prefix}: Unmounting ZFS snapshot at {snapshot_mount_path}{dry_run_label}'
             )
diff --git a/tests/unit/hooks/data_source/test_btrfs.py b/tests/unit/hooks/data_source/test_btrfs.py
index f983a66d..4a9f3fb6 100644
--- a/tests/unit/hooks/data_source/test_btrfs.py
+++ b/tests/unit/hooks/data_source/test_btrfs.py
@@ -51,9 +51,12 @@ def test_get_subvolumes_collects_subvolumes_matching_source_directories_from_all
         'btrfs', '/mnt2'
     ).and_return(('/three', '/four'))
 
+    for path in ('/one', '/two', '/three', '/four'):
+        flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive('get_contained_directories').with_args(path).and_return((path,))
+
     assert module.get_subvolumes(
         'btrfs', 'findmnt', source_directories=['/one', '/four', '/five', '/six', '/mnt2', '/mnt3']
-    ) == ('/one', '/mnt2', '/four')
+    ) == (module.Subvolume('/one'), module.Subvolume('/mnt2'), module.Subvolume('/four'))
 
 
 def test_get_subvolumes_without_source_directories_collects_all_subvolumes_from_all_filesystems():
@@ -65,13 +68,16 @@ def test_get_subvolumes_without_source_directories_collects_all_subvolumes_from_
         'btrfs', '/mnt2'
     ).and_return(('/three', '/four'))
 
+    for path in ('/one', '/two', '/three', '/four'):
+        flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive('get_contained_directories').with_args(path).and_return((path,))
+
     assert module.get_subvolumes('btrfs', 'findmnt') == (
-        '/mnt1',
-        '/one',
-        '/two',
-        '/mnt2',
-        '/three',
-        '/four',
+        module.Subvolume('/mnt1'),
+        module.Subvolume('/one'),
+        module.Subvolume('/two'),
+        module.Subvolume('/mnt2'),
+        module.Subvolume('/three'),
+        module.Subvolume('/four'),
     )