diff --git a/borgmatic/hooks/data_source/btrfs.py b/borgmatic/hooks/data_source/btrfs.py index e4ff9931..340763d7 100644 --- a/borgmatic/hooks/data_source/btrfs.py +++ b/borgmatic/hooks/data_source/btrfs.py @@ -22,26 +22,34 @@ def use_streaming(hook_config, config): # pragma: no cover return False -def list_subvolume_paths(btrfs_command, subvolume_path): +def get_contained_subvolume_paths(btrfs_command, subvolume_path): ''' - Given the path of a Btrfs subvolume, return it in a sequence with the paths of its contained - subvolumes. + Given the path of a Btrfs subvolume, return it in a sequence along with the paths of its + contained subvolumes. - If the Btrfs command errors, log that error and return an empty sequence. + If the btrfs command errors, log that error and return an empty sequence. ''' try: btrfs_output = borgmatic.execute.execute_command_and_capture_output( - tuple(btrfs_command.split(' ')) + ('subvolume', 'list', subvolume_path,), + tuple(btrfs_command.split(' ')) + + ( + 'subvolume', + 'list', + subvolume_path, + ), close_fds=True, ) except subprocess.CalledProcessError as error: - logger.debug(error) + logger.debug( + f'Ignoring Btrfs subvolume {subvolume_path} because of error listing its subvolumes: {error}' + ) return () return (subvolume_path,) + tuple( os.path.join(subvolume_path, line.split(' ')[-1]) for line in btrfs_output.splitlines() + if line.strip() ) @@ -74,9 +82,11 @@ def get_all_subvolume_paths(btrfs_command, findmnt_command): # a subvolume might exist at /mnt/subvolume but be mounted at /home/myuser. # findmnt is still useful though because it's a global way to discover all # Btrfs subvolumes—even if we have to do some additional legwork ourselves. - list_subvolume_paths(btrfs_command, filesystem['target']) if - FINDMNT_BTRFS_ROOT_SUBVOLUME_OPTION in filesystem['options'].split(',') - else (filesystem['target'],) + ( + get_contained_subvolume_paths(btrfs_command, filesystem['target']) + if FINDMNT_BTRFS_ROOT_SUBVOLUME_OPTION in filesystem['options'].split(',') + else (filesystem['target'],) + ) for filesystem in json.loads(findmnt_output)['filesystems'] ) ) @@ -91,6 +101,12 @@ Subvolume = collections.namedtuple('Subvolume', ('path', 'contained_patterns'), def get_subvolume_property(btrfs_command, subvolume_path, property_name): + ''' + Given a btrfs command, a subvolume path, and a property name to lookup, return the value of the + corresponding property. + + Raise subprocess.CalledProcessError if the btrfs command errors. + ''' output = borgmatic.execute.execute_command_and_capture_output( tuple(btrfs_command.split(' ')) + ( @@ -124,10 +140,15 @@ def omit_read_only_subvolume_paths(btrfs_command, subvolume_paths): retained_subvolume_paths = [] for subvolume_path in subvolume_paths: - if get_subvolume_property(btrfs_command, subvolume_path, 'ro'): - logger.debug(f'Ignoring Btrfs subvolume {subvolume_path} because it is read-only') - else: - retained_subvolume_paths.append(subvolume_path) + try: + if get_subvolume_property(btrfs_command, subvolume_path, 'ro'): + logger.debug(f'Ignoring Btrfs subvolume {subvolume_path} because it is read-only') + else: + retained_subvolume_paths.append(subvolume_path) + except subprocess.CalledProcessError as error: + logger.debug( + f'Error determining read-only status of Btrfs subvolume {subvolume_path}: {error}' + ) return tuple(retained_subvolume_paths) diff --git a/test_requirements.txt b/test_requirements.txt index 4f107722..64bca041 100644 --- a/test_requirements.txt +++ b/test_requirements.txt @@ -1,30 +1,30 @@ appdirs==1.4.4 -apprise==1.8.0 -attrs==23.2.0 -black==24.4.2 -certifi==2024.7.4 +apprise==1.9.3 +attrs==25.3.0 +black==25.1.0 +certifi==2025.6.15 chardet==5.2.0 -click==8.1.7 -codespell==2.2.6 -coverage==7.5.1 -flake8==7.0.0 +click==8.2.1 +codespell==2.4.1 +coverage==7.9.1 +flake8==7.3.0 flake8-quotes==3.4.0 flake8-use-fstring==1.4 flake8-variables-names==0.0.6 flexmock==0.12.1 -idna==3.7 -isort==5.13.2 -jsonschema==4.22.0 -Markdown==3.6 +idna==3.10 +isort==6.0.1 +jsonschema==4.24.0 +Markdown==3.8.2 mccabe==0.7.0 -packaging==24.0 +packaging==25.0 pathspec==0.12.1 -pluggy==1.5.0 +pluggy==1.6.0 py==1.11.0 -pycodestyle==2.11.1 -pyflakes==3.2.0 -pytest==8.2.1 -pytest-cov==5.0.0 +pycodestyle==2.14.0 +pyflakes==3.4.0 +pytest==8.4.1 +pytest-cov==6.2.1 PyYAML>5.0.0 regex requests==2.32.4 diff --git a/tests/unit/hooks/data_source/test_btrfs.py b/tests/unit/hooks/data_source/test_btrfs.py index fb44b91d..7493a004 100644 --- a/tests/unit/hooks/data_source/test_btrfs.py +++ b/tests/unit/hooks/data_source/test_btrfs.py @@ -5,7 +5,31 @@ from borgmatic.borg.pattern import Pattern, Pattern_source, Pattern_style, Patte from borgmatic.hooks.data_source import btrfs as module -def test_get_subvolume_mount_points_parses_findmnt_output(): +def test_get_contained_subvolume_paths_parses_btrfs_output(): + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output' + ).with_args(('btrfs', 'subvolume', 'list', '/mnt0'), close_fds=True).and_return( + 'ID 256 gen 28 top level 5 path @sub\nID 258 gen 17 top level 5 path snap\n\n' + ) + + assert module.get_contained_subvolume_paths('btrfs', '/mnt0') == ( + '/mnt0', + '/mnt0/@sub', + '/mnt0/snap', + ) + + +def test_get_contained_subvolume_paths_swallows_called_process_error(): + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output' + ).with_args(('btrfs', 'subvolume', 'list', '/mnt0'), close_fds=True).and_raise( + module.subprocess.CalledProcessError(1, 'btrfs') + ) + + assert module.get_contained_subvolume_paths('btrfs', '/mnt0') == () + + +def test_get_all_subvolume_paths_parses_findmnt_output(): flexmock(module.borgmatic.execute).should_receive( 'execute_command_and_capture_output' ).and_return( @@ -22,31 +46,53 @@ def test_get_subvolume_mount_points_parses_findmnt_output(): "source": "/dev/loop0", "fstype": "btrfs", "options": "rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/" + }, + { + "target": "/mnt2", + "source": "/dev/loop0", + "fstype": "btrfs", + "options": "rw,relatime,ssd,space_cache=v2,subvolid=256,subvol=/" } ] } ''' ) + flexmock(module).should_receive('get_contained_subvolume_paths').with_args( + 'btrfs', '/mnt0' + ).and_return(('/mnt0',)) + flexmock(module).should_receive('get_contained_subvolume_paths').with_args( + 'btrfs', '/mnt1' + ).and_return(('/mnt1', '/mnt1/sub')) + flexmock(module).should_receive('get_contained_subvolume_paths').with_args( + 'btrfs', '/mnt2' + ).never() - assert module.get_subvolume_mount_points('findmnt') == ('/mnt0', '/mnt1') + assert module.get_all_subvolume_paths('btrfs', 'findmnt') == ( + '/mnt0', + '/mnt1', + '/mnt1/sub', + '/mnt2', + ) -def test_get_subvolume_mount_points_with_invalid_findmnt_json_errors(): +def test_get_all_subvolume_paths_with_invalid_findmnt_json_errors(): flexmock(module.borgmatic.execute).should_receive( 'execute_command_and_capture_output' ).and_return('{') + flexmock(module).should_receive('get_contained_subvolume_paths').never() with pytest.raises(ValueError): - module.get_subvolume_mount_points('findmnt') + module.get_all_subvolume_paths('btrfs', 'findmnt') -def test_get_subvolume_mount_points_with_findmnt_json_missing_filesystems_errors(): +def test_get_all_subvolume_paths_with_findmnt_json_missing_filesystems_errors(): flexmock(module.borgmatic.execute).should_receive( 'execute_command_and_capture_output' ).and_return('{"wtf": "something is wrong here"}') + flexmock(module).should_receive('get_contained_subvolume_paths').never() with pytest.raises(ValueError): - module.get_subvolume_mount_points('findmnt') + module.get_all_subvolume_paths('btrfs', 'findmnt') def test_get_subvolume_property_with_invalid_btrfs_output_errors(): @@ -82,7 +128,7 @@ def test_get_subvolume_property_passes_through_general_value(): assert module.get_subvolume_property('btrfs', '/foo', 'thing') == 'value' -def test_omit_read_only_subvolume_mount_points_filters_out_read_only(): +def test_omit_read_only_subvolume_paths_filters_out_read_only_subvolumes(): flexmock(module).should_receive('get_subvolume_property').with_args( 'btrfs', '/foo', 'ro' ).and_return(False) @@ -93,17 +139,29 @@ def test_omit_read_only_subvolume_mount_points_filters_out_read_only(): 'btrfs', '/baz', 'ro' ).and_return(False) - assert module.omit_read_only_subvolume_mount_points('btrfs', ('/foo', '/bar', '/baz')) == ( + assert module.omit_read_only_subvolume_paths('btrfs', ('/foo', '/bar', '/baz')) == ( '/foo', '/baz', ) +def test_omit_read_only_subvolume_paths_filters_out_erroring_subvolumes(): + flexmock(module).should_receive('get_subvolume_property').with_args( + 'btrfs', '/foo', 'ro' + ).and_raise(module.subprocess.CalledProcessError(1, 'btrfs')) + flexmock(module).should_receive('get_subvolume_property').with_args( + 'btrfs', '/bar', 'ro' + ).and_return(True) + flexmock(module).should_receive('get_subvolume_property').with_args( + 'btrfs', '/baz', 'ro' + ).and_return(False) + + assert module.omit_read_only_subvolume_paths('btrfs', ('/foo', '/bar', '/baz')) == ('/baz',) + + def test_get_subvolumes_collects_subvolumes_matching_patterns(): - flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2')) - flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return( - ('/mnt1', '/mnt2') - ) + flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2')) + flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2')) contained_pattern = Pattern( '/mnt1', @@ -128,10 +186,8 @@ def test_get_subvolumes_collects_subvolumes_matching_patterns(): def test_get_subvolumes_skips_non_root_patterns(): - flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2')) - flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return( - ('/mnt1', '/mnt2') - ) + flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2')) + flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2')) flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( 'get_contained_patterns' @@ -162,10 +218,8 @@ def test_get_subvolumes_skips_non_root_patterns(): def test_get_subvolumes_skips_non_config_patterns(): - flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2')) - flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return( - ('/mnt1', '/mnt2') - ) + flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2')) + flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2')) flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( 'get_contained_patterns' @@ -196,10 +250,8 @@ def test_get_subvolumes_skips_non_config_patterns(): def test_get_subvolumes_without_patterns_collects_all_subvolumes(): - flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2')) - flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return( - ('/mnt1', '/mnt2') - ) + flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2')) + flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2')) flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive( 'get_contained_patterns'