Merge pull request #526 from YunoHost/hook_return

[enh] Allow hooks to return data

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",

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)

src/yunohost/backup.py

@@ -593,8 +593,15 @@ class BackupManager():
                             env=env_dict,
                             chdir=self.work_dir)
 
-        if ret["succeed"] != []:
+        ret_succeed = {hook: {path:result["state"] for path, result in infos.items()}
-            self.system_return = ret["succeed"]
+                       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'))
-
+        ret_succeed = [hook for hook, infos in ret.items()
-        self._need_mount = True if ret['succeed'] else False
+                       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):

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,
+                hook_return = hook_exec(path, args=hook_args, chdir=chdir, env=env,
-                          no_trace=no_trace, raise_on_error=True)
+                          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:
+            if not name in result:
-                result[state][name].append(path)
+                result[name] = {}
-            except KeyError:
+            result[name][path] = {'state' : state, 'stdreturn' : hook_return }
-                result[state][name] = [path]
     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):

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
+    # Keep only the hook names with at least one success
-    names = pre_result['succeed'].keys()
+    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