From d879d27208a985886d0943d549d1c33a98df2d03 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 5 Nov 2019 21:47:33 +0100 Subject: [PATCH 01/14] Add test-status to have a custom status check for service like postfix and yunohost-firewall --- data/templates/yunohost/services.yml | 2 ++ src/yunohost/service.py | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/data/templates/yunohost/services.yml b/data/templates/yunohost/services.yml index 1c0ee031f..a9948fc69 100644 --- a/data/templates/yunohost/services.yml +++ b/data/templates/yunohost/services.yml @@ -10,6 +10,7 @@ dovecot: log: [/var/log/mail.log,/var/log/mail.err] postfix: log: [/var/log/mail.log,/var/log/mail.err] + test-status: systemctl show postfix@- | grep -q "^SubState=running" rspamd: log: /var/log/rspamd/rspamd.log redis-server: @@ -29,6 +30,7 @@ yunohost-api: log: /var/log/yunohost/yunohost-api.log yunohost-firewall: need_lock: true + test-status: iptables -S | grep "^-A INPUT" | grep " --dport" | grep -q ACCEPT nslcd: log: /var/log/syslog glances: null diff --git a/src/yunohost/service.py b/src/yunohost/service.py index 17a3cc83e..96a7061e3 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -330,6 +330,11 @@ def service_status(names=[]): else: result[name]['active_at'] = "unknown" + # 'test-status' is an optional field to test the status of the service using a custom command + if "test-status" in services[name]: + status = os.system(services[name]["test-status"]) + result[name]["status"] = "running" if status == 0 else "failed" + if len(names) == 1: return result[names[0]] return result From e15d8e72624b225840713abf5e801be7e2c07ccf Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 5 Nov 2019 21:52:35 +0100 Subject: [PATCH 02/14] Add test about configuration validity --- data/templates/yunohost/services.yml | 3 +++ src/yunohost/service.py | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/data/templates/yunohost/services.yml b/data/templates/yunohost/services.yml index a9948fc69..fd9d06eac 100644 --- a/data/templates/yunohost/services.yml +++ b/data/templates/yunohost/services.yml @@ -1,5 +1,6 @@ nginx: log: /var/log/nginx + test-conf: nginx -t avahi-daemon: log: /var/log/daemon.log dnsmasq: @@ -20,12 +21,14 @@ mysql: alternates: ['mariadb'] ssh: log: /var/log/auth.log + test-conf: sshd -t metronome: log: [/var/log/metronome/metronome.log,/var/log/metronome/metronome.err] slapd: log: /var/log/syslog php7.0-fpm: log: /var/log/php7.0-fpm.log + test-conf: php-fpm7.0 --test yunohost-api: log: /var/log/yunohost/yunohost-api.log yunohost-firewall: diff --git a/src/yunohost/service.py b/src/yunohost/service.py index 96a7061e3..8f1e55669 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -297,6 +297,7 @@ def service_status(names=[]): 'active_at': "unknown", 'description': "Error: failed to get information for this service, it doesn't exists for systemd", 'service_file_path': "unknown", + 'configuration': "unknown", } else: @@ -318,6 +319,7 @@ def service_status(names=[]): 'active': str(status.get("ActiveState", "unknown")), 'description': description, 'service_file_path': str(status.get("FragmentPath", "unknown")), + 'configuration': "unknown", } # Fun stuff™ : to obtain the enabled/disabled status for sysv services, @@ -335,6 +337,11 @@ def service_status(names=[]): status = os.system(services[name]["test-status"]) result[name]["status"] = "running" if status == 0 else "failed" + # 'test-status' is an optional field to test the status of the service using a custom command + if "test-conf" in services[name]: + conf = os.system(services[name]["test-conf"] + " &>/dev/null") + result[name]["configuration"] = "valid" if conf == 0 else "broken" + if len(names) == 1: return result[names[0]] return result From 9b77123938f6590d01deedb7d549bf85b3b3b2f0 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 5 Nov 2019 21:54:05 +0100 Subject: [PATCH 03/14] loaded -> start_on_boot, which is more meaningful ... --- src/yunohost/service.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/yunohost/service.py b/src/yunohost/service.py index 8f1e55669..abaf58f09 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -292,7 +292,7 @@ def service_status(names=[]): logger.error("Failed to get status information via dbus for service %s, systemctl didn't recognize this service ('NoSuchUnit')." % name) result[name] = { 'status': "unknown", - 'loaded': "unknown", + 'start_on_boot': "unknown", 'active': "unknown", 'active_at': "unknown", 'description': "Error: failed to get information for this service, it doesn't exists for systemd", @@ -315,7 +315,7 @@ def service_status(names=[]): result[name] = { 'status': str(status.get("SubState", "unknown")), - 'loaded': str(status.get("UnitFileState", "unknown")), + 'start_on_boot': str(status.get("UnitFileState", "unknown")), 'active': str(status.get("ActiveState", "unknown")), 'description': description, 'service_file_path': str(status.get("FragmentPath", "unknown")), @@ -324,8 +324,8 @@ def service_status(names=[]): # Fun stuff™ : to obtain the enabled/disabled status for sysv services, # gotta do this ... cf code of /lib/systemd/systemd-sysv-install - if result[name]["loaded"] == "generated": - result[name]["loaded"] = "enabled" if glob("/etc/rc[S5].d/S??"+name) else "disabled" + if result[name]["start_on_boot"] == "generated": + result[name]["start_on_boot"] = "enabled" if glob("/etc/rc[S5].d/S??"+name) else "disabled" if "ActiveEnterTimestamp" in status: result[name]['active_at'] = datetime.utcfromtimestamp(status["ActiveEnterTimestamp"] / 1000000) From 5d46f3ef8883e451fe36af03907e5df5cad76998 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 5 Nov 2019 21:57:22 +0100 Subject: [PATCH 04/14] Imho we don't need those 'active' and 'service_file_path' info... --- src/yunohost/service.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/yunohost/service.py b/src/yunohost/service.py index abaf58f09..253e87d78 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -293,10 +293,8 @@ def service_status(names=[]): result[name] = { 'status': "unknown", 'start_on_boot': "unknown", - 'active': "unknown", 'active_at': "unknown", 'description': "Error: failed to get information for this service, it doesn't exists for systemd", - 'service_file_path': "unknown", 'configuration': "unknown", } @@ -316,9 +314,8 @@ def service_status(names=[]): result[name] = { 'status': str(status.get("SubState", "unknown")), 'start_on_boot': str(status.get("UnitFileState", "unknown")), - 'active': str(status.get("ActiveState", "unknown")), + 'active_at': "unknown", 'description': description, - 'service_file_path': str(status.get("FragmentPath", "unknown")), 'configuration': "unknown", } @@ -329,8 +326,6 @@ def service_status(names=[]): if "ActiveEnterTimestamp" in status: result[name]['active_at'] = datetime.utcfromtimestamp(status["ActiveEnterTimestamp"] / 1000000) - else: - result[name]['active_at'] = "unknown" # 'test-status' is an optional field to test the status of the service using a custom command if "test-status" in services[name]: From acba0c4a1089f228d1c839c0ca2829a7409fe570 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 7 Nov 2019 12:25:19 +0100 Subject: [PATCH 05/14] Use a proper subprocess for conf test instead of dirty os.system + store errors if there are --- src/yunohost/service.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/yunohost/service.py b/src/yunohost/service.py index 253e87d78..e3a584c41 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -334,8 +334,17 @@ def service_status(names=[]): # 'test-status' is an optional field to test the status of the service using a custom command if "test-conf" in services[name]: - conf = os.system(services[name]["test-conf"] + " &>/dev/null") - result[name]["configuration"] = "valid" if conf == 0 else "broken" + p = subprocess.Popen(services[name]["test-conf"], + shell=True, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT) + + out, _ = p.communicate() + if p.returncode == 0: + result[name]["configuration"] = "valid" + else: + result[name]["configuration"] = "broken" + result[name]["configuration-details"] = out.strip().split("\n") if len(names) == 1: return result[names[0]] From 4b3a4c7e4e997998e9e83ec8c8b29cc5649c2620 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 7 Nov 2019 13:48:42 +0100 Subject: [PATCH 06/14] Fetch the timestamp of the latest state change, not just the last time it got up and running... --- src/yunohost/service.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/yunohost/service.py b/src/yunohost/service.py index e3a584c41..850899ecc 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -293,7 +293,7 @@ def service_status(names=[]): result[name] = { 'status': "unknown", 'start_on_boot': "unknown", - 'active_at': "unknown", + 'last_state_change': "unknown", 'description': "Error: failed to get information for this service, it doesn't exists for systemd", 'configuration': "unknown", } @@ -314,7 +314,7 @@ def service_status(names=[]): result[name] = { 'status': str(status.get("SubState", "unknown")), 'start_on_boot': str(status.get("UnitFileState", "unknown")), - 'active_at': "unknown", + 'last_state_change': "unknown", 'description': description, 'configuration': "unknown", } @@ -324,8 +324,8 @@ def service_status(names=[]): if result[name]["start_on_boot"] == "generated": result[name]["start_on_boot"] = "enabled" if glob("/etc/rc[S5].d/S??"+name) else "disabled" - if "ActiveEnterTimestamp" in status: - result[name]['active_at'] = datetime.utcfromtimestamp(status["ActiveEnterTimestamp"] / 1000000) + if "StateChangeTimestamp" in status: + result[name]['last_state_change'] = datetime.utcfromtimestamp(status["StateChangeTimestamp"] / 1000000) # 'test-status' is an optional field to test the status of the service using a custom command if "test-status" in services[name]: From e578140172378d2e48e7499dc2e4e2ee10d5887a Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 7 Nov 2019 15:57:26 +0100 Subject: [PATCH 07/14] Improve service diagnoser, report details related to configuration tests --- data/hooks/diagnosis/30-services.py | 30 +++++++++------------------- data/hooks/diagnosis/70-regenconf.py | 13 ------------ locales/en.json | 5 +++-- 3 files changed, 12 insertions(+), 36 deletions(-) diff --git a/data/hooks/diagnosis/30-services.py b/data/hooks/diagnosis/30-services.py index 6589d83f2..fed0a1156 100644 --- a/data/hooks/diagnosis/30-services.py +++ b/data/hooks/diagnosis/30-services.py @@ -5,17 +5,6 @@ import os from yunohost.diagnosis import Diagnoser from yunohost.service import service_status -# TODO : all these are arbitrary, should be collectively validated -services_ignored = {"glances"} -services_critical = {"dnsmasq", "fail2ban", "yunohost-firewall", "nginx", "slapd", "ssh"} -# TODO / FIXME : we should do something about this postfix thing -# The nominal value is to be "exited" ... some daemon is actually running -# in a different thread that the thing started by systemd, which is fine -# but somehow sometimes it gets killed and there's no easy way to detect it -# Just randomly restarting it will fix ths issue. We should find some trick -# to identify the PID of the process and check it's still up or idk -services_expected_to_be_exited = {"postfix", "yunohost-firewall"} - class ServicesDiagnoser(Diagnoser): id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] @@ -28,23 +17,22 @@ class ServicesDiagnoser(Diagnoser): for service, result in all_result.items(): - if service in services_ignored: - continue - item = dict(meta={"service": service}) - expected_status = "running" if service not in services_expected_to_be_exited else "exited" - # TODO / FIXME : might also want to check that services are enabled + if result["status"] != "running": + item["status"] = "ERROR" + item["summary"] = ("diagnosis_services_bad_status", {"service": service, "status": result["status"]}) - if result["active"] != "active" or result["status"] != expected_status: - item["status"] = "WARNING" if service not in services_critical else "ERROR" - item["summary"] = ("diagnosis_services_bad_status", {"service": service, "status": result["active"] + "/" + result["status"]}) + elif result["configuration"] == "broken": + item["status"] = "WARNING" + item["summary"] = ("diagnosis_services_conf_broken", {"service": service}) - # TODO : could try to append the tail of the service log to the "details" key ... else: item["status"] = "SUCCESS" - item["summary"] = ("diagnosis_services_good_status", {"service": service, "status": result["active"] + "/" + result["status"]}) + item["summary"] = ("diagnosis_services_running", {"service": service, "status": result["status"]}) + if result["configuration"] == "broken": + item["details"] = [(d, tuple()) for d in result["configuration-details"]] yield item def main(args, env, loggers): diff --git a/data/hooks/diagnosis/70-regenconf.py b/data/hooks/diagnosis/70-regenconf.py index 105d43fa3..a04f5f98d 100644 --- a/data/hooks/diagnosis/70-regenconf.py +++ b/data/hooks/diagnosis/70-regenconf.py @@ -15,19 +15,6 @@ class RegenconfDiagnoser(Diagnoser): def run(self): - # nginx -t - p = subprocess.Popen("nginx -t".split(), - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT) - out, _ = p.communicate() - - if p.returncode != 0: - yield dict(meta={"test": "nginx-t"}, - status="ERROR", - summary=("diagnosis_regenconf_nginx_conf_broken", {}), - details=[(out, ())] - ) - regenconf_modified_files = manually_modified_files() debian_modified_files = manually_modified_files_compared_to_debian_default(ignore_handled_by_regenconf=True) diff --git a/locales/en.json b/locales/en.json index 751180a37..505bb44fd 100644 --- a/locales/en.json +++ b/locales/en.json @@ -180,8 +180,9 @@ "diagnosis_dns_bad_conf": "Bad / missing DNS configuration for domain {domain} (category {category})", "diagnosis_dns_missing_record": "According to the recommended DNS configuration, you should add a DNS record with type {0}, name {1} and value {2}", "diagnosis_dns_discrepancy": "According to the recommended DNS configuration, the value for the DNS record with type {0} and name {1} should be {2}, not {3}.", - "diagnosis_services_good_status": "Service {service} is {status} as expected!", - "diagnosis_services_bad_status": "Service {service} is {status} :/", + "diagnosis_services_running": "Service {service} is running!", + "diagnosis_services_conf_broken": "Configuration is broken for service {service}!", + "diagnosis_services_bad_status": "Service {service} is {status} :(", "diagnosis_diskusage_verylow": "Storage {mountpoint} (on device {device}) has only {free_abs_GB} GB ({free_percent}%) space remaining. You should really consider cleaning up some space.", "diagnosis_diskusage_low": "Storage {mountpoint} (on device {device}) has only {free_abs_GB} GB ({free_percent}%) space remaining. Be careful.", "diagnosis_diskusage_ok": "Storage {mountpoint} (on device {device}) still has {free_abs_GB} GB ({free_percent}%) space left!", From 3b4ad59cd7abf57a86b008a8d4aa4550da1f3b67 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 7 Nov 2019 15:59:31 +0100 Subject: [PATCH 08/14] Propagate removal of 'active' to that piece of code that uses it --- src/yunohost/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 443b14565..4e49c64dd 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -3140,7 +3140,7 @@ def _assert_system_is_sane_for_app(manifest, when): services.append("fail2ban") # List services currently down and raise an exception if any are found - faulty_services = [s for s in services if service_status(s)["active"] != "active"] + faulty_services = [s for s in services if service_status(s)["status"] != "running"] if faulty_services: if when == "pre": raise YunohostError('app_action_cannot_be_ran_because_required_services_down', From a9dd70182494eea2b13931cfe81d7009162f44ea Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 8 Nov 2019 22:29:21 +0100 Subject: [PATCH 09/14] Improve port diagnosis by adding a relation between ports and services --- data/hooks/diagnosis/14-ports.py | 32 ++++++++++++++++------------ data/templates/yunohost/services.yml | 5 +++++ locales/en.json | 2 ++ 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/data/hooks/diagnosis/14-ports.py b/data/hooks/diagnosis/14-ports.py index b953f35a9..a845174b6 100644 --- a/data/hooks/diagnosis/14-ports.py +++ b/data/hooks/diagnosis/14-ports.py @@ -5,7 +5,7 @@ import requests from yunohost.diagnosis import Diagnoser from yunohost.utils.error import YunohostError - +from yunohost.service import _get_services class PortsDiagnoser(Diagnoser): @@ -15,16 +15,18 @@ class PortsDiagnoser(Diagnoser): def run(self): - # FIXME / TODO : in the future, maybe we want to report different - # things per port depending on how important they are - # (e.g. XMPP sounds to me much less important than other ports) - # Ideally, a port could be related to a service... - # FIXME / TODO : for now this list of port is hardcoded, might want - # to fetch this from the firewall.yml in /etc/yunohost/ - ports = [22, 25, 53, 80, 443, 587, 993, 5222, 5269] + # This dict is something like : + # { 80: "nginx", + # 25: "postfix", + # 443: "nginx" + # ... } + ports = {} + for service, infos in _get_services().items(): + for port in infos.get("needs_exposed_ports", []): + ports[port] = service try: - r = requests.post('https://ynhdiagnoser.netlib.re/check-ports', json={'ports': ports}, timeout=30).json() + r = requests.post('https://ynhdiagnoser.netlib.re/check-ports', json={'ports': ports.keys()}, timeout=30).json() if "status" not in r.keys(): raise Exception("Bad syntax for response ? Raw json: %s" % str(r)) elif r["status"] == "error": @@ -37,15 +39,17 @@ class PortsDiagnoser(Diagnoser): except Exception as e: raise YunohostError("diagnosis_ports_could_not_diagnose", error=e) - for port in ports: + for port, service in ports.items(): if r["ports"].get(str(port), None) is not True: - yield dict(meta={"port": port}, + yield dict(meta={"port": port, "needed_by": service}, status="ERROR", - summary=("diagnosis_ports_unreachable", {"port": port})) + summary=("diagnosis_ports_unreachable", {"port": port}), + details=[("diagnosis_ports_needed_by", (service,)), ("diagnosis_ports_forwarding_tip", ())]) else: - yield dict(meta={}, + yield dict(meta={"port": port, "needed_by": service}, status="SUCCESS", - summary=("diagnosis_ports_ok", {"port": port})) + summary=("diagnosis_ports_ok", {"port": port}), + details=[("diagnosis_ports_needed_by", (service))]) def main(args, env, loggers): diff --git a/data/templates/yunohost/services.yml b/data/templates/yunohost/services.yml index fd9d06eac..986beb271 100644 --- a/data/templates/yunohost/services.yml +++ b/data/templates/yunohost/services.yml @@ -1,6 +1,7 @@ nginx: log: /var/log/nginx test-conf: nginx -t + needs_exposed_ports: [80, 443] avahi-daemon: log: /var/log/daemon.log dnsmasq: @@ -9,9 +10,11 @@ fail2ban: log: /var/log/fail2ban.log dovecot: log: [/var/log/mail.log,/var/log/mail.err] + needs_exposed_ports: [993] postfix: log: [/var/log/mail.log,/var/log/mail.err] test-status: systemctl show postfix@- | grep -q "^SubState=running" + needs_exposed_ports: [25, 587] rspamd: log: /var/log/rspamd/rspamd.log redis-server: @@ -22,8 +25,10 @@ mysql: ssh: log: /var/log/auth.log test-conf: sshd -t + needs_exposed_ports: [22] metronome: log: [/var/log/metronome/metronome.log,/var/log/metronome/metronome.err] + needs_exposed_ports: [5222, 5269] slapd: log: /var/log/syslog php7.0-fpm: diff --git a/locales/en.json b/locales/en.json index f3a6c662e..581f38bef 100644 --- a/locales/en.json +++ b/locales/en.json @@ -216,6 +216,8 @@ "diagnosis_ports_could_not_diagnose": "Could not diagnose if ports are reachable from outside. Error: {error}", "diagnosis_ports_unreachable": "Port {port} is not reachable from outside.", "diagnosis_ports_ok": "Port {port} is reachable from outside.", + "diagnosis_ports_needed_by": "Exposing this port is needed for service {0}", + "diagnosis_ports_forwarding_tip": "To fix this issue, most probably you need to configure port forwarding on your internet router as described in https://yunohost.org/port_forwarding", "diagnosis_http_could_not_diagnose": "Could not diagnose if domain is reachable from outside. Error: {error}", "diagnosis_http_ok": "Domain {domain} is reachable from outside.", "diagnosis_http_unreachable": "Domain {domain} is unreachable through HTTP from outside.", From 5a682503220bf4cf3a68684eb513fa2a4dbbcd94 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 8 Nov 2019 22:35:25 +0100 Subject: [PATCH 10/14] test-conf -> test_conf, and test-status -> test_status --- data/templates/yunohost/services.yml | 10 +++++----- src/yunohost/service.py | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/data/templates/yunohost/services.yml b/data/templates/yunohost/services.yml index 986beb271..351120a7d 100644 --- a/data/templates/yunohost/services.yml +++ b/data/templates/yunohost/services.yml @@ -1,6 +1,6 @@ nginx: log: /var/log/nginx - test-conf: nginx -t + test_conf: nginx -t needs_exposed_ports: [80, 443] avahi-daemon: log: /var/log/daemon.log @@ -13,7 +13,7 @@ dovecot: needs_exposed_ports: [993] postfix: log: [/var/log/mail.log,/var/log/mail.err] - test-status: systemctl show postfix@- | grep -q "^SubState=running" + test_status: systemctl show postfix@- | grep -q "^SubState=running" needs_exposed_ports: [25, 587] rspamd: log: /var/log/rspamd/rspamd.log @@ -24,7 +24,7 @@ mysql: alternates: ['mariadb'] ssh: log: /var/log/auth.log - test-conf: sshd -t + test_conf: sshd -t needs_exposed_ports: [22] metronome: log: [/var/log/metronome/metronome.log,/var/log/metronome/metronome.err] @@ -33,12 +33,12 @@ slapd: log: /var/log/syslog php7.0-fpm: log: /var/log/php7.0-fpm.log - test-conf: php-fpm7.0 --test + test_conf: php-fpm7.0 --test yunohost-api: log: /var/log/yunohost/yunohost-api.log yunohost-firewall: need_lock: true - test-status: iptables -S | grep "^-A INPUT" | grep " --dport" | grep -q ACCEPT + test_status: iptables -S | grep "^-A INPUT" | grep " --dport" | grep -q ACCEPT nslcd: log: /var/log/syslog glances: null diff --git a/src/yunohost/service.py b/src/yunohost/service.py index 850899ecc..612e73f6c 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -327,14 +327,14 @@ def service_status(names=[]): if "StateChangeTimestamp" in status: result[name]['last_state_change'] = datetime.utcfromtimestamp(status["StateChangeTimestamp"] / 1000000) - # 'test-status' is an optional field to test the status of the service using a custom command - if "test-status" in services[name]: - status = os.system(services[name]["test-status"]) + # 'test_status' is an optional field to test the status of the service using a custom command + if "test_status" in services[name]: + status = os.system(services[name]["test_status"]) result[name]["status"] = "running" if status == 0 else "failed" - # 'test-status' is an optional field to test the status of the service using a custom command - if "test-conf" in services[name]: - p = subprocess.Popen(services[name]["test-conf"], + # 'test_status' is an optional field to test the status of the service using a custom command + if "test_conf" in services[name]: + p = subprocess.Popen(services[name]["test_conf"], shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) From e70530a9dbe5ebb43e24134b3b24cd49906b9266 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 14 Nov 2019 17:08:10 +0100 Subject: [PATCH 11/14] Remove deprecated / useless option from 'service add', and add new ones for status and conf check and port exposure --- data/actionsmap/yunohost.yml | 33 ++++++++++++++++++------------- src/yunohost/service.py | 38 +++++++++++++++++++++--------------- 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml index f067cc3c3..887c7f9d6 100644 --- a/data/actionsmap/yunohost.yml +++ b/data/actionsmap/yunohost.yml @@ -1038,24 +1038,13 @@ service: arguments: name: help: Service name to add - -s: - full: --status - help: Custom status command + -d: + full: --description + help: Description of the service -l: full: --log help: Absolute path to log file to display nargs: "+" - -r: - full: --runlevel - help: Runlevel priority of the service - type: int - -n: - full: --need_lock - help: Use this option to prevent deadlocks if the service does invoke yunohost commands. - action: store_true - -d: - full: --description - help: Description of the service -t: full: --log_type help: Type of the log (file or systemd) @@ -1064,6 +1053,22 @@ service: - file - systemd default: file + --test_status: + help: Specify a custom bash command to check the status of the service. Note that it only makes sense to specify this if the corresponding systemd service does not return the proper information already. + --test_conf: + help: Specify a custom bash command to check if the configuration of the service is valid or broken, similar to nginx -t. + --needs_exposed_ports: + help: A list of ports that needs to be publicly exposed for the service to work as intended. + nargs: "+" + type: int + metavar: PORT + -n: + full: --need_lock + help: Use this option to prevent deadlocks if the service does invoke yunohost commands. + action: store_true + -s: + full: --status + help: Deprecated, old option. Does nothing anymore. Possibly check the --test_status option. ### service_remove() remove: diff --git a/src/yunohost/service.py b/src/yunohost/service.py index 612e73f6c..0125fd7e4 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -40,25 +40,24 @@ MOULINETTE_LOCK = "/var/run/moulinette_yunohost.lock" logger = log.getActionLogger('yunohost.service') -def service_add(name, status=None, log=None, runlevel=None, need_lock=False, description=None, log_type="file"): +def service_add(name, description=None, log=None, log_type="file", test_status=None, test_conf=None, needs_exposed_ports=None, need_lock=False, status=None): """ Add a custom service Keyword argument: name -- Service name to add - status -- Custom status command - log -- Absolute path to log file to display - runlevel -- Runlevel priority of the service - need_lock -- Use this option to prevent deadlocks if the service does invoke yunohost commands. description -- description of the service - log_type -- Precise if the corresponding log is a file or a systemd log + log -- Absolute path to log file to display + log_type -- Specify if the corresponding log is a file or a systemd log + test_status -- Specify a custom bash command to check the status of the service. N.B. : it only makes sense to specify this if the corresponding systemd service does not return the proper information. + test_conf -- Specify a custom bash command to check if the configuration of the service is valid or broken, similar to nginx -t. + needs_exposed_ports -- A list of ports that needs to be publicly exposed for the service to work as intended. + need_lock -- Use this option to prevent deadlocks if the service does invoke yunohost commands. + status -- Deprecated, doesn't do anything anymore. Use test_status instead. """ services = _get_services() - if not status: - services[name] = {'status': 'service'} - else: - services[name] = {'status': status} + services[name] = {} if log is not None: if not isinstance(log, list): @@ -77,15 +76,22 @@ def service_add(name, status=None, log=None, runlevel=None, need_lock=False, des else: raise YunohostError('service_add_failed', service=name) - - if runlevel is not None: - services[name]['runlevel'] = runlevel + if description: + services[name]['description'] = description + else: + logger.warning("/!\\ Packager ! You added a custom service without specifying a description. Please add --description to explain what the service does in a similar fashion to existing services.") if need_lock: services[name]['need_lock'] = True - if description is not None: - services[name]['description'] = description + if test_status: + services[name]["test_status"] = test_status + + if test_conf: + services[name]["test_conf"] = test_conf + + if needs_exposed_ports: + services[name]["needs_exposed_ports"] = needs_exposed_ports try: _save_services(services) @@ -277,7 +283,7 @@ def service_status(names=[]): # the hack was to add fake services... # we need to extract regenconf from service at some point, also because # some app would really like to use it - if "status" in services[name] and services[name]["status"] is None: + if services[name].get("status", "") is None: continue status = _get_service_information_from_systemd(name) From bb8b1b052df4d90a86b993616dc0c70249523fb9 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 17 Nov 2019 16:29:12 +0100 Subject: [PATCH 12/14] Using /var/log/daemon.log or /var/log/syslog is pointless, these files logs many different things. Instead, we shall always return the logs from journalctl --- data/hooks/conf_regen/01-yunohost | 6 ++++++ data/templates/yunohost/services.yml | 12 ++++-------- locales/en.json | 1 - src/yunohost/service.py | 8 ++++---- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/data/hooks/conf_regen/01-yunohost b/data/hooks/conf_regen/01-yunohost index f22de7a53..7b9644c2a 100755 --- a/data/hooks/conf_regen/01-yunohost +++ b/data/hooks/conf_regen/01-yunohost @@ -101,6 +101,12 @@ for service, conf in new_services.items(): if conffiles: services[service]['conffiles'] = conffiles + # Remove legacy /var/log/daemon.log and /var/log/syslog from log entries + # because they are too general. Instead, now the journalctl log is + # returned by default which is more relevant. + if "log" in services[service]: + if services[service]["log"] in ["/var/log/syslog", "/var/log/daemon.log"]: + del services[service]["log"] if updated: with open('/etc/yunohost/services.yml-new', 'w') as f: diff --git a/data/templates/yunohost/services.yml b/data/templates/yunohost/services.yml index 351120a7d..540224f3a 100644 --- a/data/templates/yunohost/services.yml +++ b/data/templates/yunohost/services.yml @@ -2,10 +2,8 @@ nginx: log: /var/log/nginx test_conf: nginx -t needs_exposed_ports: [80, 443] -avahi-daemon: - log: /var/log/daemon.log -dnsmasq: - log: /var/log/daemon.log +avahi-daemon: {} +dnsmasq: {} fail2ban: log: /var/log/fail2ban.log dovecot: @@ -29,8 +27,7 @@ ssh: metronome: log: [/var/log/metronome/metronome.log,/var/log/metronome/metronome.err] needs_exposed_ports: [5222, 5269] -slapd: - log: /var/log/syslog +slapd: {} php7.0-fpm: log: /var/log/php7.0-fpm.log test_conf: php-fpm7.0 --test @@ -39,8 +36,7 @@ yunohost-api: yunohost-firewall: need_lock: true test_status: iptables -S | grep "^-A INPUT" | grep " --dport" | grep -q ACCEPT -nslcd: - log: /var/log/syslog +nslcd: {} glances: null nsswitch: null ssl: null diff --git a/locales/en.json b/locales/en.json index 7cdd6b667..6a6f629b4 100644 --- a/locales/en.json +++ b/locales/en.json @@ -544,7 +544,6 @@ "service_disabled": "The '{service:s}' service was turned off", "service_enable_failed": "Could not turn on the service '{service:s}'\n\nRecent service logs:{logs:s}", "service_enabled": "The '{service:s}' service was turned off", - "service_no_log": "No logs to display for the service '{service:s}'", "service_regen_conf_is_deprecated": "'yunohost service regen-conf' is deprecated! Please use 'yunohost tools regen-conf' instead.", "service_remove_failed": "Could not remove the service '{service:s}'", "service_removed": "'{service:s}' service removed", diff --git a/src/yunohost/service.py b/src/yunohost/service.py index 0125fd7e4..f43d7cca5 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -395,10 +395,7 @@ def service_log(name, number=50): if name not in services.keys(): raise YunohostError('service_unknown', service=name) - if 'log' not in services[name]: - raise YunohostError('service_no_log', service=name) - - log_list = services[name]['log'] + log_list = services[name].get('log', []) log_type_list = services[name].get('log_type', []) if not isinstance(log_list, list): @@ -408,6 +405,9 @@ def service_log(name, number=50): result = {} + # First we always add the logs from journalctl / systemd + result["journalctl"] = _get_journalctl_logs(name, int(number)).splitlines() + for index, log_path in enumerate(log_list): log_type = log_type_list[index] From a7a3e7b6baccf80035b0e95c6d15fb33beb5cbd1 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 17 Nov 2019 16:39:41 +0100 Subject: [PATCH 13/14] Try to keep this service list in alphabetic order or something --- data/templates/yunohost/services.yml | 40 ++++++++++++++-------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/data/templates/yunohost/services.yml b/data/templates/yunohost/services.yml index 540224f3a..c2f57c245 100644 --- a/data/templates/yunohost/services.yml +++ b/data/templates/yunohost/services.yml @@ -1,42 +1,42 @@ +avahi-daemon: {} +dnsmasq: {} +dovecot: + log: [/var/log/mail.log,/var/log/mail.err] + needs_exposed_ports: [993] +fail2ban: + log: /var/log/fail2ban.log +metronome: + log: [/var/log/metronome/metronome.log,/var/log/metronome/metronome.err] + needs_exposed_ports: [5222, 5269] +mysql: + log: [/var/log/mysql.log,/var/log/mysql.err] + alternates: ['mariadb'] nginx: log: /var/log/nginx test_conf: nginx -t needs_exposed_ports: [80, 443] -avahi-daemon: {} -dnsmasq: {} -fail2ban: - log: /var/log/fail2ban.log -dovecot: - log: [/var/log/mail.log,/var/log/mail.err] - needs_exposed_ports: [993] +nslcd: {} +php7.0-fpm: + log: /var/log/php7.0-fpm.log + test_conf: php-fpm7.0 --test postfix: log: [/var/log/mail.log,/var/log/mail.err] test_status: systemctl show postfix@- | grep -q "^SubState=running" needs_exposed_ports: [25, 587] -rspamd: - log: /var/log/rspamd/rspamd.log redis-server: log: /var/log/redis/redis-server.log -mysql: - log: [/var/log/mysql.log,/var/log/mysql.err] - alternates: ['mariadb'] +rspamd: + log: /var/log/rspamd/rspamd.log +slapd: {} ssh: log: /var/log/auth.log test_conf: sshd -t needs_exposed_ports: [22] -metronome: - log: [/var/log/metronome/metronome.log,/var/log/metronome/metronome.err] - needs_exposed_ports: [5222, 5269] -slapd: {} -php7.0-fpm: - log: /var/log/php7.0-fpm.log - test_conf: php-fpm7.0 --test yunohost-api: log: /var/log/yunohost/yunohost-api.log yunohost-firewall: need_lock: true test_status: iptables -S | grep "^-A INPUT" | grep " --dport" | grep -q ACCEPT -nslcd: {} glances: null nsswitch: null ssl: null From 7986f61b14b68d8cad505e0ead49971ad984514f Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 17 Nov 2019 16:59:35 +0100 Subject: [PATCH 14/14] Specific shit for mysql --- data/templates/yunohost/services.yml | 2 +- src/yunohost/service.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/data/templates/yunohost/services.yml b/data/templates/yunohost/services.yml index c2f57c245..b3c406f0f 100644 --- a/data/templates/yunohost/services.yml +++ b/data/templates/yunohost/services.yml @@ -9,7 +9,7 @@ metronome: log: [/var/log/metronome/metronome.log,/var/log/metronome/metronome.err] needs_exposed_ports: [5222, 5269] mysql: - log: [/var/log/mysql.log,/var/log/mysql.err] + log: [/var/log/mysql.log,/var/log/mysql.err,/var/log/mysql/error.log] alternates: ['mariadb'] nginx: log: /var/log/nginx diff --git a/src/yunohost/service.py b/src/yunohost/service.py index f43d7cca5..548d1efda 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -408,6 +408,10 @@ def service_log(name, number=50): # First we always add the logs from journalctl / systemd result["journalctl"] = _get_journalctl_logs(name, int(number)).splitlines() + # Mysql and journalctl are fucking annoying, we gotta explictly fetch mariadb ... + if name == "mysql": + result["journalctl"] = _get_journalctl_logs("mariadb", int(number)).splitlines() + for index, log_path in enumerate(log_list): log_type = log_type_list[index]