From ab7acceff6a1fb3104f872470c837951cb328ae3 Mon Sep 17 00:00:00 2001
From: Dan Helfman <witten@torsion.org>
Date: Thu, 21 Nov 2024 19:09:30 -0800
Subject: [PATCH] Unmount and remove mounted snapshot directories, not just for
 the current process but for previous borgmatic runs as well (#261).

---
 borgmatic/config/paths.py | 26 ++++++++++++++++++++++++++
 borgmatic/hooks/zfs.py    | 34 +++++++++++++++++++++++++---------
 2 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/borgmatic/config/paths.py b/borgmatic/config/paths.py
index 0b13e65d..e5488bd8 100644
--- a/borgmatic/config/paths.py
+++ b/borgmatic/config/paths.py
@@ -33,6 +33,32 @@ def get_borgmatic_source_directory(config):
 TEMPORARY_DIRECTORY_PREFIX = 'borgmatic-'
 
 
+def replace_temporary_subdirectory_with_glob(path):
+    '''
+    Given an absolute temporary directory path, look for a subdirectory within it starting with the
+    temporary directory prefix and replace it with an appropriate glob. For instance, given:
+
+        /tmp/borgmatic-aet8kn93/borgmatic
+
+    ... replace it with:
+
+        /tmp/borgmatic-*/borgmatic
+
+    This is useful for finding previous temporary directories from prior borgmatic runs.
+    '''
+    return os.path.join(
+        '/',
+        *(
+            (
+                f'{TEMPORARY_DIRECTORY_PREFIX}*'
+                if subdirectory.startswith(TEMPORARY_DIRECTORY_PREFIX)
+                else subdirectory
+            )
+            for subdirectory in path.split(os.path.sep)
+        )
+    )
+
+
 class Runtime_directory:
     '''
     A Python context manager for creating and cleaning up the borgmatic runtime directory used for
diff --git a/borgmatic/hooks/zfs.py b/borgmatic/hooks/zfs.py
index 70b43f7a..39d4e586 100644
--- a/borgmatic/hooks/zfs.py
+++ b/borgmatic/hooks/zfs.py
@@ -1,6 +1,8 @@
+import glob
 import logging
 import os
 import shlex
+import shutil
 import subprocess
 
 import borgmatic.config.paths
@@ -65,8 +67,7 @@ def dump_data_sources(
         (dataset_name, mount_point)
         for line in list_output.splitlines()
         for (dataset_name, mount_point, user_property_value) in (line.rstrip().split('\t'),)
-        if mount_point in source_directories_set
-        or user_property_value == 'auto'
+        if mount_point in source_directories_set or user_property_value == 'auto'
     )
 
     # Snapshot each dataset, rewriting source directories to use the snapshot paths.
@@ -97,7 +98,9 @@ def dump_data_sources(
             mount_point.lstrip(os.path.sep),
         )
         snapshot_path = os.path.normpath(snapshot_path_for_borg)
-        logger.debug(f'{log_prefix}: Mounting ZFS snapshot {full_snapshot_name} at {snapshot_path}{dry_run_label}')
+        logger.debug(
+            f'{log_prefix}: Mounting ZFS snapshot {full_snapshot_name} at {snapshot_path}{dry_run_label}'
+        )
 
         if not dry_run:
             os.makedirs(snapshot_path, mode=0o700, exist_ok=True)
@@ -154,15 +157,26 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
         for line in list_datasets_output.splitlines()
         for (dataset_name, mount_point) in (line.rstrip().split('\t'),)
     )
-    # FIXME: This doesn't necessarily find snapshot mounts from previous borgmatic runs, because
-    # borgmatic_runtime_directory could be in a tempfile-created directory that has a random name.
-    snapshots_directory = os.path.join(
-        os.path.normpath(borgmatic_runtime_directory),
+    snapshots_glob = os.path.join(
+        borgmatic.config.paths.replace_temporary_subdirectory_with_glob(
+            os.path.normpath(borgmatic_runtime_directory)
+        ),
         'zfs_snapshots',
     )
-    logger.debug(f'{log_prefix}: Looking for snapshots to remove in {snapshots_directory}{dry_run_label}')
+    logger.debug(
+        f'{log_prefix}: Looking for snapshots to remove in {snapshots_glob}{dry_run_label}'
+    )
+
+    for snapshots_directory in glob.glob(snapshots_glob):
+        if not os.path.isdir(snapshots_directory):
+            continue
+
+        # This might fail if the directory is already mounted, but we swallow errors here since
+        # we'll try again below. The point of doing it here is that we don't want to try to unmount
+        # a non-mounted directory (which *will* fail), and probing for whether a directory is
+        # mounted is tough to do in a cross-platform way.
+        shutil.rmtree(snapshots_directory, ignore_errors=True)
 
-    if os.path.isdir(snapshots_directory):
         for mount_point in mount_points:
             snapshot_path = os.path.join(snapshots_directory, mount_point.lstrip(os.path.sep))
             logger.debug(f'{log_prefix}: Unmounting ZFS snapshot at {snapshot_path}{dry_run_label}')
@@ -176,6 +190,8 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
                     output_log_level=logging.DEBUG,
                 )
 
+        shutil.rmtree(snapshots_directory)
+
     # Destroy snapshots.
     list_snapshots_command = (
         zfs_command,