diff --git a/borgmatic/hooks/data_source/lvm.py b/borgmatic/hooks/data_source/lvm.py index ce9aa5a0..a88fe053 100644 --- a/borgmatic/hooks/data_source/lvm.py +++ b/borgmatic/hooks/data_source/lvm.py @@ -244,7 +244,8 @@ def delete_snapshot(lvremove_command, snapshot_device_path): # pragma: no cover Snapshot = collections.namedtuple( - 'Snapshot', ('name', 'device_path'), + 'Snapshot', + ('name', 'device_path'), ) diff --git a/borgmatic/hooks/data_source/zfs.py b/borgmatic/hooks/data_source/zfs.py index 6b688421..c472484d 100644 --- a/borgmatic/hooks/data_source/zfs.py +++ b/borgmatic/hooks/data_source/zfs.py @@ -24,7 +24,9 @@ BORGMATIC_USER_PROPERTY = 'org.torsion.borgmatic:backup' Dataset = collections.namedtuple( - 'Dataset', ('name', 'mount_point', 'auto_backup', 'contained_source_directories') + 'Dataset', + ('name', 'mount_point', 'auto_backup', 'contained_source_directories'), + defaults=(False, ()), ) @@ -69,26 +71,29 @@ def get_datasets_to_backup(zfs_command, source_directories): candidate_source_directories = set(source_directories) - return sorted( - tuple( - Dataset( - dataset.name, - dataset.mount_point, - dataset.auto_backup, - contained_source_directories, - ) - for dataset in datasets - for contained_source_directories in ( - ( - ((dataset.mount_point,) if dataset.auto_backup else ()) - + borgmatic.hooks.data_source.snapshot.get_contained_directories( - dataset.mount_point, candidate_source_directories - ) - ), - ) - if contained_source_directories - ), - key=lambda dataset: dataset.mount_point, + return tuple( + sorted( + ( + Dataset( + dataset.name, + dataset.mount_point, + dataset.auto_backup, + contained_source_directories, + ) + for dataset in datasets + for contained_source_directories in ( + ( + (dataset.mount_point,) + if dataset.auto_backup + else borgmatic.hooks.data_source.snapshot.get_contained_directories( + dataset.mount_point, candidate_source_directories + ) + ), + ) + if contained_source_directories + ), + key=lambda dataset: dataset.mount_point, + ) ) @@ -108,10 +113,7 @@ def get_all_dataset_mount_points(zfs_command): ) ) - try: - return tuple(sorted(line.rstrip() for line in list_output.splitlines())) - except ValueError: - raise ValueError('Invalid {zfs_command} list output') + return tuple(sorted(line.rstrip() for line in list_output.splitlines())) def snapshot_dataset(zfs_command, full_snapshot_name): # pragma: no cover diff --git a/tests/unit/hooks/data_source/test_zfs.py b/tests/unit/hooks/data_source/test_zfs.py index 2eea24f7..954074fe 100644 --- a/tests/unit/hooks/data_source/test_zfs.py +++ b/tests/unit/hooks/data_source/test_zfs.py @@ -1,3 +1,5 @@ +import os + import pytest from flexmock import flexmock @@ -10,10 +12,18 @@ def test_get_datasets_to_backup_filters_datasets_by_source_directories(): ).and_return( 'dataset\t/dataset\t-\nother\t/other\t-', ) + flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( + 'get_contained_directories' + ).with_args('/dataset', object).and_return(('/dataset',)) + flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( + 'get_contained_directories' + ).with_args('/other', object).and_return(()) assert module.get_datasets_to_backup( 'zfs', source_directories=('/foo', '/dataset', '/bar') - ) == (('dataset', '/dataset'),) + ) == ( + module.Dataset(name='dataset', mount_point='/dataset', contained_source_directories=('/dataset',)), + ) def test_get_datasets_to_backup_filters_datasets_by_user_property(): @@ -22,9 +32,20 @@ def test_get_datasets_to_backup_filters_datasets_by_user_property(): ).and_return( 'dataset\t/dataset\tauto\nother\t/other\t-', ) + flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( + 'get_contained_directories' + ).with_args('/dataset', object).never() + flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( + 'get_contained_directories' + ).with_args('/other', object).and_return(()) assert module.get_datasets_to_backup('zfs', source_directories=('/foo', '/bar')) == ( - ('dataset', '/dataset'), + module.Dataset( + name='dataset', + mount_point='/dataset', + auto_backup=True, + contained_source_directories=('/dataset',), + ), ) @@ -34,38 +55,39 @@ def test_get_datasets_to_backup_with_invalid_list_output_raises(): ).and_return( 'dataset', ) + flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( + 'get_contained_directories' + ).never() with pytest.raises(ValueError, match='zfs'): module.get_datasets_to_backup('zfs', source_directories=('/foo', '/bar')) -def test_get_get_all_datasets_does_not_filter_datasets(): +def test_get_all_dataset_mount_points_does_not_filter_datasets(): flexmock(module.borgmatic.execute).should_receive( 'execute_command_and_capture_output' ).and_return( - 'dataset\t/dataset\nother\t/other', + '/dataset\n/other', ) + flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( + 'get_contained_directories' + ).and_return(('/dataset',)) - assert module.get_all_datasets('zfs') == ( - ('dataset', '/dataset'), - ('other', '/other'), + assert module.get_all_dataset_mount_points('zfs') == ( + ('/dataset'), + ('/other'), ) -def test_get_all_datasets_with_invalid_list_output_raises(): - flexmock(module.borgmatic.execute).should_receive( - 'execute_command_and_capture_output' - ).and_return( - 'dataset', - ) - - with pytest.raises(ValueError, match='zfs'): - module.get_all_datasets('zfs') - - def test_dump_data_sources_snapshots_and_mounts_and_updates_source_directories(): flexmock(module).should_receive('get_datasets_to_backup').and_return( - (('dataset', '/mnt/dataset'),) + ( + flexmock( + name='dataset', + mount_point='/mnt/dataset', + contained_source_directories=('/mnt/dataset/subdir',), + ) + ) ) flexmock(module.os).should_receive('getpid').and_return(1234) full_snapshot_name = 'dataset@borgmatic-1234' @@ -79,7 +101,7 @@ def test_dump_data_sources_snapshots_and_mounts_and_updates_source_directories() full_snapshot_name, module.os.path.normpath(snapshot_mount_path), ).once() - source_directories = ['/mnt/dataset'] + source_directories = ['/mnt/dataset/subdir'] assert ( module.dump_data_sources( @@ -94,7 +116,7 @@ def test_dump_data_sources_snapshots_and_mounts_and_updates_source_directories() == [] ) - assert source_directories == [snapshot_mount_path] + assert source_directories == [os.path.join(snapshot_mount_path, 'subdir')] def test_dump_data_sources_snapshots_with_no_datasets_skips_snapshots(): @@ -122,7 +144,13 @@ def test_dump_data_sources_snapshots_with_no_datasets_skips_snapshots(): def test_dump_data_sources_uses_custom_commands(): flexmock(module).should_receive('get_datasets_to_backup').and_return( - (('dataset', '/mnt/dataset'),) + ( + flexmock( + name='dataset', + mount_point='/mnt/dataset', + contained_source_directories=('/mnt/dataset/subdir',), + ) + ) ) flexmock(module.os).should_receive('getpid').and_return(1234) full_snapshot_name = 'dataset@borgmatic-1234' @@ -136,7 +164,7 @@ def test_dump_data_sources_uses_custom_commands(): full_snapshot_name, module.os.path.normpath(snapshot_mount_path), ).once() - source_directories = ['/mnt/dataset'] + source_directories = ['/mnt/dataset/subdir'] hook_config = { 'zfs_command': '/usr/local/bin/zfs', 'mount_command': '/usr/local/bin/mount', @@ -158,12 +186,12 @@ def test_dump_data_sources_uses_custom_commands(): == [] ) - assert source_directories == [snapshot_mount_path] + assert source_directories == [os.path.join(snapshot_mount_path, 'subdir')] def test_dump_data_sources_with_dry_run_skips_commands_and_does_not_touch_source_directories(): flexmock(module).should_receive('get_datasets_to_backup').and_return( - (('dataset', '/mnt/dataset'),) + (flexmock(name='dataset', mount_point='/mnt/dataset'),) ) flexmock(module.os).should_receive('getpid').and_return(1234) flexmock(module).should_receive('snapshot_dataset').never() @@ -197,7 +225,7 @@ def test_get_all_snapshots_parses_list_output(): def test_remove_data_source_dumps_unmounts_and_destroys_snapshots(): - flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/mnt/dataset'),)) + flexmock(module).should_receive('get_all_dataset_mount_points').and_return(('/mnt/dataset',)) flexmock(module.borgmatic.config.paths).should_receive( 'replace_temporary_subdirectory_with_glob' ).and_return('/run/borgmatic') @@ -224,7 +252,7 @@ def test_remove_data_source_dumps_unmounts_and_destroys_snapshots(): def test_remove_data_source_dumps_use_custom_commands(): - flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/mnt/dataset'),)) + flexmock(module).should_receive('get_all_dataset_mount_points').and_return(('/mnt/dataset',)) flexmock(module.borgmatic.config.paths).should_receive( 'replace_temporary_subdirectory_with_glob' ).and_return('/run/borgmatic') @@ -252,7 +280,7 @@ def test_remove_data_source_dumps_use_custom_commands(): def test_remove_data_source_dumps_bails_for_missing_zfs_command(): - flexmock(module).should_receive('get_all_datasets').and_raise(FileNotFoundError) + flexmock(module).should_receive('get_all_dataset_mount_points').and_raise(FileNotFoundError) flexmock(module.borgmatic.config.paths).should_receive( 'replace_temporary_subdirectory_with_glob' ).never() @@ -268,7 +296,7 @@ def test_remove_data_source_dumps_bails_for_missing_zfs_command(): def test_remove_data_source_dumps_bails_for_zfs_command_error(): - flexmock(module).should_receive('get_all_datasets').and_raise( + flexmock(module).should_receive('get_all_dataset_mount_points').and_raise( module.subprocess.CalledProcessError(1, 'wtf') ) flexmock(module.borgmatic.config.paths).should_receive( @@ -286,7 +314,7 @@ def test_remove_data_source_dumps_bails_for_zfs_command_error(): def test_remove_data_source_dumps_bails_for_missing_umount_command(): - flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/mnt/dataset'),)) + flexmock(module).should_receive('get_all_dataset_mount_points').and_return(('/mnt/dataset',)) flexmock(module.borgmatic.config.paths).should_receive( 'replace_temporary_subdirectory_with_glob' ).and_return('/run/borgmatic') @@ -310,7 +338,7 @@ def test_remove_data_source_dumps_bails_for_missing_umount_command(): def test_remove_data_source_dumps_bails_for_umount_command_error(): - flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/mnt/dataset'),)) + flexmock(module).should_receive('get_all_dataset_mount_points').and_return(('/mnt/dataset',)) flexmock(module.borgmatic.config.paths).should_receive( 'replace_temporary_subdirectory_with_glob' ).and_return('/run/borgmatic') @@ -334,7 +362,7 @@ def test_remove_data_source_dumps_bails_for_umount_command_error(): def test_remove_data_source_dumps_skips_unmount_snapshot_directories_that_are_not_actually_directories(): - flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/mnt/dataset'),)) + flexmock(module).should_receive('get_all_dataset_mount_points').and_return(('/mnt/dataset',)) flexmock(module.borgmatic.config.paths).should_receive( 'replace_temporary_subdirectory_with_glob' ).and_return('/run/borgmatic') @@ -359,7 +387,7 @@ def test_remove_data_source_dumps_skips_unmount_snapshot_directories_that_are_no def test_remove_data_source_dumps_skips_unmount_snapshot_mount_paths_that_are_not_actually_directories(): - flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/mnt/dataset'),)) + flexmock(module).should_receive('get_all_dataset_mount_points').and_return(('/mnt/dataset',)) flexmock(module.borgmatic.config.paths).should_receive( 'replace_temporary_subdirectory_with_glob' ).and_return('/run/borgmatic') @@ -389,7 +417,7 @@ def test_remove_data_source_dumps_skips_unmount_snapshot_mount_paths_that_are_no def test_remove_data_source_dumps_with_dry_run_skips_unmount_and_destroy(): - flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/mnt/dataset'),)) + flexmock(module).should_receive('get_all_dataset_mount_points').and_return(('/mnt/dataset',)) flexmock(module.borgmatic.config.paths).should_receive( 'replace_temporary_subdirectory_with_glob' ).and_return('/run/borgmatic')