Browse Source

To avoid a hang in the database hooks, error and exit when the borgmatic runtime directory overlaps with the configured excludes (#947).

Dan Helfman 6 months ago
parent
commit
267af5b372
3 changed files with 70 additions and 24 deletions
  1. 2 0
      NEWS
  2. 28 13
      borgmatic/borg/create.py
  3. 40 11
      tests/unit/borg/test_create.py

+ 2 - 0
NEWS

@@ -1,4 +1,6 @@
 1.9.5.dev0
 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.
  * #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.
  * When the ZFS, Btrfs, or LVM hooks aren't configured, don't try to cleanup snapshots for them.
 
 

+ 28 - 13
borgmatic/borg/create.py

@@ -160,14 +160,24 @@ def any_parent_directories(path, candidate_parents):
 
 
 
 
 def collect_special_file_paths(
 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
     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
     # Omit "--exclude-nodump" from the Borg dry run command, because that flag causes Borg to open
     # files including any named pipe we've created.
     # 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')
         for path_line in paths_output.split('\n')
         if path_line and path_line.startswith('- ') or path_line.startswith('+ ')
         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):
 def check_all_source_directories_exist(source_directories):
@@ -335,9 +352,7 @@ def make_base_create_command(
             local_path,
             local_path,
             working_directory,
             working_directory,
             borg_environment,
             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:
         if special_file_paths:

+ 40 - 11
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'
         'Processing files ...\n- /foo\n+ /bar\n- /baz'
     )
     )
     flexmock(module).should_receive('special_file').and_return(True)
     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(
     assert module.collect_special_file_paths(
         ('borg', 'create'),
         ('borg', 'create'),
@@ -283,17 +284,24 @@ def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_
         local_path=None,
         local_path=None,
         working_directory=None,
         working_directory=None,
         borg_environment=None,
         borg_environment=None,
-        skip_directories=flexmock(),
+        borgmatic_runtime_directory='/run/borgmatic',
     ) == ('/foo', '/bar', '/baz')
     ) == ('/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(
     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('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)
     ).and_return(False)
 
 
     assert module.collect_special_file_paths(
     assert module.collect_special_file_paths(
@@ -302,10 +310,29 @@ def test_collect_special_file_paths_excludes_requested_directories():
         local_path=None,
         local_path=None,
         working_directory=None,
         working_directory=None,
         borg_environment=None,
         borg_environment=None,
-        skip_directories=flexmock(),
+        borgmatic_runtime_directory='/run/borgmatic',
     ) == ('/foo', '/baz')
     ) == ('/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():
 def test_collect_special_file_paths_excludes_non_special_files():
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         '+ /foo\n+ /bar\n+ /baz'
         '+ /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(
     flexmock(module).should_receive('special_file').and_return(True).and_return(False).and_return(
         True
         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(
     assert module.collect_special_file_paths(
         ('borg', 'create'),
         ('borg', 'create'),
@@ -321,7 +349,7 @@ def test_collect_special_file_paths_excludes_non_special_files():
         local_path=None,
         local_path=None,
         working_directory=None,
         working_directory=None,
         borg_environment=None,
         borg_environment=None,
-        skip_directories=flexmock(),
+        borgmatic_runtime_directory='/run/borgmatic',
     ) == ('/foo', '/baz')
     ) == ('/foo', '/baz')
 
 
 
 
@@ -335,7 +363,8 @@ def test_collect_special_file_paths_omits_exclude_no_dump_flag_from_command():
         borg_exit_codes=None,
         borg_exit_codes=None,
     ).and_return('Processing files ...\n- /foo\n+ /bar\n- /baz').once()
     ).and_return('Processing files ...\n- /foo\n+ /bar\n- /baz').once()
     flexmock(module).should_receive('special_file').and_return(True)
     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(
     module.collect_special_file_paths(
         ('borg', 'create', '--exclude-nodump'),
         ('borg', 'create', '--exclude-nodump'),
@@ -343,7 +372,7 @@ def test_collect_special_file_paths_omits_exclude_no_dump_flag_from_command():
         local_path='borg',
         local_path='borg',
         working_directory=None,
         working_directory=None,
         borg_environment=None,
         borg_environment=None,
-        skip_directories=flexmock(),
+        borgmatic_runtime_directory='/run/borgmatic',
     )
     )