Ver código fonte

Change soft failure command hooks to skip only the current repository (#921).

Dan Helfman 8 meses atrás
pai
commit
29d5b36a78

+ 7 - 5
NEWS

@@ -1,16 +1,18 @@
 1.9.0.dev0
 1.9.0.dev0
  * #914: Fix a confusing apparent hang when when the repository location changes, and instead
  * #914: Fix a confusing apparent hang when when the repository location changes, and instead
    show a helpful error message.
    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,
  * #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
    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"
    them. But if this change causes problems for you, you can always enable "one_file_system"
    explicitly.
    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.
  * #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
 1.8.14
  * #896: Fix an error in borgmatic rcreate/init on an empty repository directory with Borg 1.4.
  * #896: Fix an error in borgmatic rcreate/init on an empty repository directory with Borg 1.4.

+ 1 - 1
borgmatic/commands/borgmatic.py

@@ -172,7 +172,7 @@ def run_configuration(config_filename, config, config_paths, arguments):
                     continue
                     continue
 
 
                 if command.considered_soft_failure(config_filename, error):
                 if command.considered_soft_failure(config_filename, error):
-                    break
+                    continue
 
 
                 yield from log_error_records(
                 yield from log_error_records(
                     f'{repository.get("label", repository["path"])}: Error running actions for repository',
                     f'{repository.get("label", repository["path"])}: Error running actions for repository',

+ 1 - 1
borgmatic/hooks/command.py

@@ -90,7 +90,7 @@ def considered_soft_failure(config_filename, error):
 
 
     if exit_code == SOFT_FAIL_EXIT_CODE:
     if exit_code == SOFT_FAIL_EXIT_CODE:
         logger.info(
         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
         return True
 
 

+ 27 - 13
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
 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,
 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.
+
+<span class="minilink minilink-addedin">Prior to version 1.9.0</span> 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,
 So for instance, if you have an external drive that's only sometimes mounted,
 declare its repository in its own [separate configuration
 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
 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
 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
 Note that `before_backup` only runs on the `create` action. See below about
 optionally using `before_actions` instead.
 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
    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
    error, borgmatic does not display it in angry red text or consider it a
    failure.
    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
  * The soft failure feature also works for before/after hooks for other
    actions as well. But it is not implemented for `before_everything` or
    actions as well. But it is not implemented for `before_everything` or
    `after_everything`.
    `after_everything`.

+ 7 - 6
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).should_receive('get_skip_actions').and_return([])
     flexmock(module.borg_version).should_receive('local_borg_version').and_return(flexmock())
     flexmock(module.borg_version).should_receive('local_borg_version').and_return(flexmock())
     error = subprocess.CalledProcessError(borgmatic.hooks.command.SOFT_FAIL_EXIT_CODE, 'try again')
     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('log_error_records').never()
     flexmock(module).should_receive('run_actions').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()}
     arguments = {'global': flexmock(monitoring_verbosity=1, dry_run=False), 'create': flexmock()}
 
 
     results = list(module.run_configuration('test.yaml', config, ['/tmp/test.yaml'], arguments))
     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
     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('verbosity_to_log_level').and_return(logging.INFO)
     flexmock(module).should_receive('get_skip_actions').and_return([])
     flexmock(module).should_receive('get_skip_actions').and_return([])
     flexmock(module.borg_version).should_receive('local_borg_version').and_return(flexmock())
     flexmock(module.borg_version).should_receive('local_borg_version').and_return(flexmock())
     flexmock(module.dispatch).should_receive('call_hooks').times(5)
     flexmock(module.dispatch).should_receive('call_hooks').times(5)
     error = subprocess.CalledProcessError(borgmatic.hooks.command.SOFT_FAIL_EXIT_CODE, 'try again')
     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).should_receive('log_error_records').never()
     flexmock(module.command).should_receive('considered_soft_failure').and_return(True)
     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()}
     arguments = {'global': flexmock(monitoring_verbosity=1, dry_run=False), 'create': flexmock()}
 
 
     results = list(module.run_configuration('test.yaml', config, ['/tmp/test.yaml'], arguments))
     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():
 def test_run_configuration_logs_monitor_log_error():