diff --git a/NEWS b/NEWS
index ddf12237..5531d48d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,8 @@
2.0.3.dev0
* #1065: Fix a regression in monitoring hooks in which an error pinged the finish state instead of
the fail state.
+ * #1066: Add a "states" option to command hooks, so you can optionally skip an "after" hook if
+ borgmatic encounters an error.
2.0.2
* #1035: Document potential performance issues and workarounds with the ZFS, Btrfs, and LVM hooks:
diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py
index d5415f8b..593a8873 100644
--- a/borgmatic/commands/borgmatic.py
+++ b/borgmatic/commands/borgmatic.py
@@ -301,7 +301,10 @@ def run_configuration(config_filename, config, config_paths, arguments):
try:
command.execute_hooks(
command.filter_hooks(
- config.get('commands'), after='error', action_names=arguments.keys()
+ config.get('commands'),
+ after='error',
+ action_names=arguments.keys(),
+ state_names=['fail'],
),
config.get('umask'),
borgmatic.config.paths.get_working_directory(config),
@@ -907,6 +910,7 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments, log
# Execute the actions corresponding to each configuration file.
json_results = []
+ encountered_error = False
for config_filename, config in configs.items():
with Log_prefix(config_filename):
@@ -917,6 +921,7 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments, log
)
if error_logs:
+ encountered_error = True
yield from log_error_records('An error occurred')
yield from error_logs
else:
@@ -939,6 +944,7 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments, log
local_path=get_local_path(configs),
)
except (CalledProcessError, OSError) as error:
+ encountered_error = True
yield from log_error_records('Error unmounting mount point', error)
if json_results:
@@ -948,7 +954,10 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments, log
for config_filename, config in configs.items():
command.execute_hooks(
command.filter_hooks(
- config.get('commands'), after='everything', action_names=arguments.keys()
+ config.get('commands'),
+ after='everything',
+ action_names=arguments.keys(),
+ state_names=['fail' if encountered_error else 'finish'],
),
config.get('umask'),
borgmatic.config.paths.get_working_directory(config),
diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml
index f99a8ee2..0216833b 100644
--- a/borgmatic/config/schema.yaml
+++ b/borgmatic/config/schema.yaml
@@ -1225,6 +1225,26 @@ properties:
particular actions listed here. Defaults to
running for all actions.
example: [create, prune, compact, check]
+ states:
+ type: array
+ items:
+ type: string
+ enum:
+ - finish
+ - fail
+ description: |
+ Only trigger the hook if borgmatic encounters one
+ of the states (execution results) listed here,
+ where:
+ * "finish": No errors occurred.
+ * "fail": An error occurred.
+ This state is evaluated only for the scope of the
+ configured "action", "repository", etc., rather
+ than for the entire borgmatic run. Only available
+ for "after" hooks. Defaults to running the hook
+ for all states.
+ example:
+ - finish
run:
type: array
items:
diff --git a/borgmatic/hooks/command.py b/borgmatic/hooks/command.py
index 83eb9241..e0843da6 100644
--- a/borgmatic/hooks/command.py
+++ b/borgmatic/hooks/command.py
@@ -47,21 +47,25 @@ def make_environment(current_environment, sys_module=sys):
return environment
-def filter_hooks(command_hooks, before=None, after=None, hook_name=None, action_names=None):
+def filter_hooks(command_hooks, before=None, after=None, action_names=None, state_names=None):
'''
Given a sequence of command hook dicts from configuration and one or more filters (before name,
- after name, calling hook name, or a sequence of action names), filter down the command hooks to
- just the ones that match the given filters.
+ after name, a sequence of action names, and/or a sequence of execution result state names),
+ filter down the command hooks to just the ones that match the given filters.
'''
return tuple(
hook_config
for hook_config in command_hooks or ()
for config_action_names in (hook_config.get('when'),)
+ for config_state_names in (hook_config.get('states'),)
if before is None or hook_config.get('before') == before
if after is None or hook_config.get('after') == after
if action_names is None
or config_action_names is None
or set(config_action_names or ()).intersection(set(action_names))
+ if state_names is None
+ or config_state_names is None
+ or set(config_state_names or ()).intersection(set(state_names))
)
@@ -143,7 +147,7 @@ class Before_after_hooks:
before_after='do_stuff',
umask=config.get('umask'),
dry_run=dry_run,
- hook_name='myhook',
+ action_names=['create'],
):
do()
some()
@@ -160,22 +164,20 @@ class Before_after_hooks:
umask,
working_directory,
dry_run,
- hook_name=None,
action_names=None,
**context,
):
'''
Given a sequence of command hook configuration dicts, the before/after name, a umask to run
- commands with, a working directory to run commands with, a dry run flag, the name of the
- calling hook, a sequence of action names, and any context for the executed commands, save
- those data points for use below.
+ commands with, a working directory to run commands with, a dry run flag, a sequence of
+ action names, and any context for the executed commands, save those data points for use
+ below.
'''
self.command_hooks = command_hooks
self.before_after = before_after
self.umask = umask
self.working_directory = working_directory
self.dry_run = dry_run
- self.hook_name = hook_name
self.action_names = action_names
self.context = context
@@ -188,7 +190,6 @@ class Before_after_hooks:
borgmatic.hooks.command.filter_hooks(
self.command_hooks,
before=self.before_after,
- hook_name=self.hook_name,
action_names=self.action_names,
),
self.umask,
@@ -202,7 +203,7 @@ class Before_after_hooks:
# Trigger the after hook manually, since raising here will prevent it from being run
# otherwise.
- self.__exit__(None, None, None)
+ self.__exit__(exception_type=type(error), exception=error, traceback=None)
raise ValueError(f'Error running before {self.before_after} hook: {error}')
@@ -215,8 +216,8 @@ class Before_after_hooks:
borgmatic.hooks.command.filter_hooks(
self.command_hooks,
after=self.before_after,
- hook_name=self.hook_name,
action_names=self.action_names,
+ state_names=['fail' if exception_type else 'finish'],
),
self.umask,
self.working_directory,
diff --git a/docs/how-to/add-preparation-and-cleanup-steps-to-backups.md b/docs/how-to/add-preparation-and-cleanup-steps-to-backups.md
index 5eceba2d..505ab0cd 100644
--- a/docs/how-to/add-preparation-and-cleanup-steps-to-backups.md
+++ b/docs/how-to/add-preparation-and-cleanup-steps-to-backups.md
@@ -55,6 +55,29 @@ commands:
- "echo Backup: start"
```
+By default, an `after` command hook runs even if an error occurs in the
+corresponding `before` hook or between those two hooks. This allows you to
+perform cleanup steps that correspond to `before` preparation commands—even when
+something goes wrong. You may notice that this is a departure from the way that
+the deprecated `after_*` hooks worked in borgmatic prior to version 2.0.0.
+
+New in version 2.0.3 You can
+customize this behavior with the `states` option. For instance, here's an
+example of an `after` hook that only triggers on success and not on error:
+
+```yaml
+commands:
+ - after: action
+ when: [create]
+ states: [finish]
+ run:
+ - echo "After successful create!"
+```
+
+Additionally, when command hooks run, they respect the `working_directory`
+option if it is configured, meaning that the hook commands are run in that
+directory.
+
Each command in the `commands:` list has the following options:
* `before` or `after`: Name for the point in borgmatic's execution that the commands should be run before or after, one of:
@@ -64,18 +87,11 @@ Each command in the `commands:` list has the following options:
* `everything` runs before or after all configuration files. Errors here do not trigger `error` hooks or the `fail` state in monitoring hooks. This replaces the deprecated `before_everything` and `after_everything`.
* `error` runs after an error occurs—and it's only available for `after`. This replaces the deprecated `on_error` hook.
* `when`: Only trigger the hook when borgmatic is run with particular actions (`create`, `prune`, etc.) listed here. Defaults to running for all actions.
+ * `states`: New in version 2.0.3 Only trigger the hook if borgmatic encounters one of the states (execution results) listed here. This state is evaluated only for the scope of the configured `action`, `repository`, etc., rather than for the entire borgmatic run. Only available for `after` hooks. Defaults to running the hook for all states. One or more of:
+ * `finish`: No errors occurred.
+ * `fail`: An error occurred.
* `run`: List of one or more shell commands or scripts to run when this command hook is triggered.
-An `after` command hook runs even if an error occurs in the corresponding
-`before` hook or between those two hooks. This allows you to perform cleanup
-steps that correspond to `before` preparation commands—even when something goes
-wrong. This is a departure from the way that the deprecated `after_*` hooks
-worked in borgmatic prior to version 2.0.0.
-
-Additionally, when command hooks run, they respect the `working_directory`
-option if it is configured, meaning that the hook commands are run in that
-directory.
-
### Order of execution
diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py
index 85a5ceea..b2525076 100644
--- a/tests/unit/commands/test_borgmatic.py
+++ b/tests/unit/commands/test_borgmatic.py
@@ -1932,7 +1932,12 @@ def test_collect_highlander_action_summary_logs_error_on_run_validate_failure():
def test_collect_configuration_run_summary_logs_info_for_success():
flexmock(module.validate).should_receive('guard_configuration_contains_repository')
- flexmock(module.command).should_receive('filter_hooks')
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, before='everything', action_names=object
+ )
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, after='everything', action_names=object, state_names=['finish']
+ )
flexmock(module.command).should_receive('execute_hooks')
flexmock(module).should_receive('Log_prefix').and_return(flexmock())
flexmock(module).should_receive('run_configuration').and_return([])
@@ -1952,7 +1957,12 @@ def test_collect_configuration_run_summary_logs_info_for_success():
def test_collect_configuration_run_summary_executes_hooks_for_create():
flexmock(module.validate).should_receive('guard_configuration_contains_repository')
- flexmock(module.command).should_receive('filter_hooks')
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, before='everything', action_names=object
+ )
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, after='everything', action_names=object, state_names=['finish']
+ )
flexmock(module.command).should_receive('execute_hooks')
flexmock(module).should_receive('Log_prefix').and_return(flexmock())
flexmock(module).should_receive('run_configuration').and_return([])
@@ -1975,7 +1985,12 @@ def test_collect_configuration_run_summary_executes_hooks_for_create():
def test_collect_configuration_run_summary_logs_info_for_success_with_extract():
flexmock(module.validate).should_receive('guard_configuration_contains_repository')
- flexmock(module.command).should_receive('filter_hooks')
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, before='everything', action_names=object
+ )
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, after='everything', action_names=object, state_names=['finish']
+ )
flexmock(module.command).should_receive('execute_hooks')
flexmock(module).should_receive('Log_prefix').and_return(flexmock())
flexmock(module).should_receive('run_configuration').and_return([])
@@ -2018,7 +2033,12 @@ def test_collect_configuration_run_summary_logs_extract_with_repository_error():
def test_collect_configuration_run_summary_logs_info_for_success_with_mount():
flexmock(module.validate).should_receive('guard_configuration_contains_repository')
- flexmock(module.command).should_receive('filter_hooks')
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, before='everything', action_names=object
+ )
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, after='everything', action_names=object, state_names=['finish']
+ )
flexmock(module.command).should_receive('execute_hooks')
flexmock(module).should_receive('Log_prefix').and_return(flexmock())
flexmock(module).should_receive('run_configuration').and_return([])
@@ -2064,7 +2084,12 @@ def test_collect_configuration_run_summary_logs_mount_with_repository_error():
def test_collect_configuration_run_summary_logs_missing_configs_error():
flexmock(module.validate).should_receive('guard_configuration_contains_repository')
- flexmock(module.command).should_receive('filter_hooks')
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, before='everything', action_names=object
+ )
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, after='everything', action_names=object, state_names=['fail']
+ )
flexmock(module.command).should_receive('execute_hooks')
arguments = {'global': flexmock(config_paths=[])}
expected_logs = (flexmock(),)
@@ -2079,9 +2104,14 @@ def test_collect_configuration_run_summary_logs_missing_configs_error():
assert logs == expected_logs
-def test_collect_configuration_run_summary_logs_pre_hook_error():
+def test_collect_configuration_run_summary_logs_before_hook_error():
flexmock(module.validate).should_receive('guard_configuration_contains_repository')
- flexmock(module.command).should_receive('filter_hooks')
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, before='everything', action_names=object
+ )
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, after='everything', action_names=object, state_names=['fail']
+ )
flexmock(module.command).should_receive('execute_hooks').and_raise(ValueError)
expected_logs = (flexmock(),)
flexmock(module).should_receive('log_error_records').and_return(expected_logs)
@@ -2102,9 +2132,14 @@ def test_collect_configuration_run_summary_logs_pre_hook_error():
assert logs == expected_logs
-def test_collect_configuration_run_summary_logs_post_hook_error():
+def test_collect_configuration_run_summary_logs_after_hook_error():
flexmock(module.validate).should_receive('guard_configuration_contains_repository')
- flexmock(module.command).should_receive('filter_hooks')
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, before='everything', action_names=object
+ )
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, after='everything', action_names=object, state_names=['finish']
+ )
flexmock(module.command).should_receive('execute_hooks').and_return(None).and_raise(ValueError)
flexmock(module).should_receive('Log_prefix').and_return(flexmock())
flexmock(module).should_receive('run_configuration').and_return([])
@@ -2152,7 +2187,12 @@ def test_collect_configuration_run_summary_logs_for_list_with_archive_and_reposi
def test_collect_configuration_run_summary_logs_info_for_success_with_list():
flexmock(module.validate).should_receive('guard_configuration_contains_repository')
- flexmock(module.command).should_receive('filter_hooks')
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, before='everything', action_names=object
+ )
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, after='everything', action_names=object, state_names=['finish']
+ )
flexmock(module.command).should_receive('execute_hooks')
flexmock(module).should_receive('Log_prefix').and_return(flexmock())
flexmock(module).should_receive('run_configuration').and_return([])
@@ -2175,7 +2215,12 @@ def test_collect_configuration_run_summary_logs_info_for_success_with_list():
def test_collect_configuration_run_summary_logs_run_configuration_error_logs():
flexmock(module.validate).should_receive('guard_configuration_contains_repository')
- flexmock(module.command).should_receive('filter_hooks')
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, before='everything', action_names=object
+ )
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, after='everything', action_names=object, state_names=['fail']
+ )
flexmock(module.command).should_receive('execute_hooks')
flexmock(module).should_receive('Log_prefix').and_return(flexmock())
flexmock(module).should_receive('run_configuration').and_return(
@@ -2198,7 +2243,12 @@ def test_collect_configuration_run_summary_logs_run_configuration_error_logs():
def test_collect_configuration_run_summary_logs_run_umount_error():
flexmock(module.validate).should_receive('guard_configuration_contains_repository')
- flexmock(module.command).should_receive('filter_hooks')
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, before='everything', action_names=object
+ )
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, after='everything', action_names=object, state_names=['fail']
+ )
flexmock(module.command).should_receive('execute_hooks')
flexmock(module).should_receive('Log_prefix').and_return(flexmock())
flexmock(module).should_receive('run_configuration').and_return([])
@@ -2225,7 +2275,12 @@ def test_collect_configuration_run_summary_logs_run_umount_error():
def test_collect_configuration_run_summary_logs_outputs_merged_json_results():
flexmock(module.validate).should_receive('guard_configuration_contains_repository')
- flexmock(module.command).should_receive('filter_hooks')
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, before='everything', action_names=object
+ )
+ flexmock(module.command).should_receive('filter_hooks').with_args(
+ object, after='everything', action_names=object, state_names=['finish']
+ )
flexmock(module.command).should_receive('execute_hooks')
flexmock(module).should_receive('Log_prefix').and_return(flexmock())
flexmock(module).should_receive('run_configuration').and_return(['foo', 'bar']).and_return(
diff --git a/tests/unit/hooks/test_command.py b/tests/unit/hooks/test_command.py
index acfcd7d1..667e6ab3 100644
--- a/tests/unit/hooks/test_command.py
+++ b/tests/unit/hooks/test_command.py
@@ -167,6 +167,44 @@ def test_make_environment_with_pyinstaller_and_LD_LIBRARY_PATH_ORIG_copies_it_in
},
),
),
+ (
+ (
+ {
+ 'before': 'action',
+ 'states': ['finish'], # Not actually valid; only valid for "after".
+ 'run': ['foo'],
+ },
+ {
+ 'after': 'action',
+ 'run': ['bar'],
+ },
+ {
+ 'after': 'action',
+ 'states': ['finish'],
+ 'run': ['baz'],
+ },
+ {
+ 'after': 'action',
+ 'states': ['fail'],
+ 'run': ['quux'],
+ },
+ ),
+ {
+ 'after': 'action',
+ 'state_names': ['finish'],
+ },
+ (
+ {
+ 'after': 'action',
+ 'run': ['bar'],
+ },
+ {
+ 'after': 'action',
+ 'states': ['finish'],
+ 'run': ['baz'],
+ },
+ ),
+ ),
),
)
def test_filter_hooks(hooks, filters, expected_hooks):
@@ -336,14 +374,13 @@ def test_before_after_hooks_calls_command_hooks():
flexmock(module).should_receive('filter_hooks').with_args(
commands,
before='action',
- hook_name='myhook',
action_names=['create'],
).and_return(flexmock()).once()
flexmock(module).should_receive('filter_hooks').with_args(
commands,
after='action',
- hook_name='myhook',
action_names=['create'],
+ state_names=['finish'],
).and_return(flexmock()).once()
flexmock(module).should_receive('execute_hooks').twice()
@@ -353,7 +390,6 @@ def test_before_after_hooks_calls_command_hooks():
umask=1234,
working_directory='/working',
dry_run=False,
- hook_name='myhook',
action_names=['create'],
context1='stuff',
context2='such',
@@ -369,14 +405,13 @@ def test_before_after_hooks_with_before_error_runs_after_hook_and_raises():
flexmock(module).should_receive('filter_hooks').with_args(
commands,
before='action',
- hook_name='myhook',
action_names=['create'],
).and_return(flexmock()).once()
flexmock(module).should_receive('filter_hooks').with_args(
commands,
after='action',
- hook_name='myhook',
action_names=['create'],
+ state_names=['fail'],
).and_return(flexmock()).once()
flexmock(module).should_receive('execute_hooks').and_raise(OSError).and_return(None)
flexmock(module).should_receive('considered_soft_failure').and_return(False)
@@ -388,7 +423,6 @@ def test_before_after_hooks_with_before_error_runs_after_hook_and_raises():
umask=1234,
working_directory='/working',
dry_run=False,
- hook_name='myhook',
action_names=['create'],
context1='stuff',
context2='such',
@@ -404,14 +438,13 @@ def test_before_after_hooks_with_before_soft_failure_raises():
flexmock(module).should_receive('filter_hooks').with_args(
commands,
before='action',
- hook_name='myhook',
action_names=['create'],
).and_return(flexmock()).once()
flexmock(module).should_receive('filter_hooks').with_args(
commands,
after='action',
- hook_name='myhook',
action_names=['create'],
+ state_names=['finish'],
).never()
flexmock(module).should_receive('execute_hooks').and_raise(OSError)
flexmock(module).should_receive('considered_soft_failure').and_return(True)
@@ -423,7 +456,6 @@ def test_before_after_hooks_with_before_soft_failure_raises():
umask=1234,
working_directory='/working',
dry_run=False,
- hook_name='myhook',
action_names=['create'],
context1='stuff',
context2='such',
@@ -431,6 +463,38 @@ def test_before_after_hooks_with_before_soft_failure_raises():
pass
+def test_before_after_hooks_with_wrapped_code_error_runs_after_hook_and_raises():
+ commands = [
+ {'before': 'repository', 'run': ['foo', 'bar']},
+ {'after': 'repository', 'run': ['baz']},
+ ]
+ flexmock(module).should_receive('filter_hooks').with_args(
+ commands,
+ before='action',
+ action_names=['create'],
+ ).and_return(flexmock()).once()
+ flexmock(module).should_receive('filter_hooks').with_args(
+ commands,
+ after='action',
+ action_names=['create'],
+ state_names=['fail'],
+ ).and_return(flexmock()).once()
+ flexmock(module).should_receive('execute_hooks').twice()
+
+ with pytest.raises(ValueError):
+ with module.Before_after_hooks(
+ command_hooks=commands,
+ before_after='action',
+ umask=1234,
+ working_directory='/working',
+ dry_run=False,
+ action_names=['create'],
+ context1='stuff',
+ context2='such',
+ ):
+ raise ValueError()
+
+
def test_before_after_hooks_with_after_error_raises():
commands = [
{'before': 'repository', 'run': ['foo', 'bar']},
@@ -439,14 +503,13 @@ def test_before_after_hooks_with_after_error_raises():
flexmock(module).should_receive('filter_hooks').with_args(
commands,
before='action',
- hook_name='myhook',
action_names=['create'],
).and_return(flexmock()).once()
flexmock(module).should_receive('filter_hooks').with_args(
commands,
after='action',
- hook_name='myhook',
action_names=['create'],
+ state_names=['finish'],
).and_return(flexmock()).once()
flexmock(module).should_receive('execute_hooks').and_return(None).and_raise(OSError)
flexmock(module).should_receive('considered_soft_failure').and_return(False)
@@ -458,7 +521,6 @@ def test_before_after_hooks_with_after_error_raises():
umask=1234,
working_directory='/working',
dry_run=False,
- hook_name='myhook',
action_names=['create'],
context1='stuff',
context2='such',
@@ -474,14 +536,13 @@ def test_before_after_hooks_with_after_soft_failure_raises():
flexmock(module).should_receive('filter_hooks').with_args(
commands,
before='action',
- hook_name='myhook',
action_names=['create'],
).and_return(flexmock()).once()
flexmock(module).should_receive('filter_hooks').with_args(
commands,
after='action',
- hook_name='myhook',
action_names=['create'],
+ state_names=['finish'],
).and_return(flexmock()).once()
flexmock(module).should_receive('execute_hooks').and_return(None).and_raise(OSError)
flexmock(module).should_receive('considered_soft_failure').and_return(True)
@@ -493,7 +554,6 @@ def test_before_after_hooks_with_after_soft_failure_raises():
umask=1234,
working_directory='/working',
dry_run=False,
- hook_name='myhook',
action_names=['create'],
context1='stuff',
context2='such',