From c4aa34bf5c00a6eded73c08d748533527c89f583 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 30 Nov 2019 14:51:32 -0800 Subject: [PATCH 01/30] Fix for missing Healthchecks monitoring payload or HTTP 500 due to incorrect unicode encoding (#260). --- NEWS | 3 +++ borgmatic/hooks/healthchecks.py | 2 +- setup.py | 2 +- tests/unit/hooks/test_healthchecks.py | 16 +++++++++------- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index 7e411212..92333826 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +1.4.16 + * Fix for missing Healthchecks monitoring payload or HTTP 500 due to incorrect unicode encoding. + 1.4.15 * Fix for database dump removal incorrectly skipping some database dumps. * #123: Support for mounting an archive as a FUSE filesystem via "borgmatic mount" action, and diff --git a/borgmatic/hooks/healthchecks.py b/borgmatic/hooks/healthchecks.py index 19201d0f..a116205a 100644 --- a/borgmatic/hooks/healthchecks.py +++ b/borgmatic/hooks/healthchecks.py @@ -97,4 +97,4 @@ def ping_monitor(ping_url_or_uuid, config_filename, state, dry_run): if not dry_run: logging.getLogger('urllib3').setLevel(logging.ERROR) - requests.post(ping_url, data=payload) + requests.post(ping_url, data=payload.encode('utf-8')) diff --git a/setup.py b/setup.py index cf16442a..46871a9c 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.4.15' +VERSION = '1.4.16' setup( diff --git a/tests/unit/hooks/test_healthchecks.py b/tests/unit/hooks/test_healthchecks.py index cbb9cc76..24e8fca0 100644 --- a/tests/unit/hooks/test_healthchecks.py +++ b/tests/unit/hooks/test_healthchecks.py @@ -60,7 +60,7 @@ def test_ping_monitor_hits_ping_url_for_start_state(): flexmock(module).should_receive('Forgetful_buffering_handler') ping_url = 'https://example.com' flexmock(module.requests).should_receive('post').with_args( - '{}/{}'.format(ping_url, 'start'), data='' + '{}/{}'.format(ping_url, 'start'), data=''.encode('utf-8') ) module.ping_monitor(ping_url, 'config.yaml', state=module.monitor.State.START, dry_run=False) @@ -68,19 +68,21 @@ def test_ping_monitor_hits_ping_url_for_start_state(): def test_ping_monitor_hits_ping_url_for_finish_state(): ping_url = 'https://example.com' - payload = flexmock() + payload = 'data' flexmock(module).should_receive('format_buffered_logs_for_payload').and_return(payload) - flexmock(module.requests).should_receive('post').with_args(ping_url, data=payload) + flexmock(module.requests).should_receive('post').with_args( + ping_url, data=payload.encode('utf-8') + ) module.ping_monitor(ping_url, 'config.yaml', state=module.monitor.State.FINISH, dry_run=False) def test_ping_monitor_hits_ping_url_for_fail_state(): ping_url = 'https://example.com' - payload = flexmock() + payload = 'data' flexmock(module).should_receive('format_buffered_logs_for_payload').and_return(payload) flexmock(module.requests).should_receive('post').with_args( - '{}/{}'.format(ping_url, 'fail'), data=payload + '{}/{}'.format(ping_url, 'fail'), data=payload.encode('utf') ) module.ping_monitor(ping_url, 'config.yaml', state=module.monitor.State.FAIL, dry_run=False) @@ -88,10 +90,10 @@ def test_ping_monitor_hits_ping_url_for_fail_state(): def test_ping_monitor_with_ping_uuid_hits_corresponding_url(): ping_uuid = 'abcd-efgh-ijkl-mnop' - payload = flexmock() + payload = 'data' flexmock(module).should_receive('format_buffered_logs_for_payload').and_return(payload) flexmock(module.requests).should_receive('post').with_args( - 'https://hc-ping.com/{}'.format(ping_uuid), data=payload + 'https://hc-ping.com/{}'.format(ping_uuid), data=payload.encode('utf-8') ) module.ping_monitor(ping_uuid, 'config.yaml', state=module.monitor.State.FINISH, dry_run=False) From 9b2ca15de6ba5ad17b0aac19156c1ef6571764a7 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 30 Nov 2019 15:31:36 -0800 Subject: [PATCH 02/30] Fix for garbled Borg file listing when using "borgmatic create --progress" with verbosity level 1 or 2 (#257). --- NEWS | 5 ++++- borgmatic/borg/create.py | 6 +++++- tests/unit/borg/test_create.py | 25 +++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 92333826..d019c5bf 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,8 @@ 1.4.16 - * Fix for missing Healthchecks monitoring payload or HTTP 500 due to incorrect unicode encoding. + * #257: Fix for garbled Borg file listing when using "borgmatic create --progress" with + verbosity level 1 or 2. + * #260: Fix for missing Healthchecks monitoring payload or HTTP 500 due to incorrect unicode + encoding. 1.4.15 * Fix for database dump removal incorrectly skipping some database dumps. diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index f3e8c2ad..3190a8fc 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -170,7 +170,11 @@ def create_archive( + (('--remote-path', remote_path) if remote_path else ()) + (('--umask', str(umask)) if umask else ()) + (('--lock-wait', str(lock_wait)) if lock_wait else ()) - + (('--list', '--filter', 'AME-') if logger.isEnabledFor(logging.INFO) and not json else ()) + + ( + ('--list', '--filter', 'AME-') + if logger.isEnabledFor(logging.INFO) and not json and not progress + else () + ) + (('--info',) if logger.getEffectiveLevel() == logging.INFO and not json else ()) + ( ('--stats',) diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 156be573..d2cb12f0 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -821,6 +821,31 @@ def test_create_archive_with_stats_calls_borg_with_stats_parameter(): ) +def test_create_archive_with_progress_and_log_info_calls_borg_with_progress_parameter_and_no_list(): + flexmock(module).should_receive('borgmatic_source_directories').and_return([]) + flexmock(module).should_receive('_expand_directories').and_return(('foo', 'bar')) + flexmock(module).should_receive('_expand_home_directories').and_return(()) + flexmock(module).should_receive('_write_pattern_file').and_return(None) + flexmock(module).should_receive('_make_pattern_flags').and_return(()) + flexmock(module).should_receive('_make_exclude_flags').and_return(()) + flexmock(module).should_receive('execute_command_without_capture').with_args( + ('borg', 'create', '--info', '--stats', '--progress') + ARCHIVE_WITH_PATHS + ) + insert_logging_mock(logging.INFO) + + module.create_archive( + dry_run=False, + repository='repo', + location_config={ + 'source_directories': ['foo', 'bar'], + 'repositories': ['repo'], + 'exclude_patterns': None, + }, + storage_config={}, + progress=True, + ) + + def test_create_archive_with_progress_calls_borg_with_progress_parameter(): flexmock(module).should_receive('borgmatic_source_directories').and_return([]) flexmock(module).should_receive('_expand_directories').and_return(('foo', 'bar')) From 00f62ca023c54b46aa0b6da7e97a632c45f1c727 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 30 Nov 2019 16:55:05 -0800 Subject: [PATCH 03/30] Fix for "before_backup" hook not triggering an error when the command contains "borg" and has an exit code of 1 (#256). --- NEWS | 2 + borgmatic/borg/create.py | 4 +- borgmatic/borg/extract.py | 2 +- borgmatic/borg/info.py | 4 +- borgmatic/borg/init.py | 2 +- borgmatic/borg/list.py | 4 +- borgmatic/borg/mount.py | 4 +- borgmatic/borg/prune.py | 6 ++- borgmatic/execute.py | 18 +++---- tests/unit/borg/test_create.py | 88 ++++++++++++++++++++++++++------- tests/unit/borg/test_extract.py | 2 +- tests/unit/borg/test_info.py | 29 +++++++---- tests/unit/borg/test_init.py | 2 +- tests/unit/borg/test_list.py | 34 ++++++++----- tests/unit/borg/test_mount.py | 6 ++- tests/unit/borg/test_prune.py | 2 +- tests/unit/test_execute.py | 54 +++++++------------- 17 files changed, 165 insertions(+), 98 deletions(-) diff --git a/NEWS b/NEWS index d019c5bf..dac9e98a 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ 1.4.16 + * #256: Fix for "before_backup" hook not triggering an error when the command contains "borg" and + has an exit code of 1. * #257: Fix for garbled Borg file listing when using "borgmatic create --progress" with verbosity level 1 or 2. * #260: Fix for missing Healthchecks monitoring payload or HTTP 500 due to incorrect unicode diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 3190a8fc..879a94a1 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -196,7 +196,7 @@ def create_archive( # The progress output isn't compatible with captured and logged output, as progress messes with # the terminal directly. if progress: - execute_command_without_capture(full_command) + execute_command_without_capture(full_command, error_on_warnings=False) return if json: @@ -206,4 +206,4 @@ def create_archive( else: output_log_level = logging.INFO - return execute_command(full_command, output_log_level) + return execute_command(full_command, output_log_level, error_on_warnings=False) diff --git a/borgmatic/borg/extract.py b/borgmatic/borg/extract.py index 7a32ef6d..09af5376 100644 --- a/borgmatic/borg/extract.py +++ b/borgmatic/borg/extract.py @@ -27,7 +27,7 @@ def extract_last_archive_dry_run(repository, lock_wait=None, local_path='borg', + (repository,) ) - list_output = execute_command(full_list_command, output_log_level=None) + list_output = execute_command(full_list_command, output_log_level=None, error_on_warnings=False) try: last_archive_name = list_output.strip().splitlines()[-1] diff --git a/borgmatic/borg/info.py b/borgmatic/borg/info.py index cbc50d12..3ff9312c 100644 --- a/borgmatic/borg/info.py +++ b/borgmatic/borg/info.py @@ -39,5 +39,7 @@ def display_archives_info( ) return execute_command( - full_command, output_log_level=None if info_arguments.json else logging.WARNING + full_command, + output_log_level=None if info_arguments.json else logging.WARNING, + error_on_warnings=False, ) diff --git a/borgmatic/borg/init.py b/borgmatic/borg/init.py index cb787ae9..1ec25c07 100644 --- a/borgmatic/borg/init.py +++ b/borgmatic/borg/init.py @@ -45,4 +45,4 @@ def initialize_repository( ) # Don't use execute_command() here because it doesn't support interactive prompts. - execute_command_without_capture(init_command) + execute_command_without_capture(init_command, error_on_warnings=False) diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index 854cefff..bcbe36e2 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -46,5 +46,7 @@ def list_archives(repository, storage_config, list_arguments, local_path='borg', ) return execute_command( - full_command, output_log_level=None if list_arguments.json else logging.WARNING + full_command, + output_log_level=None if list_arguments.json else logging.WARNING, + error_on_warnings=False, ) diff --git a/borgmatic/borg/mount.py b/borgmatic/borg/mount.py index 6580717c..4e700097 100644 --- a/borgmatic/borg/mount.py +++ b/borgmatic/borg/mount.py @@ -40,7 +40,7 @@ def mount_archive( # Don't capture the output when foreground mode is used so that ctrl-C can work properly. if foreground: - execute_command_without_capture(full_command) + execute_command_without_capture(full_command, error_on_warnings=False) return - execute_command(full_command) + execute_command(full_command, error_on_warnings=False) diff --git a/borgmatic/borg/prune.py b/borgmatic/borg/prune.py index ec5963f9..584973e1 100644 --- a/borgmatic/borg/prune.py +++ b/borgmatic/borg/prune.py @@ -64,4 +64,8 @@ def prune_archives( + (repository,) ) - execute_command(full_command, output_log_level=logging.WARNING if stats else logging.INFO) + execute_command( + full_command, + output_log_level=logging.WARNING if stats else logging.INFO, + error_on_warnings=False, + ) diff --git a/borgmatic/execute.py b/borgmatic/execute.py index 4b791b9b..0d5047c1 100644 --- a/borgmatic/execute.py +++ b/borgmatic/execute.py @@ -9,15 +9,15 @@ ERROR_OUTPUT_MAX_LINE_COUNT = 25 BORG_ERROR_EXIT_CODE = 2 -def exit_code_indicates_error(command, exit_code, error_on_warnings=False): +def exit_code_indicates_error(command, exit_code, error_on_warnings=True): ''' Return True if the given exit code from running the command corresponds to an error. + If error on warnings is False, then treat exit code 1 as a warning instead of an error. ''' - # If we're running something other than Borg, treat all non-zero exit codes as errors. - if 'borg' in command[0] and not error_on_warnings: - return bool(exit_code >= BORG_ERROR_EXIT_CODE) + if error_on_warnings: + return bool(exit_code != 0) - return bool(exit_code != 0) + return bool(exit_code >= BORG_ERROR_EXIT_CODE) def log_output(command, process, output_buffer, output_log_level, error_on_warnings): @@ -65,7 +65,7 @@ def execute_command( shell=False, extra_environment=None, working_directory=None, - error_on_warnings=False, + error_on_warnings=True, ): ''' Execute the given command (a sequence of command/argument strings) and log its output at the @@ -75,7 +75,7 @@ def execute_command( file. If shell is True, execute the command within a shell. If an extra environment dict is given, then use it to augment the current environment, and pass the result into the command. If a working directory is given, use that as the present working directory when running the - command. + command. If error on warnings is False, then treat exit code 1 as a warning instead of an error. Raise subprocesses.CalledProcessError if an error occurs while running the command. ''' @@ -110,14 +110,14 @@ def execute_command( ) -def execute_command_without_capture(full_command, working_directory=None, error_on_warnings=False): +def execute_command_without_capture(full_command, working_directory=None, error_on_warnings=True): ''' Execute the given command (a sequence of command/argument strings), but don't capture or log its output in any way. This is necessary for commands that monkey with the terminal (e.g. progress display) or provide interactive prompts. If a working directory is given, use that as the present working directory when running the - command. + command. If error on warnings is False, then treat exit code 1 as a warning instead of an error. ''' logger.debug(' '.join(full_command)) diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index d2cb12f0..5819df02 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -206,7 +206,9 @@ def test_create_archive_calls_borg_with_parameters(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO + ('borg', 'create') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -232,7 +234,9 @@ def test_create_archive_with_patterns_calls_borg_with_patterns(): flexmock(module).should_receive('_make_pattern_flags').and_return(pattern_flags) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create') + pattern_flags + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO + ('borg', 'create') + pattern_flags + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -258,7 +262,9 @@ def test_create_archive_with_exclude_patterns_calls_borg_with_excludes(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(exclude_flags) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create') + exclude_flags + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO + ('borg', 'create') + exclude_flags + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -284,6 +290,7 @@ def test_create_archive_with_log_info_calls_borg_with_info_parameter(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--list', '--filter', 'AME-', '--info', '--stats') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + error_on_warnings=False, ) insert_logging_mock(logging.INFO) @@ -308,7 +315,9 @@ def test_create_archive_with_log_info_and_json_suppresses_most_borg_output(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, output_log_level=None + ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, + output_log_level=None, + error_on_warnings=False, ) insert_logging_mock(logging.INFO) @@ -336,6 +345,7 @@ def test_create_archive_with_log_debug_calls_borg_with_debug_parameter(): ('borg', 'create', '--list', '--filter', 'AME-', '--stats', '--debug', '--show-rc') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + error_on_warnings=False, ) insert_logging_mock(logging.DEBUG) @@ -359,7 +369,9 @@ def test_create_archive_with_log_debug_and_json_suppresses_most_borg_output(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, output_log_level=None + ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, + output_log_level=None, + error_on_warnings=False, ) insert_logging_mock(logging.DEBUG) @@ -385,7 +397,9 @@ def test_create_archive_with_dry_run_calls_borg_with_dry_run_parameter(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', '--dry-run') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO + ('borg', 'create', '--dry-run') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -414,6 +428,7 @@ def test_create_archive_with_dry_run_and_log_info_calls_borg_without_stats_param ('borg', 'create', '--list', '--filter', 'AME-', '--info', '--dry-run') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + error_on_warnings=False, ) insert_logging_mock(logging.INFO) @@ -443,6 +458,7 @@ def test_create_archive_with_dry_run_and_log_debug_calls_borg_without_stats_para ('borg', 'create', '--list', '--filter', 'AME-', '--debug', '--show-rc', '--dry-run') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + error_on_warnings=False, ) insert_logging_mock(logging.DEBUG) @@ -468,6 +484,7 @@ def test_create_archive_with_checkpoint_interval_calls_borg_with_checkpoint_inte flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--checkpoint-interval', '600') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -492,6 +509,7 @@ def test_create_archive_with_chunker_params_calls_borg_with_chunker_params_param flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--chunker-params', '1,2,3,4') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -516,6 +534,7 @@ def test_create_archive_with_compression_calls_borg_with_compression_parameters( flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--compression', 'rle') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -540,6 +559,7 @@ def test_create_archive_with_remote_rate_limit_calls_borg_with_remote_ratelimit_ flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--remote-ratelimit', '100') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -562,7 +582,9 @@ def test_create_archive_with_one_file_system_calls_borg_with_one_file_system_par flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', '--one-file-system') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO + ('borg', 'create', '--one-file-system') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -586,7 +608,9 @@ def test_create_archive_with_numeric_owner_calls_borg_with_numeric_owner_paramet flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', '--numeric-owner') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO + ('borg', 'create', '--numeric-owner') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -610,7 +634,9 @@ def test_create_archive_with_read_special_calls_borg_with_read_special_parameter flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', '--read-special') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO + ('borg', 'create', '--read-special') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -635,7 +661,9 @@ def test_create_archive_with_option_true_calls_borg_without_corresponding_parame flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO + ('borg', 'create') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -662,6 +690,7 @@ def test_create_archive_with_option_false_calls_borg_with_corresponding_paramete flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--no' + option_name.replace('_', '')) + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -687,6 +716,7 @@ def test_create_archive_with_files_cache_calls_borg_with_files_cache_parameters( flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--files-cache', 'ctime,size') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -710,7 +740,9 @@ def test_create_archive_with_local_path_calls_borg_via_local_path(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg1', 'create') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO + ('borg1', 'create') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -736,6 +768,7 @@ def test_create_archive_with_remote_path_calls_borg_with_remote_path_parameters( flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--remote-path', 'borg1') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -759,7 +792,9 @@ def test_create_archive_with_umask_calls_borg_with_umask_parameters(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', '--umask', '740') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO + ('borg', 'create', '--umask', '740') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -782,7 +817,9 @@ def test_create_archive_with_lock_wait_calls_borg_with_lock_wait_parameters(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', '--lock-wait', '5') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO + ('borg', 'create', '--lock-wait', '5') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -805,7 +842,9 @@ def test_create_archive_with_stats_calls_borg_with_stats_parameter(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', '--stats') + ARCHIVE_WITH_PATHS, output_log_level=logging.WARNING + ('borg', 'create', '--stats') + ARCHIVE_WITH_PATHS, + output_log_level=logging.WARNING, + error_on_warnings=False, ) module.create_archive( @@ -829,7 +868,8 @@ def test_create_archive_with_progress_and_log_info_calls_borg_with_progress_para flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command_without_capture').with_args( - ('borg', 'create', '--info', '--stats', '--progress') + ARCHIVE_WITH_PATHS + ('borg', 'create', '--info', '--stats', '--progress') + ARCHIVE_WITH_PATHS, + error_on_warnings=False, ) insert_logging_mock(logging.INFO) @@ -854,7 +894,7 @@ def test_create_archive_with_progress_calls_borg_with_progress_parameter(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command_without_capture').with_args( - ('borg', 'create', '--progress') + ARCHIVE_WITH_PATHS + ('borg', 'create', '--progress') + ARCHIVE_WITH_PATHS, error_on_warnings=False ) module.create_archive( @@ -878,7 +918,9 @@ def test_create_archive_with_json_calls_borg_with_json_parameter(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, output_log_level=None + ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, + output_log_level=None, + error_on_warnings=False, ).and_return('[]') json_output = module.create_archive( @@ -904,7 +946,9 @@ def test_create_archive_with_stats_and_json_calls_borg_without_stats_parameter() flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, output_log_level=None + ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, + output_log_level=None, + error_on_warnings=False, ).and_return('[]') json_output = module.create_archive( @@ -933,6 +977,7 @@ def test_create_archive_with_source_directories_glob_expands(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo', 'food'), output_log_level=logging.INFO, + error_on_warnings=False, ) flexmock(module.glob).should_receive('glob').with_args('foo*').and_return(['foo', 'food']) @@ -958,6 +1003,7 @@ def test_create_archive_with_non_matching_source_directories_glob_passes_through flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo*'), output_log_level=logging.INFO, + error_on_warnings=False, ) flexmock(module.glob).should_receive('glob').with_args('foo*').and_return([]) @@ -983,6 +1029,7 @@ def test_create_archive_with_glob_calls_borg_with_expanded_directories(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo', 'food'), output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -1005,7 +1052,9 @@ def test_create_archive_with_archive_name_format_calls_borg_with_archive_name(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create', 'repo::ARCHIVE_NAME', 'foo', 'bar'), output_log_level=logging.INFO + ('borg', 'create', 'repo::ARCHIVE_NAME', 'foo', 'bar'), + output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( @@ -1030,6 +1079,7 @@ def test_create_archive_with_archive_name_format_accepts_borg_placeholders(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', 'repo::Documents_{hostname}-{now}', 'foo', 'bar'), output_log_level=logging.INFO, + error_on_warnings=False, ) module.create_archive( diff --git a/tests/unit/borg/test_extract.py b/tests/unit/borg/test_extract.py index 027523c8..f6c904e3 100644 --- a/tests/unit/borg/test_extract.py +++ b/tests/unit/borg/test_extract.py @@ -15,7 +15,7 @@ def insert_execute_command_mock(command, working_directory=None, error_on_warnin def insert_execute_command_output_mock(command, result): flexmock(module).should_receive('execute_command').with_args( - command, output_log_level=None + command, output_log_level=None, error_on_warnings=False ).and_return(result).once() diff --git a/tests/unit/borg/test_info.py b/tests/unit/borg/test_info.py index 93ff843a..f09d4e8d 100644 --- a/tests/unit/borg/test_info.py +++ b/tests/unit/borg/test_info.py @@ -10,7 +10,7 @@ from ..test_verbosity import insert_logging_mock def test_display_archives_info_calls_borg_with_parameters(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'info', 'repo'), output_log_level=logging.WARNING + ('borg', 'info', 'repo'), output_log_level=logging.WARNING, error_on_warnings=False ) module.display_archives_info( @@ -20,7 +20,9 @@ def test_display_archives_info_calls_borg_with_parameters(): def test_display_archives_info_with_log_info_calls_borg_with_info_parameter(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'info', '--info', 'repo'), output_log_level=logging.WARNING + ('borg', 'info', '--info', 'repo'), + output_log_level=logging.WARNING, + error_on_warnings=False, ) insert_logging_mock(logging.INFO) module.display_archives_info( @@ -30,7 +32,7 @@ def test_display_archives_info_with_log_info_calls_borg_with_info_parameter(): def test_display_archives_info_with_log_info_and_json_suppresses_most_borg_output(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'info', '--json', 'repo'), output_log_level=None + ('borg', 'info', '--json', 'repo'), output_log_level=None, error_on_warnings=False ).and_return('[]') insert_logging_mock(logging.INFO) @@ -43,7 +45,9 @@ def test_display_archives_info_with_log_info_and_json_suppresses_most_borg_outpu def test_display_archives_info_with_log_debug_calls_borg_with_debug_parameter(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'info', '--debug', '--show-rc', 'repo'), output_log_level=logging.WARNING + ('borg', 'info', '--debug', '--show-rc', 'repo'), + output_log_level=logging.WARNING, + error_on_warnings=False, ) insert_logging_mock(logging.DEBUG) @@ -54,7 +58,7 @@ def test_display_archives_info_with_log_debug_calls_borg_with_debug_parameter(): def test_display_archives_info_with_log_debug_and_json_suppresses_most_borg_output(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'info', '--json', 'repo'), output_log_level=None + ('borg', 'info', '--json', 'repo'), output_log_level=None, error_on_warnings=False ).and_return('[]') insert_logging_mock(logging.DEBUG) @@ -67,7 +71,7 @@ def test_display_archives_info_with_log_debug_and_json_suppresses_most_borg_outp def test_display_archives_info_with_json_calls_borg_with_json_parameter(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'info', '--json', 'repo'), output_log_level=None + ('borg', 'info', '--json', 'repo'), output_log_level=None, error_on_warnings=False ).and_return('[]') json_output = module.display_archives_info( @@ -79,7 +83,7 @@ def test_display_archives_info_with_json_calls_borg_with_json_parameter(): def test_display_archives_info_with_archive_calls_borg_with_archive_parameter(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'info', 'repo::archive'), output_log_level=logging.WARNING + ('borg', 'info', 'repo::archive'), output_log_level=logging.WARNING, error_on_warnings=False ) module.display_archives_info( @@ -89,7 +93,7 @@ def test_display_archives_info_with_archive_calls_borg_with_archive_parameter(): def test_display_archives_info_with_local_path_calls_borg_via_local_path(): flexmock(module).should_receive('execute_command').with_args( - ('borg1', 'info', 'repo'), output_log_level=logging.WARNING + ('borg1', 'info', 'repo'), output_log_level=logging.WARNING, error_on_warnings=False ) module.display_archives_info( @@ -102,7 +106,9 @@ def test_display_archives_info_with_local_path_calls_borg_via_local_path(): def test_display_archives_info_with_remote_path_calls_borg_with_remote_path_parameters(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'info', '--remote-path', 'borg1', 'repo'), output_log_level=logging.WARNING + ('borg', 'info', '--remote-path', 'borg1', 'repo'), + output_log_level=logging.WARNING, + error_on_warnings=False, ) module.display_archives_info( @@ -116,7 +122,9 @@ def test_display_archives_info_with_remote_path_calls_borg_with_remote_path_para def test_display_archives_info_with_lock_wait_calls_borg_with_lock_wait_parameters(): storage_config = {'lock_wait': 5} flexmock(module).should_receive('execute_command').with_args( - ('borg', 'info', '--lock-wait', '5', 'repo'), output_log_level=logging.WARNING + ('borg', 'info', '--lock-wait', '5', 'repo'), + output_log_level=logging.WARNING, + error_on_warnings=False, ) module.display_archives_info( @@ -131,6 +139,7 @@ def test_display_archives_info_passes_through_arguments_to_borg(argument_name): flexmock(module).should_receive('execute_command').with_args( ('borg', 'info', '--' + argument_name.replace('_', '-'), 'value', 'repo'), output_log_level=logging.WARNING, + error_on_warnings=False, ) module.display_archives_info( diff --git a/tests/unit/borg/test_init.py b/tests/unit/borg/test_init.py index 33c40b33..fffdfd7f 100644 --- a/tests/unit/borg/test_init.py +++ b/tests/unit/borg/test_init.py @@ -24,7 +24,7 @@ def insert_info_command_not_found_mock(): def insert_init_command_mock(init_command, **kwargs): flexmock(module).should_receive('execute_command_without_capture').with_args( - init_command + init_command, error_on_warnings=False ).once() diff --git a/tests/unit/borg/test_list.py b/tests/unit/borg/test_list.py index 2b84d0e4..4e3de334 100644 --- a/tests/unit/borg/test_list.py +++ b/tests/unit/borg/test_list.py @@ -10,7 +10,7 @@ from ..test_verbosity import insert_logging_mock def test_list_archives_calls_borg_with_parameters(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'list', 'repo'), output_log_level=logging.WARNING + ('borg', 'list', 'repo'), output_log_level=logging.WARNING, error_on_warnings=False ) module.list_archives( @@ -22,7 +22,9 @@ def test_list_archives_calls_borg_with_parameters(): def test_list_archives_with_log_info_calls_borg_with_info_parameter(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'list', '--info', 'repo'), output_log_level=logging.WARNING + ('borg', 'list', '--info', 'repo'), + output_log_level=logging.WARNING, + error_on_warnings=False, ) insert_logging_mock(logging.INFO) @@ -35,7 +37,7 @@ def test_list_archives_with_log_info_calls_borg_with_info_parameter(): def test_list_archives_with_log_info_and_json_suppresses_most_borg_output(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'list', '--json', 'repo'), output_log_level=None + ('borg', 'list', '--json', 'repo'), output_log_level=None, error_on_warnings=False ) insert_logging_mock(logging.INFO) @@ -48,7 +50,9 @@ def test_list_archives_with_log_info_and_json_suppresses_most_borg_output(): def test_list_archives_with_log_debug_calls_borg_with_debug_parameter(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'list', '--debug', '--show-rc', 'repo'), output_log_level=logging.WARNING + ('borg', 'list', '--debug', '--show-rc', 'repo'), + output_log_level=logging.WARNING, + error_on_warnings=False, ) insert_logging_mock(logging.DEBUG) @@ -61,7 +65,7 @@ def test_list_archives_with_log_debug_calls_borg_with_debug_parameter(): def test_list_archives_with_log_debug_and_json_suppresses_most_borg_output(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'list', '--json', 'repo'), output_log_level=None + ('borg', 'list', '--json', 'repo'), output_log_level=None, error_on_warnings=False ) insert_logging_mock(logging.DEBUG) @@ -75,7 +79,9 @@ def test_list_archives_with_log_debug_and_json_suppresses_most_borg_output(): def test_list_archives_with_lock_wait_calls_borg_with_lock_wait_parameters(): storage_config = {'lock_wait': 5} flexmock(module).should_receive('execute_command').with_args( - ('borg', 'list', '--lock-wait', '5', 'repo'), output_log_level=logging.WARNING + ('borg', 'list', '--lock-wait', '5', 'repo'), + output_log_level=logging.WARNING, + error_on_warnings=False, ) module.list_archives( @@ -88,7 +94,7 @@ def test_list_archives_with_lock_wait_calls_borg_with_lock_wait_parameters(): def test_list_archives_with_archive_calls_borg_with_archive_parameter(): storage_config = {} flexmock(module).should_receive('execute_command').with_args( - ('borg', 'list', 'repo::archive'), output_log_level=logging.WARNING + ('borg', 'list', 'repo::archive'), output_log_level=logging.WARNING, error_on_warnings=False ) module.list_archives( @@ -100,7 +106,7 @@ def test_list_archives_with_archive_calls_borg_with_archive_parameter(): def test_list_archives_with_local_path_calls_borg_via_local_path(): flexmock(module).should_receive('execute_command').with_args( - ('borg1', 'list', 'repo'), output_log_level=logging.WARNING + ('borg1', 'list', 'repo'), output_log_level=logging.WARNING, error_on_warnings=False ) module.list_archives( @@ -113,7 +119,9 @@ def test_list_archives_with_local_path_calls_borg_via_local_path(): def test_list_archives_with_remote_path_calls_borg_with_remote_path_parameters(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'list', '--remote-path', 'borg1', 'repo'), output_log_level=logging.WARNING + ('borg', 'list', '--remote-path', 'borg1', 'repo'), + output_log_level=logging.WARNING, + error_on_warnings=False, ) module.list_archives( @@ -126,7 +134,9 @@ def test_list_archives_with_remote_path_calls_borg_with_remote_path_parameters() def test_list_archives_with_short_calls_borg_with_short_parameter(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'list', '--short', 'repo'), output_log_level=logging.WARNING + ('borg', 'list', '--short', 'repo'), + output_log_level=logging.WARNING, + error_on_warnings=False, ).and_return('[]') module.list_archives( @@ -154,6 +164,7 @@ def test_list_archives_passes_through_arguments_to_borg(argument_name): flexmock(module).should_receive('execute_command').with_args( ('borg', 'list', '--' + argument_name.replace('_', '-'), 'value', 'repo'), output_log_level=logging.WARNING, + error_on_warnings=False, ).and_return('[]') module.list_archives( @@ -169,6 +180,7 @@ def test_list_archives_with_successful_calls_borg_to_exclude_checkpoints(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'list', '--glob-archives', module.BORG_EXCLUDE_CHECKPOINTS_GLOB, 'repo'), output_log_level=logging.WARNING, + error_on_warnings=False, ).and_return('[]') module.list_archives( @@ -180,7 +192,7 @@ def test_list_archives_with_successful_calls_borg_to_exclude_checkpoints(): def test_list_archives_with_json_calls_borg_with_json_parameter(): flexmock(module).should_receive('execute_command').with_args( - ('borg', 'list', '--json', 'repo'), output_log_level=None + ('borg', 'list', '--json', 'repo'), output_log_level=None, error_on_warnings=False ).and_return('[]') json_output = module.list_archives( diff --git a/tests/unit/borg/test_mount.py b/tests/unit/borg/test_mount.py index 998dc648..10914063 100644 --- a/tests/unit/borg/test_mount.py +++ b/tests/unit/borg/test_mount.py @@ -8,7 +8,9 @@ from ..test_verbosity import insert_logging_mock def insert_execute_command_mock(command): - flexmock(module).should_receive('execute_command').with_args(command).once() + flexmock(module).should_receive('execute_command').with_args( + command, error_on_warnings=False + ).once() def test_mount_archive_calls_borg_with_required_parameters(): @@ -116,7 +118,7 @@ def test_mount_archive_with_log_debug_calls_borg_with_debug_parameters(): def test_mount_archive_calls_borg_with_foreground_parameter(): flexmock(module).should_receive('execute_command_without_capture').with_args( - ('borg', 'mount', '--foreground', 'repo::archive', '/mnt') + ('borg', 'mount', '--foreground', 'repo::archive', '/mnt'), error_on_warnings=False ).once() module.mount_archive( diff --git a/tests/unit/borg/test_prune.py b/tests/unit/borg/test_prune.py index d05e8c0d..ed1ee674 100644 --- a/tests/unit/borg/test_prune.py +++ b/tests/unit/borg/test_prune.py @@ -10,7 +10,7 @@ from ..test_verbosity import insert_logging_mock def insert_execute_command_mock(prune_command, output_log_level): flexmock(module).should_receive('execute_command').with_args( - prune_command, output_log_level=output_log_level + prune_command, output_log_level=output_log_level, error_on_warnings=False ).once() diff --git a/tests/unit/test_execute.py b/tests/unit/test_execute.py index f29feb57..b6fcb04c 100644 --- a/tests/unit/test_execute.py +++ b/tests/unit/test_execute.py @@ -4,44 +4,28 @@ from flexmock import flexmock from borgmatic import execute as module -def test_exit_code_indicates_error_with_borg_error_is_true(): - assert module.exit_code_indicates_error(('/usr/bin/borg1', 'init'), 2) - - -def test_exit_code_indicates_error_with_borg_warning_is_false(): - assert not module.exit_code_indicates_error(('/usr/bin/borg1', 'init'), 1) - - -def test_exit_code_indicates_error_with_borg_success_is_false(): - assert not module.exit_code_indicates_error(('/usr/bin/borg1', 'init'), 0) - - -def test_exit_code_indicates_error_with_borg_error_and_error_on_warnings_is_true(): - assert module.exit_code_indicates_error(('/usr/bin/borg1', 'init'), 2, error_on_warnings=True) - - -def test_exit_code_indicates_error_with_borg_warning_and_error_on_warnings_is_true(): - assert module.exit_code_indicates_error(('/usr/bin/borg1', 'init'), 1, error_on_warnings=True) - - -def test_exit_code_indicates_error_with_borg_success_and_error_on_warnings_is_false(): - assert not module.exit_code_indicates_error( - ('/usr/bin/borg1', 'init'), 0, error_on_warnings=True +@pytest.mark.parametrize( + 'exit_code,error_on_warnings,expected_result', + ( + (2, True, True), + (2, False, True), + (1, True, True), + (1, False, False), + (0, True, False), + (0, False, False), + ), +) +def test_exit_code_indicates_error_respects_exit_code_and_error_on_warnings( + exit_code, error_on_warnings, expected_result +): + assert ( + module.exit_code_indicates_error( + ('command',), exit_code, error_on_warnings=error_on_warnings + ) + is expected_result ) -def test_exit_code_indicates_error_with_non_borg_error_is_true(): - assert module.exit_code_indicates_error(('/usr/bin/command',), 2) - - -def test_exit_code_indicates_error_with_non_borg_warning_is_true(): - assert module.exit_code_indicates_error(('/usr/bin/command',), 1) - - -def test_exit_code_indicates_error_with_non_borg_success_is_false(): - assert not module.exit_code_indicates_error(('/usr/bin/command',), 0) - - def test_execute_command_calls_full_command(): full_command = ['foo', 'bar'] flexmock(module.os, environ={'a': 'b'}) From 0c6c61a272b3a7ba6aeedf9c0be5bbdd522801ab Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 4 Dec 2019 15:48:10 -0800 Subject: [PATCH 04/30] Pass extra options directly to particular Borg commands, handy for Borg options that borgmatic does not yet support natively (#235). --- NEWS | 5 ++++ borgmatic/borg/check.py | 2 ++ borgmatic/borg/create.py | 2 ++ borgmatic/borg/init.py | 10 +++++--- borgmatic/borg/prune.py | 2 ++ borgmatic/commands/borgmatic.py | 1 + borgmatic/config/schema.yaml | 23 ++++++++++++++++++ setup.py | 2 +- tests/unit/borg/test_check.py | 14 +++++++++++ tests/unit/borg/test_create.py | 25 +++++++++++++++++++ tests/unit/borg/test_init.py | 43 +++++++++++++++++++++++++-------- tests/unit/borg/test_prune.py | 15 ++++++++++++ 12 files changed, 130 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index dac9e98a..09def405 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,8 @@ +1.4.17 + * #235: Pass extra options directly to particular Borg commands, handy for Borg options that + borgmatic does not yet support natively. Use "extra_borg_options" in the storage configuration + section. + 1.4.16 * #256: Fix for "before_backup" hook not triggering an error when the command contains "borg" and has an exit code of 1. diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index 78f1b386..89c58555 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -103,6 +103,7 @@ def check_archives( checks = _parse_checks(consistency_config, only_checks) check_last = consistency_config.get('check_last', None) lock_wait = None + extra_borg_options = storage_config.get('extra_borg_options', {}).get('check', '') if set(checks).intersection(set(DEFAULT_CHECKS + ('data',))): remote_path_flags = ('--remote-path', remote_path) if remote_path else () @@ -123,6 +124,7 @@ def check_archives( + remote_path_flags + lock_wait_flags + verbosity_flags + + (tuple(extra_borg_options.split(' ')) if extra_borg_options else ()) + (repository,) ) diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 879a94a1..2be2eb01 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -150,6 +150,7 @@ def create_archive( files_cache = location_config.get('files_cache') default_archive_name_format = '{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f}' archive_name_format = storage_config.get('archive_name_format', default_archive_name_format) + extra_borg_options = storage_config.get('extra_borg_options', {}).get('create', '') full_command = ( (local_path, 'create') @@ -185,6 +186,7 @@ def create_archive( + (('--dry-run',) if dry_run else ()) + (('--progress',) if progress else ()) + (('--json',) if json else ()) + + (tuple(extra_borg_options.split(' ')) if extra_borg_options else ()) + ( '{repository}::{archive_name_format}'.format( repository=repository, archive_name_format=archive_name_format diff --git a/borgmatic/borg/init.py b/borgmatic/borg/init.py index 1ec25c07..152f8c1b 100644 --- a/borgmatic/borg/init.py +++ b/borgmatic/borg/init.py @@ -11,6 +11,7 @@ INFO_REPOSITORY_NOT_FOUND_EXIT_CODE = 2 def initialize_repository( repository, + storage_config, encryption_mode, append_only=None, storage_quota=None, @@ -18,9 +19,9 @@ def initialize_repository( remote_path=None, ): ''' - Given a local or remote repository path, a Borg encryption mode, whether the repository should - be append-only, and the storage quota to use, initialize the repository. If the repository - already exists, then log and skip initialization. + Given a local or remote repository path, a storage configuration dict, a Borg encryption mode, + whether the repository should be append-only, and the storage quota to use, initialize the + repository. If the repository already exists, then log and skip initialization. ''' info_command = (local_path, 'info', repository) logger.debug(' '.join(info_command)) @@ -33,6 +34,8 @@ def initialize_repository( if error.returncode != INFO_REPOSITORY_NOT_FOUND_EXIT_CODE: raise + extra_borg_options = storage_config.get('extra_borg_options', {}).get('init', '') + init_command = ( (local_path, 'init') + (('--encryption', encryption_mode) if encryption_mode else ()) @@ -41,6 +44,7 @@ def initialize_repository( + (('--info',) if logger.getEffectiveLevel() == logging.INFO else ()) + (('--debug',) if logger.isEnabledFor(logging.DEBUG) else ()) + (('--remote-path', remote_path) if remote_path else ()) + + (tuple(extra_borg_options.split(' ')) if extra_borg_options else ()) + (repository,) ) diff --git a/borgmatic/borg/prune.py b/borgmatic/borg/prune.py index 584973e1..0913cdeb 100644 --- a/borgmatic/borg/prune.py +++ b/borgmatic/borg/prune.py @@ -49,6 +49,7 @@ def prune_archives( ''' umask = storage_config.get('umask', None) lock_wait = storage_config.get('lock_wait', None) + extra_borg_options = storage_config.get('extra_borg_options', {}).get('prune', '') full_command = ( (local_path, 'prune') @@ -61,6 +62,7 @@ def prune_archives( + (('--debug', '--list', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ()) + (('--dry-run',) if dry_run else ()) + (('--stats',) if stats else ()) + + (tuple(extra_borg_options.split(' ')) if extra_borg_options else ()) + (repository,) ) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 4cd63bb4..c33b2483 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -189,6 +189,7 @@ def run_actions( logger.info('{}: Initializing repository'.format(repository)) borg_init.initialize_repository( repository, + storage, arguments['init'].encryption_mode, arguments['init'].append_only, arguments['init'].storage_quota, diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 8b3667f4..169b6432 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -245,6 +245,29 @@ map: Bypass Borg error about a previously unknown unencrypted repository. Defaults to false. example: true + extra_borg_options: + map: + init: + type: str + desc: Extra command-line options to pass to "borg init". + example: "--make-parent-dirs" + prune: + type: str + desc: Extra command-line options to pass to "borg prune". + example: "--save-space" + create: + type: str + desc: Extra command-line options to pass to "borg create". + example: "--no-files-cache" + check: + type: str + desc: Extra command-line options to pass to "borg check". + example: "--save-space" + desc: | + Additional options to pass directly to particular Borg commands, handy for Borg + options that borgmatic does not yet support natively. Note that borgmatic does + not perform any validation on these options. Running borgmatic with + "--verbosity 2" shows the exact Borg command-line invocation. retention: desc: | Retention policy for how many backups to keep in each category. See diff --git a/setup.py b/setup.py index 46871a9c..5f60951d 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.4.16' +VERSION = '1.4.17' setup( diff --git a/tests/unit/borg/test_check.py b/tests/unit/borg/test_check.py index 51d430f4..df82936b 100644 --- a/tests/unit/borg/test_check.py +++ b/tests/unit/borg/test_check.py @@ -296,3 +296,17 @@ def test_check_archives_with_retention_prefix(): module.check_archives( repository='repo', storage_config={}, consistency_config=consistency_config ) + + +def test_check_archives_with_extra_borg_options_calls_borg_with_extra_options(): + checks = ('repository',) + consistency_config = {'check_last': None} + flexmock(module).should_receive('_parse_checks').and_return(checks) + flexmock(module).should_receive('_make_check_flags').and_return(()) + insert_execute_command_mock(('borg', 'check', '--extra', '--options', 'repo')) + + module.check_archives( + repository='repo', + storage_config={'extra_borg_options': {'check': '--extra --options'}}, + consistency_config=consistency_config, + ) diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 5819df02..5a1a3e82 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -1092,3 +1092,28 @@ def test_create_archive_with_archive_name_format_accepts_borg_placeholders(): }, storage_config={'archive_name_format': 'Documents_{hostname}-{now}'}, ) + + +def test_create_archive_with_extra_borg_options_calls_borg_with_extra_options(): + flexmock(module).should_receive('borgmatic_source_directories').and_return([]) + flexmock(module).should_receive('_expand_directories').and_return(('foo', 'bar')) + flexmock(module).should_receive('_expand_home_directories').and_return(()) + flexmock(module).should_receive('_write_pattern_file').and_return(None) + flexmock(module).should_receive('_make_pattern_flags').and_return(()) + flexmock(module).should_receive('_make_exclude_flags').and_return(()) + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'create', '--extra', '--options') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + error_on_warnings=False, + ) + + module.create_archive( + dry_run=False, + repository='repo', + location_config={ + 'source_directories': ['foo', 'bar'], + 'repositories': ['repo'], + 'exclude_patterns': None, + }, + storage_config={'extra_borg_options': {'create': '--extra --options'}}, + ) diff --git a/tests/unit/borg/test_init.py b/tests/unit/borg/test_init.py index fffdfd7f..49e9933c 100644 --- a/tests/unit/borg/test_init.py +++ b/tests/unit/borg/test_init.py @@ -32,7 +32,7 @@ def test_initialize_repository_calls_borg_with_parameters(): insert_info_command_not_found_mock() insert_init_command_mock(INIT_COMMAND + ('repo',)) - module.initialize_repository(repository='repo', encryption_mode='repokey') + module.initialize_repository(repository='repo', storage_config={}, encryption_mode='repokey') def test_initialize_repository_raises_for_borg_init_error(): @@ -42,14 +42,16 @@ def test_initialize_repository_raises_for_borg_init_error(): ) with pytest.raises(subprocess.CalledProcessError): - module.initialize_repository(repository='repo', encryption_mode='repokey') + module.initialize_repository( + repository='repo', storage_config={}, encryption_mode='repokey' + ) def test_initialize_repository_skips_initialization_when_repository_already_exists(): insert_info_command_found_mock() flexmock(module).should_receive('execute_command_without_capture').never() - module.initialize_repository(repository='repo', encryption_mode='repokey') + module.initialize_repository(repository='repo', storage_config={}, encryption_mode='repokey') def test_initialize_repository_raises_for_unknown_info_command_error(): @@ -58,21 +60,27 @@ def test_initialize_repository_raises_for_unknown_info_command_error(): ) with pytest.raises(subprocess.CalledProcessError): - module.initialize_repository(repository='repo', encryption_mode='repokey') + module.initialize_repository( + repository='repo', storage_config={}, encryption_mode='repokey' + ) def test_initialize_repository_with_append_only_calls_borg_with_append_only_parameter(): insert_info_command_not_found_mock() insert_init_command_mock(INIT_COMMAND + ('--append-only', 'repo')) - module.initialize_repository(repository='repo', encryption_mode='repokey', append_only=True) + module.initialize_repository( + repository='repo', storage_config={}, encryption_mode='repokey', append_only=True + ) def test_initialize_repository_with_storage_quota_calls_borg_with_storage_quota_parameter(): insert_info_command_not_found_mock() insert_init_command_mock(INIT_COMMAND + ('--storage-quota', '5G', 'repo')) - module.initialize_repository(repository='repo', encryption_mode='repokey', storage_quota='5G') + module.initialize_repository( + repository='repo', storage_config={}, encryption_mode='repokey', storage_quota='5G' + ) def test_initialize_repository_with_log_info_calls_borg_with_info_parameter(): @@ -80,7 +88,7 @@ def test_initialize_repository_with_log_info_calls_borg_with_info_parameter(): insert_init_command_mock(INIT_COMMAND + ('--info', 'repo')) insert_logging_mock(logging.INFO) - module.initialize_repository(repository='repo', encryption_mode='repokey') + module.initialize_repository(repository='repo', storage_config={}, encryption_mode='repokey') def test_initialize_repository_with_log_debug_calls_borg_with_debug_parameter(): @@ -88,18 +96,33 @@ def test_initialize_repository_with_log_debug_calls_borg_with_debug_parameter(): insert_init_command_mock(INIT_COMMAND + ('--debug', 'repo')) insert_logging_mock(logging.DEBUG) - module.initialize_repository(repository='repo', encryption_mode='repokey') + module.initialize_repository(repository='repo', storage_config={}, encryption_mode='repokey') def test_initialize_repository_with_local_path_calls_borg_via_local_path(): insert_info_command_not_found_mock() insert_init_command_mock(('borg1',) + INIT_COMMAND[1:] + ('repo',)) - module.initialize_repository(repository='repo', encryption_mode='repokey', local_path='borg1') + module.initialize_repository( + repository='repo', storage_config={}, encryption_mode='repokey', local_path='borg1' + ) def test_initialize_repository_with_remote_path_calls_borg_with_remote_path_parameter(): insert_info_command_not_found_mock() insert_init_command_mock(INIT_COMMAND + ('--remote-path', 'borg1', 'repo')) - module.initialize_repository(repository='repo', encryption_mode='repokey', remote_path='borg1') + module.initialize_repository( + repository='repo', storage_config={}, encryption_mode='repokey', remote_path='borg1' + ) + + +def test_initialize_repository_with_extra_borg_options_calls_borg_with_extra_options(): + insert_info_command_not_found_mock() + insert_init_command_mock(INIT_COMMAND + ('--extra', '--options', 'repo')) + + module.initialize_repository( + repository='repo', + storage_config={'extra_borg_options': {'init': '--extra --options'}}, + encryption_mode='repokey', + ) diff --git a/tests/unit/borg/test_prune.py b/tests/unit/borg/test_prune.py index ed1ee674..80cce836 100644 --- a/tests/unit/borg/test_prune.py +++ b/tests/unit/borg/test_prune.py @@ -188,3 +188,18 @@ def test_prune_archives_with_lock_wait_calls_borg_with_lock_wait_parameters(): storage_config=storage_config, retention_config=retention_config, ) + + +def test_prune_archives_with_extra_borg_options_calls_borg_with_extra_options(): + retention_config = flexmock() + flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return( + BASE_PRUNE_FLAGS + ) + insert_execute_command_mock(PRUNE_COMMAND + ('--extra', '--options', 'repo'), logging.INFO) + + module.prune_archives( + dry_run=False, + repository='repo', + storage_config={'extra_borg_options': {'prune': '--extra --options'}}, + retention_config=retention_config, + ) From 2ab9daaa0f090a8a9a6951d979b049bdb818f91c Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 4 Dec 2019 16:07:00 -0800 Subject: [PATCH 05/30] Attempt to repair any inconsistencies found during a consistency check via "borgmatic check --repair" flag (#266). --- NEWS | 2 ++ borgmatic/borg/check.py | 20 +++++++++++++------- borgmatic/commands/arguments.py | 7 +++++++ borgmatic/commands/borgmatic.py | 1 + tests/unit/borg/test_check.py | 15 +++++++++++++++ 5 files changed, 38 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 09def405..c3f6a833 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ * #235: Pass extra options directly to particular Borg commands, handy for Borg options that borgmatic does not yet support natively. Use "extra_borg_options" in the storage configuration section. + * #266: Attempt to repair any inconsistencies found during a consistency check via + "borgmatic check --repair" flag. 1.4.16 * #256: Fix for "before_backup" hook not triggering an error when the command contains "borg" and diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index 89c58555..45e59f28 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -1,7 +1,7 @@ import logging from borgmatic.borg import extract -from borgmatic.execute import execute_command +from borgmatic.execute import execute_command, execute_command_without_capture DEFAULT_CHECKS = ('repository', 'archives') DEFAULT_PREFIX = '{hostname}-' @@ -91,12 +91,13 @@ def check_archives( consistency_config, local_path='borg', remote_path=None, + repair=None, only_checks=None, ): ''' Given a local or remote repository path, a storage config dict, a consistency config dict, - local/remote commands to run, and an optional list of checks to use instead of configured - checks, check the contained Borg archives for consistency. + local/remote commands to run, whether to attempt a repair, and an optional list of checks + to use instead of configured checks, check the contained Borg archives for consistency. If there are no consistency checks to run, skip running them. ''' @@ -106,9 +107,7 @@ def check_archives( extra_borg_options = storage_config.get('extra_borg_options', {}).get('check', '') if set(checks).intersection(set(DEFAULT_CHECKS + ('data',))): - remote_path_flags = ('--remote-path', remote_path) if remote_path else () lock_wait = storage_config.get('lock_wait', None) - lock_wait_flags = ('--lock-wait', str(lock_wait)) if lock_wait else () verbosity_flags = () if logger.isEnabledFor(logging.INFO): @@ -120,14 +119,21 @@ def check_archives( full_command = ( (local_path, 'check') + + (('--repair',) if repair else ()) + _make_check_flags(checks, check_last, prefix) - + remote_path_flags - + lock_wait_flags + + (('--remote-path', remote_path) if remote_path else ()) + + (('--lock-wait', str(lock_wait)) if lock_wait else ()) + verbosity_flags + (tuple(extra_borg_options.split(' ')) if extra_borg_options else ()) + (repository,) ) + # The Borg repair option trigger an interactive prompt, which won't work when output is + # captured. + if repair: + execute_command_without_capture(full_command, error_on_warnings=True) + return + execute_command(full_command, error_on_warnings=True) if 'extract' in checks: diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 98d94848..86794642 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -266,6 +266,13 @@ def parse_arguments(*unparsed_arguments): add_help=False, ) check_group = check_parser.add_argument_group('check arguments') + check_group.add_argument( + '--repair', + dest='repair', + default=False, + action='store_true', + help='Attempt to repair any inconsistencies found (experimental and only for interactive use)', + ) check_group.add_argument( '--only', metavar='CHECK', diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index c33b2483..0817a97e 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -230,6 +230,7 @@ def run_actions( consistency, local_path=local_path, remote_path=remote_path, + repair=arguments['check'].repair, only_checks=arguments['check'].only, ) if 'extract' in arguments: diff --git a/tests/unit/borg/test_check.py b/tests/unit/borg/test_check.py index df82936b..b2506aae 100644 --- a/tests/unit/borg/test_check.py +++ b/tests/unit/borg/test_check.py @@ -158,6 +158,21 @@ def test_make_check_flags_with_default_checks_and_prefix_includes_prefix_flag(): assert flags == ('--prefix', 'foo-') +def test_check_archives_with_repair_calls_borg_with_repair_parameter(): + checks = ('repository',) + consistency_config = {'check_last': None} + flexmock(module).should_receive('_parse_checks').and_return(checks) + flexmock(module).should_receive('_make_check_flags').and_return(()) + flexmock(module).should_receive('execute_command').never() + flexmock(module).should_receive('execute_command_without_capture').with_args( + ('borg', 'check', '--repair', 'repo'), error_on_warnings=True + ).once() + + module.check_archives( + repository='repo', storage_config={}, consistency_config=consistency_config, repair=True + ) + + @pytest.mark.parametrize( 'checks', ( From df2be9620b5bfac12bc192001c05873cbc17f696 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 6 Dec 2019 15:58:54 -0800 Subject: [PATCH 06/30] Mount whole repositories via "borgmatic mount" without any "--archive" flag (#253). --- NEWS | 3 +++ borgmatic/borg/mount.py | 8 ++++---- borgmatic/commands/arguments.py | 2 +- borgmatic/commands/borgmatic.py | 8 +++++++- docs/how-to/extract-a-backup.md | 6 ++++++ setup.py | 2 +- tests/integration/commands/test_arguments.py | 7 ------- 7 files changed, 22 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index c3f6a833..63185bd0 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +1.4.18 + * #253: Mount whole repositories via "borgmatic mount" without any "--archive" flag. + 1.4.17 * #235: Pass extra options directly to particular Borg commands, handy for Borg options that borgmatic does not yet support natively. Use "extra_borg_options" in the storage configuration diff --git a/borgmatic/borg/mount.py b/borgmatic/borg/mount.py index 4e700097..4fccbf9f 100644 --- a/borgmatic/borg/mount.py +++ b/borgmatic/borg/mount.py @@ -17,9 +17,9 @@ def mount_archive( remote_path=None, ): ''' - Given a local or remote repository path, an archive name, a filesystem mount point, zero or more - paths to mount from the archive, extra Borg mount options, a storage configuration dict, and - optional local and remote Borg paths, mount the archive onto the mount point. + Given a local or remote repository path, an optional archive name, a filesystem mount point, + zero or more paths to mount from the archive, extra Borg mount options, a storage configuration + dict, and optional local and remote Borg paths, mount the archive onto the mount point. ''' umask = storage_config.get('umask', None) lock_wait = storage_config.get('lock_wait', None) @@ -33,7 +33,7 @@ def mount_archive( + (('--debug', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ()) + (('--foreground',) if foreground else ()) + (('-o', options) if options else ()) - + ('::'.join((repository, archive)),) + + (('::'.join((repository, archive)),) if archive else (repository,)) + (mount_point,) + (tuple(paths) if paths else ()) ) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 86794642..b2e9eabf 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -333,7 +333,7 @@ def parse_arguments(*unparsed_arguments): '--repository', help='Path of repository to use, defaults to the configured repository if there is only one', ) - mount_group.add_argument('--archive', help='Name of archive to mount', required=True) + mount_group.add_argument('--archive', help='Name of archive to mount') mount_group.add_argument( '--mount-point', metavar='PATH', diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 0817a97e..3a1434dd 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -252,7 +252,13 @@ def run_actions( ) if 'mount' in arguments: if arguments['mount'].repository is None or repository == arguments['mount'].repository: - logger.info('{}: Mounting archive {}'.format(repository, arguments['mount'].archive)) + if arguments['mount'].archive: + logger.info( + '{}: Mounting archive {}'.format(repository, arguments['mount'].archive) + ) + else: + logger.info('{}: Mounting repository'.format(repository)) + borg_mount.mount_archive( repository, arguments['mount'].archive, diff --git a/docs/how-to/extract-a-backup.md b/docs/how-to/extract-a-backup.md index 6270d4d9..c45a3df3 100644 --- a/docs/how-to/extract-a-backup.md +++ b/docs/how-to/extract-a-backup.md @@ -100,6 +100,12 @@ borgmatic mount --archive host-2019-... --mount-point /mnt This mounts the entire archive on the given mount point `/mnt`, so that you can look in there for your files. +Omit the `--archive` flag to mount all archives (lazy-loaded): + +```bash +borgmatic mount --mount-point /mnt +``` + If you'd like to restrict the mounted filesystem to only particular paths from your archive, use the `--path` flag, similar to the `extract` action above. For instance: diff --git a/setup.py b/setup.py index 5f60951d..9ff3ba3a 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.4.17' +VERSION = '1.4.18' setup( diff --git a/tests/integration/commands/test_arguments.py b/tests/integration/commands/test_arguments.py index 5a432be0..9c5bb948 100644 --- a/tests/integration/commands/test_arguments.py +++ b/tests/integration/commands/test_arguments.py @@ -352,13 +352,6 @@ def test_parse_arguments_requires_archive_with_extract(): module.parse_arguments('--config', 'myconfig', 'extract') -def test_parse_arguments_requires_archive_with_mount(): - flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) - - with pytest.raises(SystemExit): - module.parse_arguments('--config', 'myconfig', 'mount', '--mount-point', '/mnt') - - def test_parse_arguments_requires_archive_with_restore(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) From 65cc4c9429040e10a185e78f54ff5a1d737b4918 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 6 Dec 2019 16:29:41 -0800 Subject: [PATCH 07/30] Fix "--repository" flag to accept relative paths. --- NEWS | 1 + borgmatic/commands/borgmatic.py | 20 +++++++++++---- borgmatic/config/validate.py | 25 ++++++++++++++++--- tests/unit/config/test_validate.py | 40 ++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index 63185bd0..bc4a687a 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,5 @@ 1.4.18 + * Fix "--repository" flag to accept relative paths. * #253: Mount whole repositories via "borgmatic mount" without any "--archive" flag. 1.4.17 diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 3a1434dd..5e285ae8 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -234,7 +234,9 @@ def run_actions( only_checks=arguments['check'].only, ) if 'extract' in arguments: - if arguments['extract'].repository is None or repository == arguments['extract'].repository: + if arguments['extract'].repository is None or validate.repositories_match( + repository, arguments['extract'].repository + ): logger.info( '{}: Extracting archive {}'.format(repository, arguments['extract'].archive) ) @@ -251,7 +253,9 @@ def run_actions( progress=arguments['extract'].progress, ) if 'mount' in arguments: - if arguments['mount'].repository is None or repository == arguments['mount'].repository: + if arguments['mount'].repository is None or validate.repositories_match( + repository, arguments['mount'].repository + ): if arguments['mount'].archive: logger.info( '{}: Mounting archive {}'.format(repository, arguments['mount'].archive) @@ -278,7 +282,9 @@ def run_actions( mount_point=arguments['umount'].mount_point, local_path=local_path ) if 'restore' in arguments: - if arguments['restore'].repository is None or repository == arguments['restore'].repository: + if arguments['restore'].repository is None or validate.repositories_match( + repository, arguments['restore'].repository + ): logger.info( '{}: Restoring databases from archive {}'.format( repository, arguments['restore'].archive @@ -336,7 +342,9 @@ def run_actions( global_arguments.dry_run, ) if 'list' in arguments: - if arguments['list'].repository is None or repository == arguments['list'].repository: + if arguments['list'].repository is None or validate.repositories_match( + repository, arguments['list'].repository + ): logger.info('{}: Listing archives'.format(repository)) json_output = borg_list.list_archives( repository, @@ -348,7 +356,9 @@ def run_actions( if json_output: yield json.loads(json_output) if 'info' in arguments: - if arguments['info'].repository is None or repository == arguments['info'].repository: + if arguments['info'].repository is None or validate.repositories_match( + repository, arguments['info'].repository + ): logger.info('{}: Displaying summary info for archives'.format(repository)) json_output = borg_info.display_archives_info( repository, diff --git a/borgmatic/config/validate.py b/borgmatic/config/validate.py index 8c9e8e9b..1e421d18 100644 --- a/borgmatic/config/validate.py +++ b/borgmatic/config/validate.py @@ -1,4 +1,5 @@ import logging +import os import pkg_resources import pykwalify.core @@ -112,6 +113,24 @@ def parse_configuration(config_filename, schema_filename): return parsed_result +def normalize_repository_path(repository): + ''' + Given a repository path, return the absolute path of it (for local repositories). + ''' + # A colon in the repository indicates it's a remote repository. Bail. + if ':' in repository: + return repository + + return os.path.abspath(repository) + + +def repositories_match(first, second): + ''' + Given two repository paths (relative and/or absolute), return whether they match. + ''' + return normalize_repository_path(first) == normalize_repository_path(second) + + def guard_configuration_contains_repository(repository, configurations): ''' Given a repository path and a dict mapping from config filename to corresponding parsed config @@ -133,9 +152,7 @@ def guard_configuration_contains_repository(repository, configurations): if count > 1: raise ValueError( - 'Can\'t determine which repository to use. Use --repository option to disambiguate'.format( - repository - ) + 'Can\'t determine which repository to use. Use --repository option to disambiguate' ) return @@ -145,7 +162,7 @@ def guard_configuration_contains_repository(repository, configurations): config_repository for config in configurations.values() for config_repository in config['location']['repositories'] - if repository == config_repository + if repositories_match(repository, config_repository) ) ) diff --git a/tests/unit/config/test_validate.py b/tests/unit/config/test_validate.py index bdf30bec..4fc4a620 100644 --- a/tests/unit/config/test_validate.py +++ b/tests/unit/config/test_validate.py @@ -1,4 +1,5 @@ import pytest +from flexmock import flexmock from borgmatic.config import validate as module @@ -95,7 +96,38 @@ def test_remove_examples_strips_examples_from_sequence_of_maps(): assert schema == {'seq': [{'map': {'foo': {'desc': 'thing'}}}]} +def test_normalize_repository_path_passes_through_remote_repository(): + repository = 'example.org:test.borg' + + module.normalize_repository_path(repository) == repository + + +def test_normalize_repository_path_passes_through_absolute_repository(): + repository = '/foo/bar/test.borg' + flexmock(module.os.path).should_receive('abspath').and_return(repository) + + module.normalize_repository_path(repository) == repository + + +def test_normalize_repository_path_resolves_relative_repository(): + repository = 'test.borg' + absolute = '/foo/bar/test.borg' + flexmock(module.os.path).should_receive('abspath').and_return(absolute) + + module.normalize_repository_path(repository) == absolute + + +def test_repositories_match_does_not_raise(): + flexmock(module).should_receive('normalize_repository_path') + + module.repositories_match('foo', 'bar') + + def test_guard_configuration_contains_repository_does_not_raise_when_repository_in_config(): + flexmock(module).should_receive('repositories_match').replace_with( + lambda first, second: first == second + ) + module.guard_configuration_contains_repository( repository='repo', configurations={'config.yaml': {'location': {'repositories': ['repo']}}} ) @@ -116,6 +148,10 @@ def test_guard_configuration_contains_repository_errors_when_repository_assumed_ def test_guard_configuration_contains_repository_errors_when_repository_missing_from_config(): + flexmock(module).should_receive('repositories_match').replace_with( + lambda first, second: first == second + ) + with pytest.raises(ValueError): module.guard_configuration_contains_repository( repository='nope', @@ -124,6 +160,10 @@ def test_guard_configuration_contains_repository_errors_when_repository_missing_ def test_guard_configuration_contains_repository_errors_when_repository_matches_config_twice(): + flexmock(module).should_receive('repositories_match').replace_with( + lambda first, second: first == second + ) + with pytest.raises(ValueError): module.guard_configuration_contains_repository( repository='repo', From b94999bba4bfe06bce4ce56f184cabcd004591df Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 7 Dec 2019 21:36:51 -0800 Subject: [PATCH 08/30] Fix "borgmatic umount" so it only runs Borg once instead of once per repository / configuration file. --- NEWS | 2 ++ borgmatic/commands/borgmatic.py | 24 ++++++++++++++++------- tests/unit/commands/test_borgmatic.py | 28 +++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index bc4a687a..57397842 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,7 @@ 1.4.18 * Fix "--repository" flag to accept relative paths. + * Fix "borgmatic umount" so it only runs Borg once instead of once per repository / configuration + file. * #253: Mount whole repositories via "borgmatic mount" without any "--archive" flag. 1.4.17 diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 5e285ae8..a623db74 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -274,13 +274,6 @@ def run_actions( local_path=local_path, remote_path=remote_path, ) - if 'umount' in arguments: - logger.info( - '{}: Unmounting mount point {}'.format(repository, arguments['umount'].mount_point) - ) - borg_umount.unmount_archive( - mount_point=arguments['umount'].mount_point, local_path=local_path - ) if 'restore' in arguments: if arguments['restore'].repository is None or validate.repositories_match( repository, arguments['restore'].repository @@ -447,6 +440,14 @@ def make_error_log_records(message, error=None): pass +def get_local_path(configs): + ''' + Arbitrarily return the local path from the first configuration dict. Default to "borg" if not + set. + ''' + return next(iter(configs.values())).get('location', {}).get('local_path', 'borg') + + def collect_configuration_run_summary_logs(configs, arguments): ''' Given a dict of configuration filename to corresponding parsed configuration, and parsed @@ -517,6 +518,15 @@ def collect_configuration_run_summary_logs(configs, arguments): if results: json_results.extend(results) + if 'umount' in arguments: + logger.info('Unmounting mount point {}'.format(arguments['umount'].mount_point)) + try: + borg_umount.unmount_archive( + mount_point=arguments['umount'].mount_point, local_path=get_local_path(configs) + ) + except (CalledProcessError, OSError) as error: + yield from make_error_log_records('Error unmounting mount point', error) + if json_results: sys.stdout.write(json.dumps(json_results)) diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index b0a887b1..1bdb3213 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -169,6 +169,18 @@ def test_make_error_log_records_generates_nothing_for_other_error(): assert logs == () +def test_get_local_path_uses_configuration_value(): + assert module.get_local_path({'test.yaml': {'location': {'local_path': 'borg1'}}}) == 'borg1' + + +def test_get_local_path_without_location_defaults_to_borg(): + assert module.get_local_path({'test.yaml': {}}) == 'borg' + + +def test_get_local_path_without_local_path_defaults_to_borg(): + assert module.get_local_path({'test.yaml': {'location': {}}}) == 'borg' + + def test_collect_configuration_run_summary_logs_info_for_success(): flexmock(module.command).should_receive('execute_hook').never() flexmock(module).should_receive('run_configuration').and_return([]) @@ -324,6 +336,22 @@ def test_collect_configuration_run_summary_logs_run_configuration_error(): assert {log.levelno for log in logs} == {logging.CRITICAL} +def test_collect_configuration_run_summary_logs_run_umount_error(): + flexmock(module.validate).should_receive('guard_configuration_contains_repository') + flexmock(module).should_receive('run_configuration').and_return([]) + flexmock(module.borg_umount).should_receive('unmount_archive').and_raise(OSError) + flexmock(module).should_receive('make_error_log_records').and_return( + [logging.makeLogRecord(dict(levelno=logging.CRITICAL, levelname='CRITICAL', msg='Error'))] + ) + arguments = {'umount': flexmock(mount_point='/mnt')} + + logs = tuple( + module.collect_configuration_run_summary_logs({'test.yaml': {}}, arguments=arguments) + ) + + assert {log.levelno for log in logs} == {logging.INFO, logging.CRITICAL} + + def test_collect_configuration_run_summary_logs_outputs_merged_json_results(): flexmock(module).should_receive('run_configuration').and_return(['foo', 'bar']).and_return( ['baz'] From 826e4352d1ba0d59beb0cb013e149f6d486f8ae9 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 8 Dec 2019 14:07:02 -0800 Subject: [PATCH 09/30] Filter listed paths via "borgmatic list --path" flag (#269). --- NEWS | 1 + borgmatic/borg/list.py | 3 ++- borgmatic/commands/arguments.py | 7 ++++++ tests/unit/borg/test_list.py | 41 ++++++++++++++++++++++----------- 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index 57397842..bd671266 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ * Fix "borgmatic umount" so it only runs Borg once instead of once per repository / configuration file. * #253: Mount whole repositories via "borgmatic mount" without any "--archive" flag. + * #269: Filter listed paths via "borgmatic list --path" flag. 1.4.17 * #235: Pass extra options directly to particular Borg commands, handy for Borg options that diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index bcbe36e2..845f2884 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -36,13 +36,14 @@ def list_archives(repository, storage_config, list_arguments, local_path='borg', + make_flags('remote-path', remote_path) + make_flags('lock-wait', lock_wait) + make_flags_from_arguments( - list_arguments, excludes=('repository', 'archive', 'successful') + list_arguments, excludes=('repository', 'archive', 'paths', 'successful') ) + ( '::'.join((repository, list_arguments.archive)) if list_arguments.archive else repository, ) + + (tuple(list_arguments.paths) if list_arguments.paths else ()) ) return execute_command( diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index b2e9eabf..738981ca 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -419,6 +419,13 @@ def parse_arguments(*unparsed_arguments): help='Path of repository to list, defaults to the configured repository if there is only one', ) list_group.add_argument('--archive', help='Name of archive to list') + list_group.add_argument( + '--path', + metavar='PATH', + nargs='+', + dest='paths', + help='Paths to list from archive, defaults to the entire archive', + ) list_group.add_argument( '--short', default=False, action='store_true', help='Output only archive or path names' ) diff --git a/tests/unit/borg/test_list.py b/tests/unit/borg/test_list.py index 4e3de334..40ad1229 100644 --- a/tests/unit/borg/test_list.py +++ b/tests/unit/borg/test_list.py @@ -16,7 +16,7 @@ def test_list_archives_calls_borg_with_parameters(): module.list_archives( repository='repo', storage_config={}, - list_arguments=flexmock(archive=None, json=False, successful=False), + list_arguments=flexmock(archive=None, paths=None, json=False, successful=False), ) @@ -31,7 +31,7 @@ def test_list_archives_with_log_info_calls_borg_with_info_parameter(): module.list_archives( repository='repo', storage_config={}, - list_arguments=flexmock(archive=None, json=False, successful=False), + list_arguments=flexmock(archive=None, paths=None, json=False, successful=False), ) @@ -44,7 +44,7 @@ def test_list_archives_with_log_info_and_json_suppresses_most_borg_output(): module.list_archives( repository='repo', storage_config={}, - list_arguments=flexmock(archive=None, json=True, successful=False), + list_arguments=flexmock(archive=None, paths=None, json=True, successful=False), ) @@ -59,7 +59,7 @@ def test_list_archives_with_log_debug_calls_borg_with_debug_parameter(): module.list_archives( repository='repo', storage_config={}, - list_arguments=flexmock(archive=None, json=False, successful=False), + list_arguments=flexmock(archive=None, paths=None, json=False, successful=False), ) @@ -72,7 +72,7 @@ def test_list_archives_with_log_debug_and_json_suppresses_most_borg_output(): module.list_archives( repository='repo', storage_config={}, - list_arguments=flexmock(archive=None, json=True, successful=False), + list_arguments=flexmock(archive=None, paths=None, json=True, successful=False), ) @@ -87,7 +87,7 @@ def test_list_archives_with_lock_wait_calls_borg_with_lock_wait_parameters(): module.list_archives( repository='repo', storage_config=storage_config, - list_arguments=flexmock(archive=None, json=False, successful=False), + list_arguments=flexmock(archive=None, paths=None, json=False, successful=False), ) @@ -100,7 +100,22 @@ def test_list_archives_with_archive_calls_borg_with_archive_parameter(): module.list_archives( repository='repo', storage_config=storage_config, - list_arguments=flexmock(archive='archive', json=False, successful=False), + list_arguments=flexmock(archive='archive', paths=None, json=False, successful=False), + ) + + +def test_list_archives_with_path_calls_borg_with_path_parameter(): + storage_config = {} + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'list', 'repo::archive', 'var/lib'), + output_log_level=logging.WARNING, + error_on_warnings=False, + ) + + module.list_archives( + repository='repo', + storage_config=storage_config, + list_arguments=flexmock(archive='archive', paths=['var/lib'], json=False, successful=False), ) @@ -112,7 +127,7 @@ def test_list_archives_with_local_path_calls_borg_via_local_path(): module.list_archives( repository='repo', storage_config={}, - list_arguments=flexmock(archive=None, json=False, successful=False), + list_arguments=flexmock(archive=None, paths=None, json=False, successful=False), local_path='borg1', ) @@ -127,7 +142,7 @@ def test_list_archives_with_remote_path_calls_borg_with_remote_path_parameters() module.list_archives( repository='repo', storage_config={}, - list_arguments=flexmock(archive=None, json=False, successful=False), + list_arguments=flexmock(archive=None, paths=None, json=False, successful=False), remote_path='borg1', ) @@ -142,7 +157,7 @@ def test_list_archives_with_short_calls_borg_with_short_parameter(): module.list_archives( repository='repo', storage_config={}, - list_arguments=flexmock(archive=None, json=False, successful=False, short=True), + list_arguments=flexmock(archive=None, paths=None, json=False, successful=False, short=True), ) @@ -171,7 +186,7 @@ def test_list_archives_passes_through_arguments_to_borg(argument_name): repository='repo', storage_config={}, list_arguments=flexmock( - archive=None, json=False, successful=False, **{argument_name: 'value'} + archive=None, paths=None, json=False, successful=False, **{argument_name: 'value'} ), ) @@ -186,7 +201,7 @@ def test_list_archives_with_successful_calls_borg_to_exclude_checkpoints(): module.list_archives( repository='repo', storage_config={}, - list_arguments=flexmock(archive=None, json=False, successful=True), + list_arguments=flexmock(archive=None, paths=None, json=False, successful=True), ) @@ -198,7 +213,7 @@ def test_list_archives_with_json_calls_borg_with_json_parameter(): json_output = module.list_archives( repository='repo', storage_config={}, - list_arguments=flexmock(archive=None, json=True, successful=False), + list_arguments=flexmock(archive=None, paths=None, json=True, successful=False), ) assert json_output == '[]' From 8660af745ec891e432c94ef1f8036550020a99cd Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 10 Dec 2019 16:04:34 -0800 Subject: [PATCH 10/30] Optionally change the internal database dump path via "borgmatic_source_directory" option in location configuration section (#259). --- NEWS | 4 +++ borgmatic/borg/create.py | 14 ++++++--- borgmatic/commands/borgmatic.py | 5 +++ borgmatic/config/schema.yaml | 8 +++++ borgmatic/hooks/dump.py | 13 ++++++++ borgmatic/hooks/mysql.py | 47 +++++++++++++++++++--------- borgmatic/hooks/postgresql.py | 47 +++++++++++++++++++--------- docs/how-to/backup-your-databases.md | 8 +++-- setup.py | 2 +- tests/unit/borg/test_create.py | 11 +++++-- tests/unit/hooks/test_dump.py | 8 +++++ tests/unit/hooks/test_mysql.py | 35 ++++++++++++++------- tests/unit/hooks/test_postgresql.py | 38 ++++++++++++++-------- 13 files changed, 175 insertions(+), 65 deletions(-) diff --git a/NEWS b/NEWS index bd671266..9d35de31 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,7 @@ +1.4.19 + * #259: Optionally change the internal database dump path via "borgmatic_source_directory" option + in location configuration section. + 1.4.18 * Fix "--repository" flag to accept relative paths. * Fix "borgmatic umount" so it only runs Borg once instead of once per repository / configuration diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 2be2eb01..f582fb7b 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -104,16 +104,19 @@ def _make_exclude_flags(location_config, exclude_filename=None): ) -BORGMATIC_SOURCE_DIRECTORY = '~/.borgmatic' +DEFAULT_BORGMATIC_SOURCE_DIRECTORY = '~/.borgmatic' -def borgmatic_source_directories(): +def borgmatic_source_directories(borgmatic_source_directory): ''' Return a list of borgmatic-specific source directories used for state like database backups. ''' + if not borgmatic_source_directory: + borgmatic_source_directory = DEFAULT_BORGMATIC_SOURCE_DIRECTORY + return ( - [BORGMATIC_SOURCE_DIRECTORY] - if os.path.exists(os.path.expanduser(BORGMATIC_SOURCE_DIRECTORY)) + [borgmatic_source_directory] + if os.path.exists(os.path.expanduser(borgmatic_source_directory)) else [] ) @@ -134,7 +137,8 @@ def create_archive( storage config dict, create a Borg archive and return Borg's JSON output (if any). ''' sources = _expand_directories( - location_config['source_directories'] + borgmatic_source_directories() + location_config['source_directories'] + + borgmatic_source_directories(location_config.get('borgmatic_source_directory')) ) pattern_file = _write_pattern_file(location_config.get('patterns')) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index a623db74..3b539764 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -75,6 +75,7 @@ def run_configuration(config_filename, config, arguments): hooks, config_filename, dump.DATABASE_HOOK_NAMES, + location, global_arguments.dry_run, ) except (OSError, CalledProcessError) as error: @@ -111,6 +112,7 @@ def run_configuration(config_filename, config, arguments): hooks, config_filename, dump.DATABASE_HOOK_NAMES, + location, global_arguments.dry_run, ) command.execute_hook( @@ -294,6 +296,7 @@ def run_actions( hooks, repository, dump.DATABASE_HOOK_NAMES, + location, restore_names, ) @@ -325,6 +328,7 @@ def run_actions( restore_databases, repository, dump.DATABASE_HOOK_NAMES, + location, global_arguments.dry_run, ) dispatch.call_hooks( @@ -332,6 +336,7 @@ def run_actions( restore_databases, repository, dump.DATABASE_HOOK_NAMES, + location, global_arguments.dry_run, ) if 'list' in arguments: diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 169b6432..c58ebdec 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -137,6 +137,14 @@ map: desc: | Exclude files with the NODUMP flag. Defaults to false. example: true + borgmatic_source_directory: + type: str + desc: | + Path for additional source files used for temporary internal state like + borgmatic database dumps. Note that changing this path prevents "borgmatic + restore" from finding any database dumps created before the change. Defaults + to ~/.borgmatic + example: /tmp/borgmatic storage: desc: | Repository storage options. See diff --git a/borgmatic/hooks/dump.py b/borgmatic/hooks/dump.py index 38905efd..54db1d26 100644 --- a/borgmatic/hooks/dump.py +++ b/borgmatic/hooks/dump.py @@ -2,11 +2,24 @@ import glob import logging import os +from borgmatic.borg.create import DEFAULT_BORGMATIC_SOURCE_DIRECTORY + logger = logging.getLogger(__name__) DATABASE_HOOK_NAMES = ('postgresql_databases', 'mysql_databases') +def make_database_dump_path(borgmatic_source_directory, database_hook_name): + ''' + Given a borgmatic source directory (or None) and a database hook name, construct a database dump + path. + ''' + if not borgmatic_source_directory: + borgmatic_source_directory = DEFAULT_BORGMATIC_SOURCE_DIRECTORY + + return os.path.join(borgmatic_source_directory, database_hook_name) + + def make_database_dump_filename(dump_path, name, hostname=None): ''' Based on the given dump directory path, database name, and hostname, return a filename to use diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index 5d17e1d6..b76e2bed 100644 --- a/borgmatic/hooks/mysql.py +++ b/borgmatic/hooks/mysql.py @@ -4,15 +4,24 @@ import os from borgmatic.execute import execute_command from borgmatic.hooks import dump -DUMP_PATH = '~/.borgmatic/mysql_databases' logger = logging.getLogger(__name__) -def dump_databases(databases, log_prefix, dry_run): +def make_dump_path(location_config): # pragma: no cover + ''' + Make the dump path from the given location configuration and the name of this hook. + ''' + return dump.make_database_dump_path( + location_config.get('borgmatic_source_directory'), 'mysql_databases' + ) + + +def dump_databases(databases, log_prefix, location_config, dry_run): ''' Dump the given MySQL/MariaDB databases to disk. The databases are supplied as a sequence of dicts, one dict describing each database as per the configuration schema. Use the given log - prefix in any log entries. If this is a dry run, then don't actually dump anything. + prefix in any log entries. Use the given location configuration dict to construct the + destination path. If this is a dry run, then don't actually dump anything. ''' dry_run_label = ' (dry run; not actually dumping anything)' if dry_run else '' @@ -20,7 +29,9 @@ def dump_databases(databases, log_prefix, dry_run): for database in databases: name = database['name'] - dump_filename = dump.make_database_dump_filename(DUMP_PATH, name, database.get('hostname')) + dump_filename = dump.make_database_dump_filename( + make_dump_path(location_config), name, database.get('hostname') + ) command = ( ('mysqldump', '--add-drop-database') + (('--host', database['hostname']) if 'hostname' in database else ()) @@ -44,37 +55,43 @@ def dump_databases(databases, log_prefix, dry_run): ) -def remove_database_dumps(databases, log_prefix, dry_run): # pragma: no cover +def remove_database_dumps(databases, log_prefix, location_config, dry_run): # pragma: no cover ''' Remove the database dumps for the given databases. The databases are supplied as a sequence of dicts, one dict describing each database as per the configuration schema. Use the log prefix in - any log entries. If this is a dry run, then don't actually remove anything. + any log entries. Use the given location configuration dict to construct the destination path. If + this is a dry run, then don't actually remove anything. ''' - dump.remove_database_dumps(DUMP_PATH, databases, 'MySQL', log_prefix, dry_run) + dump.remove_database_dumps( + make_dump_path(location_config), databases, 'MySQL', log_prefix, dry_run + ) -def make_database_dump_patterns(databases, log_prefix, names): +def make_database_dump_patterns(databases, log_prefix, location_config, names): ''' - Given a sequence of configurations dicts, a prefix to log with, and a sequence of database - names to match, return the corresponding glob patterns to match the database dumps in an - archive. An empty sequence of names indicates that the patterns should match all dumps. + Given a sequence of configurations dicts, a prefix to log with, a location configuration dict, + and a sequence of database names to match, return the corresponding glob patterns to match the + database dumps in an archive. An empty sequence of names indicates that the patterns should + match all dumps. ''' return [ - dump.make_database_dump_filename(DUMP_PATH, name, hostname='*') for name in (names or ['*']) + dump.make_database_dump_filename(make_dump_path(location_config), name, hostname='*') + for name in (names or ['*']) ] -def restore_database_dumps(databases, log_prefix, dry_run): +def restore_database_dumps(databases, log_prefix, location_config, dry_run): ''' Restore the given MySQL/MariaDB databases from disk. The databases are supplied as a sequence of dicts, one dict describing each database as per the configuration schema. Use the given log - prefix in any log entries. If this is a dry run, then don't actually restore anything. + prefix in any log entries. Use the given location configuration dict to construct the + destination path. If this is a dry run, then don't actually restore anything. ''' dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else '' for database in databases: dump_filename = dump.make_database_dump_filename( - DUMP_PATH, database['name'], database.get('hostname') + make_dump_path(location_config), database['name'], database.get('hostname') ) restore_command = ( ('mysql', '--batch') diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index a7a86942..7a46b268 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -4,15 +4,24 @@ import os from borgmatic.execute import execute_command from borgmatic.hooks import dump -DUMP_PATH = '~/.borgmatic/postgresql_databases' logger = logging.getLogger(__name__) -def dump_databases(databases, log_prefix, dry_run): +def make_dump_path(location_config): # pragma: no cover + ''' + Make the dump path from the given location configuration and the name of this hook. + ''' + return dump.make_database_dump_path( + location_config.get('borgmatic_source_directory'), 'postgresql_databases' + ) + + +def dump_databases(databases, log_prefix, location_config, dry_run): ''' Dump the given PostgreSQL databases to disk. The databases are supplied as a sequence of dicts, one dict describing each database as per the configuration schema. Use the given log prefix in - any log entries. If this is a dry run, then don't actually dump anything. + any log entries. Use the given location configuration dict to construct the destination path. If + this is a dry run, then don't actually dump anything. ''' dry_run_label = ' (dry run; not actually dumping anything)' if dry_run else '' @@ -20,7 +29,9 @@ def dump_databases(databases, log_prefix, dry_run): for database in databases: name = database['name'] - dump_filename = dump.make_database_dump_filename(DUMP_PATH, name, database.get('hostname')) + dump_filename = dump.make_database_dump_filename( + make_dump_path(location_config), name, database.get('hostname') + ) all_databases = bool(name == 'all') command = ( ('pg_dumpall' if all_databases else 'pg_dump', '--no-password', '--clean') @@ -44,37 +55,43 @@ def dump_databases(databases, log_prefix, dry_run): execute_command(command, extra_environment=extra_environment) -def remove_database_dumps(databases, log_prefix, dry_run): # pragma: no cover +def remove_database_dumps(databases, log_prefix, location_config, dry_run): # pragma: no cover ''' Remove the database dumps for the given databases. The databases are supplied as a sequence of dicts, one dict describing each database as per the configuration schema. Use the log prefix in - any log entries. If this is a dry run, then don't actually remove anything. + any log entries. Use the given location configuration dict to construct the destination path. If + this is a dry run, then don't actually remove anything. ''' - dump.remove_database_dumps(DUMP_PATH, databases, 'PostgreSQL', log_prefix, dry_run) + dump.remove_database_dumps( + make_dump_path(location_config), databases, 'PostgreSQL', log_prefix, dry_run + ) -def make_database_dump_patterns(databases, log_prefix, names): +def make_database_dump_patterns(databases, log_prefix, location_config, names): ''' - Given a sequence of configurations dicts, a prefix to log with, and a sequence of database - names to match, return the corresponding glob patterns to match the database dumps in an - archive. An empty sequence of names indicates that the patterns should match all dumps. + Given a sequence of configurations dicts, a prefix to log with, a location configuration dict, + and a sequence of database names to match, return the corresponding glob patterns to match the + database dumps in an archive. An empty sequence of names indicates that the patterns should + match all dumps. ''' return [ - dump.make_database_dump_filename(DUMP_PATH, name, hostname='*') for name in (names or ['*']) + dump.make_database_dump_filename(make_dump_path(location_config), name, hostname='*') + for name in (names or ['*']) ] -def restore_database_dumps(databases, log_prefix, dry_run): +def restore_database_dumps(databases, log_prefix, location_config, dry_run): ''' Restore the given PostgreSQL databases from disk. The databases are supplied as a sequence of dicts, one dict describing each database as per the configuration schema. Use the given log - prefix in any log entries. If this is a dry run, then don't actually restore anything. + prefix in any log entries. Use the given location configuration dict to construct the + destination path. If this is a dry run, then don't actually restore anything. ''' dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else '' for database in databases: dump_filename = dump.make_database_dump_filename( - DUMP_PATH, database['name'], database.get('hostname') + make_dump_path(location_config), database['name'], database.get('hostname') ) restore_command = ( ('pg_restore', '--no-password', '--clean', '--if-exists', '--exit-on-error') diff --git a/docs/how-to/backup-your-databases.md b/docs/how-to/backup-your-databases.md index de21192b..0ea771f9 100644 --- a/docs/how-to/backup-your-databases.md +++ b/docs/how-to/backup-your-databases.md @@ -23,8 +23,12 @@ hooks: ``` Prior to each backup, borgmatic dumps each configured database to a file -(located in `~/.borgmatic/`) and includes it in the backup. After the backup -completes, borgmatic removes the database dump files to recover disk space. +and includes it in the backup. After the backup completes, borgmatic removes +the database dump files to recover disk space. + +borgmatic creates these temporary dump files in `~/.borgmatic` by default. To +customize this path, set the `borgmatic_source_directory` option in the +`location` section of borgmatic's configuration. Here's a more involved example that connects to remote databases: diff --git a/setup.py b/setup.py index 9ff3ba3a..18c8ff87 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.4.18' +VERSION = '1.4.19' setup( diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 5a1a3e82..f9044145 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -184,14 +184,21 @@ def test_borgmatic_source_directories_set_when_directory_exists(): flexmock(module.os.path).should_receive('exists').and_return(True) flexmock(module.os.path).should_receive('expanduser') - assert module.borgmatic_source_directories() == [module.BORGMATIC_SOURCE_DIRECTORY] + assert module.borgmatic_source_directories('/tmp') == ['/tmp'] def test_borgmatic_source_directories_empty_when_directory_does_not_exist(): flexmock(module.os.path).should_receive('exists').and_return(False) flexmock(module.os.path).should_receive('expanduser') - assert module.borgmatic_source_directories() == [] + assert module.borgmatic_source_directories('/tmp') == [] + + +def test_borgmatic_source_directories_defaults_when_directory_not_given(): + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module.os.path).should_receive('expanduser') + + assert module.borgmatic_source_directories(None) == [module.DEFAULT_BORGMATIC_SOURCE_DIRECTORY] DEFAULT_ARCHIVE_NAME = '{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f}' diff --git a/tests/unit/hooks/test_dump.py b/tests/unit/hooks/test_dump.py index b74d5b7a..d36f8098 100644 --- a/tests/unit/hooks/test_dump.py +++ b/tests/unit/hooks/test_dump.py @@ -4,6 +4,14 @@ from flexmock import flexmock from borgmatic.hooks import dump as module +def test_make_database_dump_path_joins_arguments(): + assert module.make_database_dump_path('/tmp', 'super_databases') == '/tmp/super_databases' + + +def test_make_database_dump_path_defaults_without_source_directory(): + assert module.make_database_dump_path(None, 'super_databases') == '~/.borgmatic/super_databases' + + def test_make_database_dump_filename_uses_name_and_hostname(): flexmock(module.os.path).should_receive('expanduser').and_return('databases') diff --git a/tests/unit/hooks/test_mysql.py b/tests/unit/hooks/test_mysql.py index c1344131..6465d71d 100644 --- a/tests/unit/hooks/test_mysql.py +++ b/tests/unit/hooks/test_mysql.py @@ -8,6 +8,7 @@ from borgmatic.hooks import mysql as module def test_dump_databases_runs_mysqldump_for_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] output_file = flexmock() + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ).and_return('databases/localhost/bar') @@ -21,23 +22,25 @@ def test_dump_databases_runs_mysqldump_for_each_database(): extra_environment=None, ).once() - module.dump_databases(databases, 'test.yaml', dry_run=False) + module.dump_databases(databases, 'test.yaml', {}, dry_run=False) def test_dump_databases_with_dry_run_skips_mysqldump(): databases = [{'name': 'foo'}, {'name': 'bar'}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ).and_return('databases/localhost/bar') flexmock(module.os).should_receive('makedirs').never() flexmock(module).should_receive('execute_command').never() - module.dump_databases(databases, 'test.yaml', dry_run=True) + module.dump_databases(databases, 'test.yaml', {}, dry_run=True) def test_dump_databases_runs_mysqldump_with_hostname_and_port(): databases = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}] output_file = flexmock() + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/database.example.org/foo' ) @@ -61,12 +64,13 @@ def test_dump_databases_runs_mysqldump_with_hostname_and_port(): extra_environment=None, ).once() - module.dump_databases(databases, 'test.yaml', dry_run=False) + module.dump_databases(databases, 'test.yaml', {}, dry_run=False) def test_dump_databases_runs_mysqldump_with_username_and_password(): databases = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}] output_file = flexmock() + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ) @@ -79,12 +83,13 @@ def test_dump_databases_runs_mysqldump_with_username_and_password(): extra_environment={'MYSQL_PWD': 'trustsome1'}, ).once() - module.dump_databases(databases, 'test.yaml', dry_run=False) + module.dump_databases(databases, 'test.yaml', {}, dry_run=False) def test_dump_databases_runs_mysqldump_with_options(): databases = [{'name': 'foo', 'options': '--stuff=such'}] output_file = flexmock() + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ) @@ -97,12 +102,13 @@ def test_dump_databases_runs_mysqldump_with_options(): extra_environment=None, ).once() - module.dump_databases(databases, 'test.yaml', dry_run=False) + module.dump_databases(databases, 'test.yaml', {}, dry_run=False) def test_dump_databases_runs_mysqldump_for_all_databases(): databases = [{'name': 'all'}] output_file = flexmock() + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/all' ) @@ -115,30 +121,33 @@ def test_dump_databases_runs_mysqldump_for_all_databases(): extra_environment=None, ).once() - module.dump_databases(databases, 'test.yaml', dry_run=False) + module.dump_databases(databases, 'test.yaml', {}, dry_run=False) def test_make_database_dump_patterns_converts_names_to_glob_paths(): + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/*/foo' ).and_return('databases/*/bar') - assert module.make_database_dump_patterns(flexmock(), flexmock(), ('foo', 'bar')) == [ + assert module.make_database_dump_patterns(flexmock(), flexmock(), {}, ('foo', 'bar')) == [ 'databases/*/foo', 'databases/*/bar', ] def test_make_database_dump_patterns_treats_empty_names_as_matching_all_databases(): + flexmock(module).should_receive('make_dump_path').and_return('/dump/path') flexmock(module.dump).should_receive('make_database_dump_filename').with_args( - module.DUMP_PATH, '*', '*' + '/dump/path', '*', '*' ).and_return('databases/*/*') - assert module.make_database_dump_patterns(flexmock(), flexmock(), ()) == ['databases/*/*'] + assert module.make_database_dump_patterns(flexmock(), flexmock(), {}, ()) == ['databases/*/*'] def test_restore_database_dumps_restores_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ).and_return('databases/localhost/bar') @@ -153,11 +162,12 @@ def test_restore_database_dumps_restores_each_database(): ('mysql', '--batch'), input_file=input_file, extra_environment=None ).once() - module.restore_database_dumps(databases, 'test.yaml', dry_run=False) + module.restore_database_dumps(databases, 'test.yaml', {}, dry_run=False) def test_restore_database_dumps_runs_mysql_with_hostname_and_port(): databases = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ) @@ -182,11 +192,12 @@ def test_restore_database_dumps_runs_mysql_with_hostname_and_port(): extra_environment=None, ).once() - module.restore_database_dumps(databases, 'test.yaml', dry_run=False) + module.restore_database_dumps(databases, 'test.yaml', {}, dry_run=False) def test_restore_database_dumps_runs_mysql_with_username_and_password(): databases = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ) @@ -202,4 +213,4 @@ def test_restore_database_dumps_runs_mysql_with_username_and_password(): extra_environment={'MYSQL_PWD': 'trustsome1'}, ).once() - module.restore_database_dumps(databases, 'test.yaml', dry_run=False) + module.restore_database_dumps(databases, 'test.yaml', {}, dry_run=False) diff --git a/tests/unit/hooks/test_postgresql.py b/tests/unit/hooks/test_postgresql.py index 470a63e4..a7c23025 100644 --- a/tests/unit/hooks/test_postgresql.py +++ b/tests/unit/hooks/test_postgresql.py @@ -5,6 +5,7 @@ from borgmatic.hooks import postgresql as module def test_dump_databases_runs_pg_dump_for_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ).and_return('databases/localhost/bar') @@ -25,22 +26,24 @@ def test_dump_databases_runs_pg_dump_for_each_database(): extra_environment=None, ).once() - module.dump_databases(databases, 'test.yaml', dry_run=False) + module.dump_databases(databases, 'test.yaml', {}, dry_run=False) def test_dump_databases_with_dry_run_skips_pg_dump(): databases = [{'name': 'foo'}, {'name': 'bar'}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ).and_return('databases/localhost/bar') flexmock(module.os).should_receive('makedirs').never() flexmock(module).should_receive('execute_command').never() - module.dump_databases(databases, 'test.yaml', dry_run=True) + module.dump_databases(databases, 'test.yaml', {}, dry_run=True) def test_dump_databases_runs_pg_dump_with_hostname_and_port(): databases = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/database.example.org/foo' ) @@ -64,11 +67,12 @@ def test_dump_databases_runs_pg_dump_with_hostname_and_port(): extra_environment=None, ).once() - module.dump_databases(databases, 'test.yaml', dry_run=False) + module.dump_databases(databases, 'test.yaml', {}, dry_run=False) def test_dump_databases_runs_pg_dump_with_username_and_password(): databases = [{'name': 'foo', 'username': 'postgres', 'password': 'trustsome1'}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ) @@ -90,11 +94,12 @@ def test_dump_databases_runs_pg_dump_with_username_and_password(): extra_environment={'PGPASSWORD': 'trustsome1'}, ).once() - module.dump_databases(databases, 'test.yaml', dry_run=False) + module.dump_databases(databases, 'test.yaml', {}, dry_run=False) def test_dump_databases_runs_pg_dump_with_format(): databases = [{'name': 'foo', 'format': 'tar'}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ) @@ -114,11 +119,12 @@ def test_dump_databases_runs_pg_dump_with_format(): extra_environment=None, ).once() - module.dump_databases(databases, 'test.yaml', dry_run=False) + module.dump_databases(databases, 'test.yaml', {}, dry_run=False) def test_dump_databases_runs_pg_dump_with_options(): databases = [{'name': 'foo', 'options': '--stuff=such'}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ) @@ -139,11 +145,12 @@ def test_dump_databases_runs_pg_dump_with_options(): extra_environment=None, ).once() - module.dump_databases(databases, 'test.yaml', dry_run=False) + module.dump_databases(databases, 'test.yaml', {}, dry_run=False) def test_dump_databases_runs_pg_dumpall_for_all_databases(): databases = [{'name': 'all'}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/all' ) @@ -154,30 +161,33 @@ def test_dump_databases_runs_pg_dumpall_for_all_databases(): extra_environment=None, ).once() - module.dump_databases(databases, 'test.yaml', dry_run=False) + module.dump_databases(databases, 'test.yaml', {}, dry_run=False) def test_make_database_dump_patterns_converts_names_to_glob_paths(): + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/*/foo' ).and_return('databases/*/bar') - assert module.make_database_dump_patterns(flexmock(), flexmock(), ('foo', 'bar')) == [ + assert module.make_database_dump_patterns(flexmock(), flexmock(), {}, ('foo', 'bar')) == [ 'databases/*/foo', 'databases/*/bar', ] def test_make_database_dump_patterns_treats_empty_names_as_matching_all_databases(): + flexmock(module).should_receive('make_dump_path').and_return('/dump/path') flexmock(module.dump).should_receive('make_database_dump_filename').with_args( - module.DUMP_PATH, '*', '*' + '/dump/path', '*', '*' ).and_return('databases/*/*') - assert module.make_database_dump_patterns(flexmock(), flexmock(), ()) == ['databases/*/*'] + assert module.make_database_dump_patterns(flexmock(), flexmock(), {}, ()) == ['databases/*/*'] def test_restore_database_dumps_restores_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ).and_return('databases/localhost/bar') @@ -201,11 +211,12 @@ def test_restore_database_dumps_restores_each_database(): extra_environment=None, ).once() - module.restore_database_dumps(databases, 'test.yaml', dry_run=False) + module.restore_database_dumps(databases, 'test.yaml', {}, dry_run=False) def test_restore_database_dumps_runs_pg_restore_with_hostname_and_port(): databases = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ) @@ -244,11 +255,12 @@ def test_restore_database_dumps_runs_pg_restore_with_hostname_and_port(): extra_environment=None, ).once() - module.restore_database_dumps(databases, 'test.yaml', dry_run=False) + module.restore_database_dumps(databases, 'test.yaml', {}, dry_run=False) def test_restore_database_dumps_runs_pg_restore_with_username_and_password(): databases = [{'name': 'foo', 'username': 'postgres', 'password': 'trustsome1'}] + flexmock(module).should_receive('make_dump_path').and_return('') flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ) @@ -283,4 +295,4 @@ def test_restore_database_dumps_runs_pg_restore_with_username_and_password(): extra_environment={'PGPASSWORD': 'trustsome1'}, ).once() - module.restore_database_dumps(databases, 'test.yaml', dry_run=False) + module.restore_database_dumps(databases, 'test.yaml', {}, dry_run=False) From 9f821862b74e29a41f20ed6d0a39a4cb859d3267 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 10 Dec 2019 16:41:01 -0800 Subject: [PATCH 11/30] End-to-end tests for database dump and restore. --- .drone.yml | 45 ++++++++++++++++++ tests/end-to-end/test_borgmatic.py | 8 ++-- tests/end-to-end/test_database.py | 76 ++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 tests/end-to-end/test_database.py diff --git a/.drone.yml b/.drone.yml index 81377208..a7fa332d 100644 --- a/.drone.yml +++ b/.drone.yml @@ -2,6 +2,15 @@ kind: pipeline name: python-3-5-alpine-3-10 +services: + database: + image: postgres:11.5 + ports: + - "5432:5432" + environment: + POSTGRES_PASSWORD: test + POSTGRES_DB: test + steps: - name: build image: python:3.5-alpine3.10 @@ -12,6 +21,15 @@ steps: kind: pipeline name: python-3-6-alpine-3-10 +services: + database: + image: postgres:11.5 + ports: + - "5432:5432" + environment: + POSTGRES_PASSWORD: test + POSTGRES_DB: test + steps: - name: build image: python:3.6-alpine3.10 @@ -22,6 +40,15 @@ steps: kind: pipeline name: python-3-7-alpine-3-10 +services: + database: + image: postgres:11.5 + ports: + - "5432:5432" + environment: + POSTGRES_PASSWORD: test + POSTGRES_DB: test + steps: - name: build image: python:3.7-alpine3.10 @@ -32,6 +59,15 @@ steps: kind: pipeline name: python-3-7-alpine-3-7 +services: + database: + image: postgres:11.5 + ports: + - "5432:5432" + environment: + POSTGRES_PASSWORD: test + POSTGRES_DB: test + steps: - name: build image: python:3.7-alpine3.7 @@ -42,6 +78,15 @@ steps: kind: pipeline name: python-3-8-alpine-3-10 +services: + database: + image: postgres:11.5 + ports: + - "5432:5432" + environment: + POSTGRES_PASSWORD: test + POSTGRES_DB: test + steps: - name: build image: python:3.8-alpine3.10 diff --git a/tests/end-to-end/test_borgmatic.py b/tests/end-to-end/test_borgmatic.py index 620e67a9..0bfe7b7b 100644 --- a/tests/end-to-end/test_borgmatic.py +++ b/tests/end-to-end/test_borgmatic.py @@ -44,13 +44,13 @@ def test_borgmatic_command(): generate_configuration(config_path, repository_path) subprocess.check_call( - 'borgmatic -v 2 --config {} --init --encryption repokey'.format(config_path).split(' ') + 'borgmatic -v 2 --config {} init --encryption repokey'.format(config_path).split(' ') ) # Run borgmatic to generate a backup archive, and then list it to make sure it exists. subprocess.check_call('borgmatic --config {}'.format(config_path).split(' ')) output = subprocess.check_output( - 'borgmatic --config {} --list --json'.format(config_path).split(' ') + 'borgmatic --config {} list --json'.format(config_path).split(' ') ).decode(sys.stdout.encoding) parsed_output = json.loads(output) @@ -61,7 +61,7 @@ def test_borgmatic_command(): # Extract the created archive into the current (temporary) directory, and confirm that the # extracted file looks right. output = subprocess.check_output( - 'borgmatic --config {} --extract --archive {}'.format(config_path, archive_name).split( + 'borgmatic --config {} extract --archive {}'.format(config_path, archive_name).split( ' ' ) ).decode(sys.stdout.encoding) @@ -70,7 +70,7 @@ def test_borgmatic_command(): # Exercise the info flag. output = subprocess.check_output( - 'borgmatic --config {} --info --json'.format(config_path).split(' ') + 'borgmatic --config {} info --json'.format(config_path).split(' ') ).decode(sys.stdout.encoding) parsed_output = json.loads(output) diff --git a/tests/end-to-end/test_database.py b/tests/end-to-end/test_database.py new file mode 100644 index 00000000..b5a34135 --- /dev/null +++ b/tests/end-to-end/test_database.py @@ -0,0 +1,76 @@ +import json +import os +import shutil +import subprocess +import sys +import tempfile + + +def write_configuration(config_path, repository_path, borgmatic_source_directory): + ''' + Write out borgmatic configuration into a file at the config path. Set the options so as to work + for testing. This includes injecting the given repository path, borgmatic source directory for + storing database dumps, and encryption passphrase. + ''' + config = ''' +location: + source_directories: + - {} + repositories: + - {} + borgmatic_source_directory: {} + +storage: + encryption_passphrase: "test" + +hooks: + postgresql_databases: + - name: test + hostname: localhost + username: postgres + password: test +'''.format(config_path, repository_path, borgmatic_source_directory) + + config_file = open(config_path, 'w') + config_file.write(config) + config_file.close() + + +def test_database_dump_and_restore(): + # Create a Borg repository. + temporary_directory = tempfile.mkdtemp() + repository_path = os.path.join(temporary_directory, 'test.borg') + borgmatic_source_directory = os.path.join(temporary_directory, '.borgmatic') + + original_working_directory = os.getcwd() + + try: + config_path = os.path.join(temporary_directory, 'test.yaml') + write_configuration(config_path, repository_path, borgmatic_source_directory) + + subprocess.check_call( + 'borgmatic -v 2 --config {} init --encryption repokey'.format(config_path).split(' ') + ) + + # Run borgmatic to generate a backup archive including a database dump + subprocess.check_call('borgmatic create --config {} -v 2'.format(config_path).split(' ')) + + # Get the created archive name. + output = subprocess.check_output( + 'borgmatic --config {} list --json'.format(config_path).split(' ') + ).decode(sys.stdout.encoding) + parsed_output = json.loads(output) + + assert len(parsed_output) == 1 + assert len(parsed_output[0]['archives']) == 1 + archive_name = parsed_output[0]['archives'][0]['archive'] + + # Restore the database from the archive. + subprocess.check_call( + 'borgmatic --config {} restore --archive {}'.format(config_path, archive_name).split( + ' ' + ) + ) + finally: + os.chdir(original_working_directory) + shutil.rmtree(temporary_directory) From 01e2cf08d1938bd1adf2650f2e2e57253b4dc5e5 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 10 Dec 2019 16:43:43 -0800 Subject: [PATCH 12/30] Fix Drone CI services syntax. --- .drone.yml | 10 +++++----- tests/end-to-end/test_database.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.drone.yml b/.drone.yml index a7fa332d..426c0292 100644 --- a/.drone.yml +++ b/.drone.yml @@ -3,7 +3,7 @@ kind: pipeline name: python-3-5-alpine-3-10 services: - database: + - name: postgresql image: postgres:11.5 ports: - "5432:5432" @@ -22,7 +22,7 @@ kind: pipeline name: python-3-6-alpine-3-10 services: - database: + - name: postgresql image: postgres:11.5 ports: - "5432:5432" @@ -41,7 +41,7 @@ kind: pipeline name: python-3-7-alpine-3-10 services: - database: + - name: postgresql image: postgres:11.5 ports: - "5432:5432" @@ -60,7 +60,7 @@ kind: pipeline name: python-3-7-alpine-3-7 services: - database: + - name: postgresql image: postgres:11.5 ports: - "5432:5432" @@ -79,7 +79,7 @@ kind: pipeline name: python-3-8-alpine-3-10 services: - database: + - name: postgresql image: postgres:11.5 ports: - "5432:5432" diff --git a/tests/end-to-end/test_database.py b/tests/end-to-end/test_database.py index b5a34135..d12b03d7 100644 --- a/tests/end-to-end/test_database.py +++ b/tests/end-to-end/test_database.py @@ -26,7 +26,7 @@ storage: hooks: postgresql_databases: - name: test - hostname: localhost + hostname: postgresql username: postgres password: test '''.format(config_path, repository_path, borgmatic_source_directory) From abd31a94fb0bcc0f37632f0a88c9d8712d5ab79d Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 10 Dec 2019 16:47:09 -0800 Subject: [PATCH 13/30] Ports fix? --- .drone.yml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/.drone.yml b/.drone.yml index 426c0292..f929564f 100644 --- a/.drone.yml +++ b/.drone.yml @@ -5,8 +5,6 @@ name: python-3-5-alpine-3-10 services: - name: postgresql image: postgres:11.5 - ports: - - "5432:5432" environment: POSTGRES_PASSWORD: test POSTGRES_DB: test @@ -24,8 +22,6 @@ name: python-3-6-alpine-3-10 services: - name: postgresql image: postgres:11.5 - ports: - - "5432:5432" environment: POSTGRES_PASSWORD: test POSTGRES_DB: test @@ -43,8 +39,6 @@ name: python-3-7-alpine-3-10 services: - name: postgresql image: postgres:11.5 - ports: - - "5432:5432" environment: POSTGRES_PASSWORD: test POSTGRES_DB: test @@ -62,8 +56,6 @@ name: python-3-7-alpine-3-7 services: - name: postgresql image: postgres:11.5 - ports: - - "5432:5432" environment: POSTGRES_PASSWORD: test POSTGRES_DB: test @@ -81,8 +73,6 @@ name: python-3-8-alpine-3-10 services: - name: postgresql image: postgres:11.5 - ports: - - "5432:5432" environment: POSTGRES_PASSWORD: test POSTGRES_DB: test From 2e5be3d3f1d4acb42851ea04a66b6adcaa62cf5f Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 10 Dec 2019 16:52:59 -0800 Subject: [PATCH 14/30] Add missing psql. --- NEWS | 1 + scripts/run-tests | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 9d35de31..9c439f1c 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ 1.4.19 * #259: Optionally change the internal database dump path via "borgmatic_source_directory" option in location configuration section. + * Add end-to-end tests for database dump and restore. 1.4.18 * Fix "--repository" flag to accept relative paths. diff --git a/scripts/run-tests b/scripts/run-tests index d2a91c26..ec0901a9 100755 --- a/scripts/run-tests +++ b/scripts/run-tests @@ -9,5 +9,5 @@ set -e python -m pip install --upgrade pip==19.3.1 pip install tox==3.14.0 tox -apk add --no-cache borgbackup +apk add --no-cache borgbackup postgresql-client tox -e end-to-end From 68281339b77e204d6e4390fe62706fefcfa43888 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 10 Dec 2019 16:57:12 -0800 Subject: [PATCH 15/30] Black. --- tests/end-to-end/test_database.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/end-to-end/test_database.py b/tests/end-to-end/test_database.py index d12b03d7..83c9e949 100644 --- a/tests/end-to-end/test_database.py +++ b/tests/end-to-end/test_database.py @@ -29,7 +29,9 @@ hooks: hostname: postgresql username: postgres password: test -'''.format(config_path, repository_path, borgmatic_source_directory) +'''.format( + config_path, repository_path, borgmatic_source_directory + ) config_file = open(config_path, 'w') config_file.write(config) From b8b888090d6820c795119579c5679ed5a27175b0 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 10 Dec 2019 21:41:15 -0800 Subject: [PATCH 16/30] Select Postgres service to work with particular client version. --- .drone.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.drone.yml b/.drone.yml index f929564f..57ba82cf 100644 --- a/.drone.yml +++ b/.drone.yml @@ -4,7 +4,7 @@ name: python-3-5-alpine-3-10 services: - name: postgresql - image: postgres:11.5 + image: postgres:11.6-alpine environment: POSTGRES_PASSWORD: test POSTGRES_DB: test @@ -21,7 +21,7 @@ name: python-3-6-alpine-3-10 services: - name: postgresql - image: postgres:11.5 + image: postgres:11.6-alpine environment: POSTGRES_PASSWORD: test POSTGRES_DB: test @@ -38,7 +38,7 @@ name: python-3-7-alpine-3-10 services: - name: postgresql - image: postgres:11.5 + image: postgres:11.6-alpine environment: POSTGRES_PASSWORD: test POSTGRES_DB: test @@ -55,7 +55,7 @@ name: python-3-7-alpine-3-7 services: - name: postgresql - image: postgres:11.5 + image: postgres:10.11-alpine environment: POSTGRES_PASSWORD: test POSTGRES_DB: test @@ -72,7 +72,7 @@ name: python-3-8-alpine-3-10 services: - name: postgresql - image: postgres:11.5 + image: postgres:11.6-alpine environment: POSTGRES_PASSWORD: test POSTGRES_DB: test From 14e5cfc8f84de63d977a95030fdddbb9486fe9ca Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 11 Dec 2019 12:12:25 -0800 Subject: [PATCH 17/30] Support piping "borgmatic list" output to grep. Retain colored output when piping/redirecting (#271). --- NEWS | 3 +++ borgmatic/logger.py | 52 +++++++++++++++++++++++++++++++++++++-- tests/unit/test_logger.py | 40 +++++++++++++++++++++++++++--- 3 files changed, 90 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index 9d35de31..0137cf80 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,9 @@ 1.4.19 * #259: Optionally change the internal database dump path via "borgmatic_source_directory" option in location configuration section. + * #271: Support piping "borgmatic list" output to grep by logging certain log levels to console + stdout and others to stderr. + * Retain colored output when piping or redirecting output in an interactive terminal. 1.4.18 * Fix "--repository" flag to accept relative paths. diff --git a/borgmatic/logger.py b/borgmatic/logger.py index ce9be04a..b20f89e3 100644 --- a/borgmatic/logger.py +++ b/borgmatic/logger.py @@ -26,7 +26,7 @@ def interactive_console(): Return whether the current console is "interactive". Meaning: Capable of user input and not just something like a cron job. ''' - return sys.stdout.isatty() and os.environ.get('TERM') != 'dumb' + return sys.stderr.isatty() and os.environ.get('TERM') != 'dumb' def should_do_markup(no_color, configs): @@ -48,6 +48,42 @@ def should_do_markup(no_color, configs): return interactive_console() +class Multi_stream_handler(logging.Handler): + ''' + A logging handler that dispatches each log record to one of multiple stream handlers depending + on the record's log level. + ''' + + def __init__(self, log_level_to_stream_handler): + super(Multi_stream_handler, self).__init__() + self.log_level_to_handler = log_level_to_stream_handler + self.handlers = set(self.log_level_to_handler.values()) + + def flush(self): # pragma: no cover + super(Multi_stream_handler, self).flush() + + for handler in self.handlers: + handler.flush() + + def emit(self, record): + ''' + Dispatch the log record to the approriate stream handler for the record's log level. + ''' + self.log_level_to_handler[record.levelno].emit(record) + + def setFormatter(self, formatter): # pragma: no cover + super(Multi_stream_handler, self).setFormatter(formatter) + + for handler in self.handlers: + handler.setFormatter(formatter) + + def setLevel(self, level): # pragma: no cover + super(Multi_stream_handler, self).setLevel(level) + + for handler in self.handlers: + handler.setLevel(level) + + LOG_LEVEL_TO_COLOR = { logging.CRITICAL: colorama.Fore.RED, logging.ERROR: colorama.Fore.RED, @@ -87,7 +123,19 @@ def configure_logging( if log_file_log_level is None: log_file_log_level = console_log_level - console_handler = logging.StreamHandler() + # Log certain log levels to console stderr and others to stdout. This supports use cases like + # grepping (non-error) output. + console_error_handler = logging.StreamHandler(sys.stderr) + console_standard_handler = logging.StreamHandler(sys.stdout) + console_handler = Multi_stream_handler( + { + logging.CRITICAL: console_error_handler, + logging.ERROR: console_error_handler, + logging.WARN: console_standard_handler, + logging.INFO: console_standard_handler, + logging.DEBUG: console_standard_handler, + } + ) console_handler.setFormatter(Console_color_formatter()) console_handler.setLevel(console_log_level) diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index 8eeb6be6..707376bc 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -22,14 +22,14 @@ def test_to_bool_passes_none_through(): def test_interactive_console_false_when_not_isatty(capsys): with capsys.disabled(): - flexmock(module.sys.stdout).should_receive('isatty').and_return(False) + flexmock(module.sys.stderr).should_receive('isatty').and_return(False) assert module.interactive_console() is False def test_interactive_console_false_when_TERM_is_dumb(capsys): with capsys.disabled(): - flexmock(module.sys.stdout).should_receive('isatty').and_return(True) + flexmock(module.sys.stderr).should_receive('isatty').and_return(True) flexmock(module.os.environ).should_receive('get').with_args('TERM').and_return('dumb') assert module.interactive_console() is False @@ -37,7 +37,7 @@ def test_interactive_console_false_when_TERM_is_dumb(capsys): def test_interactive_console_true_when_isatty_and_TERM_is_not_dumb(capsys): with capsys.disabled(): - flexmock(module.sys.stdout).should_receive('isatty').and_return(True) + flexmock(module.sys.stderr).should_receive('isatty').and_return(True) flexmock(module.os.environ).should_receive('get').with_args('TERM').and_return('smart') assert module.interactive_console() is True @@ -113,6 +113,17 @@ def test_should_do_markup_prefers_PY_COLORS_to_interactive_console_value(): assert module.should_do_markup(no_color=False, configs={}) is True +def test_multi_stream_handler_logs_to_handler_for_log_level(): + error_handler = flexmock() + error_handler.should_receive('emit').once() + info_handler = flexmock() + + multi_handler = module.Multi_stream_handler( + {module.logging.ERROR: error_handler, module.logging.INFO: info_handler} + ) + multi_handler.emit(flexmock(levelno=module.logging.ERROR)) + + def test_console_color_formatter_format_includes_log_message(): plain_message = 'uh oh' record = flexmock(levelno=logging.CRITICAL, msg=plain_message) @@ -132,6 +143,9 @@ def test_color_text_without_color_does_not_raise(): def test_configure_logging_probes_for_log_socket_on_linux(): + flexmock(module).should_receive('Multi_stream_handler').and_return( + flexmock(setFormatter=lambda formatter: None, setLevel=lambda level: None) + ) flexmock(module).should_receive('Console_color_formatter') flexmock(module).should_receive('interactive_console').and_return(False) flexmock(module.logging).should_receive('basicConfig').with_args( @@ -147,6 +161,9 @@ def test_configure_logging_probes_for_log_socket_on_linux(): def test_configure_logging_probes_for_log_socket_on_macos(): + flexmock(module).should_receive('Multi_stream_handler').and_return( + flexmock(setFormatter=lambda formatter: None, setLevel=lambda level: None) + ) flexmock(module).should_receive('Console_color_formatter') flexmock(module).should_receive('interactive_console').and_return(False) flexmock(module.logging).should_receive('basicConfig').with_args( @@ -163,6 +180,9 @@ def test_configure_logging_probes_for_log_socket_on_macos(): def test_configure_logging_sets_global_logger_to_most_verbose_log_level(): + flexmock(module).should_receive('Multi_stream_handler').and_return( + flexmock(setFormatter=lambda formatter: None, setLevel=lambda level: None) + ) flexmock(module).should_receive('Console_color_formatter') flexmock(module.logging).should_receive('basicConfig').with_args( level=logging.DEBUG, handlers=tuple @@ -173,6 +193,9 @@ def test_configure_logging_sets_global_logger_to_most_verbose_log_level(): def test_configure_logging_skips_syslog_if_not_found(): + flexmock(module).should_receive('Multi_stream_handler').and_return( + flexmock(setFormatter=lambda formatter: None, setLevel=lambda level: None) + ) flexmock(module).should_receive('Console_color_formatter') flexmock(module.logging).should_receive('basicConfig').with_args( level=logging.INFO, handlers=tuple @@ -184,6 +207,9 @@ def test_configure_logging_skips_syslog_if_not_found(): def test_configure_logging_skips_syslog_if_interactive_console(): + flexmock(module).should_receive('Multi_stream_handler').and_return( + flexmock(setFormatter=lambda formatter: None, setLevel=lambda level: None) + ) flexmock(module).should_receive('Console_color_formatter') flexmock(module).should_receive('interactive_console').and_return(True) flexmock(module.logging).should_receive('basicConfig').with_args( @@ -196,6 +222,10 @@ def test_configure_logging_skips_syslog_if_interactive_console(): def test_configure_logging_to_logfile_instead_of_syslog(): + flexmock(module).should_receive('Multi_stream_handler').and_return( + flexmock(setFormatter=lambda formatter: None, setLevel=lambda level: None) + ) + # syslog skipped in non-interactive console if --log-file argument provided flexmock(module).should_receive('interactive_console').and_return(False) flexmock(module.logging).should_receive('basicConfig').with_args( @@ -214,6 +244,10 @@ def test_configure_logging_to_logfile_instead_of_syslog(): def test_configure_logging_skips_logfile_if_argument_is_none(): + flexmock(module).should_receive('Multi_stream_handler').and_return( + flexmock(setFormatter=lambda formatter: None, setLevel=lambda level: None) + ) + # No WatchedFileHandler added if argument --log-file is None flexmock(module).should_receive('interactive_console').and_return(False) flexmock(module.logging).should_receive('basicConfig').with_args( From 464ff2fe96868b7a17f4c27c7cb0fa8d89acf068 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 11 Dec 2019 16:43:01 -0800 Subject: [PATCH 18/30] Run end-to-end tests on developer machines with Docker Compose for approximate parity with continuous integration tests. --- .drone.yml | 35 ++++++++++++++++++++++++---- NEWS | 5 ++-- docs/how-to/develop-on-borgmatic.md | 16 +++++++++---- scripts/run-full-dev-tests | 13 +++++++++++ scripts/run-full-tests | 19 +++++++++++++++ scripts/run-tests | 13 ----------- tests/end-to-end/docker-compose.yaml | 23 ++++++++++++++++++ tests/end-to-end/test_database.py | 5 ++++ tox.ini | 2 +- 9 files changed, 106 insertions(+), 25 deletions(-) create mode 100755 scripts/run-full-dev-tests create mode 100755 scripts/run-full-tests delete mode 100755 scripts/run-tests create mode 100644 tests/end-to-end/docker-compose.yaml diff --git a/.drone.yml b/.drone.yml index 57ba82cf..89987e3e 100644 --- a/.drone.yml +++ b/.drone.yml @@ -8,13 +8,18 @@ services: environment: POSTGRES_PASSWORD: test POSTGRES_DB: test + - name: mysql + image: mariadb:10.3 + environment: + MYSQL_ROOT_PASSWORD: test + MYSQL_DATABASE: test steps: - name: build image: python:3.5-alpine3.10 pull: always commands: - - scripts/run-tests + - scripts/run-full-tests --- kind: pipeline name: python-3-6-alpine-3-10 @@ -25,13 +30,18 @@ services: environment: POSTGRES_PASSWORD: test POSTGRES_DB: test + - name: mysql + image: mariadb:10.3 + environment: + MYSQL_ROOT_PASSWORD: test + MYSQL_DATABASE: test steps: - name: build image: python:3.6-alpine3.10 pull: always commands: - - scripts/run-tests + - scripts/run-full-tests --- kind: pipeline name: python-3-7-alpine-3-10 @@ -42,13 +52,18 @@ services: environment: POSTGRES_PASSWORD: test POSTGRES_DB: test + - name: mysql + image: mariadb:10.3 + environment: + MYSQL_ROOT_PASSWORD: test + MYSQL_DATABASE: test steps: - name: build image: python:3.7-alpine3.10 pull: always commands: - - scripts/run-tests + - scripts/run-full-tests --- kind: pipeline name: python-3-7-alpine-3-7 @@ -59,13 +74,18 @@ services: environment: POSTGRES_PASSWORD: test POSTGRES_DB: test + - name: mysql + image: mariadb:10.1 + environment: + MYSQL_ROOT_PASSWORD: test + MYSQL_DATABASE: test steps: - name: build image: python:3.7-alpine3.7 pull: always commands: - - scripts/run-tests + - scripts/run-full-tests --- kind: pipeline name: python-3-8-alpine-3-10 @@ -76,13 +96,18 @@ services: environment: POSTGRES_PASSWORD: test POSTGRES_DB: test + - name: mysql + image: mariadb:10.3 + environment: + MYSQL_ROOT_PASSWORD: test + MYSQL_DATABASE: test steps: - name: build image: python:3.8-alpine3.10 pull: always commands: - - scripts/run-tests + - scripts/run-full-tests --- kind: pipeline name: documentation diff --git a/NEWS b/NEWS index fc24cd3d..a0eac709 100644 --- a/NEWS +++ b/NEWS @@ -3,8 +3,9 @@ in location configuration section. * #271: Support piping "borgmatic list" output to grep by logging certain log levels to console stdout and others to stderr. - * Retain colored output when piping or redirecting output in an interactive terminal. - * Add end-to-end tests for database dump and restore. + * Retain colored output when piping or redirecting in an interactive terminal. + * Add end-to-end tests for database dump and restore. These are run on developer machines with + Docker Compose for approximate parity with continuous integration tests. 1.4.18 * Fix "--repository" flag to accept relative paths. diff --git a/docs/how-to/develop-on-borgmatic.md b/docs/how-to/develop-on-borgmatic.md index 5a52352e..9e9fee72 100644 --- a/docs/how-to/develop-on-borgmatic.md +++ b/docs/how-to/develop-on-borgmatic.md @@ -75,14 +75,22 @@ tox -e isort ### End-to-end tests borgmatic additionally includes some end-to-end tests that integration test -with Borg for a few representative scenarios. These tests don't run by default -because they're relatively slow and depend on Borg. If you would like to run -them: +with Borg and supported databases for a few representative scenarios. These +tests don't run by default when running `tox`, because they're relatively slow +and depend on Docker containers for runtime dependencies. These tests tests do +run on the continuous integration (CI) server, and running them on your +developer machine is the closest thing to CI test parity. + +If you would like to run the full test suite, first install Docker and [Docker +Compose](https://docs.docker.com/compose/install/). Then run: ```bash -tox -e end-to-end +scripts/run-full-dev-tests ``` +Note that this scripts assumes you have permission to run Docker. If you +don't, then you may need to run with `sudo`. + ## Code style Start with [PEP 8](https://www.python.org/dev/peps/pep-0008/). But then, apply diff --git a/scripts/run-full-dev-tests b/scripts/run-full-dev-tests new file mode 100755 index 00000000..2c69a09f --- /dev/null +++ b/scripts/run-full-dev-tests @@ -0,0 +1,13 @@ +#!/bin/sh + +# This script is for running all tests, including end-to-end tests, on a developer machine. It sets +# up database containers to run tests against, runs the tests, and then tears down the containers. +# +# Run this script from the root directory of the borgmatic source. +# +# For more information, see: +# https://torsion.org/borgmatic/docs/how-to/develop-on-borgmatic/ + +set -e + +docker-compose --file tests/end-to-end/docker-compose.yaml up --abort-on-container-exit diff --git a/scripts/run-full-tests b/scripts/run-full-tests new file mode 100755 index 00000000..04c77c9d --- /dev/null +++ b/scripts/run-full-tests @@ -0,0 +1,19 @@ +#!/bin/sh + +# This script installs test dependencies and runs all tests, including end-to-end tests. It +# is designed to run inside a test container, and presumes that other test infrastructure like +# databases are already running. Therefore, on a developer machine, you should not run this script +# directly. Instead, run scripts/run-full-dev-tests +# +# For more information, see: +# https://torsion.org/borgmatic/docs/how-to/develop-on-borgmatic/ + +set -e + +python -m pip install --upgrade pip==19.3.1 +pip install tox==3.14.0 +tox +apk add --no-cache borgbackup postgresql-client mariadb-client +working_directory="$PWD" +adduser --disabled-password tests +su - tests --command "cd $working_directory && tox --workdir /tmp -e end-to-end" diff --git a/scripts/run-tests b/scripts/run-tests deleted file mode 100755 index ec0901a9..00000000 --- a/scripts/run-tests +++ /dev/null @@ -1,13 +0,0 @@ -#!/bin/sh - -# This script is intended to be run from the continuous integration build -# server, and not on a developer machine. For that, see: -# https://torsion.org/borgmatic/docs/how-to/develop-on-borgmatic/ - -set -e - -python -m pip install --upgrade pip==19.3.1 -pip install tox==3.14.0 -tox -apk add --no-cache borgbackup postgresql-client -tox -e end-to-end diff --git a/tests/end-to-end/docker-compose.yaml b/tests/end-to-end/docker-compose.yaml new file mode 100644 index 00000000..1e7ad582 --- /dev/null +++ b/tests/end-to-end/docker-compose.yaml @@ -0,0 +1,23 @@ +version: '3' +services: + postgresql: + image: postgres:11.6-alpine + environment: + POSTGRES_PASSWORD: test + POSTGRES_DB: test + mysql: + image: mariadb:10.4 + environment: + MYSQL_ROOT_PASSWORD: test + MYSQL_DATABASE: test + tests: + image: python:3.7-alpine3.10 + volumes: + - "../..:/app" + tty: true + working_dir: /app + command: + - /app/scripts/run-full-tests + depends_on: + - postgresql + - mysql diff --git a/tests/end-to-end/test_database.py b/tests/end-to-end/test_database.py index 83c9e949..a011c6f3 100644 --- a/tests/end-to-end/test_database.py +++ b/tests/end-to-end/test_database.py @@ -29,6 +29,11 @@ hooks: hostname: postgresql username: postgres password: test + mysql_databases: + - name: test + hostname: mysql + username: root + password: test '''.format( config_path, repository_path, borgmatic_source_directory ) diff --git a/tox.ini b/tox.ini index 92c3a33f..8d453c21 100644 --- a/tox.ini +++ b/tox.ini @@ -11,7 +11,7 @@ whitelist_externals = find sh commands_pre = - find {toxinidir} -type f -not -path '{toxinidir}/.tox/*' -path '*/__pycache__/*' -name '*.py[c|o]' -delete + find {envdir} -type f -not -path '*/__pycache__/*' -name '*.py[c|o]' -delete commands = pytest {posargs} py36,py37,py38: black --check . From d2df224da800bf050cba0fd02e5dd8e052e17ecb Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 11 Dec 2019 16:46:24 -0800 Subject: [PATCH 19/30] Use busybox short option to su. --- scripts/run-full-tests | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/run-full-tests b/scripts/run-full-tests index 04c77c9d..6afea000 100755 --- a/scripts/run-full-tests +++ b/scripts/run-full-tests @@ -16,4 +16,4 @@ tox apk add --no-cache borgbackup postgresql-client mariadb-client working_directory="$PWD" adduser --disabled-password tests -su - tests --command "cd $working_directory && tox --workdir /tmp -e end-to-end" +su - tests -c "cd $working_directory && tox --workdir /tmp -e end-to-end" From 78aa4626fab000ecb48a8c7a75324db38c36ff00 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 11 Dec 2019 16:58:08 -0800 Subject: [PATCH 20/30] Remove user switch in container due to CI permission issue. --- scripts/run-full-tests | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/run-full-tests b/scripts/run-full-tests index 6afea000..a88b4a45 100755 --- a/scripts/run-full-tests +++ b/scripts/run-full-tests @@ -14,6 +14,4 @@ python -m pip install --upgrade pip==19.3.1 pip install tox==3.14.0 tox apk add --no-cache borgbackup postgresql-client mariadb-client -working_directory="$PWD" -adduser --disabled-password tests -su - tests -c "cd $working_directory && tox --workdir /tmp -e end-to-end" +tox --workdir /tmp -e end-to-end From c6cb21a7488afb8da303dee6e778c8051762ce69 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 11 Dec 2019 21:24:37 -0800 Subject: [PATCH 21/30] Switch to read-only container filesystem to avoid *.pyc getting created with busted permissions. --- scripts/run-full-dev-tests | 3 ++- scripts/run-full-tests | 7 ++++--- tests/end-to-end/docker-compose.yaml | 4 +++- tox.ini | 6 +++--- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/scripts/run-full-dev-tests b/scripts/run-full-dev-tests index 2c69a09f..2c682f02 100755 --- a/scripts/run-full-dev-tests +++ b/scripts/run-full-dev-tests @@ -10,4 +10,5 @@ set -e -docker-compose --file tests/end-to-end/docker-compose.yaml up --abort-on-container-exit +docker-compose --file tests/end-to-end/docker-compose.yaml up --force-recreate \ + --abort-on-container-exit diff --git a/scripts/run-full-tests b/scripts/run-full-tests index a88b4a45..21035ee4 100755 --- a/scripts/run-full-tests +++ b/scripts/run-full-tests @@ -11,7 +11,8 @@ set -e python -m pip install --upgrade pip==19.3.1 -pip install tox==3.14.0 -tox +pip install tox==3.14.1 +export COVERAGE_FILE=/tmp/.coverage +tox --workdir /tmp/.tox apk add --no-cache borgbackup postgresql-client mariadb-client -tox --workdir /tmp -e end-to-end +tox --workdir /tmp/.tox -e end-to-end diff --git a/tests/end-to-end/docker-compose.yaml b/tests/end-to-end/docker-compose.yaml index 1e7ad582..156596a8 100644 --- a/tests/end-to-end/docker-compose.yaml +++ b/tests/end-to-end/docker-compose.yaml @@ -13,7 +13,9 @@ services: tests: image: python:3.7-alpine3.10 volumes: - - "../..:/app" + - "../..:/app:ro" + tmpfs: + - "/app/borgmatic.egg-info" tty: true working_dir: /app command: diff --git a/tox.ini b/tox.ini index 8d453c21..b6b0537d 100644 --- a/tox.ini +++ b/tox.ini @@ -2,7 +2,7 @@ envlist = py35,py36,py37,py38 skip_missing_interpreters = True skipsdist = True -minversion = 3.14.0 +minversion = 3.14.1 [testenv] usedevelop = True @@ -10,8 +10,7 @@ deps = -rtest_requirements.txt whitelist_externals = find sh -commands_pre = - find {envdir} -type f -not -path '*/__pycache__/*' -name '*.py[c|o]' -delete +passenv = COVERAGE_FILE commands = pytest {posargs} py36,py37,py38: black --check . @@ -28,6 +27,7 @@ commands = [testenv:end-to-end] deps = -rtest_requirements.txt +passenv = COVERAGE_FILE commands = pytest {posargs} --no-cov tests/end-to-end From dd16504329489c0921427f3ae414231039b08bd7 Mon Sep 17 00:00:00 2001 From: Matthew Daley Date: Fri, 13 Dec 2019 15:42:46 +1300 Subject: [PATCH 22/30] Use --remote-path, --debug and --info when checking for repo existence These are currently not being used in the call to `borg info` performed as part of the borgmatic init command to check whether or not the repo already exists. --- borgmatic/borg/init.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/borgmatic/borg/init.py b/borgmatic/borg/init.py index 152f8c1b..08256aef 100644 --- a/borgmatic/borg/init.py +++ b/borgmatic/borg/init.py @@ -23,7 +23,13 @@ def initialize_repository( whether the repository should be append-only, and the storage quota to use, initialize the repository. If the repository already exists, then log and skip initialization. ''' - info_command = (local_path, 'info', repository) + info_command = ( + (local_path, 'info') + + (('--info',) if logger.getEffectiveLevel() == logging.INFO else ()) + + (('--debug',) if logger.isEnabledFor(logging.DEBUG) else ()) + + (('--remote-path', remote_path) if remote_path else ()) + + (repository,) + ) logger.debug(' '.join(info_command)) try: From f1358d52aaa9e6926636a8660f57669fa2abe8ae Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 12 Dec 2019 21:50:24 -0800 Subject: [PATCH 23/30] Add "borgmatic init" repository probing fix to NEWS. --- NEWS | 3 +++ setup.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index a0eac709..49a167f9 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +1.4.20 + * Fix repository probing during "borgmatic init" to respect verbosity flag and remote_path option. + 1.4.19 * #259: Optionally change the internal database dump path via "borgmatic_source_directory" option in location configuration section. diff --git a/setup.py b/setup.py index 18c8ff87..e61d3ac8 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.4.19' +VERSION = '1.4.20' setup( From e009bfeaa20d173ebe189cecb47c43819f747aa0 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 12 Dec 2019 22:54:45 -0800 Subject: [PATCH 24/30] Update Healthchecks/Cronitor/Cronhub monitoring integrations to fire for "check" and "prune" actions, not just "create" (#249). --- NEWS | 2 + borgmatic/commands/borgmatic.py | 65 ++++++++++++++------------- docs/how-to/monitor-your-backups.md | 31 +++++++------ tests/unit/commands/test_borgmatic.py | 35 ++++++++++++++- 4 files changed, 87 insertions(+), 46 deletions(-) diff --git a/NEWS b/NEWS index 49a167f9..61abaf45 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,7 @@ 1.4.20 * Fix repository probing during "borgmatic init" to respect verbosity flag and remote_path option. + * #249: Update Healthchecks/Cronitor/Cronhub monitoring integrations to fire for "check" and + "prune" actions, not just "create". 1.4.19 * #259: Optionally change the internal database dump path via "borgmatic_source_directory" option diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 3b539764..f3da1618 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -53,8 +53,8 @@ def run_configuration(config_filename, config, arguments): encountered_error = None error_repository = '' - if 'create' in arguments: - try: + try: + if {'prune', 'create', 'check'}.intersection(arguments): dispatch.call_hooks( 'ping_monitor', hooks, @@ -63,6 +63,7 @@ def run_configuration(config_filename, config, arguments): monitor.State.START, global_arguments.dry_run, ) + if 'create' in arguments: command.execute_hook( hooks.get('before_backup'), hooks.get('umask'), @@ -78,11 +79,11 @@ def run_configuration(config_filename, config, arguments): location, global_arguments.dry_run, ) - except (OSError, CalledProcessError) as error: - encountered_error = error - yield from make_error_log_records( - '{}: Error running pre-backup hook'.format(config_filename), error - ) + except (OSError, CalledProcessError) as error: + encountered_error = error + yield from make_error_log_records( + '{}: Error running pre-backup hook'.format(config_filename), error + ) if not encountered_error: for repository_path in location['repositories']: @@ -105,31 +106,33 @@ def run_configuration(config_filename, config, arguments): '{}: Error running actions for repository'.format(repository_path), error ) - if 'create' in arguments and not encountered_error: + if not encountered_error: try: - dispatch.call_hooks( - 'remove_database_dumps', - hooks, - config_filename, - dump.DATABASE_HOOK_NAMES, - location, - global_arguments.dry_run, - ) - command.execute_hook( - hooks.get('after_backup'), - hooks.get('umask'), - config_filename, - 'post-backup', - global_arguments.dry_run, - ) - dispatch.call_hooks( - 'ping_monitor', - hooks, - config_filename, - monitor.MONITOR_HOOK_NAMES, - monitor.State.FINISH, - global_arguments.dry_run, - ) + if 'create' in arguments: + dispatch.call_hooks( + 'remove_database_dumps', + hooks, + config_filename, + dump.DATABASE_HOOK_NAMES, + location, + global_arguments.dry_run, + ) + command.execute_hook( + hooks.get('after_backup'), + hooks.get('umask'), + config_filename, + 'post-backup', + global_arguments.dry_run, + ) + if {'prune', 'create', 'check'}.intersection(arguments): + dispatch.call_hooks( + 'ping_monitor', + hooks, + config_filename, + monitor.MONITOR_HOOK_NAMES, + monitor.State.FINISH, + global_arguments.dry_run, + ) except (OSError, CalledProcessError) as error: encountered_error = error yield from make_error_log_records( diff --git a/docs/how-to/monitor-your-backups.md b/docs/how-to/monitor-your-backups.md index cddadb01..f2f105da 100644 --- a/docs/how-to/monitor-your-backups.md +++ b/docs/how-to/monitor-your-backups.md @@ -116,21 +116,22 @@ hooks: With this hook in place, borgmatic pings your Healthchecks project when a backup begins, ends, or errors. Specifically, before the `before_backup` -hooks run, borgmatic lets Healthchecks know that a backup has started. +hooks run, borgmatic lets Healthchecks know that it has started if any of +the `prune`, `create`, or `check` actions are run. -Then, if the backup completes successfully, borgmatic notifies Healthchecks of +Then, if the actions complete successfully, borgmatic notifies Healthchecks of the success after the `after_backup` hooks run, and includes borgmatic logs in the payload data sent to Healthchecks. This means that borgmatic logs show up in the Healthchecks UI, although be aware that Healthchecks currently has a 10-kilobyte limit for the logs in each ping. -If an error occurs during the backup, borgmatic notifies Healthchecks after +If an error occurs during any action, borgmatic notifies Healthchecks after the `on_error` hooks run, also tacking on logs including the error itself. But -the logs are only included for errors that occur within the borgmatic `create` -action (and not other actions). +the logs are only included for errors that occur when a `prune`, `create`, or +`check` action is run. Note that borgmatic sends logs to Healthchecks by applying the maximum of any -other borgmatic verbosity level (`--verbosity`, `--syslog-verbosity`, etc.), +other borgmatic verbosity levels (`--verbosity`, `--syslog-verbosity`, etc.), as there is not currently a dedicated Healthchecks verbosity setting. You can configure Healthchecks to notify you by a [variety of @@ -155,10 +156,11 @@ hooks: With this hook in place, borgmatic pings your Cronitor monitor when a backup begins, ends, or errors. Specifically, before the `before_backup` -hooks run, borgmatic lets Cronitor know that a backup has started. Then, -if the backup completes successfully, borgmatic notifies Cronitor of the -success after the `after_backup` hooks run. And if an error occurs during the -backup, borgmatic notifies Cronitor after the `on_error` hooks run. +hooks run, borgmatic lets Cronitor know that it has started if any of the +`prune`, `create`, or `check` actions are run. Then, if the actions complete +successfully, borgmatic notifies Cronitor of the success after the +`after_backup` hooks run. And if an error occurs during any action, borgmatic +notifies Cronitor after the `on_error` hooks run. You can configure Cronitor to notify you by a [variety of mechanisms](https://cronitor.io/docs/cron-job-notifications) when backups fail @@ -182,10 +184,11 @@ hooks: With this hook in place, borgmatic pings your Cronhub monitor when a backup begins, ends, or errors. Specifically, before the `before_backup` -hooks run, borgmatic lets Cronhub know that a backup has started. Then, -if the backup completes successfully, borgmatic notifies Cronhub of the -success after the `after_backup` hooks run. And if an error occurs during the -backup, borgmatic notifies Cronhub after the `on_error` hooks run. +hooks run, borgmatic lets Cronhub know that it has started if any of the +`prune`, `create`, or `check` actions are run. Then, if the actions complete +successfully, borgmatic notifies Cronhub of the success after the +`after_backup` hooks run. And if an error occurs during any action, borgmatic +notifies Cronhub after the `on_error` hooks run. Note that even though you configure borgmatic with the "start" variant of the ping URL, borgmatic substitutes the correct state into the URL when pinging diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index 1bdb3213..a2b98f37 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -20,7 +20,18 @@ def test_run_configuration_runs_actions_for_each_repository(): assert results == expected_results -def test_run_configuration_executes_hooks_for_create_action(): +def test_run_configuration_calls_hooks_for_prune_action(): + flexmock(module.borg_environment).should_receive('initialize') + flexmock(module.command).should_receive('execute_hook').never() + flexmock(module.dispatch).should_receive('call_hooks').at_least().twice() + flexmock(module).should_receive('run_actions').and_return([]) + config = {'location': {'repositories': ['foo']}} + arguments = {'global': flexmock(dry_run=False), 'prune': flexmock()} + + list(module.run_configuration('test.yaml', config, arguments)) + + +def test_run_configuration_executes_and_calls_hooks_for_create_action(): flexmock(module.borg_environment).should_receive('initialize') flexmock(module.command).should_receive('execute_hook').twice() flexmock(module.dispatch).should_receive('call_hooks').at_least().twice() @@ -31,6 +42,28 @@ def test_run_configuration_executes_hooks_for_create_action(): list(module.run_configuration('test.yaml', config, arguments)) +def test_run_configuration_calls_hooks_for_check_action(): + flexmock(module.borg_environment).should_receive('initialize') + flexmock(module.command).should_receive('execute_hook').never() + flexmock(module.dispatch).should_receive('call_hooks').at_least().twice() + flexmock(module).should_receive('run_actions').and_return([]) + config = {'location': {'repositories': ['foo']}} + arguments = {'global': flexmock(dry_run=False), 'check': flexmock()} + + list(module.run_configuration('test.yaml', config, arguments)) + + +def test_run_configuration_does_not_trigger_hooks_for_list_action(): + flexmock(module.borg_environment).should_receive('initialize') + flexmock(module.command).should_receive('execute_hook').never() + flexmock(module.dispatch).should_receive('call_hooks').never() + flexmock(module).should_receive('run_actions').and_return([]) + config = {'location': {'repositories': ['foo']}} + arguments = {'global': flexmock(dry_run=False), 'list': flexmock()} + + list(module.run_configuration('test.yaml', config, arguments)) + + def test_run_configuration_logs_actions_error(): flexmock(module.borg_environment).should_receive('initialize') flexmock(module.command).should_receive('execute_hook') From afaabd14a862764a7e8d57377deb3eb7c02799ba Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 13 Dec 2019 11:42:17 -0800 Subject: [PATCH 25/30] Clarify documentation on how /etc/borgmatic.d/ configuration files are interpreted. --- docs/how-to/make-per-application-backups.md | 5 +++++ docs/how-to/set-up-backups.md | 12 ++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/how-to/make-per-application-backups.md b/docs/how-to/make-per-application-backups.md index f962b813..840e2a6d 100644 --- a/docs/how-to/make-per-application-backups.md +++ b/docs/how-to/make-per-application-backups.md @@ -22,6 +22,11 @@ When you set up multiple configuration files like this, borgmatic will run each one in turn from a single borgmatic invocation. This includes, by default, the traditional `/etc/borgmatic/config.yaml` as well. +Each configuration file is interpreted independently, as if you ran borgmatic +for each configuration file one at a time. In other words, borgmatic does not +perform any merging of configuration files by default. If you'd like borgmatic +to merge your configuration files, see below about configuration includes. + And if you need even more customizability, you can specify alternate configuration paths on the command-line with borgmatic's `--config` option. See `borgmatic --help` for more information. diff --git a/docs/how-to/set-up-backups.md b/docs/how-to/set-up-backups.md index 2a6704dd..bc78237c 100644 --- a/docs/how-to/set-up-backups.md +++ b/docs/how-to/set-up-backups.md @@ -3,15 +3,11 @@ title: How to set up backups with borgmatic --- ## Installation -To get up and running, first [install -Borg](https://borgbackup.readthedocs.io/en/stable/installation.html), at -least version 1.1. +First, [install +Borg](https://borgbackup.readthedocs.io/en/stable/installation.html), at least +version 1.1. -By default, borgmatic looks for its configuration files in `/etc/borgmatic/` -and `/etc/borgmatic.d/`, where the root user typically has read access. - -So, to download and install borgmatic as the root user, run the following -commands: +Then, download and install borgmatic by running the following command: ```bash sudo pip3 install --user --upgrade borgmatic From f787dfe809561cb6d27005b43b9d58bd958bb818 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 17 Dec 2019 11:46:27 -0800 Subject: [PATCH 26/30] Override particular configuration options from the command-line via "--override" flag (#268). --- NEWS | 5 ++ borgmatic/commands/arguments.py | 7 ++ borgmatic/commands/borgmatic.py | 6 +- borgmatic/config/override.py | 71 ++++++++++++++++++ borgmatic/config/validate.py | 13 ++-- docs/how-to/make-per-application-backups.md | 34 +++++++++ setup.py | 2 +- tests/integration/config/test_override.py | 40 ++++++++++ tests/integration/config/test_validate.py | 27 +++++++ tests/unit/config/test_override.py | 82 +++++++++++++++++++++ 10 files changed, 278 insertions(+), 9 deletions(-) create mode 100644 borgmatic/config/override.py create mode 100644 tests/integration/config/test_override.py create mode 100644 tests/unit/config/test_override.py diff --git a/NEWS b/NEWS index 61abaf45..dd1c9539 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,8 @@ +1.4.21.dev0 + * #268: Override particular configuration options from the command-line via "--override" flag. See + the documentation for more information: + https://torsion.org/borgmatic/docs/how-to/make-per-application-backups/#configuration-overrides + 1.4.20 * Fix repository probing during "borgmatic init" to respect verbosity flag and remote_path option. * #249: Update Healthchecks/Cronitor/Cronhub monitoring integrations to fire for "check" and diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 738981ca..63030066 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -164,6 +164,13 @@ def parse_arguments(*unparsed_arguments): default=None, help='Write log messages to this file instead of syslog', ) + global_group.add_argument( + '--override', + metavar='SECTION.OPTION=VALUE', + nargs='+', + dest='overrides', + help='One or more configuration file options to override with specified values', + ) global_group.add_argument( '--version', dest='version', diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index f3da1618..ce3719d8 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -372,7 +372,7 @@ def run_actions( yield json.loads(json_output) -def load_configurations(config_filenames): +def load_configurations(config_filenames, overrides=None): ''' Given a sequence of configuration filenames, load and validate each configuration file. Return the results as a tuple of: dict of configuration filename to corresponding parsed configuration, @@ -386,7 +386,7 @@ def load_configurations(config_filenames): for config_filename in config_filenames: try: configs[config_filename] = validate.parse_configuration( - config_filename, validate.schema_filename() + config_filename, validate.schema_filename(), overrides ) except (ValueError, OSError, validate.Validation_error) as error: logs.extend( @@ -584,7 +584,7 @@ def main(): # pragma: no cover sys.exit(0) config_filenames = tuple(collect.collect_config_filenames(global_arguments.config_paths)) - configs, parse_logs = load_configurations(config_filenames) + configs, parse_logs = load_configurations(config_filenames, global_arguments.overrides) colorama.init(autoreset=True, strip=not should_do_markup(global_arguments.no_color, configs)) try: diff --git a/borgmatic/config/override.py b/borgmatic/config/override.py new file mode 100644 index 00000000..eb86077b --- /dev/null +++ b/borgmatic/config/override.py @@ -0,0 +1,71 @@ +import io + +import ruamel.yaml + + +def set_values(config, keys, value): + ''' + Given a hierarchy of configuration dicts, a sequence of parsed key strings, and a string value, + descend into the hierarchy based on the keys to set the value into the right place. + ''' + if not keys: + return + + first_key = keys[0] + if len(keys) == 1: + config[first_key] = value + return + + if first_key not in config: + config[first_key] = {} + + set_values(config[first_key], keys[1:], value) + + +def convert_value_type(value): + ''' + Given a string value, determine its logical type (string, boolean, integer, etc.), and return it + converted to that type. + ''' + return ruamel.yaml.YAML(typ='safe').load(io.StringIO(value)) + + +def parse_overrides(raw_overrides): + ''' + Given a sequence of configuration file override strings in the form of "section.option=value", + parse and return a sequence of tuples (keys, values), where keys is a sequence of strings. For + instance, given the following raw overrides: + + ['section.my_option=value1', 'section.other_option=value2'] + + ... return this: + + ( + (('section', 'my_option'), 'value1'), + (('section', 'other_option'), 'value2'), + ) + + Raise ValueError if an override can't be parsed. + ''' + if not raw_overrides: + return () + + try: + return tuple( + (tuple(raw_keys.split('.')), convert_value_type(value)) + for raw_override in raw_overrides + for raw_keys, value in (raw_override.split('=', 1),) + ) + except ValueError: + raise ValueError('Invalid override. Make sure you use the form: SECTION.OPTION=VALUE') + + +def apply_overrides(config, raw_overrides): + ''' + Given a sequence of configuration file override strings in the form of "section.option=value" + and a configuration dict, parse each override and set it the configuration dict. + ''' + overrides = parse_overrides(raw_overrides) + + for (keys, value) in overrides: + set_values(config, keys, value) diff --git a/borgmatic/config/validate.py b/borgmatic/config/validate.py index 1e421d18..b7d34a9f 100644 --- a/borgmatic/config/validate.py +++ b/borgmatic/config/validate.py @@ -6,7 +6,7 @@ import pykwalify.core import pykwalify.errors import ruamel.yaml -from borgmatic.config import load +from borgmatic.config import load, override def schema_filename(): @@ -82,11 +82,12 @@ def remove_examples(schema): return schema -def parse_configuration(config_filename, schema_filename): +def parse_configuration(config_filename, schema_filename, overrides=None): ''' - Given the path to a config filename in YAML format and the path to a schema filename in - pykwalify YAML schema format, return the parsed configuration as a data structure of nested - dicts and lists corresponding to the schema. Example return value: + Given the path to a config filename in YAML format, the path to a schema filename in pykwalify + YAML schema format, a sequence of configuration file override strings in the form of + "section.option=value", return the parsed configuration as a data structure of nested dicts and + lists corresponding to the schema. Example return value: {'location': {'source_directories': ['/home', '/etc'], 'repository': 'hostname.borg'}, 'retention': {'keep_daily': 7}, 'consistency': {'checks': ['repository', 'archives']}} @@ -102,6 +103,8 @@ def parse_configuration(config_filename, schema_filename): except (ruamel.yaml.error.YAMLError, RecursionError) as error: raise Validation_error(config_filename, (str(error),)) + override.apply_overrides(config, overrides) + validator = pykwalify.core.Core(source_data=config, schema_data=remove_examples(schema)) parsed_result = validator.validate(raise_exception=False) diff --git a/docs/how-to/make-per-application-backups.md b/docs/how-to/make-per-application-backups.md index 840e2a6d..cd257460 100644 --- a/docs/how-to/make-per-application-backups.md +++ b/docs/how-to/make-per-application-backups.md @@ -115,6 +115,40 @@ Note that this `<<` include merging syntax is only for merging in mappings directly, please see the section above about standard includes. +## Configuration overrides + +In more complex multi-application setups, you may want to override particular +borgmatic configuration file options at the time you run borgmatic. For +instance, you could reuse a common configuration file for multiple +applications, but then set the repository for each application at runtime. Or +you might want to try a variant of an option for testing purposes without +actually touching your configuration file. + +Whatever the reason, you can override borgmatic configuration options at the +command-line via the `--override` flag. Here's an example: + +```bash +borgmatic create --override location.remote_path=borg1 +``` + +What this does is load your configuration files, and for each one, disregard +the configured value for the `remote_path` option in the `location` section, +and use the value of `borg1` instead. + +Note that the value is parsed as an actual YAML string, so you can even set +list values by using brackets. For instance: + +```bash +borgmatic create --override location.repositories=[test1.borg,test2.borg] +``` + +There is not currently a way to override a single element of a list without +replacing the whole list. + +Be sure to quote your overrides if they contain spaces or other characters +that your shell may interpret. + + ## Related documentation * [Set up backups with borgmatic](https://torsion.org/borgmatic/docs/how-to/set-up-backups/) diff --git a/setup.py b/setup.py index e61d3ac8..0d87bc59 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.4.20' +VERSION = '1.4.21.dev0' setup( diff --git a/tests/integration/config/test_override.py b/tests/integration/config/test_override.py new file mode 100644 index 00000000..cfcd3394 --- /dev/null +++ b/tests/integration/config/test_override.py @@ -0,0 +1,40 @@ +import pytest + +from borgmatic.config import override as module + + +@pytest.mark.parametrize( + 'value,expected_result', + ( + ('thing', 'thing'), + ('33', 33), + ('33b', '33b'), + ('true', True), + ('false', False), + ('[foo]', ['foo']), + ('[foo, bar]', ['foo', 'bar']), + ), +) +def test_convert_value_type_coerces_values(value, expected_result): + assert module.convert_value_type(value) == expected_result + + +def test_apply_overrides_updates_config(): + raw_overrides = [ + 'section.key=value1', + 'other_section.thing=value2', + 'section.nested.key=value3', + 'new.foo=bar', + ] + config = { + 'section': {'key': 'value', 'other': 'other_value'}, + 'other_section': {'thing': 'thing_value'}, + } + + module.apply_overrides(config, raw_overrides) + + assert config == { + 'section': {'key': 'value1', 'other': 'other_value', 'nested': {'key': 'value3'}}, + 'other_section': {'thing': 'value2'}, + 'new': {'foo': 'bar'}, + } diff --git a/tests/integration/config/test_validate.py b/tests/integration/config/test_validate.py index 706743bc..cbd4f5ba 100644 --- a/tests/integration/config/test_validate.py +++ b/tests/integration/config/test_validate.py @@ -212,3 +212,30 @@ def test_parse_configuration_raises_for_validation_error(): with pytest.raises(module.Validation_error): module.parse_configuration('config.yaml', 'schema.yaml') + + +def test_parse_configuration_applies_overrides(): + mock_config_and_schema( + ''' + location: + source_directories: + - /home + + repositories: + - hostname.borg + + local_path: borg1 + ''' + ) + + result = module.parse_configuration( + 'config.yaml', 'schema.yaml', overrides=['location.local_path=borg2'] + ) + + assert result == { + 'location': { + 'source_directories': ['/home'], + 'repositories': ['hostname.borg'], + 'local_path': 'borg2', + } + } diff --git a/tests/unit/config/test_override.py b/tests/unit/config/test_override.py new file mode 100644 index 00000000..6925ca42 --- /dev/null +++ b/tests/unit/config/test_override.py @@ -0,0 +1,82 @@ +import pytest +from flexmock import flexmock + +from borgmatic.config import override as module + + +def test_set_values_with_empty_keys_bails(): + config = {} + + module.set_values(config, keys=(), value='value') + + assert config == {} + + +def test_set_values_with_one_key_sets_it_into_config(): + config = {} + + module.set_values(config, keys=('key',), value='value') + + assert config == {'key': 'value'} + + +def test_set_values_with_one_key_overwrites_existing_key(): + config = {'key': 'old_value', 'other': 'other_value'} + + module.set_values(config, keys=('key',), value='value') + + assert config == {'key': 'value', 'other': 'other_value'} + + +def test_set_values_with_multiple_keys_creates_hierarchy(): + config = {} + + module.set_values(config, ('section', 'key'), 'value') + + assert config == {'section': {'key': 'value'}} + + +def test_set_values_with_multiple_keys_updates_hierarchy(): + config = {'section': {'other': 'other_value'}} + module.set_values(config, ('section', 'key'), 'value') + + assert config == {'section': {'key': 'value', 'other': 'other_value'}} + + +def test_parse_overrides_splits_keys_and_values(): + flexmock(module).should_receive('convert_value_type').replace_with(lambda value: value) + raw_overrides = ['section.my_option=value1', 'section.other_option=value2'] + expected_result = ( + (('section', 'my_option'), 'value1'), + (('section', 'other_option'), 'value2'), + ) + + module.parse_overrides(raw_overrides) == expected_result + + +def test_parse_overrides_allows_value_with_equal_sign(): + flexmock(module).should_receive('convert_value_type').replace_with(lambda value: value) + raw_overrides = ['section.option=this===value'] + expected_result = ((('section', 'option'), 'this===value'),) + + module.parse_overrides(raw_overrides) == expected_result + + +def test_parse_overrides_raises_on_missing_equal_sign(): + flexmock(module).should_receive('convert_value_type').replace_with(lambda value: value) + raw_overrides = ['section.option'] + + with pytest.raises(ValueError): + module.parse_overrides(raw_overrides) + + +def test_parse_overrides_allows_value_with_single_key(): + flexmock(module).should_receive('convert_value_type').replace_with(lambda value: value) + raw_overrides = ['option=value'] + expected_result = ((('option',), 'value'),) + + module.parse_overrides(raw_overrides) == expected_result + + +def test_parse_overrides_handles_empty_overrides(): + module.parse_overrides(raw_overrides=None) == () From ed2ca9f47691702e7c8348cb009af907999e7308 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 17 Dec 2019 20:06:25 -0800 Subject: [PATCH 27/30] Sign release files. --- scripts/release | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/release b/scripts/release index 0477b6ba..afc99c30 100755 --- a/scripts/release +++ b/scripts/release @@ -23,10 +23,11 @@ git push github $version rm -fr dist python3 setup.py bdist_wheel python3 setup.py sdist +gpg --detach-sign --armor dist/* twine upload -r pypi dist/borgmatic-*.tar.gz twine upload -r pypi dist/borgmatic-*-py3-none-any.whl -# Set release changelogs on projects.evoworx.org and GitHub. +# Set release changelogs on projects.torsion.org and GitHub. release_changelog="$(cat NEWS | sed '/^$/q' | grep -v '^\S')" escaped_release_changelog="$(echo "$release_changelog" | sed -z 's/\n/\\n/g' | sed -z 's/\"/\\"/g')" curl --silent --request POST \ From d64bcd5e83990e9682815ba8f4abad73c4ca564f Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 17 Dec 2019 20:12:41 -0800 Subject: [PATCH 28/30] When pruning with verbosity level 1, list pruned and kept archives. --- NEWS | 2 ++ borgmatic/borg/prune.py | 2 +- tests/unit/borg/test_prune.py | 4 +++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index dd1c9539..f27b9b1e 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ * #268: Override particular configuration options from the command-line via "--override" flag. See the documentation for more information: https://torsion.org/borgmatic/docs/how-to/make-per-application-backups/#configuration-overrides + * When pruning with verbosity level 1, list pruned and kept archives. Previously, this information + was only shown at verbosity level 2. 1.4.20 * Fix repository probing during "borgmatic init" to respect verbosity flag and remote_path option. diff --git a/borgmatic/borg/prune.py b/borgmatic/borg/prune.py index 0913cdeb..2c4811eb 100644 --- a/borgmatic/borg/prune.py +++ b/borgmatic/borg/prune.py @@ -58,7 +58,7 @@ def prune_archives( + (('--umask', str(umask)) if umask else ()) + (('--lock-wait', str(lock_wait)) if lock_wait else ()) + (('--stats',) if not dry_run and logger.isEnabledFor(logging.INFO) else ()) - + (('--info',) if logger.getEffectiveLevel() == logging.INFO else ()) + + (('--info', '--list') if logger.getEffectiveLevel() == logging.INFO else ()) + (('--debug', '--list', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ()) + (('--dry-run',) if dry_run else ()) + (('--stats',) if stats else ()) diff --git a/tests/unit/borg/test_prune.py b/tests/unit/borg/test_prune.py index 80cce836..b2b4785a 100644 --- a/tests/unit/borg/test_prune.py +++ b/tests/unit/borg/test_prune.py @@ -75,7 +75,9 @@ def test_prune_archives_with_log_info_calls_borg_with_info_parameter(): flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return( BASE_PRUNE_FLAGS ) - insert_execute_command_mock(PRUNE_COMMAND + ('--stats', '--info', 'repo'), logging.INFO) + insert_execute_command_mock( + PRUNE_COMMAND + ('--stats', '--info', '--list', 'repo'), logging.INFO + ) insert_logging_mock(logging.INFO) module.prune_archives( From 6bfa0783b933cbb8282c219f84355c6b9c2285b0 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 17 Dec 2019 20:16:13 -0800 Subject: [PATCH 29/30] Clarify that the documentation suggestion form is only for documentation. --- docs/_includes/components/suggestion-form.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/_includes/components/suggestion-form.html b/docs/_includes/components/suggestion-form.html index c4e59b20..8e3a73a6 100644 --- a/docs/_includes/components/suggestion-form.html +++ b/docs/_includes/components/suggestion-form.html @@ -1,12 +1,12 @@

Improve this documentation

Have an idea on how to make this documentation even better? Send your -feedback below! (But if you need help installing or using borgmatic, please -use our issue tracker -instead.)

+feedback below! But if you need help with borgmatic, or have an idea for a +borgmatic feature, please use our issue +tracker instead.

-
+
From 911668f0c8737ba7b3c955aa57f718a206a4f331 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 20 Dec 2019 13:58:02 -0800 Subject: [PATCH 30/30] Only trigger "on_error" hooks and monitoring failures for "prune", "create", and "check" actions, and not for other actions (#270). --- NEWS | 2 ++ borgmatic/commands/borgmatic.py | 5 +++-- docs/how-to/monitor-your-backups.md | 17 +++++++++-------- tests/unit/commands/test_borgmatic.py | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index f27b9b1e..275dcf3b 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ * #268: Override particular configuration options from the command-line via "--override" flag. See the documentation for more information: https://torsion.org/borgmatic/docs/how-to/make-per-application-backups/#configuration-overrides + * #270: Only trigger "on_error" hooks and monitoring failures for "prune", "create", and "check" + actions, and not for other actions. * When pruning with verbosity level 1, list pruned and kept archives. Previously, this information was only shown at verbosity level 2. diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index ce3719d8..b39668ad 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -52,9 +52,10 @@ def run_configuration(config_filename, config, arguments): borg_environment.initialize(storage) encountered_error = None error_repository = '' + prune_create_or_check = {'prune', 'create', 'check'}.intersection(arguments) try: - if {'prune', 'create', 'check'}.intersection(arguments): + if prune_create_or_check: dispatch.call_hooks( 'ping_monitor', hooks, @@ -139,7 +140,7 @@ def run_configuration(config_filename, config, arguments): '{}: Error running post-backup hook'.format(config_filename), error ) - if encountered_error: + if encountered_error and prune_create_or_check: try: command.execute_hook( hooks.get('on_error'), diff --git a/docs/how-to/monitor-your-backups.md b/docs/how-to/monitor-your-backups.md index f2f105da..6f6f1505 100644 --- a/docs/how-to/monitor-your-backups.md +++ b/docs/how-to/monitor-your-backups.md @@ -57,10 +57,10 @@ tests](https://torsion.org/borgmatic/docs/how-to/extract-a-backup/). ## Error hooks -When an error occurs during a backup or another action, 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: +When an error occurs during a `prune`, `create`, or `check` action, 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: @@ -91,10 +91,11 @@ here: * `output`: output of the command that failed (may be blank if an error occurred without running a command) -Note that borgmatic runs the `on_error` hooks for any action in which an error -occurs, not just the `create` action. But 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 +Note that borgmatic runs the `on_error` hooks only for `prune`, `create`, or +`check` actions or hooks in which an error occurs, and not other actions. +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/), especially the security information. diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index a2b98f37..ccc63754 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -119,7 +119,7 @@ def test_run_configuration_logs_on_error_hook_error(): ).and_return(expected_results[1:]) flexmock(module).should_receive('run_actions').and_raise(OSError) config = {'location': {'repositories': ['foo']}} - arguments = {'global': flexmock(dry_run=False)} + arguments = {'global': flexmock(dry_run=False), 'create': flexmock()} results = list(module.run_configuration('test.yaml', config, arguments))