ソースを参照

When the ZFS, Btrfs, or LVM hooks aren't configured, don't try to cleanup snapshots for them.

Dan Helfman 5 ヶ月 前
コミット
48dca28c74

+ 1 - 0
NEWS

@@ -1,5 +1,6 @@
 1.9.5.dev0
  * #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.
 
 1.9.4
  * #80 (beta): Add an LVM hook for snapshotting and backing up LVM logical volumes. See the

+ 1 - 1
borgmatic/hooks/data_source/bootstrap.py

@@ -34,7 +34,7 @@ def dump_data_sources(
 
     Return an empty sequence, since there are no ongoing dump processes from this hook.
     '''
-    if hook_config.get('store_config_files') is False:
+    if hook_config and hook_config.get('store_config_files') is False:
         return []
 
     borgmatic_manifest_path = os.path.join(

+ 5 - 1
borgmatic/hooks/data_source/btrfs.py

@@ -266,8 +266,12 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
     '''
     Given a Btrfs configuration dict, a configuration dict, a log prefix, the borgmatic runtime
     directory, and whether this is a dry run, delete any Btrfs snapshots created by borgmatic. Use
-    the log prefix in any log entries. If this is a dry run, then don't actually remove anything.
+    the log prefix in any log entries. If this is a dry run or Btrfs isn't configured in borgmatic's
+    configuration, then don't actually remove anything.
     '''
+    if hook_config is None:
+        return
+
     dry_run_label = ' (dry run; not actually removing anything)' if dry_run else ''
 
     btrfs_command = hook_config.get('btrfs_command', 'btrfs')

+ 5 - 2
borgmatic/hooks/data_source/lvm.py

@@ -288,9 +288,12 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
     '''
     Given an LVM configuration dict, a configuration dict, a log prefix, the borgmatic runtime
     directory, and whether this is a dry run, unmount and delete any LVM snapshots created by
-    borgmatic. Use the log prefix in any log entries. If this is a dry run, then don't actually
-    remove anything.
+    borgmatic. Use the log prefix in any log entries. If this is a dry run or LVM isn't configured
+    in borgmatic's configuration, then don't actually remove anything.
     '''
+    if hook_config is None:
+        return
+
     dry_run_label = ' (dry run; not actually removing anything)' if dry_run else ''
 
     # Unmount snapshots.

+ 5 - 2
borgmatic/hooks/data_source/zfs.py

@@ -283,9 +283,12 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
     '''
     Given a ZFS configuration dict, a configuration dict, a log prefix, the borgmatic runtime
     directory, and whether this is a dry run, unmount and destroy any ZFS snapshots created by
-    borgmatic. Use the log prefix in any log entries. If this is a dry run, then don't actually
-    remove anything.
+    borgmatic. Use the log prefix in any log entries. If this is a dry run or ZFS isn't configured
+    in borgmatic's configuration, then don't actually remove anything.
     '''
+    if hook_config is None:
+        return
+
     dry_run_label = ' (dry run; not actually removing anything)' if dry_run else ''
 
     # Unmount snapshots.

+ 5 - 1
borgmatic/hooks/dispatch.py

@@ -32,7 +32,11 @@ def call_hook(function_name, config, log_prefix, hook_name, *args, **kwargs):
     Raise AttributeError if the function name is not found in the module.
     Raise anything else that the called function raises.
     '''
-    hook_config = config.get(hook_name) or config.get(f'{hook_name}_databases') or {}
+    if hook_name in config or f'{hook_name}_databases' in config:
+        hook_config = config.get(hook_name) or config.get(f'{hook_name}_databases') or {}
+    else:
+        hook_config = None
+
     module_name = hook_name.split('_databases')[0]
 
     # Probe for a data source or monitoring hook module corresponding to the hook name.

+ 6 - 6
tests/unit/hooks/data_source/test_bootstrap.py

@@ -22,7 +22,7 @@ def test_dump_data_sources_creates_manifest_file():
     ).once()
 
     module.dump_data_sources(
-        hook_config={},
+        hook_config=None,
         config={},
         log_prefix='test',
         config_paths=('test.yaml',),
@@ -53,7 +53,7 @@ def test_dump_data_sources_with_dry_run_does_not_create_manifest_file():
     flexmock(module.json).should_receive('dump').never()
 
     module.dump_data_sources(
-        hook_config={},
+        hook_config=None,
         config={},
         log_prefix='test',
         config_paths=('test.yaml',),
@@ -74,7 +74,7 @@ def test_remove_data_source_dumps_deletes_manifest_and_parent_directory():
     flexmock(module.os).should_receive('rmdir').with_args('/run/borgmatic/bootstrap').once()
 
     module.remove_data_source_dumps(
-        hook_config={},
+        hook_config=None,
         config={},
         log_prefix='test',
         borgmatic_runtime_directory='/run/borgmatic',
@@ -91,7 +91,7 @@ def test_remove_data_source_dumps_with_dry_run_bails():
     flexmock(module.os).should_receive('rmdir').never()
 
     module.remove_data_source_dumps(
-        hook_config={},
+        hook_config=None,
         config={},
         log_prefix='test',
         borgmatic_runtime_directory='/run/borgmatic',
@@ -110,7 +110,7 @@ def test_remove_data_source_dumps_swallows_manifest_file_not_found_error():
     flexmock(module.os).should_receive('rmdir').with_args('/run/borgmatic/bootstrap').once()
 
     module.remove_data_source_dumps(
-        hook_config={},
+        hook_config=None,
         config={},
         log_prefix='test',
         borgmatic_runtime_directory='/run/borgmatic',
@@ -131,7 +131,7 @@ def test_remove_data_source_dumps_swallows_manifest_parent_directory_not_found_e
     ).once()
 
     module.remove_data_source_dumps(
-        hook_config={},
+        hook_config=None,
         config={},
         log_prefix='test',
         borgmatic_runtime_directory='/run/borgmatic',

+ 18 - 0
tests/unit/hooks/data_source/test_btrfs.py

@@ -485,6 +485,24 @@ def test_remove_data_source_dumps_deletes_snapshots():
     )
 
 
+def test_remove_data_source_dumps_without_hook_configuration_bails():
+    flexmock(module).should_receive('get_subvolumes').never()
+    flexmock(module).should_receive('make_snapshot_path').never()
+    flexmock(module.borgmatic.config.paths).should_receive(
+        'replace_temporary_subdirectory_with_glob'
+    ).never()
+    flexmock(module).should_receive('delete_snapshot').never()
+    flexmock(module.shutil).should_receive('rmtree').never()
+
+    module.remove_data_source_dumps(
+        hook_config=None,
+        config={'source_directories': '/mnt/subvolume'},
+        log_prefix='test',
+        borgmatic_runtime_directory='/run/borgmatic',
+        dry_run=False,
+    )
+
+
 def test_remove_data_source_dumps_with_get_subvolumes_file_not_found_error_bails():
     config = {'btrfs': {}}
     flexmock(module).should_receive('get_subvolumes').and_raise(FileNotFoundError)

+ 17 - 0
tests/unit/hooks/data_source/test_lvm.py

@@ -673,6 +673,23 @@ def test_remove_data_source_dumps_unmounts_and_remove_snapshots():
     )
 
 
+def test_remove_data_source_dumps_bails_for_missing_lvm_configuration():
+    flexmock(module).should_receive('get_logical_volumes').never()
+    flexmock(module.borgmatic.config.paths).should_receive(
+        'replace_temporary_subdirectory_with_glob'
+    ).never()
+    flexmock(module).should_receive('unmount_snapshot').never()
+    flexmock(module).should_receive('remove_snapshot').never()
+
+    module.remove_data_source_dumps(
+        hook_config=None,
+        config={'source_directories': '/mnt/lvolume'},
+        log_prefix='test',
+        borgmatic_runtime_directory='/run/borgmatic',
+        dry_run=False,
+    )
+
+
 def test_remove_data_source_dumps_bails_for_missing_lsblk_command():
     config = {'lvm': {}}
     flexmock(module).should_receive('get_logical_volumes').and_raise(FileNotFoundError)

+ 15 - 0
tests/unit/hooks/data_source/test_zfs.py

@@ -321,6 +321,21 @@ def test_remove_data_source_dumps_use_custom_commands():
     )
 
 
+def test_remove_data_source_dumps_bails_for_missing_hook_configuration():
+    flexmock(module).should_receive('get_all_dataset_mount_points').never()
+    flexmock(module.borgmatic.config.paths).should_receive(
+        'replace_temporary_subdirectory_with_glob'
+    ).never()
+
+    module.remove_data_source_dumps(
+        hook_config=None,
+        config={'source_directories': '/mnt/dataset'},
+        log_prefix='test',
+        borgmatic_runtime_directory='/run/borgmatic',
+        dry_run=False,
+    )
+
+
 def test_remove_data_source_dumps_bails_for_missing_zfs_command():
     flexmock(module).should_receive('get_all_dataset_mount_points').and_raise(FileNotFoundError)
     flexmock(module.borgmatic.config.paths).should_receive(

+ 1 - 1
tests/unit/hooks/test_dispatch.py

@@ -95,7 +95,7 @@ def test_call_hook_without_hook_config_invokes_module_function_with_arguments_an
         'borgmatic.hooks.monitoring.super_hook'
     ).and_return(test_module)
     flexmock(test_module).should_receive('hook_function').with_args(
-        {}, config, 'prefix', 55, value=66
+        None, config, 'prefix', 55, value=66
     ).and_return(expected_return_value).once()
 
     return_value = module.call_hook('hook_function', config, 'prefix', 'super_hook', 55, value=66)