From a2c139245d19d8c3b1be871bdc321aba67920361 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 22 Jun 2024 16:19:06 -0700 Subject: [PATCH] Add a "--max-duration" flag to the "check" action and a "max_duration" option to the repository check configuration (#817). --- NEWS | 3 + borgmatic/borg/check.py | 31 +++- borgmatic/commands/arguments.py | 5 + borgmatic/config/schema.yaml | 45 ++++- tests/unit/borg/test_check.py | 285 +++++++++++++++++++++++++++----- 5 files changed, 319 insertions(+), 50 deletions(-) diff --git a/NEWS b/NEWS index def29f6..31ff50f 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,7 @@ 1.8.12.dev0 + * #817: Add a "--max-duration" flag to the "check" action and a "max_duration" option to the + repository check configuration. This tells Borg to interrupt a repository check after a certain + duration. * #860: Fix interaction between environment variable interpolation in constants and shell escaping. * #863: When color output is disabled (explicitly or implicitly), don't prefix each log line with the log level. diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index a58c60d..46984b9 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -50,10 +50,10 @@ def make_archive_filter_flags(local_borg_version, config, checks, check_argument return () -def make_check_flags(checks, archive_filter_flags): +def make_check_name_flags(checks, archive_filter_flags): ''' - Given a parsed checks set and a sequence of flags to filter archives, - transform the checks into tuple of command-line check flags. + Given parsed checks set and a sequence of flags to filter archives, transform the checks into + tuple of command-line check flags. For example, given parsed checks of: @@ -134,10 +134,30 @@ def check_archives( if logger.isEnabledFor(logging.DEBUG): verbosity_flags = ('--debug', '--show-rc') + try: + repository_check_config = next( + check for check in config.get('checks', ()) if check.get('name') == 'repository' + ) + except StopIteration: + repository_check_config = {} + + if check_arguments.max_duration and 'archives' in checks: + raise ValueError('The archives check cannot run when the --max-duration flag is used') + if repository_check_config.get('max_duration') and 'archives' in checks: + raise ValueError( + 'The archives check cannot run when the repository check has the max_duration option set' + ) + + max_duration = check_arguments.max_duration or repository_check_config.get('max_duration') + + borg_environment = environment.make_environment(config) + borg_exit_codes = config.get('borg_exit_codes') + full_command = ( (local_path, 'check') + (('--repair',) if check_arguments.repair else ()) - + make_check_flags(checks, archive_filter_flags) + + (('--max-duration', str(max_duration)) if max_duration else ()) + + make_check_name_flags(checks, archive_filter_flags) + (('--remote-path', remote_path) if remote_path else ()) + (('--log-json',) if global_arguments.log_json else ()) + (('--lock-wait', str(lock_wait)) if lock_wait else ()) @@ -147,9 +167,6 @@ def check_archives( + flags.make_repository_flags(repository_path, local_borg_version) ) - borg_environment = environment.make_environment(config) - borg_exit_codes = config.get('borg_exit_codes') - # The Borg repair option triggers an interactive prompt, which won't work when output is # captured. And progress messes with the terminal directly. if check_arguments.repair or check_arguments.progress: diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 2cf357e..0f3b7b5 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -661,6 +661,11 @@ def make_parsers(): action='store_true', help='Attempt to repair any inconsistencies found (for interactive use)', ) + check_group.add_argument( + '--max-duration', + metavar='SECONDS', + help='How long to check the repository before interrupting the check, defaults to no interruption', + ) check_group.add_argument( '-a', '--match-archives', diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index d8a3548..5d02771 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -511,7 +511,6 @@ properties: name: type: string enum: - - repository - archives - data - extract @@ -542,6 +541,50 @@ properties: "always": running this check every time checks are run. example: 2 weeks + - required: [name] + additionalProperties: false + properties: + name: + type: string + enum: + - repository + description: | + Name of consistency check to run: "repository", + "archives", "data", "spot", and/or "extract". + "repository" checks the consistency of the + repository, "archives" checks all of the + archives, "data" verifies the integrity of the + data within the archives, "spot" checks that + some percentage of source files are found in the + most recent archive (with identical contents), + and "extract" does an extraction dry-run of the + most recent archive. Note that "data" implies + "archives". See "skip_actions" for disabling + checks altogether. + example: spot + frequency: + type: string + description: | + How frequently to run this type of consistency + check (as a best effort). The value is a number + followed by a unit of time. E.g., "2 weeks" to + run this consistency check no more than every + two weeks for a given repository or "1 month" to + run it no more than monthly. Defaults to + "always": running this check every time checks + are run. + example: 2 weeks + max_duration: + type: integer + description: | + How many seconds to check the repository before + interrupting the check. Useful for splitting a + long-running repository check into multiple + partial checks. Defaults to no interruption. Only + applies to the "repository" check, does not check + the repository index, and is not compatible with a + simultaneous "archives" check or "--repair" flag. + example: 3600 - required: - name - count_tolerance_percentage diff --git a/tests/unit/borg/test_check.py b/tests/unit/borg/test_check.py index d34b4a2..5e335f0 100644 --- a/tests/unit/borg/test_check.py +++ b/tests/unit/borg/test_check.py @@ -222,35 +222,35 @@ def test_make_archive_filter_flags_with_default_checks_and_prefix_includes_match assert flags == ('--match-archives', 'sh:foo-*') -def test_make_check_flags_with_repository_check_returns_flag(): - flags = module.make_check_flags({'repository'}, ()) +def test_make_check_name_flags_with_repository_check_returns_flag(): + flags = module.make_check_name_flags({'repository'}, ()) assert flags == ('--repository-only',) -def test_make_check_flags_with_archives_check_returns_flag(): - flags = module.make_check_flags({'archives'}, ()) +def test_make_check_name_flags_with_archives_check_returns_flag(): + flags = module.make_check_name_flags({'archives'}, ()) assert flags == ('--archives-only',) -def test_make_check_flags_with_archives_check_and_archive_filter_flags_includes_those_flags(): - flags = module.make_check_flags({'archives'}, ('--match-archives', 'sh:foo-*')) +def test_make_check_name_flags_with_archives_check_and_archive_filter_flags_includes_those_flags(): + flags = module.make_check_name_flags({'archives'}, ('--match-archives', 'sh:foo-*')) assert flags == ('--archives-only', '--match-archives', 'sh:foo-*') -def test_make_check_flags_without_archives_check_and_with_archive_filter_flags_includes_those_flags(): - flags = module.make_check_flags({'repository'}, ('--match-archives', 'sh:foo-*')) +def test_make_check_name_flags_without_archives_check_and_with_archive_filter_flags_includes_those_flags(): + flags = module.make_check_name_flags({'repository'}, ('--match-archives', 'sh:foo-*')) assert flags == ('--repository-only',) -def test_make_check_flags_with_data_check_returns_flag_and_implies_archives(): +def test_make_check_name_flags_with_data_check_returns_flag_and_implies_archives(): flexmock(module.feature).should_receive('available').and_return(True) flexmock(module.flags).should_receive('make_match_archives_flags').and_return(()) - flags = module.make_check_flags({'data'}, ()) + flags = module.make_check_name_flags({'data'}, ()) assert flags == ( '--archives-only', @@ -258,20 +258,20 @@ def test_make_check_flags_with_data_check_returns_flag_and_implies_archives(): ) -def test_make_check_flags_with_extract_omits_extract_flag(): +def test_make_check_name_flags_with_extract_omits_extract_flag(): flexmock(module.feature).should_receive('available').and_return(True) flexmock(module.flags).should_receive('make_match_archives_flags').and_return(()) - flags = module.make_check_flags({'extract'}, ()) + flags = module.make_check_name_flags({'extract'}, ()) assert flags == () -def test_make_check_flags_with_repository_and_data_checks_does_not_return_repository_only(): +def test_make_check_name_flags_with_repository_and_data_checks_does_not_return_repository_only(): flexmock(module.feature).should_receive('available').and_return(True) flexmock(module.flags).should_receive('make_match_archives_flags').and_return(()) - flags = module.make_check_flags( + flags = module.make_check_name_flags( { 'repository', 'data', @@ -332,8 +332,7 @@ def test_get_repository_id_with_missing_json_keys_raises(): def test_check_archives_with_progress_passes_through_to_borg(): config = {} - flexmock(module).should_receive('make_check_flags').and_return(()) - flexmock(module).should_receive('execute_command').never() + flexmock(module).should_receive('make_check_name_flags').and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) flexmock(module.environment).should_receive('make_environment') flexmock(module).should_receive('execute_command').with_args( @@ -349,7 +348,12 @@ def test_check_archives_with_progress_passes_through_to_borg(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=True, repair=None, only_checks=None, force=None, match_archives=None + progress=True, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, ), global_arguments=flexmock(log_json=False), checks={'repository'}, @@ -359,8 +363,7 @@ def test_check_archives_with_progress_passes_through_to_borg(): def test_check_archives_with_repair_passes_through_to_borg(): config = {} - flexmock(module).should_receive('make_check_flags').and_return(()) - flexmock(module).should_receive('execute_command').never() + flexmock(module).should_receive('make_check_name_flags').and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) flexmock(module.environment).should_receive('make_environment') flexmock(module).should_receive('execute_command').with_args( @@ -376,7 +379,148 @@ def test_check_archives_with_repair_passes_through_to_borg(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=True, only_checks=None, force=None, match_archives=None + progress=None, + repair=True, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, + ), + global_arguments=flexmock(log_json=False), + checks={'repository'}, + archive_filter_flags=(), + ) + + +def test_check_archives_with_max_duration_flag_passes_through_to_borg(): + config = {} + flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) + flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'check', '--max-duration', '33', 'repo'), + extra_environment=None, + borg_local_path='borg', + borg_exit_codes=None, + ).once() + + module.check_archives( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + check_arguments=flexmock( + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=33, + ), + global_arguments=flexmock(log_json=False), + checks={'repository'}, + archive_filter_flags=(), + ) + + +def test_check_archives_with_max_duration_flag_and_archives_check_errors(): + config = {} + flexmock(module).should_receive('execute_command').never() + + with pytest.raises(ValueError): + module.check_archives( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + check_arguments=flexmock( + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=33, + ), + global_arguments=flexmock(log_json=False), + checks={'repository', 'archives'}, + archive_filter_flags=(), + ) + + +def test_check_archives_with_max_duration_option_passes_through_to_borg(): + config = {'checks': [{'name': 'repository', 'max_duration': 33}]} + flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) + flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'check', '--max-duration', '33', 'repo'), + extra_environment=None, + borg_local_path='borg', + borg_exit_codes=None, + ).once() + + module.check_archives( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + check_arguments=flexmock( + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, + ), + global_arguments=flexmock(log_json=False), + checks={'repository'}, + archive_filter_flags=(), + ) + + +def test_check_archives_with_max_duration_option_and_archives_check_errors(): + config = {'checks': [{'name': 'repository', 'max_duration': 33}]} + flexmock(module).should_receive('execute_command').never() + + with pytest.raises(ValueError): + module.check_archives( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + check_arguments=flexmock( + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, + ), + global_arguments=flexmock(log_json=False), + checks={'repository', 'archives'}, + archive_filter_flags=(), + ) + + +def test_check_archives_with_max_duration_flag_overrides_max_duration_option(): + config = {'checks': [{'name': 'repository', 'max_duration': 33}]} + flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) + flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'check', '--max-duration', '44', 'repo'), + extra_environment=None, + borg_local_path='borg', + borg_exit_codes=None, + ).once() + + module.check_archives( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + check_arguments=flexmock( + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=44, ), global_arguments=flexmock(log_json=False), checks={'repository'}, @@ -395,7 +539,7 @@ def test_check_archives_with_repair_passes_through_to_borg(): ) def test_check_archives_calls_borg_with_parameters(checks): config = {} - flexmock(module).should_receive('make_check_flags').with_args(checks, ()).and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args(checks, ()).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_execute_command_mock(('borg', 'check', 'repo')) @@ -404,7 +548,12 @@ def test_check_archives_calls_borg_with_parameters(checks): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=None, only_checks=None, force=None, match_archives=None + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, ), global_arguments=flexmock(log_json=False), checks=checks, @@ -414,7 +563,7 @@ def test_check_archives_calls_borg_with_parameters(checks): def test_check_archives_with_log_info_passes_through_to_borg(): config = {} - flexmock(module).should_receive('make_check_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_logging_mock(logging.INFO) insert_execute_command_mock(('borg', 'check', '--info', 'repo')) @@ -424,7 +573,12 @@ def test_check_archives_with_log_info_passes_through_to_borg(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=None, only_checks=None, force=None, match_archives=None + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, ), global_arguments=flexmock(log_json=False), checks={'repository'}, @@ -434,7 +588,7 @@ def test_check_archives_with_log_info_passes_through_to_borg(): def test_check_archives_with_log_debug_passes_through_to_borg(): config = {} - flexmock(module).should_receive('make_check_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_logging_mock(logging.DEBUG) insert_execute_command_mock(('borg', 'check', '--debug', '--show-rc', 'repo')) @@ -444,7 +598,12 @@ def test_check_archives_with_log_debug_passes_through_to_borg(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=None, only_checks=None, force=None, match_archives=None + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, ), global_arguments=flexmock(log_json=False), checks={'repository'}, @@ -455,7 +614,7 @@ def test_check_archives_with_log_debug_passes_through_to_borg(): def test_check_archives_with_local_path_calls_borg_via_local_path(): checks = {'repository'} config = {} - flexmock(module).should_receive('make_check_flags').with_args(checks, ()).and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args(checks, ()).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_execute_command_mock(('borg1', 'check', 'repo')) @@ -464,7 +623,12 @@ def test_check_archives_with_local_path_calls_borg_via_local_path(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=None, only_checks=None, force=None, match_archives=None + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, ), global_arguments=flexmock(log_json=False), checks=checks, @@ -477,7 +641,7 @@ def test_check_archives_with_exit_codes_calls_borg_using_them(): checks = {'repository'} borg_exit_codes = flexmock() config = {'borg_exit_codes': borg_exit_codes} - flexmock(module).should_receive('make_check_flags').with_args(checks, ()).and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args(checks, ()).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_execute_command_mock(('borg', 'check', 'repo'), borg_exit_codes=borg_exit_codes) @@ -486,7 +650,12 @@ def test_check_archives_with_exit_codes_calls_borg_using_them(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=None, only_checks=None, force=None, match_archives=None + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, ), global_arguments=flexmock(log_json=False), checks=checks, @@ -497,7 +666,7 @@ def test_check_archives_with_exit_codes_calls_borg_using_them(): def test_check_archives_with_remote_path_passes_through_to_borg(): checks = {'repository'} config = {} - flexmock(module).should_receive('make_check_flags').with_args(checks, ()).and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args(checks, ()).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_execute_command_mock(('borg', 'check', '--remote-path', 'borg1', 'repo')) @@ -506,7 +675,12 @@ def test_check_archives_with_remote_path_passes_through_to_borg(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=None, only_checks=None, force=None, match_archives=None + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, ), global_arguments=flexmock(log_json=False), checks=checks, @@ -518,7 +692,7 @@ def test_check_archives_with_remote_path_passes_through_to_borg(): def test_check_archives_with_log_json_passes_through_to_borg(): checks = {'repository'} config = {} - flexmock(module).should_receive('make_check_flags').with_args(checks, ()).and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args(checks, ()).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_execute_command_mock(('borg', 'check', '--log-json', 'repo')) @@ -527,7 +701,12 @@ def test_check_archives_with_log_json_passes_through_to_borg(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=None, only_checks=None, force=None, match_archives=None + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, ), global_arguments=flexmock(log_json=True), checks=checks, @@ -538,7 +717,7 @@ def test_check_archives_with_log_json_passes_through_to_borg(): def test_check_archives_with_lock_wait_passes_through_to_borg(): checks = {'repository'} config = {'lock_wait': 5} - flexmock(module).should_receive('make_check_flags').with_args(checks, ()).and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args(checks, ()).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_execute_command_mock(('borg', 'check', '--lock-wait', '5', 'repo')) @@ -547,7 +726,12 @@ def test_check_archives_with_lock_wait_passes_through_to_borg(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=None, only_checks=None, force=None, match_archives=None + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, ), global_arguments=flexmock(log_json=False), checks=checks, @@ -559,7 +743,7 @@ def test_check_archives_with_retention_prefix(): checks = {'repository'} prefix = 'foo-' config = {'prefix': prefix} - flexmock(module).should_receive('make_check_flags').with_args(checks, ()).and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args(checks, ()).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_execute_command_mock(('borg', 'check', 'repo')) @@ -568,7 +752,12 @@ def test_check_archives_with_retention_prefix(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=None, only_checks=None, force=None, match_archives=None + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, ), global_arguments=flexmock(log_json=False), checks=checks, @@ -578,7 +767,7 @@ def test_check_archives_with_retention_prefix(): def test_check_archives_with_extra_borg_options_passes_through_to_borg(): config = {'extra_borg_options': {'check': '--extra --options'}} - flexmock(module).should_receive('make_check_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_execute_command_mock(('borg', 'check', '--extra', '--options', 'repo')) @@ -587,7 +776,12 @@ def test_check_archives_with_extra_borg_options_passes_through_to_borg(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=None, only_checks=None, force=None, match_archives=None + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, ), global_arguments=flexmock(log_json=False), checks={'repository'}, @@ -597,7 +791,9 @@ def test_check_archives_with_extra_borg_options_passes_through_to_borg(): def test_check_archives_with_match_archives_passes_through_to_borg(): config = {} - flexmock(module).should_receive('make_check_flags').and_return(('--match-archives', 'foo-*')) + flexmock(module).should_receive('make_check_name_flags').and_return( + ('--match-archives', 'foo-*') + ) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) flexmock(module.environment).should_receive('make_environment') flexmock(module).should_receive('execute_command').with_args( @@ -612,7 +808,12 @@ def test_check_archives_with_match_archives_passes_through_to_borg(): config=config, local_borg_version='1.2.3', check_arguments=flexmock( - progress=None, repair=None, only_checks=None, force=None, match_archives='foo-*' + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives='foo-*', + max_duration=None, ), global_arguments=flexmock(log_json=False), checks={'archives'},