From 1d37b14356f1efda17fb9e3c6a005be7c8b901f0 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 1 Oct 2019 12:23:16 -0700 Subject: [PATCH] More detailed error alerting via runtime context available in "on_error" hook (#174). --- NEWS | 3 ++ borgmatic/commands/borgmatic.py | 13 ++++-- borgmatic/hook.py | 19 +++++++- ...reparation-and-cleanup-steps-to-backups.md | 15 ++---- docs/how-to/inspect-your-backups.md | 46 ++++++++++++++++++- setup.py | 2 +- tests/unit/test_hook.py | 29 ++++++++++++ 7 files changed, 108 insertions(+), 19 deletions(-) diff --git a/NEWS b/NEWS index ab67d6f5b..2fdc12f52 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +1.3.23 + * #174: More detailed error alerting via runtime context available in "on_error" hook. + 1.3.22 * #144: When backups to one of several repositories fails, keep backing up to the other repositories and report errors afterwards. diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index e59898d08..a37355057 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -48,7 +48,8 @@ def run_configuration(config_filename, config, arguments): local_path = location.get('local_path', 'borg') remote_path = location.get('remote_path') borg_environment.initialize(storage) - encountered_error = False + encountered_error = None + error_repository = '' if 'create' in arguments: try: @@ -60,7 +61,7 @@ def run_configuration(config_filename, config, arguments): global_arguments.dry_run, ) except (OSError, CalledProcessError) as error: - encountered_error = True + encountered_error = error yield from make_error_log_records( '{}: Error running pre-backup hook'.format(config_filename), error ) @@ -79,7 +80,8 @@ def run_configuration(config_filename, config, arguments): repository_path=repository_path, ) except (OSError, CalledProcessError) as error: - encountered_error = True + encountered_error = error + error_repository = repository_path yield from make_error_log_records( '{}: Error running actions for repository'.format(repository_path), error ) @@ -94,7 +96,7 @@ def run_configuration(config_filename, config, arguments): global_arguments.dry_run, ) except (OSError, CalledProcessError) as error: - encountered_error = True + encountered_error = error yield from make_error_log_records( '{}: Error running post-backup hook'.format(config_filename), error ) @@ -107,6 +109,9 @@ def run_configuration(config_filename, config, arguments): config_filename, 'on-error', global_arguments.dry_run, + repository=error_repository, + error=encountered_error, + output=getattr(encountered_error, 'output', ''), ) except (OSError, CalledProcessError) as error: yield from make_error_log_records( diff --git a/borgmatic/hook.py b/borgmatic/hook.py index 49f74d0e2..16dc3769f 100644 --- a/borgmatic/hook.py +++ b/borgmatic/hook.py @@ -6,12 +6,26 @@ from borgmatic import execute logger = logging.getLogger(__name__) -def execute_hook(commands, umask, config_filename, description, dry_run): +def interpolate_context(command, context): + ''' + Given a single hook command and a dict of context names/values, interpolate the values by + "{name}" into the command and return the result. + ''' + for name, value in context.items(): + command = command.replace('{%s}' % name, str(value)) + + return command + + +def execute_hook(commands, umask, config_filename, description, dry_run, **context): ''' Given a list of hook commands to execute, a umask to execute with (or None), a config filename, a hook description, and whether this is a dry run, run the given commands. Or, don't run them if this is a dry run. + The context contains optional values interpolated by name into the hook commands. Currently, + this only applies to the on_error hook. + Raise ValueError if the umask cannot be parsed. Raise subprocesses.CalledProcessError if an error occurs in a hook. ''' @@ -21,6 +35,9 @@ def execute_hook(commands, umask, config_filename, description, dry_run): dry_run_label = ' (dry run; not actually running hooks)' if dry_run else '' + context['configuration_filename'] = config_filename + commands = [interpolate_context(command, context) for command in commands] + if len(commands) == 1: logger.info( '{}: Running command for {} hook{}'.format(config_filename, description, dry_run_label) 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 dcf84bd4b..8e609878d 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 @@ -47,19 +47,10 @@ but only if there is a `create` action. It runs even if an error occurs during a backup or a backup hook, but not if an error occurs during a `before_everything` hook. -## Error hooks - borgmatic also runs `on_error` hooks if an error occurs, either when creating -a backup or running a backup hook. Here's an example configuration: - -```yaml -hooks: - on_error: - - echo "Error while creating a backup or running a backup hook." -``` - -Note however that borgmatic does not run `on_error` hooks if an error occurs -within a `before_everything` or `after_everything` hook. +a backup or running a backup hook. See the [error alerting +documentation](https://torsion.org/borgmatic/docs/how-to/inspect-your-backups.md) +for more information. ## Hook output diff --git a/docs/how-to/inspect-your-backups.md b/docs/how-to/inspect-your-backups.md index ac33f2529..c810b9073 100644 --- a/docs/how-to/inspect-your-backups.md +++ b/docs/how-to/inspect-your-backups.md @@ -24,7 +24,7 @@ borgmatic --verbosity 2 If you're less concerned with progress during a backup, and you just want to see the summary of archive statistics at the end, you can use the stats -option: +option when performing a backup: ```bash borgmatic --stats @@ -82,6 +82,49 @@ Note that the [sample borgmatic systemd service file](https://torsion.org/borgmatic/docs/how-to/set-up-backups/#systemd) already has this rate limit disabled. +## Error alerting + +When an error occurs during a backup, borgmatic can run configurable shell +commands to fire off custom error notifications or take other actions, so you +can get alerted as soon as something goes wrong. Here's a not-so-useful +example: + +```yaml +hooks: + on_error: + - echo "Error while creating a backup or running a backup hook." +``` + +The `on_error` hook supports interpolating particular runtime variables into +the hook command. Here's an example that assumes you provide a separate shell +script to handle the alerting: + +```yaml +hooks: + on_error: + - send-text-message.sh "{configuration_filename}" "{repository}" +``` + +In this example, when the error occurs, borgmatic interpolates a few runtime +values into the hook command: the borgmatic configuration filename, and the +path of the repository. Here's the full set of supported variables you can use +here: + + * `configuration_filename`: borgmatic configuration filename in which the + error occurred + * `repository`: path of the repository in which the error occurred (may be + blank if the error occurs in a hook) + * `error`: the error message itself + * `output`: output of the command that failed (may be blank if an error + occurred without running a command) + +Note that borgmatic does not run `on_error` hooks if an error occurs within a +`before_everything` or `after_everything` hook. For more about hooks, see the +[borgmatic hooks +documentation](https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups.md), +especially the security information. + + ## Scripting borgmatic To consume the output of borgmatic in other software, you can include an @@ -96,4 +139,5 @@ output only shows up at the console, and not in syslog. ## Related documentation * [Set up backups with borgmatic](https://torsion.org/borgmatic/docs/how-to/set-up-backups.md) + * [Add preparation and cleanup steps to backups](https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups.md) * [Develop on borgmatic](https://torsion.org/borgmatic/docs/how-to/develop-on-borgmatic.md) diff --git a/setup.py b/setup.py index 013fb6fe0..daceb552e 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.3.22' +VERSION = '1.3.23' setup( diff --git a/tests/unit/test_hook.py b/tests/unit/test_hook.py index 135e07e14..647e92bbc 100644 --- a/tests/unit/test_hook.py +++ b/tests/unit/test_hook.py @@ -5,7 +5,24 @@ from flexmock import flexmock from borgmatic import hook as module +def test_interpolate_context_passes_through_command_without_variable(): + assert module.interpolate_context('ls', {'foo': 'bar'}) == 'ls' + + +def test_interpolate_context_passes_through_command_with_unknown_variable(): + assert module.interpolate_context('ls {baz}', {'foo': 'bar'}) == 'ls {baz}' + + +def test_interpolate_context_interpolates_variables(): + context = {'foo': 'bar', 'baz': 'quux'} + + assert module.interpolate_context('ls {foo}{baz} {baz}', context) == 'ls barquux quux' + + def test_execute_hook_invokes_each_command(): + flexmock(module).should_receive('interpolate_context').replace_with( + lambda command, context: command + ) flexmock(module.execute).should_receive('execute_command').with_args( [':'], output_log_level=logging.WARNING, shell=True ).once() @@ -14,6 +31,9 @@ def test_execute_hook_invokes_each_command(): def test_execute_hook_with_multiple_commands_invokes_each_command(): + flexmock(module).should_receive('interpolate_context').replace_with( + lambda command, context: command + ) flexmock(module.execute).should_receive('execute_command').with_args( [':'], output_log_level=logging.WARNING, shell=True ).once() @@ -25,6 +45,9 @@ def test_execute_hook_with_multiple_commands_invokes_each_command(): def test_execute_hook_with_umask_sets_that_umask(): + flexmock(module).should_receive('interpolate_context').replace_with( + lambda command, context: command + ) flexmock(module.os).should_receive('umask').with_args(0o77).and_return(0o22).once() flexmock(module.os).should_receive('umask').with_args(0o22).once() flexmock(module.execute).should_receive('execute_command').with_args( @@ -35,6 +58,9 @@ def test_execute_hook_with_umask_sets_that_umask(): def test_execute_hook_with_dry_run_skips_commands(): + flexmock(module).should_receive('interpolate_context').replace_with( + lambda command, context: command + ) flexmock(module.execute).should_receive('execute_command').never() module.execute_hook([':', 'true'], None, 'config.yaml', 'pre-backup', dry_run=True) @@ -45,6 +71,9 @@ def test_execute_hook_with_empty_commands_does_not_raise(): def test_execute_hook_on_error_logs_as_error(): + flexmock(module).should_receive('interpolate_context').replace_with( + lambda command, context: command + ) flexmock(module.execute).should_receive('execute_command').with_args( [':'], output_log_level=logging.ERROR, shell=True ).once()