From 92e87d839da779808537a1c18835ad8c1d7a75a0 Mon Sep 17 00:00:00 2001 From: Dmitrii Tishchenko Date: Thu, 13 Feb 2025 16:12:58 +0000 Subject: [PATCH 1/2] Fix path handling error when handling btrfs '/' submodule --- borgmatic/hooks/data_source/btrfs.py | 8 +-- tests/unit/hooks/data_source/test_btrfs.py | 57 ++++++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/borgmatic/hooks/data_source/btrfs.py b/borgmatic/hooks/data_source/btrfs.py index a7d8f2f5..d703d332 100644 --- a/borgmatic/hooks/data_source/btrfs.py +++ b/borgmatic/hooks/data_source/btrfs.py @@ -299,9 +299,11 @@ 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) + continue def make_data_source_dump_patterns( diff --git a/tests/unit/hooks/data_source/test_btrfs.py b/tests/unit/hooks/data_source/test_btrfs.py index c69d5ac1..8e7055c2 100644 --- a/tests/unit/hooks/data_source/test_btrfs.py +++ b/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, + ) From 653d8c0946bbc299b8a259bc0ccd6b10c961022f Mon Sep 17 00:00:00 2001 From: Dmitrii Tishchenko Date: Thu, 13 Feb 2025 21:44:45 +0000 Subject: [PATCH 2/2] Remove unneeded 'continue' --- borgmatic/hooks/data_source/btrfs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/borgmatic/hooks/data_source/btrfs.py b/borgmatic/hooks/data_source/btrfs.py index d703d332..99e75b26 100644 --- a/borgmatic/hooks/data_source/btrfs.py +++ b/borgmatic/hooks/data_source/btrfs.py @@ -303,7 +303,6 @@ def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, d snapshot_parent_dir = snapshot_path.rsplit(subvolume.path, 1)[0] if os.path.isdir(snapshot_parent_dir): shutil.rmtree(snapshot_parent_dir) - continue def make_data_source_dump_patterns(