Adapt diagnosers code to load them as python modules instead of hooks

src/diagnosis.py

@@ -27,6 +27,8 @@
 import re
 import os
 import time
+import glob
+from importlib import import_module
 
 from moulinette import m18n, Moulinette
 from moulinette.utils import log
@@ -38,7 +40,6 @@ from moulinette.utils.filesystem import (
 )
 
 from yunohost.utils.error import YunohostError, YunohostValidationError
-from yunohost.hook import hook_list, hook_exec
 
 logger = log.getActionLogger("yunohost.diagnosis")
 
@@ -48,15 +49,13 @@ DIAGNOSIS_SERVER = "diagnosis.yunohost.org"
 
 
 def diagnosis_list():
-    all_categories_names = [h for h, _ in _list_diagnosis_categories()]
+    return {"categories": _list_diagnosis_categories()}
-    return {"categories": all_categories_names}
 
 
 def diagnosis_get(category, item):
 
     # Get all the categories
-    all_categories = _list_diagnosis_categories()
+    all_categories_names = _list_diagnosis_categories()
-    all_categories_names = [c for c, _ in all_categories]
 
     if category not in all_categories_names:
         raise YunohostValidationError(
@@ -84,8 +83,7 @@ def diagnosis_show(
         return
 
     # Get all the categories
-    all_categories = _list_diagnosis_categories()
+    all_categories_names = _list_diagnosis_categories()
-    all_categories_names = [category for category, _ in all_categories]
 
     # Check the requested category makes sense
     if categories == []:
@@ -174,8 +172,7 @@ def diagnosis_run(
         return
 
     # Get all the categories
-    all_categories = _list_diagnosis_categories()
+    all_categories_names = _list_diagnosis_categories()
-    all_categories_names = [category for category, _ in all_categories]
 
     # Check the requested category makes sense
     if categories == []:
@@ -192,10 +189,11 @@ def diagnosis_run(
     diagnosed_categories = []
     for category in categories:
         logger.debug("Running diagnosis for %s ..." % category)
-        path = [p for n, p in all_categories if n == category][0]
+
+        diagnoser = _load_diagnoser(category)
 
         try:
-            code, report = hook_exec(path, args={"force": force}, env=None)
+            code, report = diagnoser.diagnose(force=force)
         except Exception:
             import traceback
 
@@ -275,8 +273,7 @@ def _diagnosis_ignore(add_filter=None, remove_filter=None, list=False):
     def validate_filter_criterias(filter_):
 
         # Get all the categories
-        all_categories = _list_diagnosis_categories()
+        all_categories_names = _list_diagnosis_categories()
-        all_categories_names = [category for category, _ in all_categories]
 
         # Sanity checks for the provided arguments
         if len(filter_) == 0:
@@ -404,12 +401,8 @@ def add_ignore_flag_to_issues(report):
 
 
 class Diagnoser:
-    def __init__(self, args, env, loggers):
+    def __init__(self):
 
-        # FIXME ? That stuff with custom loggers is weird ... (mainly inherited from the bash hooks, idk)
-        self.logger_debug, self.logger_warning, self.logger_info = loggers
-        self.env = env
-        self.args = args or {}
         self.cache_file = Diagnoser.cache_file(self.id_)
         self.description = Diagnoser.get_description(self.id_)
 
@@ -424,10 +417,10 @@ class Diagnoser:
             os.makedirs(DIAGNOSIS_CACHE)
         return write_to_json(self.cache_file, report)
 
-    def diagnose(self):
+    def diagnose(self, force=False):
 
         if (
-            not self.args.get("force", False)
+            not force
             and self.cached_time_ago() < self.cache_duration
         ):
             self.logger_debug("Cache still valid : %s" % self.cache_file)
@@ -666,13 +659,36 @@ class Diagnoser:
 
 
 def _list_diagnosis_categories():
-    hooks_raw = hook_list("diagnosis", list_by="priority", show_info=True)["hooks"]
-    hooks = []
-    for _, some_hooks in sorted(hooks_raw.items(), key=lambda h: int(h[0])):
-        for name, info in some_hooks.items():
-            hooks.append((name, info["path"]))
 
-    return hooks
+    paths = glob.glob(os.path.dirname(__file__) + "/diagnosis/??-*.py")
+    names = sorted([os.path.basename(path)[: -len(".py")] for path in paths])
+
+    return names
+
+
+def _load_diagnoser(diagnoser_name):
+
+    logger.debug(f"Loading diagnoser {diagnoser_name}")
+
+    paths = glob.glob(os.path.dirname(__file__) + f"/diagnosis/??-{diagnoser_name}.py")
+
+    if len(paths) != 1:
+        raise YunohostError(f"Uhoh, found several matches (or none?) for diagnoser {diagnoser_name} : {paths}", raw_msg=True)
+
+    module_id = os.path.basename(paths[0][: -len(".py")])
+
+    try:
+        # this is python builtin method to import a module using a name, we
+        # use that to import the migration as a python object so we'll be
+        # able to run it in the next loop
+        module = import_module("yunohost.diagnosis.{}".format(module_id))
+        return module.MyDiagnoser()
+    except Exception as e:
+        import traceback
+
+        traceback.print_exc()
+
+        raise YunohostError(f"Failed to load diagnoser {diagnoser_name} : {e}", raw_msg=True)
 
 
 def _email_diagnosis_issues():

src/diagnosis/00-basesystem.py

@@ -4,13 +4,16 @@ import os
 import json
 import subprocess
 
+from moulinette.utils import log
 from moulinette.utils.process import check_output
 from moulinette.utils.filesystem import read_file, read_json, write_to_json
 from yunohost.diagnosis import Diagnoser
 from yunohost.utils.packages import ynh_packages_version
 
+logger = log.getActionLogger("yunohost.diagnosis")
 
-class BaseSystemDiagnoser(Diagnoser):
+
+class MyDiagnoser(Diagnoser):
 
     id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
     cache_duration = 600
@@ -172,7 +175,7 @@ class BaseSystemDiagnoser(Diagnoser):
         try:
             return int(n_failures)
         except Exception:
-            self.logger_warning(
+            logger.warning(
                 "Failed to parse number of recent auth failures, expected an int, got '%s'"
                 % n_failures
             )
@@ -196,7 +199,7 @@ class BaseSystemDiagnoser(Diagnoser):
             if not os.path.exists(dpkg_log) or os.path.getmtime(
                 cache_file
             ) > os.path.getmtime(dpkg_log):
-                self.logger_debug(
+                logger.debug(
                     "Using cached results for meltdown checker, from %s" % cache_file
                 )
                 return read_json(cache_file)[0]["VULNERABLE"]
@@ -209,7 +212,7 @@ class BaseSystemDiagnoser(Diagnoser):
         # example output from the script:
         # [{"NAME":"MELTDOWN","CVE":"CVE-2017-5754","VULNERABLE":false,"INFOS":"PTI mitigates the vulnerability"}]
         try:
-            self.logger_debug("Running meltdown vulnerability checker")
+            logger.debug("Running meltdown vulnerability checker")
             call = subprocess.Popen(
                 "bash %s --batch json --variant 3" % SCRIPT_PATH,
                 shell=True,
@@ -231,7 +234,7 @@ class BaseSystemDiagnoser(Diagnoser):
             # stuff which should be the last line
             output = output.strip()
             if "\n" in output:
-                self.logger_debug("Original meltdown checker output : %s" % output)
+                logger.debug("Original meltdown checker output : %s" % output)
                 output = output.split("\n")[-1]
 
             CVEs = json.loads(output)
@@ -241,18 +244,14 @@ class BaseSystemDiagnoser(Diagnoser):
             import traceback
 
             traceback.print_exc()
-            self.logger_warning(
+            logger.warning(
                 "Something wrong happened when trying to diagnose Meltdown vunerability, exception: %s"
                 % e
             )
             raise Exception("Command output for failed meltdown check: '%s'" % output)
 
-        self.logger_debug(
+        logger.debug(
             "Writing results from meltdown checker to cache file, %s" % cache_file
         )
         write_to_json(cache_file, CVEs)
         return CVEs[0]["VULNERABLE"]
-
-
-def main(args, env, loggers):
-    return BaseSystemDiagnoser(args, env, loggers).diagnose()

src/diagnosis/10-ip.py

@@ -4,6 +4,7 @@ import re
 import os
 import random
 
+from moulinette.utils import log
 from moulinette.utils.network import download_text
 from moulinette.utils.process import check_output
 from moulinette.utils.filesystem import read_file
@@ -11,8 +12,10 @@ from moulinette.utils.filesystem import read_file
 from yunohost.diagnosis import Diagnoser
 from yunohost.utils.network import get_network_interfaces
 
+logger = log.getActionLogger("yunohost.diagnosis")
 
-class IPDiagnoser(Diagnoser):
+
+class MyDiagnoser(Diagnoser):
 
     id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
     cache_duration = 600
@@ -144,7 +147,7 @@ class IPDiagnoser(Diagnoser):
             )
 
         if not any(is_default_route(r) for r in routes):
-            self.logger_debug(
+            logger.debug(
                 "No default route for IPv%s, so assuming there's no IP address for that version"
                 % protocol
             )
@@ -220,11 +223,7 @@ class IPDiagnoser(Diagnoser):
         try:
             return download_text(url, timeout=30).strip()
         except Exception as e:
-            self.logger_debug(
+            logger.debug(
                 "Could not get public IPv%s : %s" % (str(protocol), str(e))
             )
             return None
-
-
-def main(args, env, loggers):
-    return IPDiagnoser(args, env, loggers).diagnose()

src/diagnosis/12-dnsrecords.py

@@ -6,6 +6,7 @@ import re
 from datetime import datetime, timedelta
 from publicsuffix2 import PublicSuffixList
 
+from moulinette.utils import log
 from moulinette.utils.process import check_output
 
 from yunohost.utils.dns import (
@@ -18,8 +19,9 @@ from yunohost.diagnosis import Diagnoser
 from yunohost.domain import domain_list, _get_maindomain
 from yunohost.dns import _build_dns_conf, _get_dns_zone_for_domain
 
+logger = log.getActionLogger("yunohost.diagnosis")
 
-class DNSRecordsDiagnoser(Diagnoser):
+class MyDiagnoser(Diagnoser):
 
     id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
     cache_duration = 600
@@ -31,7 +33,7 @@ class DNSRecordsDiagnoser(Diagnoser):
 
         major_domains = domain_list(exclude_subdomains=True)["domains"]
         for domain in major_domains:
-            self.logger_debug("Diagnosing DNS conf for %s" % domain)
+            logger.debug("Diagnosing DNS conf for %s" % domain)
 
             for report in self.check_domain(
                 domain,
@@ -223,7 +225,7 @@ class DNSRecordsDiagnoser(Diagnoser):
                         )
                     )
                 else:
-                    self.logger_debug("Dyndns domain: %s" % (domain))
+                    logger.debug("Dyndns domain: %s" % (domain))
                 continue
 
             expire_in = expire_date - datetime.now()
@@ -298,7 +300,3 @@ class DNSRecordsDiagnoser(Diagnoser):
                 return datetime.strptime(match.group(1), "%d-%b-%Y")
 
         return "expiration_not_found"
-
-
-def main(args, env, loggers):
-    return DNSRecordsDiagnoser(args, env, loggers).diagnose()

src/diagnosis/14-ports.py

@@ -6,7 +6,7 @@ from yunohost.diagnosis import Diagnoser
 from yunohost.service import _get_services
 
 
-class PortsDiagnoser(Diagnoser):
+class MyDiagnoser(Diagnoser):
 
     id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
     cache_duration = 600
@@ -146,7 +146,3 @@ class PortsDiagnoser(Diagnoser):
                             "diagnosis_ports_forwarding_tip",
                         ],
                     )
-
-
-def main(args, env, loggers):
-    return PortsDiagnoser(args, env, loggers).diagnose()

src/diagnosis/21-web.py

@@ -13,7 +13,7 @@ from yunohost.utils.dns import is_special_use_tld
 DIAGNOSIS_SERVER = "diagnosis.yunohost.org"
 
 
-class WebDiagnoser(Diagnoser):
+class MyDiagnoser(Diagnoser):
 
     id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
     cache_duration = 600
@@ -198,7 +198,3 @@ class WebDiagnoser(Diagnoser):
                         summary="diagnosis_http_partially_unreachable",
                         details=[detail.replace("error_http_check", "diagnosis_http")],
                     )
-
-
-def main(args, env, loggers):
-    return WebDiagnoser(args, env, loggers).diagnose()

src/diagnosis/24-mail.py

@@ -6,6 +6,7 @@ import re
 
 from subprocess import CalledProcessError
 
+from moulinette.utils import log
 from moulinette.utils.process import check_output
 from moulinette.utils.filesystem import read_yaml
 
@@ -16,8 +17,10 @@ from yunohost.utils.dns import dig
 
 DEFAULT_DNS_BLACKLIST = "/usr/share/yunohost/dnsbl_list.yml"
 
+logger = log.getActionLogger("yunohost.diagnosis")
 
-class MailDiagnoser(Diagnoser):
+
+class MyDiagnoser(Diagnoser):
 
     id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
     cache_duration = 600
@@ -42,7 +45,7 @@ class MailDiagnoser(Diagnoser):
             "check_queue",  # i18n: diagnosis_mail_queue_ok
         ]
         for check in checks:
-            self.logger_debug("Running " + check)
+            logger.debug("Running " + check)
             reports = list(getattr(self, check)())
             for report in reports:
                 yield report
@@ -292,7 +295,3 @@ class MailDiagnoser(Diagnoser):
                 if global_ipv6:
                     outgoing_ips.append(global_ipv6)
         return (outgoing_ipversions, outgoing_ips)
-
-
-def main(args, env, loggers):
-    return MailDiagnoser(args, env, loggers).diagnose()

src/diagnosis/30-services.py

@@ -6,7 +6,7 @@ from yunohost.diagnosis import Diagnoser
 from yunohost.service import service_status
 
 
-class ServicesDiagnoser(Diagnoser):
+class MyDiagnoser(Diagnoser):
 
     id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
     cache_duration = 300
@@ -41,7 +41,3 @@ class ServicesDiagnoser(Diagnoser):
                 item["summary"] = "diagnosis_services_running"
 
             yield item
-
-
-def main(args, env, loggers):
-    return ServicesDiagnoser(args, env, loggers).diagnose()

src/diagnosis/50-systemresources.py

@@ -9,7 +9,7 @@ from moulinette.utils.process import check_output
 from yunohost.diagnosis import Diagnoser
 
 
-class SystemResourcesDiagnoser(Diagnoser):
+class MyDiagnoser(Diagnoser):
 
     id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
     cache_duration = 300
@@ -214,7 +214,3 @@ def round_(n):
     if n > 10:
         n = int(round(n))
     return n
-
-
-def main(args, env, loggers):
-    return SystemResourcesDiagnoser(args, env, loggers).diagnose()

src/diagnosis/70-regenconf.py

@@ -9,7 +9,7 @@ from yunohost.regenconf import _get_regenconf_infos, _calculate_hash
 from moulinette.utils.filesystem import read_file
 
 
-class RegenconfDiagnoser(Diagnoser):
+class MyDiagnoser(Diagnoser):
 
     id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
     cache_duration = 300
@@ -70,7 +70,3 @@ class RegenconfDiagnoser(Diagnoser):
             for path, hash_ in infos["conffiles"].items():
                 if hash_ != _calculate_hash(path):
                     yield {"path": path, "category": category}
-
-
-def main(args, env, loggers):
-    return RegenconfDiagnoser(args, env, loggers).diagnose()

src/diagnosis/80-apps.py

@@ -7,7 +7,7 @@ from yunohost.app import app_list
 from yunohost.diagnosis import Diagnoser
 
 
-class AppDiagnoser(Diagnoser):
+class MyDiagnoser(Diagnoser):
 
     id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
     cache_duration = 300
@@ -90,7 +90,3 @@ class AppDiagnoser(Diagnoser):
             == 0
         ):
             yield ("error", "diagnosis_apps_deprecated_practices")
-
-
-def main(args, env, loggers):
-    return AppDiagnoser(args, env, loggers).diagnose()