From 29d5b36a78e394bd1f2a42bc5d26a1fe410d1813 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 6 Oct 2024 17:39:02 -0700 Subject: [PATCH] Change soft failure command hooks to skip only the current repository (#921). --- NEWS | 12 +++--- borgmatic/commands/borgmatic.py | 2 +- borgmatic/hooks/command.py | 2 +- ...movable-drive-or-an-intermittent-server.md | 40 +++++++++++++------ tests/unit/commands/test_borgmatic.py | 13 +++--- 5 files changed, 43 insertions(+), 26 deletions(-) diff --git a/NEWS b/NEWS index 153e9b98..1ef844d7 100644 --- a/NEWS +++ b/NEWS @@ -1,16 +1,18 @@ 1.9.0.dev0 * #914: Fix a confusing apparent hang when when the repository location changes, and instead show a helpful error message. - * #919: Clarify the command-line help for the "--config" flag. - * #919: Document a policy for versioning and breaking changes: - https://torsion.org/borgmatic/docs/how-to/upgrade/#versioning-and-breaking-changes + * #915: BREAKING: Rename repository actions like "rcreate" to more explicit names like + "repo-create" for compatibility with recent changes in Borg 2.0.0b10. * #918: BREAKING: When databases are configured, don't auto-enable the "one_file_system" option, as existing auto-excludes of special files should be sufficient to prevent Borg from hanging on them. But if this change causes problems for you, you can always enable "one_file_system" explicitly. + * #919: Clarify the command-line help for the "--config" flag. + * #919: Document a policy for versioning and breaking changes: + https://torsion.org/borgmatic/docs/how-to/upgrade/#versioning-and-breaking-changes * #911: Add a "key change-passphrase" action to change the passphrase protecting a repository key. - * #915: BREAKING: Rename repository actions like "rcreate" to more explicit names like - "repo-create" for compatibility with recent changes in Borg 2.0.0b10. + * #921: BREAKING: Change soft failure command hooks to skip only the current repository rather than + all repositories in the configuration file. 1.8.14 * #896: Fix an error in borgmatic rcreate/init on an empty repository directory with Borg 1.4. diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 7b6388b4..d7c2aad8 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -172,7 +172,7 @@ def run_configuration(config_filename, config, config_paths, arguments): continue if command.considered_soft_failure(config_filename, error): - break + continue yield from log_error_records( f'{repository.get("label", repository["path"])}: Error running actions for repository', diff --git a/borgmatic/hooks/command.py b/borgmatic/hooks/command.py index 421d4d7f..8eafb31e 100644 --- a/borgmatic/hooks/command.py +++ b/borgmatic/hooks/command.py @@ -90,7 +90,7 @@ def considered_soft_failure(config_filename, error): if exit_code == SOFT_FAIL_EXIT_CODE: logger.info( - f'{config_filename}: Command hook exited with soft failure exit code ({SOFT_FAIL_EXIT_CODE}); skipping remaining actions', + f'{config_filename}: Command hook exited with soft failure exit code ({SOFT_FAIL_EXIT_CODE}); skipping remaining repository actions', ) return True diff --git a/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md b/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md index a7c9b20a..31c987b0 100644 --- a/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md +++ b/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md @@ -34,9 +34,14 @@ test in the form of a borgmatic hook to see if backups should proceed or not. The way the test works is that if any of your hook commands return a special exit status of 75, that indicates to borgmatic that it's a temporary failure, -and borgmatic should skip all subsequent actions for that configuration file. -If you return any other status, then it's a standard success or error. (Zero is -success; anything else other than 75 is an error). +and borgmatic should skip all subsequent actions for the current repository. + +Prior to version 1.9.0 Soft +failures skipped subsequent actions for *all* repositories in the +configuration file, rather than just for the current repository. + +If you return any status besides 75, then it's a standard success or error. +(Zero is success; anything else other than 75 is an error). So for instance, if you have an external drive that's only sometimes mounted, declare its repository in its own [separate configuration @@ -71,9 +76,15 @@ option in the `hooks:` section of your configuration. What this does is check if the `findmnt` command errors when probing for a particular mount point. If it does error, then it returns exit code 75 to -borgmatic. borgmatic logs the soft failure, skips all further actions in that -configurable file, and proceeds onward to any other borgmatic configuration -files you may have. +borgmatic. borgmatic logs the soft failure, skips all further actions for the +current repository, and proceeds onward to any other repositories and/or +configuration files you may have. + +If you'd prefer not to use a separate configuration file, and you'd rather +have multiple repositories in a single configuration file, you can make your +`before_backup` soft failure test [vary by +repository](https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/#variable-interpolation). +That might require calling out to a separate script though. Note that `before_backup` only runs on the `create` action. See below about optionally using `before_actions` instead. @@ -121,13 +132,16 @@ There are some caveats you should be aware of with this feature. executing. So, like a standard error, it is an "early out". Unlike a standard error, borgmatic does not display it in angry red text or consider it a failure. - * The soft failure only applies to the scope of a single borgmatic - configuration file. So put anything that you don't want soft-failed, like - always-online cloud backups, in separate configuration files from your - soft-failing repositories. - * The soft failure doesn't have to apply to a repository. You can even perform - a test to make sure that individual source directories are mounted and - available. Use your imagination! + * Any given soft failure only applies to the a single borgmatic repository + (as of borgmatic 1.9.0). So if you have other repositories you don't want + soft-failed, then make your soft fail test [vary by + repository](https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/#variable-interpolation)—or + put anything that you don't want soft-failed (like always-online cloud + backups) in separate configuration files from your soft-failing + repositories. + * The soft failure doesn't have to test anything related to a repository. You + can even perform a test to make sure that individual source directories are + mounted and available. Use your imagination! * The soft failure feature also works for before/after hooks for other actions as well. But it is not implemented for `before_everything` or `after_everything`. diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index b6b57598..f0832b45 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -90,10 +90,10 @@ def test_run_configuration_bails_for_monitor_start_soft_failure(): flexmock(module).should_receive('get_skip_actions').and_return([]) flexmock(module.borg_version).should_receive('local_borg_version').and_return(flexmock()) error = subprocess.CalledProcessError(borgmatic.hooks.command.SOFT_FAIL_EXIT_CODE, 'try again') - flexmock(module.dispatch).should_receive('call_hooks').and_raise(error) + flexmock(module.dispatch).should_receive('call_hooks').and_raise(error).and_return(None) flexmock(module).should_receive('log_error_records').never() flexmock(module).should_receive('run_actions').never() - config = {'repositories': [{'path': 'foo'}]} + config = {'repositories': [{'path': 'foo'}, {'path': 'bar'}]} arguments = {'global': flexmock(monitoring_verbosity=1, dry_run=False), 'create': flexmock()} results = list(module.run_configuration('test.yaml', config, ['/tmp/test.yaml'], arguments)) @@ -118,21 +118,22 @@ def test_run_configuration_logs_actions_error(): assert results == expected_results -def test_run_configuration_skips_remaining_actions_for_actions_soft_failure_but_still_pings_monitor(): +def test_run_configuration_skips_remaining_actions_for_actions_soft_failure_but_still_runs_next_repository_actions(): flexmock(module).should_receive('verbosity_to_log_level').and_return(logging.INFO) flexmock(module).should_receive('get_skip_actions').and_return([]) flexmock(module.borg_version).should_receive('local_borg_version').and_return(flexmock()) flexmock(module.dispatch).should_receive('call_hooks').times(5) error = subprocess.CalledProcessError(borgmatic.hooks.command.SOFT_FAIL_EXIT_CODE, 'try again') - flexmock(module).should_receive('run_actions').and_raise(error) + log = flexmock() + flexmock(module).should_receive('run_actions').twice().and_raise(error).and_yield(log) flexmock(module).should_receive('log_error_records').never() flexmock(module.command).should_receive('considered_soft_failure').and_return(True) - config = {'repositories': [{'path': 'foo'}]} + config = {'repositories': [{'path': 'foo'}, {'path': 'bar'}]} arguments = {'global': flexmock(monitoring_verbosity=1, dry_run=False), 'create': flexmock()} results = list(module.run_configuration('test.yaml', config, ['/tmp/test.yaml'], arguments)) - assert results == [] + assert results == [log] def test_run_configuration_logs_monitor_log_error():