From 1d1df99ee8d0bc4ec587fd2bfd412f4bb0edd200 Mon Sep 17 00:00:00 2001 From: Andrew Burkett Date: Tue, 26 Nov 2019 21:22:27 -0800 Subject: [PATCH 1/8] Add locking of borgmatic config file This prevents multiple instances of borgmatic from running against the same config file. This is particularly important when there are pre-backup scripts with side effects --- borgmatic/commands/borgmatic.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 4cd63bb4..94d709f2 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -566,9 +566,25 @@ def main(): # pragma: no cover logger.debug('Ensuring legacy configuration is upgraded') convert.guard_configuration_upgraded(LEGACY_CONFIG_PATH, config_filenames) + locks = [] + for config_filename,config in configs.items(): + lock_config = config.get("location",dict()).get("lock_config",False) + if lock_config: + f = open(config_filename) + try: + fcntl.flock(f.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + except IOError: + logger.critical("Failed to acquire lock for {}".format(config_filename)) + sys.exit(0)) + locks.append(f) + summary_logs = parse_logs + list(collect_configuration_run_summary_logs(configs, arguments)) summary_logs_max_level = max(log.levelno for log in summary_logs) + # this removes the reference to the open files which python will garbage collect the file objects + # and close them + locks = None + for message in ('', 'summary:'): log_record( levelno=summary_logs_max_level, -- 2.40.1 From 8d24b0a5864d02fa30013b171b21d32fc5a28228 Mon Sep 17 00:00:00 2001 From: Andrew Burkett Date: Tue, 26 Nov 2019 21:31:04 -0800 Subject: [PATCH 2/8] Fix a couple bugs - Remove extra closed parenthesis - Forgot to import fcntl - Add option to config schema --- borgmatic/commands/borgmatic.py | 3 ++- borgmatic/config/schema.yaml | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 94d709f2..4b68826e 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -3,6 +3,7 @@ import json import logging import os import sys +import fcntl from subprocess import CalledProcessError import colorama @@ -575,7 +576,7 @@ def main(): # pragma: no cover fcntl.flock(f.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) except IOError: logger.critical("Failed to acquire lock for {}".format(config_filename)) - sys.exit(0)) + sys.exit(0) locks.append(f) summary_logs = parse_logs + list(collect_configuration_run_summary_logs(configs, arguments)) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 8b3667f4..f8aa33bf 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -32,6 +32,10 @@ map: type: bool desc: Stay in same file system (do not cross mount points). Defaults to false. example: true + lock_config: + type: bool + desc: Lock config when running borgmatic to prevent multiple instances from running simultaneously + example: true numeric_owner: type: bool desc: Only store/extract numeric user and group identifiers. Defaults to false. -- 2.40.1 From 4fb6a49b3344d3e29d42b19fd3e217d49cacc1d4 Mon Sep 17 00:00:00 2001 From: Andrew Burkett Date: Tue, 3 Dec 2019 11:25:13 -0800 Subject: [PATCH 3/8] Changed lock_config to lock_client. Moved locking to run_configuration --- borgmatic/commands/borgmatic.py | 24 ++++++++---------------- borgmatic/config/schema.yaml | 2 +- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 4b68826e..18be8069 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -48,6 +48,14 @@ def run_configuration(config_filename, config, arguments): ) global_arguments = arguments['global'] + if location.get("lock_client",False): + lock_f = open(config_filename) + try: + fcntl.flock(lock_f.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + except IOError: + logger.critical("Failed to acquire lock for {}".format(config_filename)) + sys.exit(1) + local_path = location.get('local_path', 'borg') remote_path = location.get('remote_path') borg_environment.initialize(storage) @@ -567,25 +575,9 @@ def main(): # pragma: no cover logger.debug('Ensuring legacy configuration is upgraded') convert.guard_configuration_upgraded(LEGACY_CONFIG_PATH, config_filenames) - locks = [] - for config_filename,config in configs.items(): - lock_config = config.get("location",dict()).get("lock_config",False) - if lock_config: - f = open(config_filename) - try: - fcntl.flock(f.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) - except IOError: - logger.critical("Failed to acquire lock for {}".format(config_filename)) - sys.exit(0) - locks.append(f) - summary_logs = parse_logs + list(collect_configuration_run_summary_logs(configs, arguments)) summary_logs_max_level = max(log.levelno for log in summary_logs) - # this removes the reference to the open files which python will garbage collect the file objects - # and close them - locks = None - for message in ('', 'summary:'): log_record( levelno=summary_logs_max_level, diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index f8aa33bf..d8c663b2 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -32,7 +32,7 @@ map: type: bool desc: Stay in same file system (do not cross mount points). Defaults to false. example: true - lock_config: + lock_client: type: bool desc: Lock config when running borgmatic to prevent multiple instances from running simultaneously example: true -- 2.40.1 From 877b1c440e1098f82b3fe5e9603e3d6abb81e85e Mon Sep 17 00:00:00 2001 From: Andrew Burkett Date: Wed, 4 Dec 2019 07:52:51 -0800 Subject: [PATCH 4/8] Update error handling for lock_client --- borgmatic/commands/borgmatic.py | 70 +++++++++++++++++---------------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 18be8069..cc21fc11 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -42,56 +42,60 @@ def run_configuration(config_filename, config, arguments): * JSON output strings from successfully executing any actions that produce JSON * logging.LogRecord instances containing errors from any actions or backup hooks that fail ''' + (location, storage, retention, consistency, hooks) = ( config.get(section_name, {}) for section_name in ('location', 'storage', 'retention', 'consistency', 'hooks') ) global_arguments = arguments['global'] - if location.get("lock_client",False): - lock_f = open(config_filename) - try: - fcntl.flock(lock_f.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) - except IOError: - logger.critical("Failed to acquire lock for {}".format(config_filename)) - sys.exit(1) - local_path = location.get('local_path', 'borg') remote_path = location.get('remote_path') borg_environment.initialize(storage) encountered_error = None error_repository = '' - if 'create' in arguments: + if location.get("lock_client",False): + lock_f = open(config_filename) try: - dispatch.call_hooks( - 'ping_monitor', - hooks, - config_filename, - monitor.MONITOR_HOOK_NAMES, - monitor.State.START, - global_arguments.dry_run, - ) - command.execute_hook( - hooks.get('before_backup'), - hooks.get('umask'), - config_filename, - 'pre-backup', - global_arguments.dry_run, - ) - dispatch.call_hooks( - 'dump_databases', - hooks, - config_filename, - dump.DATABASE_HOOK_NAMES, - global_arguments.dry_run, - ) - except (OSError, CalledProcessError) as error: + fcntl.flock(lock_f.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + except IOError as error: encountered_error = error yield from make_error_log_records( - '{}: Error running pre-backup hook'.format(config_filename), error + '{}: Failed to acquire lock'.format(config_filename), error ) + if not encountered_error: + if 'create' in arguments: + try: + dispatch.call_hooks( + 'ping_monitor', + hooks, + config_filename, + monitor.MONITOR_HOOK_NAMES, + monitor.State.START, + global_arguments.dry_run, + ) + command.execute_hook( + hooks.get('before_backup'), + hooks.get('umask'), + config_filename, + 'pre-backup', + global_arguments.dry_run, + ) + dispatch.call_hooks( + 'dump_databases', + hooks, + config_filename, + dump.DATABASE_HOOK_NAMES, + 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 + ) + if not encountered_error: for repository_path in location['repositories']: try: -- 2.40.1 From f8dec72e9849bfaa459d4ae5b5d69433f04e5db4 Mon Sep 17 00:00:00 2001 From: Andrew Burkett Date: Tue, 24 Dec 2019 11:45:23 -0800 Subject: [PATCH 5/8] Formatting --- borgmatic/commands/borgmatic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index cc21fc11..105b4c2b 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -1,9 +1,9 @@ import collections +import fcntl import json import logging import os import sys -import fcntl from subprocess import CalledProcessError import colorama @@ -55,7 +55,7 @@ def run_configuration(config_filename, config, arguments): encountered_error = None error_repository = '' - if location.get("lock_client",False): + if location.get("lock_client", False): lock_f = open(config_filename) try: fcntl.flock(lock_f.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) -- 2.40.1 From 398c1294111c84f2fed8042c28671a0a8c1e9ab2 Mon Sep 17 00:00:00 2001 From: Andrew Burkett Date: Tue, 24 Dec 2019 11:50:26 -0800 Subject: [PATCH 6/8] Update schema text and merge conditionals --- borgmatic/commands/borgmatic.py | 57 ++++++++++++++++----------------- borgmatic/config/schema.yaml | 2 +- 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 105b4c2b..d20d1c0d 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -65,35 +65,34 @@ def run_configuration(config_filename, config, arguments): '{}: Failed to acquire lock'.format(config_filename), error ) - if not encountered_error: - if 'create' in arguments: - try: - dispatch.call_hooks( - 'ping_monitor', - hooks, - config_filename, - monitor.MONITOR_HOOK_NAMES, - monitor.State.START, - global_arguments.dry_run, - ) - command.execute_hook( - hooks.get('before_backup'), - hooks.get('umask'), - config_filename, - 'pre-backup', - global_arguments.dry_run, - ) - dispatch.call_hooks( - 'dump_databases', - hooks, - config_filename, - dump.DATABASE_HOOK_NAMES, - 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 + if not encountered_error and 'create' in arguments: + try: + dispatch.call_hooks( + 'ping_monitor', + hooks, + config_filename, + monitor.MONITOR_HOOK_NAMES, + monitor.State.START, + global_arguments.dry_run, + ) + command.execute_hook( + hooks.get('before_backup'), + hooks.get('umask'), + config_filename, + 'pre-backup', + global_arguments.dry_run, + ) + dispatch.call_hooks( + 'dump_databases', + hooks, + config_filename, + dump.DATABASE_HOOK_NAMES, + 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 ) if not encountered_error: diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index d8c663b2..6212a901 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -34,7 +34,7 @@ map: example: true lock_client: type: bool - desc: Lock config when running borgmatic to prevent multiple instances from running simultaneously + desc: Lock config when running borgmatic to prevent multiple instances from running simultaneously. Defaults to false. example: true numeric_owner: type: bool -- 2.40.1 From 5ea7e1eaa6a490f5a21f4570a4eac021183e04fd Mon Sep 17 00:00:00 2001 From: Andrew Burkett Date: Tue, 24 Dec 2019 11:51:25 -0800 Subject: [PATCH 7/8] Formatting --- borgmatic/commands/borgmatic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index d20d1c0d..e48f5c60 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -93,7 +93,7 @@ def run_configuration(config_filename, config, arguments): 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']: -- 2.40.1 From 16c378507d5a47e33b17c6ad050953f13ed00aa4 Mon Sep 17 00:00:00 2001 From: Andrew Burkett Date: Tue, 24 Dec 2019 11:52:39 -0800 Subject: [PATCH 8/8] Formatting --- borgmatic/commands/borgmatic.py | 1 - 1 file changed, 1 deletion(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index e48f5c60..9ee20a53 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -42,7 +42,6 @@ def run_configuration(config_filename, config, arguments): * JSON output strings from successfully executing any actions that produce JSON * logging.LogRecord instances containing errors from any actions or backup hooks that fail ''' - (location, storage, retention, consistency, hooks) = ( config.get(section_name, {}) for section_name in ('location', 'storage', 'retention', 'consistency', 'hooks') -- 2.40.1