From 7cca83b698ae105247e8f1b3726db6d175a7acb4 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 17 Nov 2024 10:15:58 -0800 Subject: [PATCH] Log the path of the borgmatic runtime directory used (#934). --- borgmatic/actions/check.py | 34 +++++++++++++++------------ borgmatic/actions/config/bootstrap.py | 3 ++- borgmatic/actions/create.py | 8 +++++-- borgmatic/actions/restore.py | 9 +++---- borgmatic/config/paths.py | 15 +++++++----- tests/unit/actions/test_check.py | 14 +++++------ tests/unit/config/test_paths.py | 18 +++++++------- 7 files changed, 57 insertions(+), 44 deletions(-) diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index 48cd0e88..928ac481 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -451,7 +451,7 @@ def compare_spot_check_hashes( global_arguments, local_path, remote_path, - log_label, + log_prefix, source_paths, ): ''' @@ -475,7 +475,7 @@ def compare_spot_check_hashes( if os.path.exists(os.path.join(working_directory or '', source_path)) } logger.debug( - f'{log_label}: Sampling {sample_count} source paths (~{spot_check_config["data_sample_percentage"]}%) for spot check' + f'{log_prefix}: Sampling {sample_count} source paths (~{spot_check_config["data_sample_percentage"]}%) for spot check' ) source_sample_paths_iterator = iter(source_sample_paths) @@ -563,8 +563,8 @@ def spot_check( disk to those stored in the latest archive. If any differences are beyond configured tolerances, then the check fails. ''' - log_label = f'{repository.get("label", repository["path"])}' - logger.debug(f'{log_label}: Running spot check') + log_prefix = f'{repository.get("label", repository["path"])}' + logger.debug(f'{log_prefix}: Running spot check') try: spot_check_config = next( @@ -586,7 +586,7 @@ def spot_check( local_path, remote_path, ) - logger.debug(f'{log_label}: {len(source_paths)} total source paths for spot check') + logger.debug(f'{log_prefix}: {len(source_paths)} total source paths for spot check') archive = borgmatic.borg.repo_list.resolve_archive_name( repository['path'], @@ -597,7 +597,7 @@ def spot_check( local_path, remote_path, ) - logger.debug(f'{log_label}: Using archive {archive} for spot check') + logger.debug(f'{log_prefix}: Using archive {archive} for spot check') archive_paths = collect_spot_check_archive_paths( repository, @@ -609,7 +609,7 @@ def spot_check( remote_path, borgmatic_runtime_directory, ) - logger.debug(f'{log_label}: {len(archive_paths)} total archive paths for spot check') + logger.debug(f'{log_prefix}: {len(archive_paths)} total archive paths for spot check') # Calculate the percentage delta between the source paths count and the archive paths count, and # compare that delta to the configured count tolerance percentage. @@ -617,10 +617,10 @@ def spot_check( if count_delta_percentage > spot_check_config['count_tolerance_percentage']: logger.debug( - f'{log_label}: Paths in source paths but not latest archive: {", ".join(set(source_paths) - set(archive_paths)) or "none"}' + f'{log_prefix}: Paths in source paths but not latest archive: {", ".join(set(source_paths) - set(archive_paths)) or "none"}' ) logger.debug( - f'{log_label}: Paths in latest archive but not source paths: {", ".join(set(archive_paths) - set(source_paths)) or "none"}' + f'{log_prefix}: Paths in latest archive but not source paths: {", ".join(set(archive_paths) - set(source_paths)) or "none"}' ) raise ValueError( f'Spot check failed: {count_delta_percentage:.2f}% file count delta between source paths and latest archive (tolerance is {spot_check_config["count_tolerance_percentage"]}%)' @@ -634,25 +634,25 @@ def spot_check( global_arguments, local_path, remote_path, - log_label, + log_prefix, source_paths, ) # Error if the percentage of failing hashes exceeds the configured tolerance percentage. - logger.debug(f'{log_label}: {len(failing_paths)} non-matching spot check hashes') + logger.debug(f'{log_prefix}: {len(failing_paths)} non-matching spot check hashes') data_tolerance_percentage = spot_check_config['data_tolerance_percentage'] failing_percentage = (len(failing_paths) / len(source_paths)) * 100 if failing_percentage > data_tolerance_percentage: logger.debug( - f'{log_label}: Source paths with data not matching the latest archive: {", ".join(failing_paths)}' + f'{log_prefix}: Source paths with data not matching the latest archive: {", ".join(failing_paths)}' ) raise ValueError( f'Spot check failed: {failing_percentage:.2f}% of source paths with data not matching the latest archive (tolerance is {data_tolerance_percentage}%)' ) logger.info( - f'{log_label}: Spot check passed with a {count_delta_percentage:.2f}% file count delta and a {failing_percentage:.2f}% file data delta' + f'{log_prefix}: Spot check passed with a {count_delta_percentage:.2f}% file count delta and a {failing_percentage:.2f}% file data delta' ) @@ -686,7 +686,9 @@ def run_check( **hook_context, ) - logger.info(f'{repository.get("label", repository["path"])}: Running consistency checks') + log_prefix = repository.get('label', repository['path']) + logger.info(f'{log_prefix}: Running consistency checks') + repository_id = borgmatic.borg.check.get_repository_id( repository['path'], config, @@ -738,7 +740,9 @@ def run_check( write_check_time(make_check_time_path(config, repository_id, 'extract')) if 'spot' in checks: - with borgmatic.config.paths.Runtime_directory(config) as borgmatic_runtime_directory: + with borgmatic.config.paths.Runtime_directory( + config, log_prefix + ) as borgmatic_runtime_directory: spot_check( repository, config, diff --git a/borgmatic/actions/config/bootstrap.py b/borgmatic/actions/config/bootstrap.py index 87095bcb..d49ccd1f 100644 --- a/borgmatic/actions/config/bootstrap.py +++ b/borgmatic/actions/config/bootstrap.py @@ -44,7 +44,8 @@ def get_config_paths(archive_name, bootstrap_arguments, global_arguments, local_ # borgmatic runtime directory (which get stored as just "/borgmatic" with Borg 1.4+). But we # still want to support reading the manifest from previously created archives as well. with borgmatic.config.paths.Runtime_directory( - {'user_runtime_directory': bootstrap_arguments.user_runtime_directory} + {'user_runtime_directory': bootstrap_arguments.user_runtime_directory}, + bootstrap_arguments.repository, ) as borgmatic_runtime_directory: for base_directory in ( 'borgmatic', diff --git a/borgmatic/actions/create.py b/borgmatic/actions/create.py index 51168650..03a5ca52 100644 --- a/borgmatic/actions/create.py +++ b/borgmatic/actions/create.py @@ -71,9 +71,13 @@ def run_create( global_arguments.dry_run, **hook_context, ) - logger.info(f'{repository.get("label", repository["path"])}: Creating archive{dry_run_label}') - with borgmatic.config.paths.Runtime_directory(config) as borgmatic_runtime_directory: + log_prefix = repository.get('label', repository['path']) + logger.info(f'{log_prefix}: Creating archive{dry_run_label}') + + with borgmatic.config.paths.Runtime_directory( + config, log_prefix + ) as borgmatic_runtime_directory: borgmatic.hooks.dispatch.call_hooks_even_if_unconfigured( 'remove_data_source_dumps', config, diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index b961a66d..7856e12b 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -344,11 +344,12 @@ def run_restore( ): return - logger.info( - f'{repository.get("label", repository["path"])}: Restoring data sources from archive {restore_arguments.archive}' - ) + log_prefix = repository.get('label', repository['path']) + logger.info(f'{log_prefix}: Restoring data sources from archive {restore_arguments.archive}') - with borgmatic.config.paths.Runtime_directory(config) as borgmatic_runtime_directory: + with borgmatic.config.paths.Runtime_directory( + config, log_prefix + ) as borgmatic_runtime_directory: borgmatic.hooks.dispatch.call_hooks_even_if_unconfigured( 'remove_data_source_dumps', config, diff --git a/borgmatic/config/paths.py b/borgmatic/config/paths.py index 54eb0921..9f5dd59d 100644 --- a/borgmatic/config/paths.py +++ b/borgmatic/config/paths.py @@ -44,13 +44,14 @@ class Runtime_directory: automatically gets cleaned up as necessary. ''' - def __init__(self, config): + def __init__(self, config, log_prefix): ''' - Given a configuration dict, determine the borgmatic runtime directory, creating a secure, - temporary directory within it if necessary. Defaults to $XDG_RUNTIME_DIR/./borgmatic or - $RUNTIME_DIRECTORY/./borgmatic or $TMPDIR/borgmatic-[random]/./borgmatic or - $TEMP/borgmatic-[random]/./borgmatic or /tmp/borgmatic-[random]/./borgmatic where "[random]" - is a randomly generated string intended to avoid path collisions. + Given a configuration dict and a log prefix, determine the borgmatic runtime directory, + creating a secure, temporary directory within it if necessary. Defaults to + $XDG_RUNTIME_DIR/./borgmatic or $RUNTIME_DIRECTORY/./borgmatic or + $TMPDIR/borgmatic-[random]/./borgmatic or $TEMP/borgmatic-[random]/./borgmatic or + /tmp/borgmatic-[random]/./borgmatic where "[random]" is a randomly generated string intended + to avoid path collisions. If XDG_RUNTIME_DIR or RUNTIME_DIRECTORY is set and already ends in "/borgmatic", then don't tack on a second "/borgmatic" path component. @@ -83,6 +84,8 @@ class Runtime_directory: ) os.makedirs(self.runtime_path, mode=0o700, exist_ok=True) + logger.debug(f'{log_prefix}: Using runtime directory {os.path.normpath(self.runtime_path)}') + def __enter__(self): ''' Return the borgmatic runtime path as a string. diff --git a/tests/unit/actions/test_check.py b/tests/unit/actions/test_check.py index 917d8a4a..e7299522 100644 --- a/tests/unit/actions/test_check.py +++ b/tests/unit/actions/test_check.py @@ -879,7 +879,7 @@ def test_compare_spot_check_hashes_returns_paths_having_failing_hashes(): global_arguments=flexmock(), local_path=flexmock(), remote_path=flexmock(), - log_label='repo', + log_prefix='repo', source_paths=('/foo', '/bar', '/baz', '/quux'), ) == ('/bar',) @@ -920,7 +920,7 @@ def test_compare_spot_check_hashes_handles_data_sample_percentage_above_100(): global_arguments=flexmock(), local_path=flexmock(), remote_path=flexmock(), - log_label='repo', + log_prefix='repo', source_paths=('/foo', '/bar'), ) == ('/foo', '/bar') @@ -958,7 +958,7 @@ def test_compare_spot_check_hashes_uses_xxh64sum_command_option(): global_arguments=flexmock(), local_path=flexmock(), remote_path=flexmock(), - log_label='repo', + log_prefix='repo', source_paths=('/foo', '/bar', '/baz', '/quux'), ) == ('/bar',) @@ -995,7 +995,7 @@ def test_compare_spot_check_hashes_considers_path_missing_from_archive_as_not_ma global_arguments=flexmock(), local_path=flexmock(), remote_path=flexmock(), - log_label='repo', + log_prefix='repo', source_paths=('/foo', '/bar', '/baz', '/quux'), ) == ('/bar',) @@ -1031,7 +1031,7 @@ def test_compare_spot_check_hashes_considers_non_existent_path_as_not_matching() global_arguments=flexmock(), local_path=flexmock(), remote_path=flexmock(), - log_label='repo', + log_prefix='repo', source_paths=('/foo', '/bar', '/baz', '/quux'), ) == ('/bar',) @@ -1078,7 +1078,7 @@ def test_compare_spot_check_hashes_with_too_many_paths_feeds_them_to_commands_in global_arguments=flexmock(), local_path=flexmock(), remote_path=flexmock(), - log_label='repo', + log_prefix='repo', source_paths=('/foo', '/bar', '/baz', '/quux'), ) == ('/quux',) @@ -1121,7 +1121,7 @@ def test_compare_spot_check_hashes_uses_working_directory_to_access_source_paths global_arguments=flexmock(), local_path=flexmock(), remote_path=flexmock(), - log_label='repo', + log_prefix='repo', source_paths=('foo', 'bar', 'baz', 'quux'), ) == ('bar',) diff --git a/tests/unit/config/test_paths.py b/tests/unit/config/test_paths.py index 33288f55..d121f952 100644 --- a/tests/unit/config/test_paths.py +++ b/tests/unit/config/test_paths.py @@ -38,7 +38,7 @@ def test_runtime_directory_uses_config_option(): flexmock(module.os).should_receive('makedirs') config = {'user_runtime_directory': '/run', 'borgmatic_source_directory': '/nope'} - with module.Runtime_directory(config) as borgmatic_runtime_directory: + with module.Runtime_directory(config, 'prefix') as borgmatic_runtime_directory: assert borgmatic_runtime_directory == '/run/./borgmatic' @@ -47,7 +47,7 @@ def test_runtime_directory_uses_config_option_without_adding_duplicate_borgmatic flexmock(module.os).should_receive('makedirs') config = {'user_runtime_directory': '/run/borgmatic', 'borgmatic_source_directory': '/nope'} - with module.Runtime_directory(config) as borgmatic_runtime_directory: + with module.Runtime_directory(config, 'prefix') as borgmatic_runtime_directory: assert borgmatic_runtime_directory == '/run/./borgmatic' @@ -58,7 +58,7 @@ def test_runtime_directory_falls_back_to_xdg_runtime_dir(): ) flexmock(module.os).should_receive('makedirs') - with module.Runtime_directory({}) as borgmatic_runtime_directory: + with module.Runtime_directory({}, 'prefix') as borgmatic_runtime_directory: assert borgmatic_runtime_directory == '/run/./borgmatic' @@ -69,7 +69,7 @@ def test_runtime_directory_falls_back_to_xdg_runtime_dir_without_adding_duplicat ) flexmock(module.os).should_receive('makedirs') - with module.Runtime_directory({}) as borgmatic_runtime_directory: + with module.Runtime_directory({}, 'prefix') as borgmatic_runtime_directory: assert borgmatic_runtime_directory == '/run/./borgmatic' @@ -81,7 +81,7 @@ def test_runtime_directory_falls_back_to_runtime_directory(): ) flexmock(module.os).should_receive('makedirs') - with module.Runtime_directory({}) as borgmatic_runtime_directory: + with module.Runtime_directory({}, 'prefix') as borgmatic_runtime_directory: assert borgmatic_runtime_directory == '/run/./borgmatic' @@ -93,7 +93,7 @@ def test_runtime_directory_falls_back_to_runtime_directory_without_adding_duplic ) flexmock(module.os).should_receive('makedirs') - with module.Runtime_directory({}) as borgmatic_runtime_directory: + with module.Runtime_directory({}, 'prefix') as borgmatic_runtime_directory: assert borgmatic_runtime_directory == '/run/./borgmatic' @@ -111,7 +111,7 @@ def test_runtime_directory_falls_back_to_tmpdir_and_adds_temporary_subdirectory_ ).and_return(temporary_directory) flexmock(module.os).should_receive('makedirs') - with module.Runtime_directory({}) as borgmatic_runtime_directory: + with module.Runtime_directory({}, 'prefix') as borgmatic_runtime_directory: assert borgmatic_runtime_directory == '/run/borgmatic-1234/./borgmatic' @@ -130,7 +130,7 @@ def test_runtime_directory_falls_back_to_temp_and_adds_temporary_subdirectory_th ).and_return(temporary_directory) flexmock(module.os).should_receive('makedirs') - with module.Runtime_directory({}) as borgmatic_runtime_directory: + with module.Runtime_directory({}, 'prefix') as borgmatic_runtime_directory: assert borgmatic_runtime_directory == '/run/borgmatic-1234/./borgmatic' @@ -149,7 +149,7 @@ def test_runtime_directory_falls_back_to_hard_coded_tmp_path_and_adds_temporary_ ).and_return(temporary_directory) flexmock(module.os).should_receive('makedirs') - with module.Runtime_directory({}) as borgmatic_runtime_directory: + with module.Runtime_directory({}, 'prefix') as borgmatic_runtime_directory: assert borgmatic_runtime_directory == '/tmp/borgmatic-1234/./borgmatic'