diff --git a/locales/en.json b/locales/en.json index f7a65a883..47e452450 100644 --- a/locales/en.json +++ b/locales/en.json @@ -219,6 +219,7 @@ "good_practices_about_user_password": "You are now about to define a new user password. The password should be at least 8 characters - though it is good practice to use longer password (i.e. a passphrase) and/or to use various kind of characters (uppercase, lowercase, digits and special characters).", "hook_exec_failed": "Script execution failed: {path:s}", "hook_exec_not_terminated": "Script execution did not finish properly: {path:s}", + "hook_json_return_error": "Failed to read return from hook {path:s}. Error: {msg:s}. Raw content: {raw_content}", "hook_list_by_invalid": "Invalid property to list hook by", "hook_name_unknown": "Unknown hook name '{name:s}'", "installation_complete": "Installation complete", diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 8561e2667..de759f04f 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -523,7 +523,7 @@ def app_change_url(operation_logger, auth, app, domain, path): os.system('chmod +x %s' % os.path.join(os.path.join(APP_TMP_FOLDER, "scripts", "change_url"))) if hook_exec(os.path.join(APP_TMP_FOLDER, 'scripts/change_url'), - args=args_list, env=env_dict) != 0: + args=args_list, env=env_dict)[0] != 0: msg = "Failed to change '%s' url." % app logger.error(msg) operation_logger.error(msg) @@ -654,7 +654,7 @@ def app_upgrade(auth, app=[], url=None, file=None): # Execute App upgrade script os.system('chown -hR admin: %s' % INSTALL_TMP) if hook_exec(extracted_app_folder + '/scripts/upgrade', - args=args_list, env=env_dict) != 0: + args=args_list, env=env_dict)[0] != 0: msg = m18n.n('app_upgrade_failed', app=app_instance_name) not_upgraded_apps.append(app_instance_name) logger.error(msg) @@ -847,7 +847,7 @@ def app_install(operation_logger, auth, app, label=None, args=None, no_remove_on install_retcode = hook_exec( os.path.join(extracted_app_folder, 'scripts/install'), args=args_list, env=env_dict - ) + )[0] except (KeyboardInterrupt, EOFError): install_retcode = -1 except Exception: @@ -872,7 +872,7 @@ def app_install(operation_logger, auth, app, label=None, args=None, no_remove_on remove_retcode = hook_exec( os.path.join(extracted_app_folder, 'scripts/remove'), args=[app_instance_name], env=env_dict_remove - ) + )[0] if remove_retcode != 0: msg = m18n.n('app_not_properly_removed', app=app_instance_name) @@ -963,7 +963,7 @@ def app_remove(operation_logger, auth, app): operation_logger.flush() if hook_exec('/tmp/yunohost_remove/scripts/remove', args=args_list, - env=env_dict) == 0: + env=env_dict)[0] == 0: logger.success(m18n.n('app_removed', app=app)) hook_callback('post_app_remove', args=args_list, env=env_dict) @@ -1562,7 +1562,7 @@ def app_action_run(app, action, args=None): env=env_dict, chdir=cwd, user=action_declaration.get("user", "root"), - ) + )[0] if retcode not in action_declaration.get("accepted_return_codes", [0]): raise YunohostError("Error while executing action '%s' of app '%s': return code %s" % (action, app, retcode), raw_msg=True) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index f9505fb66..53919f2cc 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -593,8 +593,15 @@ class BackupManager(): env=env_dict, chdir=self.work_dir) - if ret["succeed"] != []: - self.system_return = ret["succeed"] + ret_succeed = {hook: {path:result["state"] for path, result in infos.items()} + for hook, infos in ret.items() + if any(result["state"] == "succeed" for result in infos.values())} + ret_failed = {hook: {path:result["state"] for path, result in infos.items.items()} + for hook, infos in ret.items() + if any(result["state"] == "failed" for result in infos.values())} + + if ret_succeed.keys() != []: + self.system_return = ret_succeed # Add files from targets (which they put in the CSV) to the list of # files to backup @@ -610,7 +617,7 @@ class BackupManager(): restore_hooks = hook_list("restore")["hooks"] - for part in ret['succeed'].keys(): + for part in ret_succeed.keys(): if part in restore_hooks: part_restore_hooks = hook_info("restore", part)["hooks"] for hook in part_restore_hooks: @@ -620,7 +627,7 @@ class BackupManager(): logger.warning(m18n.n('restore_hook_unavailable', hook=part)) self.targets.set_result("system", part, "Warning") - for part in ret['failed'].keys(): + for part in ret_failed.keys(): logger.error(m18n.n('backup_system_part_failed', part=part)) self.targets.set_result("system", part, "Error") @@ -682,7 +689,7 @@ class BackupManager(): subprocess.call(['install', '-Dm555', app_script, tmp_script]) hook_exec(tmp_script, args=[tmp_app_bkp_dir, app], - raise_on_error=True, chdir=tmp_app_bkp_dir, env=env_dict) + raise_on_error=True, chdir=tmp_app_bkp_dir, env=env_dict)[0] self._import_to_list_to_backup(env_dict["YNH_BACKUP_CSV"]) except: @@ -1177,16 +1184,21 @@ class RestoreManager(): env=env_dict, chdir=self.work_dir) - for part in ret['succeed'].keys(): + ret_succeed = [hook for hook, infos in ret.items() + if any(result["state"] == "succeed" for result in infos.values())] + ret_failed = [hook for hook, infos in ret.items() + if any(result["state"] == "failed" for result in infos.values())] + + for part in ret_succeed: self.targets.set_result("system", part, "Success") error_part = [] - for part in ret['failed'].keys(): + for part in ret_failed: logger.error(m18n.n('restore_system_part_failed', part=part)) self.targets.set_result("system", part, "Error") error_part.append(part) - if ret['failed']: + if ret_failed: operation_logger.error(m18n.n('restore_system_part_failed', part=', '.join(error_part))) else: operation_logger.success() @@ -1301,7 +1313,7 @@ class RestoreManager(): args=[app_backup_in_archive, app_instance_name], chdir=app_backup_in_archive, raise_on_error=True, - env=env_dict) + env=env_dict)[0] except: msg = m18n.n('restore_app_failed', app=app_instance_name) logger.exception(msg) @@ -1326,7 +1338,7 @@ class RestoreManager(): # Execute remove script # TODO: call app_remove instead if hook_exec(remove_script, args=[app_instance_name], - env=env_dict_remove) != 0: + env=env_dict_remove)[0] != 0: msg = m18n.n('app_not_properly_removed', app=app_instance_name) logger.warning(msg) operation_logger.error(msg) @@ -1932,8 +1944,9 @@ class CustomBackupMethod(BackupMethod): ret = hook_callback('backup_method', [self.method], args=self._get_args('need_mount')) - - self._need_mount = True if ret['succeed'] else False + ret_succeed = [hook for hook, infos in ret.items() + if any(result["state"] == "succeed" for result in infos.values())] + self._need_mount = True if ret_succeed else False return self._need_mount def backup(self): @@ -1946,7 +1959,10 @@ class CustomBackupMethod(BackupMethod): ret = hook_callback('backup_method', [self.method], args=self._get_args('backup')) - if ret['failed']: + + ret_failed = [hook for hook, infos in ret.items() + if any(result["state"] == "failed" for result in infos.values())] + if ret_failed: raise YunohostError('backup_custom_backup_error') def mount(self, restore_manager): @@ -1959,7 +1975,10 @@ class CustomBackupMethod(BackupMethod): super(CustomBackupMethod, self).mount(restore_manager) ret = hook_callback('backup_method', [self.method], args=self._get_args('mount')) - if ret['failed']: + + ret_failed = [hook for hook, infos in ret.items() + if any(result["state"] == "failed" for result in infos.values())] + if ret_failed: raise YunohostError('backup_custom_mount_error') def _get_args(self, action): diff --git a/src/yunohost/hook.py b/src/yunohost/hook.py index ca93c7f03..c4605b6e8 100644 --- a/src/yunohost/hook.py +++ b/src/yunohost/hook.py @@ -31,6 +31,7 @@ from glob import iglob from moulinette import m18n from yunohost.utils.error import YunohostError from moulinette.utils import log +from moulinette.utils.filesystem import read_json HOOK_FOLDER = '/usr/share/yunohost/hooks/' CUSTOM_HOOK_FOLDER = '/etc/yunohost/hooks.d/' @@ -228,7 +229,7 @@ def hook_callback(action, hooks=[], args=None, no_trace=False, chdir=None, (name, priority, path, succeed) as arguments """ - result = {'succeed': {}, 'failed': {}} + result = {} hooks_dict = {} # Retrieve hooks @@ -278,20 +279,20 @@ def hook_callback(action, hooks=[], args=None, no_trace=False, chdir=None, try: hook_args = pre_callback(name=name, priority=priority, path=path, args=args) - hook_exec(path, args=hook_args, chdir=chdir, env=env, - no_trace=no_trace, raise_on_error=True) + hook_return = hook_exec(path, args=hook_args, chdir=chdir, env=env, + no_trace=no_trace, raise_on_error=True)[1] except YunohostError as e: state = 'failed' + hook_return = {} logger.error(e.strerror, exc_info=1) post_callback(name=name, priority=priority, path=path, succeed=False) else: post_callback(name=name, priority=priority, path=path, succeed=True) - try: - result[state][name].append(path) - except KeyError: - result[state][name] = [path] + if not name in result: + result[name] = {} + result[name][path] = {'state' : state, 'stdreturn' : hook_return } return result @@ -339,6 +340,11 @@ def hook_exec(path, args=None, raise_on_error=False, no_trace=False, stdinfo = os.path.join(tempfile.mkdtemp(), "stdinfo") env['YNH_STDINFO'] = stdinfo + stdreturn = os.path.join(tempfile.mkdtemp(), "stdreturn") + with open(stdreturn, 'w') as f: + f.write('') + env['YNH_STDRETURN'] = stdreturn + # Construct command to execute if user == "root": command = ['sh', '-c'] @@ -385,10 +391,27 @@ def hook_exec(path, args=None, raise_on_error=False, no_trace=False, raise YunohostError('hook_exec_not_terminated', path=path) else: logger.error(m18n.n('hook_exec_not_terminated', path=path)) - return 1 + return 1, {} elif raise_on_error and returncode != 0: raise YunohostError('hook_exec_failed', path=path) - return returncode + + raw_content = None + try: + with open(stdreturn, 'r') as f: + raw_content = f.read() + if raw_content != '': + returnjson = read_json(stdreturn) + else: + returnjson = {} + except Exception as e: + raise YunohostError('hook_json_return_error', path=path, msg=str(e), + raw_content=raw_content) + finally: + stdreturndir = os.path.split(stdreturn)[0] + os.remove(stdreturn) + os.rmdir(stdreturndir) + + return returncode, returnjson def _extract_filename_parts(filename): diff --git a/src/yunohost/service.py b/src/yunohost/service.py index 60729053b..61274aaac 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -493,12 +493,16 @@ def service_regen_conf(operation_logger, names=[], with_diff=False, force=False, pre_result = hook_callback('conf_regen', names, pre_callback=_pre_call) - # Update the services name - names = pre_result['succeed'].keys() + # Keep only the hook names with at least one success + names = [hook for hook, infos in pre_result.items() + if any(result["state"] == "succeed" for result in infos.values())] + # FIXME : what do in case of partial success/failure ... if not names: + ret_failed = [hook for hook, infos in pre_result.items() + if any(result["state"] == "failed" for result in infos.values())] raise YunohostError('service_regenconf_failed', - services=', '.join(pre_result['failed'])) + services=', '.join(ret_failed)) # Set the processing method _regen = _process_regen_conf if not dry_run else lambda *a, **k: True