diff --git a/CHANGELOG b/CHANGELOG index 4845a0e..2129f41 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,8 @@ - Fix config fails when restic password is an int - Fix empty config files did not show a proper error message - Fix various config file malformation will break execution + - Fix backup hangs when no restic password is given (restic asks for password in backgroud job) + - Fix error message in logs when repo is not initialized ## v2.2.0 - rc1 - 02/02/2023 - Added a full auto-upgrade solution: diff --git a/TODO.md b/TODO.md index c300f9e..13c65ec 100644 --- a/TODO.md +++ b/TODO.md @@ -9,30 +9,7 @@ - Fallback server when primary repo is not available - Windows installer (NSIS ?) - Linux installer script -- Shall we also include the recent backup job verification ? - - Example of a bad remote repo path: - - Fatal: unable to open config file: Head "https:/user:***@bad.example.tld/user/config": dial tcp: lookup bad.example.tld: no such host - - - Example of a bad auth: - - Fatal: unable to open config file: unexpected HTTP response (401): 401 Unauthorized -Is there a repository at the following location? - - - Example of a good path, good auth but no repo initialized: - - Fatal: unable to open config file: does not exist -Is there a repository at the following location? - - - Example: bad password - - Fatal: wrong password or no key found - - - Example: non reachable server: - - Fatal: unable to open config file: Head "https://user:***@good.example.tld/user/config": dial tcp [ipv6]:443: connectex: Une tentative de connexion a échoué car le parti connecté n’a pas répondu convenablement au-delà d’une certaine durée ou une connexion établie a échoué car l’hôte de connexion n’a pas répondu. -Is there a repository at the following location? -rest:https://user:***@good.example.tld/user/ + diff --git a/npbackup/core/runner.py b/npbackup/core/runner.py index 9f7b4fa..7f79b1c 100644 --- a/npbackup/core/runner.py +++ b/npbackup/core/runner.py @@ -123,6 +123,7 @@ class NPBackupRunner: self._minimim_backup_age = None self._exec_time = None + self.is_ready = False # Create an instance of restic wrapper self.create_restic_runner() # Configure that instance @@ -197,13 +198,22 @@ class NPBackupRunner: return wrapper def create_restic_runner(self) -> None: + can_run = True try: repository = self.config_dict["repo"]["repository"] + if not repository: + raise KeyError + except (KeyError, AttributeError): + logger.error("Repo cannot be empty") + can_run = False + try: password = self.config_dict["repo"]["password"] - except KeyError as exc: - logger.error("Missing repo information: {}".format(exc)) - return None - + if not password: + raise KeyError + except (KeyError, AttributeError): + logger.error("Repo password cannot be empty") + can_run = False + self.is_ready = can_run self.restic_runner = ResticRunner( repository=repository, password=password, @@ -219,7 +229,7 @@ class NPBackupRunner: self.restic_runner.binary = binary def apply_config_to_restic_runner(self) -> None: - if not self.restic_runner: + if not self.is_ready: return None try: if self.config_dict["repo"]["upload_speed"]: @@ -309,12 +319,16 @@ class NPBackupRunner: @exec_timer def list(self) -> Optional[dict]: + if not self.is_ready: + return False logger.info("Listing snapshots") snapshots = self.restic_runner.snapshots() return snapshots @exec_timer def find(self, path: str) -> bool: + if not self.is_ready: + return False logger.info("Searching for path {}".format(path)) result = self.restic_runner.find(path=path) if result: @@ -326,12 +340,21 @@ class NPBackupRunner: @exec_timer def ls(self, snapshot: str) -> Optional[dict]: + if not self.is_ready: + return False logger.info("Showing content of snapshot {}".format(snapshot)) result = self.restic_runner.ls(snapshot) return result @exec_timer def check_recent_backups(self) -> bool: + """ + Checks for backups in timespan + Returns True or False if found or not + Returns None if no information is available + """ + if not self.is_ready: + return None logger.info( "Searching for a backup newer than {} ago.".format( str(datetime.timedelta(minutes=self.minimum_backup_age)) @@ -345,13 +368,15 @@ class NPBackupRunner: logger.info("No recent backup found.") elif result is None: logger.error("Cannot connect to repository.") - return False + return result @exec_timer def backup(self, force: bool = False) -> bool: """ Run backup after checking if no recent backup exists, unless force == True """ + if not self.is_ready: + return False # Preflight checks try: paths = self.config_dict["backup"]["paths"] @@ -509,6 +534,8 @@ class NPBackupRunner: @exec_timer def restore(self, snapshot: str, target: str, restore_includes: List[str]) -> bool: + if not self.is_ready: + return False logger.info("Launching restore to {}".format(target)) result = self.restic_runner.restore( snapshot=snapshot, @@ -519,12 +546,15 @@ class NPBackupRunner: @exec_timer def forget(self, snapshot: str) -> bool: + if not self.is_ready: + return False logger.info("Forgetting snapshot {}".format(snapshot)) result = self.restic_runner.forget(snapshot) return result @exec_timer def raw(self, command: str) -> bool: + logger.info("Running raw command: {}".format(command)) result = self.restic_runner.raw(command=command) return result diff --git a/npbackup/restic_wrapper/__init__.py b/npbackup/restic_wrapper/__init__.py index e5b69b2..82a4be4 100644 --- a/npbackup/restic_wrapper/__init__.py +++ b/npbackup/restic_wrapper/__init__.py @@ -213,7 +213,6 @@ class ResticRunner: priority=self._priority, io_priority=self._priority, ) - # Don't keep protected environment variables in memory when not necessary self._remove_env() @@ -242,6 +241,17 @@ class ResticRunner: return True, output # TEMP-FIX-4155-END self.last_command_status = False + + # From here, we assume that we have errors + # Before going back to errors, let's analyze current output + + # Cannot connect to repo (port ?) + #if re.match("Fatal: unable to open config file: Head .*: dial tcp .*: connect .*", output): + # logger.error("Cannot connect to repo.") + + #if re.match("Is there a repository at the following location\?", output): + # We did achieve to get to the repo + if not errors_allowed and output: logger.error(output) return False, output @@ -421,6 +431,8 @@ class ResticRunner: """ Returns json list of snapshots """ + if not self.is_init: + return None cmd = "list {} --json".format(obj) result, output = self.executor(cmd) if result: @@ -435,6 +447,8 @@ class ResticRunner: """ Returns json list of objects """ + if not self.is_init: + return None cmd = "ls {} --json".format(snapshot) result, output = self.executor(cmd) if result and output: @@ -458,6 +472,8 @@ class ResticRunner: """ Returns json list of snapshots """ + if not self.is_init: + return None cmd = "snapshots --json" result, output = self.executor(cmd) if result: @@ -484,6 +500,8 @@ class ResticRunner: """ Executes restic backup after interpreting all arguments """ + if not self.is_init: + return None # make sure path is a list and does not have trailing slashes cmd = "backup {}".format( " ".join(['"{}"'.format(path.rstrip("/\\")) for path in paths]) @@ -535,6 +553,8 @@ class ResticRunner: """ Returns find command """ + if not self.is_init: + return None cmd = 'find "{}" --json'.format(path) result, output = self.executor(cmd) if result: @@ -551,6 +571,8 @@ class ResticRunner: """ Restore given snapshot to directory """ + if not self.is_init: + return None case_ignore_param = "" # Always use case ignore excludes under windows if os.name == "nt": @@ -570,6 +592,8 @@ class ResticRunner: """ Execute forget command for given snapshot """ + if not self.is_init: + return None cmd = "forget {}".format(snapshot) # We need to be verbose here since server errors will not stop client from deletion attempts verbose = self.verbose @@ -586,6 +610,8 @@ class ResticRunner: """ Execute plain restic command without any interpretation" """ + if not self.is_init: + return None result, output = self.executor(command) if result: logger.info("successfully run raw command:\n{}".format(output)) @@ -601,11 +627,11 @@ class ResticRunner: returns False is too old snapshots exit returns None if no info available """ + if not self.is_init: + return None try: snapshots = self.snapshots() - if self.last_command_status is False: - return None - if not snapshots: + if self.last_command_status is False or not snapshots: return False tz_aware_timestamp = datetime.now(timezone.utc).astimezone()