From a65d5a53c73d05c8a5a89efbfaebae9fd9a11703 Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Wed, 29 Nov 2023 21:42:25 +0100 Subject: [PATCH 1/9] Fix '/' cannot be backed up because we remove trailing slashes --- npbackup/restic_wrapper/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/npbackup/restic_wrapper/__init__.py b/npbackup/restic_wrapper/__init__.py index 0372bd8..645f4ba 100644 --- a/npbackup/restic_wrapper/__init__.py +++ b/npbackup/restic_wrapper/__init__.py @@ -7,8 +7,8 @@ __intname__ = "npbackup.restic_wrapper" __author__ = "Orsiris de Jong" __copyright__ = "Copyright (C) 2022-2023 NetInvent" __license__ = "GPL-3.0-only" -__build__ = "2023082801" -__version__ = "1.7.2" +__build__ = "2023112901" +__version__ = "1.7.3" from typing import Tuple, List, Optional, Callable, Union @@ -560,9 +560,9 @@ class ResticRunner: for path in paths: cmd += ' {} "{}"'.format(source_parameter, path) else: - # make sure path is a list and does not have trailing slashes + # make sure path is a list and does not have trailing slashes, unless we're backing up root cmd = "backup {}".format( - " ".join(['"{}"'.format(path.rstrip("/\\")) for path in paths]) + " ".join(['"{}"'.format(path.rstrip("/\\")) if path != '/' else path for path in paths]) ) case_ignore_param = "" From 5c62f7d07e26399aca886ef342155fd7cb9e5fe4 Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Wed, 29 Nov 2023 21:49:48 +0100 Subject: [PATCH 2/9] Make sure we catch repository init errors --- npbackup/core/runner.py | 4 +++- npbackup/restic_wrapper/__init__.py | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/npbackup/core/runner.py b/npbackup/core/runner.py index 7ddb70d..68f40c3 100644 --- a/npbackup/core/runner.py +++ b/npbackup/core/runner.py @@ -571,7 +571,9 @@ class NPBackupRunner: # Check if backup is required self.restic_runner.verbose = False if not self.restic_runner.is_init: - self.restic_runner.init() + if not self.restic_runner.init(): + logger.error("Cannot continue.") + return False if self.check_recent_backups() and not force: logger.info("No backup necessary.") return True diff --git a/npbackup/restic_wrapper/__init__.py b/npbackup/restic_wrapper/__init__.py index 645f4ba..67923f2 100644 --- a/npbackup/restic_wrapper/__init__.py +++ b/npbackup/restic_wrapper/__init__.py @@ -447,6 +447,10 @@ class ResticRunner: if re.search(".*already exists", output, re.IGNORECASE): logger.info("Repo already initialized.") self.is_init = True + return True + if re.search(".*server response.*\(\d+\)", output, re.IGNORECASE): + logger.error(f"Cannot contact repo: {output}") + return False return False @property From 6607d5e78a572e05a4b4a624f1cd5c4342101649 Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Wed, 29 Nov 2023 22:25:07 +0100 Subject: [PATCH 3/9] Make sure we analyze the same logger instance in execution_logs() --- npbackup/__main__.py | 6 +++++- npbackup/configuration.py | 2 +- npbackup/core/i18n_helper.py | 2 +- npbackup/core/runner.py | 2 +- npbackup/core/upgrade_runner.py | 2 +- npbackup/gui/config.py | 2 +- npbackup/gui/main.py | 2 +- npbackup/restic_wrapper/__init__.py | 2 +- npbackup/upgrade_client/requestor.py | 2 +- npbackup/upgrade_client/upgrader.py | 2 +- npbackup/windows/task.py | 2 +- upgrade_server/upgrade_server/configuration.py | 2 +- upgrade_server/upgrade_server/crud.py | 2 +- 13 files changed, 17 insertions(+), 13 deletions(-) diff --git a/npbackup/__main__.py b/npbackup/__main__.py index 9e34f60..d69566b 100644 --- a/npbackup/__main__.py +++ b/npbackup/__main__.py @@ -88,7 +88,11 @@ def execution_logs(start_time: datetime) -> None: 10 = debug, 20 = info, 30 = warning, 40 = error, 50 = critical so "if 30 in logger._cache" checks if warning has been triggered ATTENTION: logger._cache does only contain cache of current main, not modules, deprecated in favor of - ofunctions.ContextFilterWorstLevel + ofunctions.logger_utils.ContextFilterWorstLevel + + ATTENTION: For ofunctions.logger_utils.ContextFilterWorstLevel will only check current logger instance + So using logger = getLogger("anotherinstance") will create a separate instance from the one we can inspect + Makes sense ;) """ end_time = datetime.utcnow() diff --git a/npbackup/configuration.py b/npbackup/configuration.py index 252470d..1c3ba9e 100644 --- a/npbackup/configuration.py +++ b/npbackup/configuration.py @@ -52,7 +52,7 @@ except ImportError: sys.exit(1) -logger = getLogger(__name__) +logger = getLogger() # NPF-SEC-00003: Avoid password command divulgation ENCRYPTED_OPTIONS = [ diff --git a/npbackup/core/i18n_helper.py b/npbackup/core/i18n_helper.py index e73585c..e56b3aa 100644 --- a/npbackup/core/i18n_helper.py +++ b/npbackup/core/i18n_helper.py @@ -17,7 +17,7 @@ import i18n from npbackup.path_helper import BASEDIR -logger = getLogger(__intname__) +logger = getLogger() TRANSLATIONS_DIR = os.path.join(BASEDIR, "translations") diff --git a/npbackup/core/runner.py b/npbackup/core/runner.py index 68f40c3..59d1760 100644 --- a/npbackup/core/runner.py +++ b/npbackup/core/runner.py @@ -26,7 +26,7 @@ from npbackup.__main__ import __intname__ as NAME, __version__ as VERSION from npbackup import configuration -logger = logging.getLogger(__intname__) +logger = logging.getLogger() def metric_writer( diff --git a/npbackup/core/upgrade_runner.py b/npbackup/core/upgrade_runner.py index c6189b4..b3beff6 100644 --- a/npbackup/core/upgrade_runner.py +++ b/npbackup/core/upgrade_runner.py @@ -16,7 +16,7 @@ from npbackup.upgrade_client.upgrader import auto_upgrader, _check_new_version from npbackup.__main__ import __version__ as npbackup_version -logger = getLogger(__intname__) +logger = getLogger() def check_new_version(config_dict: dict) -> bool: diff --git a/npbackup/gui/config.py b/npbackup/gui/config.py index 1f749ea..4b3d0ba 100644 --- a/npbackup/gui/config.py +++ b/npbackup/gui/config.py @@ -22,7 +22,7 @@ from npbackup.core.nuitka_helper import IS_COMPILED if os.name == "nt": from npbackup.windows.task import create_scheduled_task -logger = getLogger(__intname__) +logger = getLogger() def ask_backup_admin_password(config_dict) -> bool: diff --git a/npbackup/gui/main.py b/npbackup/gui/main.py index 613fcd6..c560259 100644 --- a/npbackup/gui/main.py +++ b/npbackup/gui/main.py @@ -42,7 +42,7 @@ from npbackup.core.i18n_helper import _t from npbackup.core.upgrade_runner import run_upgrade, check_new_version -logger = getLogger(__intname__) +logger = getLogger() # Let's use mutable to get a cheap way of transfering data from thread to main program # There are no possible race conditions since we don't modifiy the data from anywhere outside the thread diff --git a/npbackup/restic_wrapper/__init__.py b/npbackup/restic_wrapper/__init__.py index 67923f2..6c76cb9 100644 --- a/npbackup/restic_wrapper/__init__.py +++ b/npbackup/restic_wrapper/__init__.py @@ -22,7 +22,7 @@ import queue from command_runner import command_runner -logger = getLogger(__intname__) +logger = getLogger() # Arbitrary timeout for init / init checks. # If init takes more than a minute, we really have a problem diff --git a/npbackup/upgrade_client/requestor.py b/npbackup/upgrade_client/requestor.py index 8b5d7e3..dcdfb6a 100644 --- a/npbackup/upgrade_client/requestor.py +++ b/npbackup/upgrade_client/requestor.py @@ -16,7 +16,7 @@ import json import requests -logger = getLogger(__intname__) +logger = getLogger() class Requestor: diff --git a/npbackup/upgrade_client/upgrader.py b/npbackup/upgrade_client/upgrader.py index e61e24d..57c0c50 100644 --- a/npbackup/upgrade_client/upgrader.py +++ b/npbackup/upgrade_client/upgrader.py @@ -25,7 +25,7 @@ from npbackup.path_helper import CURRENT_DIR, CURRENT_EXECUTABLE from npbackup.core.nuitka_helper import IS_COMPILED from npbackup.__main__ import __version__ as npbackup_version -logger = getLogger(__intname__) +logger = getLogger() UPGRADE_DEFER_TIME = 60 # Wait x seconds before we actually do the upgrade so current program could quit before being erased diff --git a/npbackup/windows/task.py b/npbackup/windows/task.py index 4d4808e..758ae3c 100644 --- a/npbackup/windows/task.py +++ b/npbackup/windows/task.py @@ -17,7 +17,7 @@ import tempfile from command_runner import command_runner from npbackup.customization import PROGRAM_NAME -logger = getLogger(__intname__) +logger = getLogger() # This is the path to a onefile executable binary diff --git a/upgrade_server/upgrade_server/configuration.py b/upgrade_server/upgrade_server/configuration.py index 42d7858..a2bb7fc 100644 --- a/upgrade_server/upgrade_server/configuration.py +++ b/upgrade_server/upgrade_server/configuration.py @@ -18,7 +18,7 @@ from logging import getLogger ROOT_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir)) config_file = os.path.join(ROOT_DIR, "upgrade_server.conf") -logger = getLogger(__intname__) +logger = getLogger() def load_config(config_file: str = config_file): diff --git a/upgrade_server/upgrade_server/crud.py b/upgrade_server/upgrade_server/crud.py index f6730b2..14ab95d 100644 --- a/upgrade_server/upgrade_server/crud.py +++ b/upgrade_server/upgrade_server/crud.py @@ -39,7 +39,7 @@ else: config_dict = configuration.load_config() -logger = getLogger(__intname__) +logger = getLogger() def sha256sum_data(data): From 90895b1ec29125da94803307b0b2db23a417f6ce Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Wed, 29 Nov 2023 22:25:57 +0100 Subject: [PATCH 4/9] Make sure we always update is_init on init --- npbackup/restic_wrapper/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/npbackup/restic_wrapper/__init__.py b/npbackup/restic_wrapper/__init__.py index 6c76cb9..a0a34e1 100644 --- a/npbackup/restic_wrapper/__init__.py +++ b/npbackup/restic_wrapper/__init__.py @@ -442,6 +442,7 @@ class ResticRunner: if re.search( r"created restic repository ([a-z0-9]+) at .+", output, re.IGNORECASE ): + self.is_init = True return True else: if re.search(".*already exists", output, re.IGNORECASE): @@ -450,7 +451,9 @@ class ResticRunner: return True if re.search(".*server response.*\(\d+\)", output, re.IGNORECASE): logger.error(f"Cannot contact repo: {output}") + self.is_init = False return False + self.is_init = False return False @property From 0c48848b7ebf8accb21d32ddc03c68ffa7a87a9a Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Wed, 29 Nov 2023 22:35:52 +0100 Subject: [PATCH 5/9] Fix additional parameters should be run on any command --- npbackup/restic_wrapper/__init__.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/npbackup/restic_wrapper/__init__.py b/npbackup/restic_wrapper/__init__.py index a0a34e1..49e1398 100644 --- a/npbackup/restic_wrapper/__init__.py +++ b/npbackup/restic_wrapper/__init__.py @@ -70,7 +70,7 @@ class ResticRunner: except AttributeError: self._backend_type = None self._ignore_cloud_files = True - self._addition_parameters = None + self._additional_parameters = None self._environment_variables = {} self._stop_on = ( @@ -201,7 +201,8 @@ class ResticRunner: """ start_time = datetime.utcnow() self._executor_finished = False - _cmd = '"{}" {}{}'.format(self._binary, cmd, self.generic_arguments) + additional_parameters = f" {self.additional_parameters.strip()} " if self.additional_parameters else "" + _cmd = f'"{self._binary}" {additional_parameters}{cmd}{self.generic_arguments}' if self.dry_run: _cmd += " --dry-run" logger.debug("Running command: [{}]".format(_cmd)) @@ -352,11 +353,11 @@ class ResticRunner: @property def additional_parameters(self): - return self._addition_parameters + return self._additional_parameters @additional_parameters.setter def additional_parameters(self, value: str): - self._addition_parameters = value + self._additional_parameters = value @property def priority(self): @@ -543,7 +544,7 @@ class ResticRunner: use_fs_snapshot: bool = False, tags: List[str] = [], one_file_system: bool = False, - additional_parameters: str = None, + additional_backup_only_parameters: str = None, ) -> Tuple[bool, str]: """ Executes restic backup after interpreting all arguments @@ -599,8 +600,8 @@ class ResticRunner: if tag: tag = tag.strip() cmd += " --tag {}".format(tag) - if additional_parameters: - cmd += " {}".format(additional_parameters) + if additional_backup_only_parameters: + cmd += " {}".format(additional_backup_only_parameters) result, output = self.executor(cmd, live_stream=True) if ( From 443a13a24d1f2e6886130aaae33f000b964ab69e Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Wed, 29 Nov 2023 22:43:28 +0100 Subject: [PATCH 6/9] Separate additional parameters for all restic calls and for backup only --- npbackup/core/runner.py | 14 +++++++++++--- npbackup/gui/config.py | 4 ++++ npbackup/translations/config_gui.en.yml | 1 + npbackup/translations/config_gui.fr.yml | 1 + 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/npbackup/core/runner.py b/npbackup/core/runner.py index 59d1760..eeedd6b 100644 --- a/npbackup/core/runner.py +++ b/npbackup/core/runner.py @@ -347,6 +347,14 @@ class NPBackupRunner: except ValueError: logger.warning("Bogus ignore_cloud_files value given") + + try: + if self.config_dict["backup"]["additional_parameters"]: + self.restic_runner.additional_parameters = self.config_dict["backup"]["additional_parameters"] + except KeyError: + pass + except ValueError: + logger.warning("Bogus additional parameters given") self.restic_runner.stdout = self.stdout try: @@ -564,9 +572,9 @@ class NPBackupRunner: tags = None try: - additional_parameters = self.config_dict["backup"]["additional_parameters"] + additional_backup_only_parameters = self.config_dict["backup"]["additional_backup_only_parameters"] except KeyError: - additional_parameters = None + additional_backup_only_parameters = None # Check if backup is required self.restic_runner.verbose = False @@ -615,7 +623,7 @@ class NPBackupRunner: one_file_system=one_file_system, use_fs_snapshot=use_fs_snapshot, tags=tags, - additional_parameters=additional_parameters, + additional_backup_only_parameters=additional_backup_only_parameters, ) logger.debug("Restic output:\n{}".format(result_string)) metric_writer( diff --git a/npbackup/gui/config.py b/npbackup/gui/config.py index 4b3d0ba..46bf3a0 100644 --- a/npbackup/gui/config.py +++ b/npbackup/gui/config.py @@ -285,6 +285,10 @@ def config_gui(config_dict: dict, config_file: str): sg.Text(_t("config_gui.additional_parameters"), size=(40, 1)), sg.Input(key="backup---additional_parameters", size=(50, 1)), ], + [ + sg.Text(_t("config_gui.additional_backup_only_parameters"), size=(40, 1)), + sg.Input(key="backup---additional_backup_only_parameters", size=(50, 1)), + ], ] repo_col = [ diff --git a/npbackup/translations/config_gui.en.yml b/npbackup/translations/config_gui.en.yml index 2ef35c8..618b96b 100644 --- a/npbackup/translations/config_gui.en.yml +++ b/npbackup/translations/config_gui.en.yml @@ -20,6 +20,7 @@ en: one_per_line: one per line backup_priority: Backup priority additional_parameters: Additional parameters + additional_backup_only_parameters: Additional backup only parmas backup_destination: Backup destination minimum_backup_age: Minimum delay between two backups diff --git a/npbackup/translations/config_gui.fr.yml b/npbackup/translations/config_gui.fr.yml index 455852b..40c589d 100644 --- a/npbackup/translations/config_gui.fr.yml +++ b/npbackup/translations/config_gui.fr.yml @@ -20,6 +20,7 @@ fr: one_per_line: un par ligne backup_priority: Priorité de sauvegarde additional_parameters: Paramètres supplémentaires + additional_backup_only_parameters: Paramètres supp. sauvegarde backup_destination: Destination de sauvegarde minimum_backup_age: Délai minimal entre deux sauvegardes From a93ff76d6df9fa3049abc8545142bb45fbf847bc Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Wed, 29 Nov 2023 23:00:05 +0100 Subject: [PATCH 7/9] Make sure we log an error when init fails --- npbackup/restic_wrapper/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/npbackup/restic_wrapper/__init__.py b/npbackup/restic_wrapper/__init__.py index 49e1398..b17ea97 100644 --- a/npbackup/restic_wrapper/__init__.py +++ b/npbackup/restic_wrapper/__init__.py @@ -450,13 +450,13 @@ class ResticRunner: logger.info("Repo already initialized.") self.is_init = True return True - if re.search(".*server response.*\(\d+\)", output, re.IGNORECASE): - logger.error(f"Cannot contact repo: {output}") - self.is_init = False - return False + logger.error(f"Cannot contact repo: {output}") + self.is_init = False + return False self.is_init = False return False + @property def is_init(self): if self._is_init is None: From c00567a741c7acf5905b9a170d143ab3ee91544f Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Wed, 29 Nov 2023 23:02:18 +0100 Subject: [PATCH 8/9] Reformat files with black --- npbackup/core/runner.py | 9 ++++++--- npbackup/restic_wrapper/__init__.py | 14 +++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/npbackup/core/runner.py b/npbackup/core/runner.py index eeedd6b..e78aafb 100644 --- a/npbackup/core/runner.py +++ b/npbackup/core/runner.py @@ -347,10 +347,11 @@ class NPBackupRunner: except ValueError: logger.warning("Bogus ignore_cloud_files value given") - try: if self.config_dict["backup"]["additional_parameters"]: - self.restic_runner.additional_parameters = self.config_dict["backup"]["additional_parameters"] + self.restic_runner.additional_parameters = self.config_dict["backup"][ + "additional_parameters" + ] except KeyError: pass except ValueError: @@ -572,7 +573,9 @@ class NPBackupRunner: tags = None try: - additional_backup_only_parameters = self.config_dict["backup"]["additional_backup_only_parameters"] + additional_backup_only_parameters = self.config_dict["backup"][ + "additional_backup_only_parameters" + ] except KeyError: additional_backup_only_parameters = None diff --git a/npbackup/restic_wrapper/__init__.py b/npbackup/restic_wrapper/__init__.py index b17ea97..53fd221 100644 --- a/npbackup/restic_wrapper/__init__.py +++ b/npbackup/restic_wrapper/__init__.py @@ -201,7 +201,11 @@ class ResticRunner: """ start_time = datetime.utcnow() self._executor_finished = False - additional_parameters = f" {self.additional_parameters.strip()} " if self.additional_parameters else "" + additional_parameters = ( + f" {self.additional_parameters.strip()} " + if self.additional_parameters + else "" + ) _cmd = f'"{self._binary}" {additional_parameters}{cmd}{self.generic_arguments}' if self.dry_run: _cmd += " --dry-run" @@ -456,7 +460,6 @@ class ResticRunner: self.is_init = False return False - @property def is_init(self): if self._is_init is None: @@ -570,7 +573,12 @@ class ResticRunner: else: # make sure path is a list and does not have trailing slashes, unless we're backing up root cmd = "backup {}".format( - " ".join(['"{}"'.format(path.rstrip("/\\")) if path != '/' else path for path in paths]) + " ".join( + [ + '"{}"'.format(path.rstrip("/\\")) if path != "/" else path + for path in paths + ] + ) ) case_ignore_param = "" From 363ae7282bcbcce5e9ffcdef1fe5ad6b77e7653e Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Thu, 7 Dec 2023 17:35:17 +0100 Subject: [PATCH 9/9] Log successful pre/post script executions --- npbackup/core/runner.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/npbackup/core/runner.py b/npbackup/core/runner.py index e78aafb..087aa0e 100644 --- a/npbackup/core/runner.py +++ b/npbackup/core/runner.py @@ -609,8 +609,8 @@ class NPBackupRunner: if pre_exec_failure_is_fatal: return False else: - logger.debug( - "Pre-execution of command {} success with\n{}.".format( + logger.info( + "Pre-execution of command {} success with:\n{}.".format( pre_exec_command, output ) ) @@ -646,8 +646,8 @@ class NPBackupRunner: if post_exec_failure_is_fatal: return False else: - logger.debug( - "Post-execution of command {} success with\n{}.".format( + logger.info( + "Post-execution of command {} success with:\n{}.".format( post_exec_command, output ) )