From 267af5b372cd28025d2db286ca4cd8cb9bf51b71 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 19 Dec 2024 20:59:57 -0800 Subject: [PATCH] To avoid a hang in the database hooks, error and exit when the borgmatic runtime directory overlaps with the configured excludes (#947). --- NEWS | 2 ++ borgmatic/borg/create.py | 41 ++++++++++++++++++--------- tests/unit/borg/test_create.py | 51 ++++++++++++++++++++++++++-------- 3 files changed, 70 insertions(+), 24 deletions(-) diff --git a/NEWS b/NEWS index 925c8b28..af57248d 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ 1.9.5.dev0 + * #947: To avoid a hang in the database hooks, error and exit when the borgmatic runtime + directory overlaps with the configured excludes. * #954: Fix findmnt command error in the Btrfs hook by switching to parsing JSON output. * When the ZFS, Btrfs, or LVM hooks aren't configured, don't try to cleanup snapshots for them. diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 49f0ea89..5ed0cfaf 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -160,14 +160,24 @@ def any_parent_directories(path, candidate_parents): def collect_special_file_paths( - create_command, config, local_path, working_directory, borg_environment, skip_directories + create_command, + config, + local_path, + working_directory, + borg_environment, + borgmatic_runtime_directory, ): ''' Given a Borg create command as a tuple, a configuration dict, a local Borg path, a working - directory, a dict of environment variables to pass to Borg, and a sequence of parent directories - to skip, collect the paths for any special files (character devices, block devices, and named - pipes / FIFOs) that Borg would encounter during a create. These are all paths that could cause - Borg to hang if its --read-special flag is used. + directory, a dict of environment variables to pass to Borg, and the borgmatic runtime directory, + collect the paths for any special files (character devices, block devices, and named pipes / + FIFOs) that Borg would encounter during a create. These are all paths that could cause Borg to + hang if its --read-special flag is used. + + Skip looking for special files in the given borgmatic runtime directory, as borgmatic creates + its own special files there for database dumps. And if the borgmatic runtime directory is + configured to be excluded from the files Borg backs up, error, because this means Borg won't be + able to consume any database dumps and therefore borgmatic will hang. ''' # Omit "--exclude-nodump" from the Borg dry run command, because that flag causes Borg to open # files including any named pipe we've created. @@ -186,12 +196,19 @@ def collect_special_file_paths( for path_line in paths_output.split('\n') if path_line and path_line.startswith('- ') or path_line.startswith('+ ') ) + skip_paths = {} - return tuple( - path - for path in paths - if special_file(path) and not any_parent_directories(path, skip_directories) - ) + if os.path.exists(borgmatic_runtime_directory): + skip_paths = { + path for path in paths if any_parent_directories(path, (borgmatic_runtime_directory,)) + } + + if not skip_paths: + raise ValueError( + f'The runtime directory {os.path.normpath(borgmatic_runtime_directory)} overlaps with the configured excludes. Please remove it from excludes or change the runtime directory.' + ) + + return tuple(path for path in paths if special_file(path) if path not in skip_paths) def check_all_source_directories_exist(source_directories): @@ -335,9 +352,7 @@ def make_base_create_command( local_path, working_directory, borg_environment, - skip_directories=( - [borgmatic_runtime_directory] if os.path.exists(borgmatic_runtime_directory) else [] - ), + borgmatic_runtime_directory=borgmatic_runtime_directory, ) if special_file_paths: diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index bc91986d..b6c088fc 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -275,7 +275,8 @@ def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_ 'Processing files ...\n- /foo\n+ /bar\n- /baz' ) flexmock(module).should_receive('special_file').and_return(True) - flexmock(module).should_receive('any_parent_directories').and_return(False) + flexmock(module.os.path).should_receive('exists').and_return(False) + flexmock(module).should_receive('any_parent_directories').never() assert module.collect_special_file_paths( ('borg', 'create'), @@ -283,17 +284,24 @@ def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_ local_path=None, working_directory=None, borg_environment=None, - skip_directories=flexmock(), + borgmatic_runtime_directory='/run/borgmatic', ) == ('/foo', '/bar', '/baz') -def test_collect_special_file_paths_excludes_requested_directories(): +def test_collect_special_file_paths_skips_borgmatic_runtime_directory(): flexmock(module).should_receive('execute_command_and_capture_output').and_return( - '+ /foo\n- /bar\n- /baz' + '+ /foo\n- /run/borgmatic/bar\n- /baz' ) flexmock(module).should_receive('special_file').and_return(True) - flexmock(module).should_receive('any_parent_directories').and_return(False).and_return( - True + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module).should_receive('any_parent_directories').with_args( + '/foo', ('/run/borgmatic',) + ).and_return(False) + flexmock(module).should_receive('any_parent_directories').with_args( + '/run/borgmatic/bar', ('/run/borgmatic',) + ).and_return(True) + flexmock(module).should_receive('any_parent_directories').with_args( + '/baz', ('/run/borgmatic',) ).and_return(False) assert module.collect_special_file_paths( @@ -302,10 +310,29 @@ def test_collect_special_file_paths_excludes_requested_directories(): local_path=None, working_directory=None, borg_environment=None, - skip_directories=flexmock(), + borgmatic_runtime_directory='/run/borgmatic', ) == ('/foo', '/baz') +def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors(): + flexmock(module).should_receive('execute_command_and_capture_output').and_return( + '+ /foo\n- /bar\n- /baz' + ) + flexmock(module).should_receive('special_file').and_return(True) + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module).should_receive('any_parent_directories').and_return(False) + + with pytest.raises(ValueError): + module.collect_special_file_paths( + ('borg', 'create'), + config={}, + local_path=None, + working_directory=None, + borg_environment=None, + borgmatic_runtime_directory='/run/borgmatic', + ) + + def test_collect_special_file_paths_excludes_non_special_files(): flexmock(module).should_receive('execute_command_and_capture_output').and_return( '+ /foo\n+ /bar\n+ /baz' @@ -313,7 +340,8 @@ def test_collect_special_file_paths_excludes_non_special_files(): flexmock(module).should_receive('special_file').and_return(True).and_return(False).and_return( True ) - flexmock(module).should_receive('any_parent_directories').and_return(False) + flexmock(module.os.path).should_receive('exists').and_return(False) + flexmock(module).should_receive('any_parent_directories').never() assert module.collect_special_file_paths( ('borg', 'create'), @@ -321,7 +349,7 @@ def test_collect_special_file_paths_excludes_non_special_files(): local_path=None, working_directory=None, borg_environment=None, - skip_directories=flexmock(), + borgmatic_runtime_directory='/run/borgmatic', ) == ('/foo', '/baz') @@ -335,7 +363,8 @@ def test_collect_special_file_paths_omits_exclude_no_dump_flag_from_command(): borg_exit_codes=None, ).and_return('Processing files ...\n- /foo\n+ /bar\n- /baz').once() flexmock(module).should_receive('special_file').and_return(True) - flexmock(module).should_receive('any_parent_directories').and_return(False) + flexmock(module.os.path).should_receive('exists').and_return(False) + flexmock(module).should_receive('any_parent_directories').never() module.collect_special_file_paths( ('borg', 'create', '--exclude-nodump'), @@ -343,7 +372,7 @@ def test_collect_special_file_paths_omits_exclude_no_dump_flag_from_command(): local_path='borg', working_directory=None, borg_environment=None, - skip_directories=flexmock(), + borgmatic_runtime_directory='/run/borgmatic', )