From 6d3fcd6cc3804752c088fe0076b36f99132bd4a6 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 17 Mar 2021 20:24:01 +0100 Subject: [PATCH] Improve error management for app restore, similar to what's done in app install --- locales/ca.json | 2 +- locales/de.json | 2 +- locales/en.json | 2 +- locales/eo.json | 2 +- locales/es.json | 2 +- locales/eu.json | 2 +- locales/fr.json | 2 +- locales/it.json | 2 +- locales/nl.json | 2 +- locales/oc.json | 2 +- src/yunohost/backup.py | 151 ++++++++++++++--------- src/yunohost/tests/test_backuprestore.py | 2 +- 12 files changed, 101 insertions(+), 72 deletions(-) diff --git a/locales/ca.json b/locales/ca.json index b6888d391..ceaa6791e 100644 --- a/locales/ca.json +++ b/locales/ca.json @@ -327,7 +327,7 @@ "regenconf_failed": "No s'ha pogut regenerar la configuració per la/les categoria/es : {categories}", "regenconf_pending_applying": "Aplicació de la configuració pendent per la categoria «{category}»...", "restore_already_installed_app": "Una aplicació amb la ID «{app:s}» ja està instal·lada", - "restore_app_failed": "No s'ha pogut restaurar {app:s}", + "app_restore_failed": "No s'ha pogut restaurar {app:s}: {error:s}", "restore_cleaning_failed": "No s'ha pogut netejar el directori temporal de restauració", "restore_complete": "Restauració completada", "restore_confirm_yunohost_installed": "Esteu segur de voler restaurar un sistema ja instal·lat? [{answers:s}]", diff --git a/locales/de.json b/locales/de.json index bfc9c36a4..b3617e476 100644 --- a/locales/de.json +++ b/locales/de.json @@ -96,7 +96,7 @@ "port_already_closed": "Der Port {port:d} wurde bereits für {ip_version:s} Verbindungen geschlossen", "port_already_opened": "Der Port {port:d} wird bereits von {ip_version:s} benutzt", "restore_already_installed_app": "Es ist bereits eine App mit der ID '{app:s}' installiet", - "restore_app_failed": "App '{app:s}' konnte nicht wiederhergestellt werden", + "app_restore_failed": "App '{app:s}' konnte nicht wiederhergestellt werden: {error:s}", "restore_cleaning_failed": "Das temporäre Wiederherstellungsverzeichnis konnte nicht geleert werden", "restore_complete": "Wiederherstellung abgeschlossen", "restore_confirm_yunohost_installed": "Möchtest du die Wiederherstellung wirklich starten? [{answers:s}]", diff --git a/locales/en.json b/locales/en.json index be31c7599..cbab5e382 100644 --- a/locales/en.json +++ b/locales/en.json @@ -523,7 +523,7 @@ "regex_with_only_domain": "You can't use a regex for domain, only for path", "restore_already_installed_app": "An app with the ID '{app:s}' is already installed", "restore_already_installed_apps": "The following apps can't be restored because they are already installed: {apps}", - "restore_app_failed": "Could not restore {app:s}", + "restore_app_failed": "Could not restore {app:s}: {error: s}", "restore_backup_too_old": "This backup archive can not be restored because it comes from a too-old YunoHost version.", "restore_cleaning_failed": "Could not clean up the temporary restoration directory", "restore_complete": "Restoration completed", diff --git a/locales/eo.json b/locales/eo.json index 1a27831f2..95030d8fa 100644 --- a/locales/eo.json +++ b/locales/eo.json @@ -479,7 +479,7 @@ "service_restarted": "Servo '{service:s}' rekomencis", "pattern_username": "Devas esti minuskulaj literoj kaj minuskloj nur", "extracting": "Eltirante…", - "restore_app_failed": "Ne povis restarigi la programon '{app:s}'", + "app_restore_failed": "Ne povis restarigi la programon '{app:s}': {error:s}", "yunohost_configured": "YunoHost nun estas agordita", "certmanager_self_ca_conf_file_not_found": "Ne povis trovi agorddosieron por mem-subskriba aŭtoritato (dosiero: {file:s})", "log_app_remove": "Forigu la aplikon '{}'", diff --git a/locales/es.json b/locales/es.json index cfcca071f..a93c3f244 100644 --- a/locales/es.json +++ b/locales/es.json @@ -107,7 +107,7 @@ "port_already_closed": "El puerto {port:d} ya está cerrado para las conexiones {ip_version:s}", "port_already_opened": "El puerto {port:d} ya está abierto para las conexiones {ip_version:s}", "restore_already_installed_app": "Una aplicación con el ID «{app:s}» ya está instalada", - "restore_app_failed": "No se pudo restaurar la aplicación «{app:s}»", + "app_restore_failed": "No se pudo restaurar la aplicación «{app:s}»: {error:s}", "restore_cleaning_failed": "No se pudo limpiar el directorio temporal de restauración", "restore_complete": "Restaurada", "restore_confirm_yunohost_installed": "¿Realmente desea restaurar un sistema ya instalado? [{answers:s}]", diff --git a/locales/eu.json b/locales/eu.json index 1891e00a3..539fb9157 100644 --- a/locales/eu.json +++ b/locales/eu.json @@ -1,3 +1,3 @@ { "password_too_simple_1": "Pasahitzak gutxienez 8 karaktere izan behar ditu" -} \ No newline at end of file +} diff --git a/locales/fr.json b/locales/fr.json index 8982d7ccc..e60e17d0e 100644 --- a/locales/fr.json +++ b/locales/fr.json @@ -107,7 +107,7 @@ "port_already_closed": "Le port {port:d} est déjà fermé pour les connexions {ip_version:s}", "port_already_opened": "Le port {port:d} est déjà ouvert pour les connexions {ip_version:s}", "restore_already_installed_app": "Une application est déjà installée avec l’identifiant '{app:s}'", - "restore_app_failed": "Impossible de restaurer '{app:s}'", + "app_restore_failed": "Impossible de restaurer '{app:s}': {error:s}", "restore_cleaning_failed": "Impossible de nettoyer le dossier temporaire de restauration", "restore_complete": "Restauration terminée", "restore_confirm_yunohost_installed": "Voulez-vous vraiment restaurer un système déjà installé ? [{answers:s}]", diff --git a/locales/it.json b/locales/it.json index 45b85d548..22248367a 100644 --- a/locales/it.json +++ b/locales/it.json @@ -120,7 +120,7 @@ "pattern_username": "Caratteri minuscoli alfanumerici o trattini bassi soli", "port_already_closed": "La porta {port:d} è già chiusa per le connessioni {ip_version:s}", "restore_already_installed_app": "Un'applicazione con l'ID '{app:s}' è già installata", - "restore_app_failed": "Impossibile ripristinare l'applicazione '{app:s}'", + "app_restore_failed": "Impossibile ripristinare l'applicazione '{app:s}': {error:s}", "restore_cleaning_failed": "Impossibile pulire la directory temporanea di ripristino", "restore_complete": "Ripristino completo", "restore_confirm_yunohost_installed": "Sei sicuro di volere ripristinare un sistema già installato? {answers:s}", diff --git a/locales/nl.json b/locales/nl.json index 59de95f58..23ad8d0cb 100644 --- a/locales/nl.json +++ b/locales/nl.json @@ -54,7 +54,7 @@ "pattern_password": "Wachtwoord moet tenminste 3 karakters lang zijn", "port_already_closed": "Poort {port:d} is al gesloten voor {ip_version:s} verbindingen", "port_already_opened": "Poort {port:d} is al open voor {ip_version:s} verbindingen", - "restore_app_failed": "De app '{app:s}' kon niet worden terug gezet", + "app_restore_failed": "De app '{app:s}' kon niet worden terug gezet: {error:s}", "restore_hook_unavailable": "De herstel-hook '{part:s}' is niet beschikbaar op dit systeem", "service_add_failed": "Kan service '{service:s}' niet toevoegen", "service_already_started": "Service '{service:s}' draait al", diff --git a/locales/oc.json b/locales/oc.json index 7fd423617..3bed9cd5a 100644 --- a/locales/oc.json +++ b/locales/oc.json @@ -170,7 +170,7 @@ "port_already_closed": "Lo pòrt {port:d} es ja tampat per las connexions {ip_version:s}", "port_already_opened": "Lo pòrt {port:d} es ja dubèrt per las connexions {ip_version:s}", "restore_already_installed_app": "Una aplicacion es ja installada amb l’id « {app:s} »", - "restore_app_failed": "Impossible de restaurar l’aplicacion « {app:s} »", + "app_restore_failed": "Impossible de restaurar l’aplicacion « {app:s} »: {error:s}", "backup_ask_for_copying_if_needed": "Volètz far una salvagarda en utilizant {size:s} Mo temporàriament ? (Aqueste biais de far es emplegat perque unes fichièrs an pas pogut èsser preparats amb un metòde mai eficaç.)", "yunohost_not_installed": "YunoHost es pas installat o corrèctament installat. Mercés d’executar « yunohost tools postinstall »", "backup_output_directory_forbidden": "Causissètz un repertòri de destinacion deferent. Las salvagardas pòdon pas se realizar dins los repertòris bin, /boot, /dev, /etc, /lib, /root, /run, /sbin, /sys, /usr, /var ou /home/yunohost.backup/archives", diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index ed96dac16..b88c6e124 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -1392,7 +1392,6 @@ class RestoreManager: self.targets.set_result("apps", app_instance_name, "Warning") return - logger.debug(m18n.n("restore_running_app_script", app=app_instance_name)) try: # Restore app settings app_settings_new_path = os.path.join( @@ -1401,7 +1400,7 @@ class RestoreManager: app_scripts_new_path = os.path.join(app_settings_new_path, "scripts") shutil.copytree(app_settings_in_archive, app_settings_new_path) filesystem.chmod(app_settings_new_path, 0o400, 0o400, True) - filesystem.chown(app_scripts_new_path, "admin", None, True) + filesystem.chown(app_scripts_new_path, "root", None, True) # Copy the app scripts to a writable temporary folder # FIXME : use 'install -Dm555' or something similar to what's done @@ -1409,7 +1408,7 @@ class RestoreManager: tmp_folder_for_app_restore = tempfile.mkdtemp(prefix="restore") copytree(app_scripts_in_archive, tmp_folder_for_app_restore) filesystem.chmod(tmp_folder_for_app_restore, 0o550, 0o550, True) - filesystem.chown(tmp_folder_for_app_restore, "admin", None, True) + filesystem.chown(tmp_folder_for_app_restore, "root", None, True) restore_script = os.path.join(tmp_folder_for_app_restore, "restore") # Restore permissions @@ -1454,81 +1453,111 @@ class RestoreManager: os.remove("%s/permissions.yml" % app_settings_new_path) _tools_migrations_run_before_app_restore(backup_version=self.info["from_yunohost_version"], app_id=app_instance_name) - - # Prepare env. var. to pass to script - env_dict = _make_environment_for_app_script(app_instance_name) - env_dict.update( - { - "YNH_BACKUP_DIR": self.work_dir, - "YNH_BACKUP_CSV": os.path.join(self.work_dir, "backup.csv"), - "YNH_APP_BACKUP_DIR": os.path.join( - self.work_dir, "apps", app_instance_name, "backup" - ), - } - ) - - operation_logger.extra["env"] = env_dict - operation_logger.flush() - - # Execute app restore script - hook_exec( - restore_script, - chdir=app_backup_in_archive, - raise_on_error=True, - env=env_dict, - )[0] except Exception: - msg = m18n.n("restore_app_failed", app=app_instance_name) + import traceback + error = m18n.n("unexpected_error", error="\n" + traceback.format_exc()) + msg = m18n.n("app_restore_failed", app=app_instance_name, error=error) logger.error(msg) operation_logger.error(msg) - if msettings.get("interface") != "api": - dump_app_log_extract_for_debugging(operation_logger) - self.targets.set_result("apps", app_instance_name, "Error") - remove_script = os.path.join(app_scripts_in_archive, "remove") - - # Setup environment for remove script - env_dict_remove = _make_environment_for_app_script(app_instance_name) - - operation_logger = OperationLogger( - "remove_on_failed_restore", - [("app", app_instance_name)], - env=env_dict_remove, - ) - operation_logger.start() - - # Execute remove script - if hook_exec(remove_script, env=env_dict_remove)[0] != 0: - msg = m18n.n("app_not_properly_removed", app=app_instance_name) - logger.warning(msg) - operation_logger.error(msg) - else: - operation_logger.success() - - # Cleaning app directory + # Cleanup shutil.rmtree(app_settings_new_path, ignore_errors=True) + shutil.rmtree(tmp_folder_for_app_restore, ignore_errors=True) - # Remove all permission in LDAP for this app - for permission_name in user_permission_list()["permissions"].keys(): - if permission_name.startswith(app_instance_name + "."): - permission_delete(permission_name, force=True) + return - # TODO Cleaning app hooks - else: - self.targets.set_result("apps", app_instance_name, "Success") - operation_logger.success() + logger.debug(m18n.n("restore_running_app_script", app=app_instance_name)) + + # Prepare env. var. to pass to script + env_dict = _make_environment_for_app_script(app_instance_name) + env_dict.update( + { + "YNH_BACKUP_DIR": self.work_dir, + "YNH_BACKUP_CSV": os.path.join(self.work_dir, "backup.csv"), + "YNH_APP_BACKUP_DIR": os.path.join( + self.work_dir, "apps", app_instance_name, "backup" + ), + } + ) + + operation_logger.extra["env"] = env_dict + operation_logger.flush() + + # Execute the app install script + restore_failed = True + try: + restore_retcode = hook_exec( + restore_script, + chdir=app_backup_in_archive, + env=env_dict, + )[0] + # "Common" app restore failure : the script failed and returned exit code != 0 + restore_failed = True if restore_retcode != 0 else False + if restore_failed: + error = m18n.n("app_restore_script_failed") + logger.error(m18n.n("app_restore_failed", app=app_instance_name, error=error)) + failure_message_with_debug_instructions = operation_logger.error(error) + if msettings.get("interface") != "api": + dump_app_log_extract_for_debugging(operation_logger) + # Script got manually interrupted ... N.B. : KeyboardInterrupt does not inherit from Exception + except (KeyboardInterrupt, EOFError): + error = m18n.n("operation_interrupted") + logger.error(m18n.n("app_restore_failed", app=app_instance_name, error=error)) + failure_message_with_debug_instructions = operation_logger.error(error) + # Something wrong happened in Yunohost's code (most probably hook_exec) + except Exception: + import traceback + error = m18n.n("unexpected_error", error="\n" + traceback.format_exc()) + logger.error(m18n.n("app_restore_failed", app=app_instance_name, error=error)) + failure_message_with_debug_instructions = operation_logger.error(error) finally: # Cleaning temporary scripts directory shutil.rmtree(tmp_folder_for_app_restore, ignore_errors=True) + if not restore_failed: + self.targets.set_result("apps", app_instance_name, "Success") + operation_logger.success() + else: + + self.targets.set_result("apps", app_instance_name, "Error") + + remove_script = os.path.join(app_scripts_in_archive, "remove") + + # Setup environment for remove script + env_dict_remove = _make_environment_for_app_script(app_instance_name) + + remove_operation_logger = OperationLogger( + "remove_on_failed_restore", + [("app", app_instance_name)], + env=env_dict_remove, + ) + remove_operation_logger.start() + + # Execute remove script + if hook_exec(remove_script, env=env_dict_remove)[0] != 0: + msg = m18n.n("app_not_properly_removed", app=app_instance_name) + logger.warning(msg) + remove_operation_logger.error(msg) + else: + remove_operation_logger.success() + + # Cleaning app directory + shutil.rmtree(app_settings_new_path, ignore_errors=True) + + # Remove all permission in LDAP for this app + for permission_name in user_permission_list()["permissions"].keys(): + if permission_name.startswith(app_instance_name + "."): + permission_delete(permission_name, force=True) + + # TODO Cleaning app hooks + + logger.error(failure_message_with_debug_instructions) # # Backup methods # # - - class BackupMethod(object): """ diff --git a/src/yunohost/tests/test_backuprestore.py b/src/yunohost/tests/test_backuprestore.py index 8af9f7149..fa3e180bd 100644 --- a/src/yunohost/tests/test_backuprestore.py +++ b/src/yunohost/tests/test_backuprestore.py @@ -473,7 +473,7 @@ def test_restore_app_script_failure_handling(monkeypatch, mocker): assert not _is_installed("wordpress") - with message(mocker, "restore_app_failed", app="wordpress"): + with message(mocker, "app_restore_failed", app="wordpress"): with raiseYunohostError(mocker, "restore_nothings_done"): backup_restore( system=None, name=backup_list()["archives"][0], apps=["wordpress"]