From 0d6e30ff6a1c9003a3d5ba0a03578e4301c071e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20BERSAC?= Date: Fri, 28 Jun 2024 08:39:24 +0200 Subject: [PATCH 01/11] services: Delegate signals and background setup to service Let services handle keeping background object, registering perf to signal manager, etc. instead of adding a forest of if in services.py --- agent/temboardagent/web/service.py | 2 +- ui/temboardui/cli/app.py | 12 +++++++----- ui/temboardui/toolkit/services.py | 8 ++------ ui/temboardui/toolkit/taskmanager.py | 11 ++++++----- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/agent/temboardagent/web/service.py b/agent/temboardagent/web/service.py index 80116a1b2..d1de50297 100644 --- a/agent/temboardagent/web/service.py +++ b/agent/temboardagent/web/service.py @@ -22,7 +22,7 @@ def __str__(self): return self.name # for services.run - def setup(self): + def setup(self, *_, **__): ServerHandler.server_software = "temBoard-agent/%s" % __version__ try: diff --git a/ui/temboardui/cli/app.py b/ui/temboardui/cli/app.py index 0b7e4ac84..d2f727c33 100644 --- a/ui/temboardui/cli/app.py +++ b/ui/temboardui/cli/app.py @@ -270,7 +270,6 @@ def __init__(self, app): self.app = app # Ref to services.BackgroundManager self.background = None - # For services.run() self.perf = perf.PerfCounters.setup(service=self.name) def __str__(self): @@ -280,8 +279,10 @@ def __str__(self): def create_loop(self): return tornado.ioloop.IOLoop.instance() - def setup(self): + def setup(self, sgm, bg): + self.background = bg if self.perf: + sgm.register(self.perf) self.perf.run() flask_app.vitejs.read_manifest() @@ -347,9 +348,10 @@ def _setup_autoreload(self): autoreload.watch(path) def _autoreload_hook(self): - if self.background: - logger.debug("Stopping background service before reloading.") - self.background.stop() + if not self.background: + return + logger.debug("Stopping background service before reloading.") + self.background.stop() def _iter_template_files(self): rootpkg = __import__(__name__) diff --git a/ui/temboardui/toolkit/services.py b/ui/temboardui/toolkit/services.py index 47809c0a1..c726d5931 100644 --- a/ui/temboardui/toolkit/services.py +++ b/ui/temboardui/toolkit/services.py @@ -61,17 +61,13 @@ def run(main, *backgrounds): sgm.register(LoopStopper(loop)) sgm.register(main) - # for tornado autoreload. See TornadoService._autoreload_hook. - main.background = bg = BackgroundManager(loop) + bg = BackgroundManager(loop) if backgrounds: for service in backgrounds: bg.add(service) sgm.register(bg) - if getattr(main, "perf", None): - sgm.register(main.perf) - - main.setup() + main.setup(sgm, bg) with sgm, bg: logger.debug("Entering %s loop.", main) loop.start() diff --git a/ui/temboardui/toolkit/taskmanager.py b/ui/temboardui/toolkit/taskmanager.py index 314e3c50c..451262900 100644 --- a/ui/temboardui/toolkit/taskmanager.py +++ b/ui/temboardui/toolkit/taskmanager.py @@ -443,7 +443,6 @@ def __init__(self, app, task_queue, event_queue): self.event_queue = event_queue self.scheduler = None self.task_list_engine = None - # For services.run self.perf = PerfCounters.setup(service=self.name) @property @@ -451,10 +450,11 @@ def address(self): return os.path.join(self.app.config.temboard.home, ".tm.socket") # interface for services.run - def setup(self): + def setup(self, sgm, *_, **__): self.teardown() self.scheduler.setup() if self.perf: + sgm.register(self.perf) self.perf.run() # interface for syncio.Loop @@ -535,7 +535,6 @@ def __init__(self, task_queue, event_queue): self.task_queue = task_queue self.event_queue = event_queue self.workers = {} - # For service.run() self.perf = None def _abort_job(self, task_id): @@ -789,13 +788,15 @@ class WorkerPoolService(syncio.Service): def __init__(self, app, task_queue, event_queue): super().__init__(app) self.worker_pool = WorkerPool(task_queue, event_queue) - # For service.run self.perf = PerfCounters.setup(service=self.name) # interface for services.run - def setup(self): + def setup(self, sgm, *_, **__): self.worker_pool.perf = self.perf self.worker_pool.setup() + if self.perf: + sgm.register(self.perf) + self.perf.run() def teardown(self): logger.info("Aborting jobs.") From b89bb8e2ffb8a6d2331a83e77efdfa2949cc3bd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20BERSAC?= Date: Fri, 28 Jun 2024 09:45:49 +0200 Subject: [PATCH 02/11] services: Handle external command as service --- ui/temboardui/toolkit/services.py | 75 +++++++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/ui/temboardui/toolkit/services.py b/ui/temboardui/toolkit/services.py index c726d5931..0ca353785 100644 --- a/ui/temboardui/toolkit/services.py +++ b/ui/temboardui/toolkit/services.py @@ -42,7 +42,7 @@ def run(main, *backgrounds): - """Execute a main service object function in this process. + """Run a main service object function in this process. Handle signals for INT, TERM, HUP, CHLD and ALRM. Handle background services. @@ -77,6 +77,25 @@ def run(main, *backgrounds): return 0 +def execute(service): + """Execute an external command in this process. + + Replace current Python program by a service.command. + Does not return. service.command continues process life. + """ + + if hasattr(service, "setup"): + service.setup() + + # Close all files except stdin, stdout, stderr. + for fd in range(3, os.sysconf("SC_OPEN_MAX")): + try: + os.close(fd) + except OSError: + pass + os.execvp(service.command[0], service.command) + + class LoopStopper: def __init__(self, loop): self.loop = loop @@ -106,9 +125,20 @@ def __init__(self, loop): def add(self, service): self.services[str(service)] = service + def __bool__(self): + return bool(self.services) + def __enter__(self): if not self.services: return + + self._read_pids() + + if self.pids: + logger.debug("Cleaning previous background services.") + self.kill() + if self.wait(): + raise Exception("Background services are still alive.") self.start() def __exit__(self, *a): @@ -128,11 +158,40 @@ def fork(self, service): pid = os.fork() if pid: # Parent process logger.debug("Background service started. service=%s pid=%d", service, pid) - self.pids[str(service)] = pid + self._save_pid(service, pid) return pid # Child process - os._exit(run(service)) + if hasattr(service, "command"): + execute(service) + else: + os._exit(run(service)) + + def _read_pids(self): + for name, service in self.services.items(): + if not hasattr(service, "pidfile"): + continue + if not os.path.exists(service.pidfile): + continue + with open(service.pidfile) as fo: + self.pids[name] = int(fo.read().strip()) + logger.debug( + "Read pid from pidfile. service=%s pid=%d", name, self.pids[name] + ) + + def _save_pid(self, service, pid): + self.pids[str(service)] = pid + if hasattr(service, "pidfile"): + with open(service.pidfile, "w") as fo: + fo.write(str(pid)) + + def _drop_pid(self, name): + del self.pids[name] + s = self.services[name] + if not hasattr(s, "pidfile"): + return + if os.path.exists(s.pidfile): + os.unlink(s.pidfile) def stop(self): self.stopping = True @@ -156,9 +215,9 @@ def wait(self, timeout=5, step=0.5): try: pid, status = os.waitpid(-1, os.WNOHANG) except ChildProcessError: - break + pass if pid: - del self.pids[name] + self._drop_pid(name) time.sleep(step) timeout -= step @@ -191,11 +250,9 @@ def sigchld_handler(self, *a): pass logger.warning( - "Background service dead. Restarting. service=%s pid=%s", - name, - self.pids[name], + "Background service dead. Restarting. service=%s pid=%s", name, pid ) - del self.pids[name] + self._drop_pid(name) self.start() From c029b9550ea4db00f19ab64dfe867b528b4acabe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20BERSAC?= Date: Wed, 24 Jul 2024 13:57:38 +0200 Subject: [PATCH 03/11] services: Send arbitrary signal to children --- ui/temboardui/toolkit/services.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ui/temboardui/toolkit/services.py b/ui/temboardui/toolkit/services.py index 0ca353785..23fa5bd95 100644 --- a/ui/temboardui/toolkit/services.py +++ b/ui/temboardui/toolkit/services.py @@ -224,11 +224,16 @@ def wait(self, timeout=5, step=0.5): return bool(self.pids) - def kill(self): + def kill(self, sig=signal.SIGKILL): for name, pid in self.pids.items(): - logger.warning("Killing background service. service=%s pid=%s", name, pid) + logger.warning( + "Signaling background service. service=%s pid=%s signal=%s", + name, + pid, + sig, + ) try: - os.kill(pid, signal.SIGKILL) + os.kill(pid, sig) except ProcessLookupError: pass From 9a5a90c2f3dff4f3a9c88f8e2672d15b5093e96c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20BERSAC?= Date: Wed, 24 Jul 2024 13:56:23 +0200 Subject: [PATCH 04/11] services: Accept to kill a terminated child --- ui/temboardui/toolkit/services.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/temboardui/toolkit/services.py b/ui/temboardui/toolkit/services.py index 23fa5bd95..70d8306d3 100644 --- a/ui/temboardui/toolkit/services.py +++ b/ui/temboardui/toolkit/services.py @@ -197,7 +197,10 @@ def stop(self): self.stopping = True for name, pid in self.pids.items(): logger.debug("Terminating background service. service=%s pid=%d", name, pid) - os.kill(pid, signal.SIGTERM) + try: + os.kill(pid, signal.SIGTERM) + except ProcessLookupError: + pass time.sleep(0.125) if self.wait(): self.kill() From 66c0682e1589e13bde22169c8b90b24ddc955e26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20BERSAC?= Date: Mon, 22 Jul 2024 11:19:45 +0200 Subject: [PATCH 05/11] services: Always clean signal fd after fork --- ui/temboardui/toolkit/services.py | 6 ++++++ ui/temboardui/toolkit/syncio.py | 2 -- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ui/temboardui/toolkit/services.py b/ui/temboardui/toolkit/services.py index 70d8306d3..993ee04ef 100644 --- a/ui/temboardui/toolkit/services.py +++ b/ui/temboardui/toolkit/services.py @@ -118,6 +118,7 @@ def sigterm_handler(self, *a): class BackgroundManager: def __init__(self, loop): + self.loop = loop self.services = {} self.pids = {} self.stopping = False # Whether to restart on SIGCHLD @@ -162,6 +163,11 @@ def fork(self, service): return pid # Child process + if hasattr(self.loop, "asyncio_loop"): + # Cleanup parent signals handling. + # See https://bugs.python.org/issue22087 and https://bugs.python.org/issue21998 for details about asyncio and fork. + signal.set_wakeup_fd(-1) + if hasattr(service, "command"): execute(service) else: diff --git a/ui/temboardui/toolkit/syncio.py b/ui/temboardui/toolkit/syncio.py index 037406baa..35a76eb3b 100644 --- a/ui/temboardui/toolkit/syncio.py +++ b/ui/temboardui/toolkit/syncio.py @@ -74,8 +74,6 @@ class SignalManager(dict): # Context manager for synchronous signal handling. def __enter__(self): - # forking from asyncio loop requires reset of wakeup_fd. - signal.set_wakeup_fd(-1) for sig, handler in self.items(): signal.signal(sig, handler) self._registered = self.keys() From 7778c86c3b65cf58f4918d8f3139f7c358ef4f3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20BERSAC?= Date: Wed, 24 Jul 2024 13:58:29 +0200 Subject: [PATCH 06/11] services: Tell whether we stop on error --- ui/temboardui/toolkit/services.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ui/temboardui/toolkit/services.py b/ui/temboardui/toolkit/services.py index 993ee04ef..8dc69046e 100644 --- a/ui/temboardui/toolkit/services.py +++ b/ui/temboardui/toolkit/services.py @@ -142,9 +142,11 @@ def __enter__(self): raise Exception("Background services are still alive.") self.start() - def __exit__(self, *a): + def __exit__(self, etype, evalue, etb): if not self.services: return + if etype: + logger.warning("Exiting on error. err=%s", evalue) self.stop() def start(self): From 904fbc1ebe57d306a9cdc6c0b85075382b95f311 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20BERSAC?= Date: Wed, 19 Jun 2024 16:42:22 +0200 Subject: [PATCH 07/11] ui: Move generate_secret as ApiKey method Prometheus manager will use this method to generate Prometheus API. --- ui/temboardui/cli/apikey.py | 11 +---------- ui/temboardui/model/orm.py | 8 ++++++++ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/ui/temboardui/cli/apikey.py b/ui/temboardui/cli/apikey.py index e6e2f56c8..8c8120f51 100644 --- a/ui/temboardui/cli/apikey.py +++ b/ui/temboardui/cli/apikey.py @@ -1,7 +1,5 @@ import logging -import string import sys -from secrets import choice from textwrap import dedent from ..model import Session @@ -31,7 +29,7 @@ def define_arguments(self, parser): def main(self, args): session = Session() key = ( - ApiKeys.insert(secret=generate_secret(), comment=args.comment) + ApiKeys.insert(secret=ApiKeys.generate_secret(), comment=args.comment) .with_session(session) .scalar() ) @@ -101,10 +99,3 @@ def main(self, args): logger.info("Purged %d keys.", count) else: logger.info("No expired keys to purge.") - - -_SECRET_LETTERS = string.ascii_letters + string.digits + "+/-_" - - -def generate_secret(length=40): - return "".join(choice(_SECRET_LETTERS) for _ in range(length)) diff --git a/ui/temboardui/model/orm.py b/ui/temboardui/model/orm.py index 2ee71161c..c2b7ba374 100644 --- a/ui/temboardui/model/orm.py +++ b/ui/temboardui/model/orm.py @@ -1,4 +1,6 @@ import datetime +import string +from secrets import choice from past.builtins import basestring from past.utils import old_div @@ -137,6 +139,12 @@ class ApiKeys(Model): cdate = Column(TIMESTAMP(timezone=True)) edate = Column(TIMESTAMP(timezone=True)) + _SECRET_LETTERS = string.ascii_letters + string.digits + "+/-_" + + @classmethod + def generate_secret(cls, length=40): + return "".join(choice(cls._SECRET_LETTERS) for _ in range(length)) + # See # https://docs.sqlalchemy.org/en/14/orm/queryguide.html#getting-orm-results-from-textual-and-core-statements From 667cd1ec5e2061e524ca51395f795dd908e75081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20BERSAC?= Date: Fri, 28 Jun 2024 14:21:22 +0200 Subject: [PATCH 08/11] Upgrade Prometheus --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 965e7713a..6a7c3ee72 100644 --- a/Makefile +++ b/Makefile @@ -56,7 +56,7 @@ install-%: venv-% dev/venv-py$*/bin/temboard-agent --version # smoke test # LTS -PROMETHEUS_VERSION=2.45.1 +PROMETHEUS_VERSION=2.53.0 dev/downloads/prometheus-%.linux-amd64.tar.gz: mkdir -p $(dir $@) curl --fail --silent -L "https://github.com/prometheus/prometheus/releases/download/v$*/$(notdir $@)" --output $@ From 66b5c0126b9437938c2993ab4cd8eb8805ca0429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20BERSAC?= Date: Fri, 28 Jun 2024 14:22:17 +0200 Subject: [PATCH 09/11] Install promtool --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6a7c3ee72..5c55fdd62 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,7 @@ develop-%:: .env @dev/bin/checkdocker $(DOCKER_MAX_VERSION) git config blame.ignoreRevsFile .git-blame-ignore-revs if [ -d ~/.config/lnav/formats ] ; then ln -fsTv $$PWD/dev/lnav/formats ~/.config/lnav/formats/temboard ; fi - $(MAKE) -j 2 install-$* dev/bin/prometheus + $(MAKE) -j 2 install-$* dev/bin/prometheus dev/bin/promtool mkdir -p dev/temboard cd ui/; npm install-clean cd ui/; npm run build From 357a707d56b19764629af813bc07545ff1b0f393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20BERSAC?= Date: Fri, 28 Jun 2024 14:34:46 +0200 Subject: [PATCH 10/11] Extract prometheus in ui/ project --- .config/temboard.conf | 2 +- Makefile | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.config/temboard.conf b/.config/temboard.conf index 74de0c6c8..56dee556a 100644 --- a/.config/temboard.conf +++ b/.config/temboard.conf @@ -49,7 +49,7 @@ method = stderr level = DEBUG [monitoring] -prometheus = dev/bin/prometheus +prometheus = ui/build/bin/prometheus [notifications] # SMTP host diff --git a/Makefile b/Makefile index 5c55fdd62..c7393289d 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,7 @@ develop-%:: .env @dev/bin/checkdocker $(DOCKER_MAX_VERSION) git config blame.ignoreRevsFile .git-blame-ignore-revs if [ -d ~/.config/lnav/formats ] ; then ln -fsTv $$PWD/dev/lnav/formats ~/.config/lnav/formats/temboard ; fi - $(MAKE) -j 2 install-$* dev/bin/prometheus dev/bin/promtool + $(MAKE) -j 2 install-$* ui/build/bin/prometheus ui/build/bin/promtool mkdir -p dev/temboard cd ui/; npm install-clean cd ui/; npm run build @@ -61,7 +61,8 @@ dev/downloads/prometheus-%.linux-amd64.tar.gz: mkdir -p $(dir $@) curl --fail --silent -L "https://github.com/prometheus/prometheus/releases/download/v$*/$(notdir $@)" --output $@ -dev/bin/prometheus dev/bin/promtool: dev/downloads/prometheus-$(PROMETHEUS_VERSION).linux-amd64.tar.gz +ui/build/bin/prometheus ui/build/bin/promtool: dev/downloads/prometheus-$(PROMETHEUS_VERSION).linux-amd64.tar.gz + mkdir -p $(dir $@) tar --extract --file "$<" --directory "$(dir $@)" --strip-component=1 --touch "prometheus-$(PROMETHEUS_VERSION).linux-amd64/$(notdir $@)" "$@" --version # Smoketest @@ -69,7 +70,7 @@ clean: #: Trash venv and containers. docker compose down --volumes --remove-orphans docker rmi --force dalibo/temboard-agent:dev rm -rf dev/venv-py* .venv-py* dev/build/ dev/prometheus/targets/temboard-dev.yaml - rm -vf dev/bin/prometheus dev/bin/promtool + rm -vf ui/build/bin/prometheus ui/build/bin/promtool rm -rf agent/build/ .env agent/.coverage rm -rvf ui/build/ ui/.coverage $(MAKE) clean-static From 7b9df9e30b342a0de0b9b280df5443ad0d0957b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20BERSAC?= Date: Wed, 24 Jul 2024 14:21:31 +0200 Subject: [PATCH 11/11] agent: Handle missing signing headers --- agent/temboardagent/web/app.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/agent/temboardagent/web/app.py b/agent/temboardagent/web/app.py index 01f881271..8af0943d4 100644 --- a/agent/temboardagent/web/app.py +++ b/agent/temboardagent/web/app.py @@ -16,6 +16,7 @@ ) from .. import notification +from ..toolkit.errors import TemboardError from ..toolkit.http import format_date from ..toolkit.signing import InvalidSignature, canonicalize_request, verify_v1 from ..toolkit.utils import JSONEncoder, utcnow @@ -154,12 +155,17 @@ def wrapper(*a, **kw): def authenticate(self): app = default_app().temboard - date = request.headers["x-temboard-date"] + date = request.headers.get("x-temboard-date") + if not date: + raise HTTPError(400, "Missing X-TemBoard-Date header.") + oldest_date = format_date(utcnow() - timedelta(hours=2)) if date < oldest_date: raise HTTPError(400, "Request older than 2 hours.") - signature = request.headers["x-temboard-signature"] + signature = request.headers.get("x-temboard-signature") + if not signature: + raise HTTPError(400, "Missing X-TemBoard-Signature header.") version, _, signature = signature.partition(":") if "v1" != version: raise HTTPError(400, "Unsupported signature format") @@ -170,9 +176,12 @@ def authenticate(self): path = request.environ["RAW_PATH_INFO"] if request.environ["QUERY_STRING"]: path = path + "?" + request.environ["QUERY_STRING"] - canonical_request = canonicalize_request( - request.method, path, request.headers, request.body.read() - ) + try: + canonical_request = canonicalize_request( + request.method, path, request.headers, request.body.read() + ) + except TemboardError as e: + raise HTTPError(400, str(e)) try: verify_v1(app.config.signing_key, signature, canonical_request)