Browse Source

Fix path handling error when handling btrfs '/' subvolume.

Merge pull request #89 from dmitry-t7ko/btrfs-root-submodule-fix
Dan Helfman 3 months ago
parent
commit
bf850b9d38
2 changed files with 61 additions and 3 deletions
  1. 4 3
      borgmatic/hooks/data_source/btrfs.py
  2. 57 0
      tests/unit/hooks/data_source/test_btrfs.py

+ 4 - 3
borgmatic/hooks/data_source/btrfs.py

@@ -299,9 +299,10 @@ def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, d
                 logger.debug(error)
                 return
 
-            # Strip off the subvolume path from the end of the snapshot path and then delete the
-            # resulting directory.
-            shutil.rmtree(snapshot_path.rsplit(subvolume.path, 1)[0])
+            # Remove snapshot parent directory if it still exists (might not exist if snapshot was for '/')
+            snapshot_parent_dir = snapshot_path.rsplit(subvolume.path, 1)[0]
+            if os.path.isdir(snapshot_parent_dir):
+                shutil.rmtree(snapshot_parent_dir)
 
 
 def make_data_source_dump_patterns(

+ 57 - 0
tests/unit/hooks/data_source/test_btrfs.py

@@ -520,6 +520,18 @@ def test_remove_data_source_dumps_deletes_snapshots():
     flexmock(module).should_receive('delete_snapshot').with_args(
         'btrfs', '/mnt/subvol2/.borgmatic-5678/mnt/subvol2'
     ).never()
+    flexmock(module.os.path).should_receive('isdir').with_args(
+        '/mnt/subvol1/.borgmatic-1234'
+    ).and_return(True)
+    flexmock(module.os.path).should_receive('isdir').with_args(
+        '/mnt/subvol1/.borgmatic-5678'
+    ).and_return(True)
+    flexmock(module.os.path).should_receive('isdir').with_args(
+        '/mnt/subvol2/.borgmatic-1234'
+    ).and_return(True)
+    flexmock(module.os.path).should_receive('isdir').with_args(
+        '/mnt/subvol2/.borgmatic-5678'
+    ).and_return(True)
     flexmock(module.shutil).should_receive('rmtree').with_args(
         '/mnt/subvol1/.borgmatic-1234'
     ).once()
@@ -846,3 +858,48 @@ def test_remove_data_source_dumps_with_delete_snapshot_called_process_error_bail
         borgmatic_runtime_directory='/run/borgmatic',
         dry_run=False,
     )
+
+
+def test_remove_data_source_dumps_with_root_subvolume():
+    config = {'btrfs': {}}
+    flexmock(module).should_receive('get_subvolumes').and_return(
+        (module.Subvolume('/', contained_patterns=(Pattern('/etc'),)),)
+    )
+
+    flexmock(module).should_receive('make_snapshot_path').with_args('/').and_return(
+        '/.borgmatic-1234'
+    )
+
+    flexmock(module.borgmatic.config.paths).should_receive(
+        'replace_temporary_subdirectory_with_glob'
+    ).with_args(
+        '/.borgmatic-1234',
+        temporary_directory_prefix=module.BORGMATIC_SNAPSHOT_PREFIX,
+    ).and_return(
+        '/.borgmatic-*'
+    )
+
+    flexmock(module.glob).should_receive('glob').with_args('/.borgmatic-*').and_return(
+        ('/.borgmatic-1234', '/.borgmatic-5678')
+    )
+
+    flexmock(module.os.path).should_receive('isdir').with_args('/.borgmatic-1234').and_return(
+        True
+    ).and_return(False)
+    flexmock(module.os.path).should_receive('isdir').with_args('/.borgmatic-5678').and_return(
+        True
+    ).and_return(False)
+
+    flexmock(module).should_receive('delete_snapshot').with_args('btrfs', '/.borgmatic-1234').once()
+    flexmock(module).should_receive('delete_snapshot').with_args('btrfs', '/.borgmatic-5678').once()
+
+    flexmock(module.os.path).should_receive('isdir').with_args('').and_return(False)
+
+    flexmock(module.shutil).should_receive('rmtree').never()
+
+    module.remove_data_source_dumps(
+        hook_config=config['btrfs'],
+        config=config,
+        borgmatic_runtime_directory='/run/borgmatic',
+        dry_run=False,
+    )