diff --git a/NEWS b/NEWS index d06f0ff5..f0480a6a 100644 --- a/NEWS +++ b/NEWS @@ -1,7 +1,9 @@ 1.9.9.dev0 - * #981: Fix "spot" check file count delta error. + * #981: Fix a "spot" check file count delta error. * #982: Fix for borgmatic "exclude_patterns" and "exclude_from" recursing into excluded subdirectories. + * #983: Fix the Btrfs hook to support subvolumes with names like "@home", different from their + mount points. 1.9.8 * #979: Fix root patterns so they don't have an invalid "sh:" prefix before getting passed to Borg. diff --git a/borgmatic/hooks/data_source/btrfs.py b/borgmatic/hooks/data_source/btrfs.py index 55728205..2552608d 100644 --- a/borgmatic/hooks/data_source/btrfs.py +++ b/borgmatic/hooks/data_source/btrfs.py @@ -21,9 +21,9 @@ def use_streaming(hook_config, config, log_prefix): # pragma: no cover return False -def get_filesystem_mount_points(findmnt_command): +def get_subvolume_mount_points(findmnt_command): ''' - Given a findmnt command to run, get all top-level Btrfs filesystem mount points. + Given a findmnt command to run, get all sorted Btrfs subvolume mount points. ''' findmnt_output = borgmatic.execute.execute_command_and_capture_output( tuple(findmnt_command.split(' ')) @@ -37,7 +37,7 @@ def get_filesystem_mount_points(findmnt_command): try: return tuple( - filesystem['target'] for filesystem in json.loads(findmnt_output)['filesystems'] + sorted(filesystem['target'] for filesystem in json.loads(findmnt_output)['filesystems']) ) except json.JSONDecodeError as error: raise ValueError(f'Invalid {findmnt_command} JSON output: {error}') @@ -45,34 +45,6 @@ def get_filesystem_mount_points(findmnt_command): raise ValueError(f'Invalid {findmnt_command} output: Missing key "{error}"') -def get_subvolumes_for_filesystem(btrfs_command, filesystem_mount_point): - ''' - Given a Btrfs command to run and a Btrfs filesystem mount point, get the sorted subvolumes for - that filesystem. Include the filesystem itself. - ''' - btrfs_output = borgmatic.execute.execute_command_and_capture_output( - tuple(btrfs_command.split(' ')) - + ( - 'subvolume', - 'list', - filesystem_mount_point, - ) - ) - - if not filesystem_mount_point.strip(): - return () - - return (filesystem_mount_point,) + tuple( - sorted( - subvolume_path - for line in btrfs_output.splitlines() - for subvolume_subpath in (line.rstrip().split(' ')[-1],) - for subvolume_path in (os.path.join(filesystem_mount_point, subvolume_subpath),) - if subvolume_subpath.strip() - ) - ) - - Subvolume = collections.namedtuple('Subvolume', ('path', 'contained_patterns'), defaults=((),)) @@ -89,20 +61,16 @@ def get_subvolumes(btrfs_command, findmnt_command, patterns=None): candidate_patterns = set(patterns or ()) subvolumes = [] - # For each filesystem mount point, find its subvolumes and match them against the given patterns - # to find the subvolumes to backup. And within this loop, sort the subvolumes from longest to - # shortest mount points, so longer mount points get a whack at the candidate pattern piñata - # before their parents do. (Patterns are consumed during this process, so no two subvolumes end - # up with the same contained patterns.) - for mount_point in get_filesystem_mount_points(findmnt_command): + # For each subvolume mount point, match it against the given patterns to find the subvolumes to + # backup. Sort the subvolumes from longest to shortest mount points, so longer mount points get + # a whack at the candidate pattern piñata before their parents do. (Patterns are consumed during + # this process, so no two subvolumes end up with the same contained patterns.) + for mount_point in reversed(get_subvolume_mount_points(findmnt_command)): subvolumes.extend( - Subvolume(subvolume_path, contained_patterns) - for subvolume_path in reversed( - get_subvolumes_for_filesystem(btrfs_command, mount_point) - ) + Subvolume(mount_point, contained_patterns) for contained_patterns in ( borgmatic.hooks.data_source.snapshot.get_contained_patterns( - subvolume_path, candidate_patterns + mount_point, candidate_patterns ), ) if patterns is None or contained_patterns diff --git a/tests/unit/hooks/data_source/test_btrfs.py b/tests/unit/hooks/data_source/test_btrfs.py index 904c94cb..63dfccad 100644 --- a/tests/unit/hooks/data_source/test_btrfs.py +++ b/tests/unit/hooks/data_source/test_btrfs.py @@ -5,7 +5,7 @@ from borgmatic.borg.pattern import Pattern, Pattern_style, Pattern_type from borgmatic.hooks.data_source import btrfs as module -def test_get_filesystem_mount_points_parses_findmnt_output(): +def test_get_subvolume_mount_points_parses_findmnt_output(): flexmock(module.borgmatic.execute).should_receive( 'execute_command_and_capture_output' ).and_return( @@ -28,113 +28,60 @@ def test_get_filesystem_mount_points_parses_findmnt_output(): ''' ) - assert module.get_filesystem_mount_points('findmnt') == ('/mnt0', '/mnt1') + assert module.get_subvolume_mount_points('findmnt') == ('/mnt0', '/mnt1') -def test_get_filesystem_mount_points_with_invalid_findmnt_json_errors(): +def test_get_subvolume_mount_points_with_invalid_findmnt_json_errors(): flexmock(module.borgmatic.execute).should_receive( 'execute_command_and_capture_output' ).and_return('{') with pytest.raises(ValueError): - module.get_filesystem_mount_points('findmnt') + module.get_subvolume_mount_points('findmnt') -def test_get_filesystem_mount_points_with_findmnt_json_missing_filesystems_errors(): +def test_get_subvolume_mount_points_with_findmnt_json_missing_filesystems_errors(): flexmock(module.borgmatic.execute).should_receive( 'execute_command_and_capture_output' ).and_return('{"wtf": "something is wrong here"}') with pytest.raises(ValueError): - module.get_filesystem_mount_points('findmnt') + module.get_subvolume_mount_points('findmnt') -def test_get_subvolumes_for_filesystem_parses_subvolume_list_output(): - flexmock(module.borgmatic.execute).should_receive( - 'execute_command_and_capture_output' - ).and_return( - 'ID 270 gen 107 top level 5 path subvol1\nID 272 gen 74 top level 5 path subvol2\n' - ) +def test_get_subvolumes_collects_subvolumes_matching_patterns(): + flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2')) - assert module.get_subvolumes_for_filesystem('btrfs', '/mnt') == ( - '/mnt', - '/mnt/subvol1', - '/mnt/subvol2', - ) - - -def test_get_subvolumes_for_filesystem_skips_empty_subvolume_paths(): - flexmock(module.borgmatic.execute).should_receive( - 'execute_command_and_capture_output' - ).and_return('\n \nID 272 gen 74 top level 5 path subvol2\n') - - assert module.get_subvolumes_for_filesystem('btrfs', '/mnt') == ('/mnt', '/mnt/subvol2') - - -def test_get_subvolumes_for_filesystem_skips_empty_filesystem_mount_points(): - flexmock(module.borgmatic.execute).should_receive( - 'execute_command_and_capture_output' - ).and_return( - 'ID 270 gen 107 top level 5 path subvol1\nID 272 gen 74 top level 5 path subvol2\n' - ) - - assert module.get_subvolumes_for_filesystem('btrfs', ' ') == () - - -def test_get_subvolumes_collects_subvolumes_matching_patterns_from_all_filesystems(): - flexmock(module).should_receive('get_filesystem_mount_points').and_return(('/mnt1', '/mnt2')) - flexmock(module).should_receive('get_subvolumes_for_filesystem').with_args( - 'btrfs', '/mnt1' - ).and_return(('/one', '/two')) - flexmock(module).should_receive('get_subvolumes_for_filesystem').with_args( - 'btrfs', '/mnt2' - ).and_return(('/three', '/four')) - - for path in ('/one', '/four'): - flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( - 'get_contained_patterns' - ).with_args(path, object).and_return((Pattern(path),)) - for path in ('/two', '/three'): - flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( - 'get_contained_patterns' - ).with_args(path, object).and_return(()) + flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( + 'get_contained_patterns' + ).with_args('/mnt1', object).and_return((Pattern('/mnt1'),)) + flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( + 'get_contained_patterns' + ).with_args('/mnt2', object).and_return(()) assert module.get_subvolumes( 'btrfs', 'findmnt', patterns=[ - Pattern('/one'), - Pattern('/four'), - Pattern('/five'), - Pattern('/six'), - Pattern('/mnt2'), + Pattern('/mnt1'), Pattern('/mnt3'), ], - ) == ( - module.Subvolume('/four', contained_patterns=(Pattern('/four'),)), - module.Subvolume('/one', contained_patterns=(Pattern('/one'),)), - ) + ) == (module.Subvolume('/mnt1', contained_patterns=(Pattern('/mnt1'),)),) -def test_get_subvolumes_without_patterns_collects_all_subvolumes_from_all_filesystems(): - flexmock(module).should_receive('get_filesystem_mount_points').and_return(('/mnt1', '/mnt2')) - flexmock(module).should_receive('get_subvolumes_for_filesystem').with_args( - 'btrfs', '/mnt1' - ).and_return(('/one', '/two')) - flexmock(module).should_receive('get_subvolumes_for_filesystem').with_args( - 'btrfs', '/mnt2' - ).and_return(('/three', '/four')) +def test_get_subvolumes_without_patterns_collects_all_subvolumes(): + flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2')) - for path in ('/one', '/two', '/three', '/four'): - flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( - 'get_contained_patterns' - ).with_args(path, object).and_return((Pattern(path),)) + flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( + 'get_contained_patterns' + ).with_args('/mnt1', object).and_return((Pattern('/mnt1'),)) + flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( + 'get_contained_patterns' + ).with_args('/mnt2', object).and_return((Pattern('/mnt2'),)) assert module.get_subvolumes('btrfs', 'findmnt') == ( - module.Subvolume('/four', contained_patterns=(Pattern('/four'),)), - module.Subvolume('/one', contained_patterns=(Pattern('/one'),)), - module.Subvolume('/three', contained_patterns=(Pattern('/three'),)), - module.Subvolume('/two', contained_patterns=(Pattern('/two'),)), + module.Subvolume('/mnt1', contained_patterns=(Pattern('/mnt1'),)), + module.Subvolume('/mnt2', contained_patterns=(Pattern('/mnt2'),)), )