From 76c7b3b3dbb2ce1178261f4ba4712a6dda119b9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Lebleu?= Date: Mon, 5 Oct 2015 15:01:40 +0200 Subject: [PATCH] [fix] Restructure app restauration and catch app script failure * Do not copy again the restore app script * Set app folder permissions and properly remove it on failure * Add a raise_on_error argument to hook_exec * Review displayed messages --- data/actionsmap/yunohost.yml | 3 + lib/yunohost/backup.py | 109 +++++++++++++++++++---------------- lib/yunohost/hook.py | 5 +- locales/en.json | 9 +-- 4 files changed, 71 insertions(+), 55 deletions(-) diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml index 66744525b..1693ac271 100644 --- a/data/actionsmap/yunohost.yml +++ b/data/actionsmap/yunohost.yml @@ -1346,3 +1346,6 @@ hook: -a: full: --args help: Arguments to pass to the script + --raise-on-error: + help: Raise if the script returns a non-zero exit code + action: store_true diff --git a/lib/yunohost/backup.py b/lib/yunohost/backup.py index 0e9ab63c5..7a911ddec 100644 --- a/lib/yunohost/backup.py +++ b/lib/yunohost/backup.py @@ -162,31 +162,21 @@ def backup_create(name=None, description=None, output_directory=None, for app_id in apps_filtered: app_setting_path = '/etc/yunohost/apps/' + app_id - tmp_app_dir = '{:s}/apps/{:s}'.format(tmp_dir, app_id) - - # Check if the app has a backup script + # Check if the app has a backup and restore script app_script = app_setting_path + '/scripts/backup' + app_restore_script = app_setting_path + '/scripts/restore' if not os.path.isfile(app_script): logger.warning("backup script '%s' not found", app_script) - msignals.display(m18n.n('unbackup_app', app_id), + msignals.display(m18n.n('unbackup_app', app=app_id), 'warning') continue - - # Copy the app restore script - app_restore_script = app_setting_path + '/scripts/restore' - if os.path.isfile(app_script): - try: - filesystem.mkdir(tmp_app_dir, 0750, True, uid='admin') - shutil.copy(app_restore_script, tmp_app_dir) - except: - logger.exception("error while copying restore script of '%s'", app_id) - msignals.display(m18n.n('restore_app_copy_failed', app=app_id), - 'warning') - else: - logger.warning("restore script '%s' not found", app_script) - msignals.display(m18n.n('unrestorable_app', app_id), + elif not os.path.isfile(app_restore_script): + logger.warning("restore script '%s' not found", + app_restore_script) + msignals.display(m18n.n('unrestore_app', app=app_id), 'warning') + tmp_app_dir = '{:s}/apps/{:s}'.format(tmp_dir, app_id) tmp_app_bkp_dir = tmp_app_dir + '/backup' msignals.display(m18n.n('backup_running_app_script', app_id)) try: @@ -196,7 +186,8 @@ def backup_create(name=None, description=None, output_directory=None, # Copy app backup script in a temporary folder and execute it subprocess.call(['install', '-Dm555', app_script, tmp_script]) - hook_exec(tmp_script, args=[tmp_app_bkp_dir, app_id]) + hook_exec(tmp_script, args=[tmp_app_bkp_dir, app_id], + raise_on_error=True) except: logger.exception("error while executing backup of '%s'", app_id) msignals.display(m18n.n('backup_app_failed', app=app_id), @@ -351,7 +342,7 @@ def backup_restore(name, hooks=[], apps=[], ignore_apps=False, ignore_hooks=Fals if not ignore_hooks: if hooks is None or len(hooks)==0: hooks=info['hooks'].keys() - + hooks_filtered=list(set(hooks) & set(info['hooks'].keys())) hooks_unexecuted=set(hooks) - set(info['hooks'].keys()) for hook in hooks_unexecuted: @@ -359,49 +350,67 @@ def backup_restore(name, hooks=[], apps=[], ignore_apps=False, ignore_hooks=Fals msignals.display(m18n.n('backup_hook_unavailable', hook), 'warning') msignals.display(m18n.n('restore_running_hooks')) hook_callback('restore', hooks_filtered, args=[tmp_dir]) - + # Add apps restore hook if not ignore_apps: + from yunohost.app import _is_installed + # Filter applications to restore apps_list = set(info['apps'].keys()) apps_filtered = set() - if not apps: - apps=apps_list - - from yunohost.app import _is_installed - for app_id in apps: - if app_id not in apps_list: - logger.warning("app '%s' not found", app_id) - msignals.display(m18n.n('unrestore_app', app_id), 'warning') - elif _is_installed(app_id): + if apps: + for a in apps: + if a not in apps_list: + logger.warning("app '%s' not found in the backup '%s'", + a, archive_file) + msignals.display(m18n.n('backup_archive_app_not_found', + app=a), + 'error') + else: + apps_filtered.add(a) + else: + apps_filtered = apps_list + + for app_id in apps_filtered: + tmp_app_dir = '{:s}/apps/{:s}'.format(tmp_dir, app_id) + + # Check if the app is not already installed + if _is_installed(app_id): logger.warning("app '%s' already installed", app_id) - msignals.display(m18n.n('restore_already_installed_app', app=app_id), 'warning') - elif not os.path.isfile('{:s}/apps/{:s}/restore'.format(tmp_dir, app_id)): - logger.warning("backup for '%s' doesn't contain a restore script", app_id) - msignals.display(m18n.n('no_restore_script', app=app_id), 'warning') - else: - apps_filtered.add(app_id) + msignals.display(m18n.n('restore_already_installed_app', + app=app_id), + 'error') + continue - for app_id in apps_filtered: - app_bkp_dir='{:s}/apps/{:s}'.format(tmp_dir, app_id) + # Check if the app has a restore script + app_script = tmp_app_dir + '/settings/scripts/restore' + if not os.path.isfile(app_script): + logger.warning("restore script for the app '%s' not found " \ + "in the backup '%s'", app_id, archive_file) + msignals.display(m18n.n('unrestore_app', app=app_id), 'warning') + continue + + tmp_script = '/tmp/restore_' + app_id + app_setting_path = '/etc/yunohost/apps/' + app_id + msignals.display(m18n.n('restore_running_app_script', app=app_id)) try: - # Copy app settings - app_setting_path = '/etc/yunohost/apps/' + app_id - shutil.copytree(app_bkp_dir + '/settings', app_setting_path ) - - # Execute app restore script - app_restore_script=app_bkp_dir+'/restore' - tmp_script = '/tmp/restore_%s_%s' % (name,app_id) - subprocess.call(['install', '-Dm555', app_restore_script, tmp_script]) - hook_exec(tmp_script, args=[app_bkp_dir+'/backup', app_id]) + # Copy app settings and set permissions + shutil.copytree(tmp_app_dir + '/settings', app_setting_path) + filesystem.chmod(app_setting_path, 0555, 0444, True) + filesystem.chmod(app_setting_path + '/settings.yml', 0400) + # Execute app restore script + subprocess.call(['install', '-Dm555', app_script, tmp_script]) + hook_exec(tmp_script, args=[tmp_app_dir + '/backup', app_id], + raise_on_error=True) except: logger.exception("error while restoring backup of '%s'", app_id) msignals.display(m18n.n('restore_app_failed', app=app_id), 'error') - # Cleaning settings directory - shutil.rmtree(app_setting_path + '/settings', ignore_errors=True) - + # Cleaning app directory + shutil.rmtree(app_setting_path, ignore_errors=True) + finally: + filesystem.rm(tmp_script, force=True) # Remove temporary directory os.system('rm -rf %s' % tmp_dir) diff --git a/lib/yunohost/hook.py b/lib/yunohost/hook.py index 96ffff35d..1661edc83 100644 --- a/lib/yunohost/hook.py +++ b/lib/yunohost/hook.py @@ -260,13 +260,14 @@ def hook_check(file): return {} -def hook_exec(file, args=None): +def hook_exec(file, args=None, raise_on_error=False): """ Execute hook from a file with arguments Keyword argument: file -- Script to execute args -- Arguments to pass to the script + raise_on_error -- Raise if the script returns a non-zero exit code """ from moulinette.utils.stream import NonBlockingStreamReader @@ -344,6 +345,8 @@ def hook_exec(file, args=None): msignals.display(line.rstrip(), 'log') stream.close() + if raise_on_error and returncode != 0: + raise MoulinetteError(m18n.n('hook_exec_failed')) return returncode diff --git a/locales/en.json b/locales/en.json index ba4e6b072..1890f5c73 100644 --- a/locales/en.json +++ b/locales/en.json @@ -86,6 +86,7 @@ "hook_name_unknown" : "Unknown hook name '{:s}'", "hook_choice_invalid" : "Invalid choice '{:s}'", "hook_argument_missing" : "Missing argument '{:s}'", + "hook_exec_failed" : "Script execution failed", "mountpoint_unknown" : "Unknown mountpoint", "unit_unknown" : "Unknown unit '{:s}'", @@ -150,6 +151,7 @@ "backup_archive_open_failed" : "Unable to open the backup archive", "backup_archive_name_unknown" : "Unknown local backup archive named '{:s}'", "backup_archive_name_exists" : "Backup archive name already exists", + "backup_archive_app_not_found" : "App '{app:s}' not found in the backup archive", "backup_app_failed" : "Unable to back up the app '{app:s}'", "backup_nothings_done" : "There is nothing to save", "backup_cleaning_failed" : "Unable to clean backup directory", @@ -159,14 +161,13 @@ "restore_confirm_yunohost_installed" : "Do you really want to restore an already installed system? [{answers:s}]", "restore_app_failed" : "Unable to restore the app '{app:s}'", "restore_running_hooks" : "Running restoration hooks...", + "restore_running_app_script" : "Running restore script of app '{app:s}'...", "restore_failed" : "Unable to restore the system", "restore_complete" : "Restore complete", "restore_already_installed_app": "An app is already installed with the id '{app:s}'", - "unbackup_app" : "App '{:s}' will not be saved", - "unrestorable_app" : "App '{:s}' will not be restored", - "restore_app_copy_failed" : "Unable to copy the restore script of app '{app:s}'", + "unbackup_app" : "App '{app:s}' will not be saved", "no_restore_script": "No restore script found for the app '{app:s}'", - "unrestore_app" : "App '{:s}' will not be restored", + "unrestore_app" : "App '{app:s}' will not be restored", "backup_delete_error" : "Unable to delete '{:s}'", "backup_deleted" : "Backup successfully deleted",