peter/borgmatic
1
0
Fork 0
mirror of https://github.com/borgmatic-collective/borgmatic.git synced 2026-08-01 06:03:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

Add a "states" option to command hooks, so you can optionally skip an "after" hook if borgmatic encounters an error (#1066).

Browse Source
This commit is contained in:
Dan Helfman
2025-04-12 15:02:47 -07:00
parent 81db67c759
commit a573e606a5
7 changed files with 215 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
NEWS
+2
View File
@@ -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:
borgmatic/commands/borgmatic.py
+11 -2
View File
@@ -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),
borgmatic/config/schema.yaml
+20
View File
@@ -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:
borgmatic/hooks/command.py
+13 -12
View File
@@ -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,
docs/how-to/add-preparation-and-cleanup-steps-to-backups.md
+26 -10
View File
@@ -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.
<span class="minilink minilink-addedin">New in version 2.0.3</span> 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`: <span class="minilink minilink-addedin">New in version 2.0.3</span> 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
tests/unit/commands/test_borgmatic.py
+68 -13
View File
@@ -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(
tests/unit/hooks/test_command.py
+75 -15
View File
@@ -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',