From b937a57602be71ffae23ce178e90bd7267c70ada Mon Sep 17 00:00:00 2001 From: patrick Date: Fri, 6 Nov 2026 13:00:36 +0100 Subject: [PATCH] =?UTF-8?q?feat:=20Selbsttest=20bei=20Versionswechsel=20un?= =?UTF-8?q?d=20Spaltenpr=C3=BCfung=20ohne=20Probleme?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Der Runner prüft die Checkmk-Version alle 5 Minuten und startet bei einem Wechsel sofort einen Selbsttest (Cooldown, Erstlauf ausgenommen). Die Spaltenprüfung fragt bei leerer Problemliste eine ungefilterte Stichprobe ab; leeres Checkmk gilt als nicht prüfbar, nicht als Fehler. Co-Authored-By: Claude Sonnet 5.5 --- CLAUDE.md | 3 ++ app/checkmk.py | 16 +++++++++ app/selftest/checks.py | 61 +++++++++++++++++++++++++-------- app/selftest/constants.py | 4 +++ app/selftest/runner.py | 53 +++++++++++++++++++++++++++-- app/selftest/service.py | 10 ++++++ tests/test_selftest.py | 51 ++++++++++++++++++++++++++++ tests/test_selftest_runner.py | 64 ++++++++++++++++++++++++++++++++++- 8 files changed, 243 insertions(+), 19 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1461e68..40f55f9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -97,6 +97,9 @@ tests/ - `app/checkmk.py`: `_items`/`extract_extensions` akzeptieren `value` oder `members`; `_is_hard` über die benannte Konstante `HARD_STATE_TYPES` (`"1"`/`"hard"`, alles andere inkl. `None` gilt als soft, stürzt nicht ab); `CheckmkError` bekommt zusätzlich `.hint` (lesbarer Hinweis je HTTP-Status 401/403/404/422/5xx über `status_hint`), `.brief` bleibt wortgleich wie bisher, damit bestehende Tests und Chat-Texte unverändert bleiben. Neue, nur für den Selbsttest gedachte Zusatzpfade, die das reguläre Verhalten nicht verändern: `problem_hosts_with_fallback`/`selftest_hosts` (POST scheitert → GET mit `columns=` und clientseitigem Filter `state != 0`, der funktionierende Weg wird in `_host_query_mode` gemerkt und von `problem_hosts()` beim nächsten Aufruf zuerst versucht), `selftest_services` (rohe Zeilen plus best-effort Parse, Parsefehler werden abgefangen statt den Selbsttest abzubrechen), `downtimes_with_fallback` (scheitert `downtime_type` → GET mit `query` als JSON-String, `is_service` 0/1), `downtimes_from_states` (reine Funktion, leitet Downtimes aus `scheduled_downtime_depth` der schon gelesenen Service-/Host-Listen ab, letzter Ausweg). - `app/health.py`: neuer Teilcheck `checkmk_api`, liest nur den letzten gespeicherten Selbsttest-Lauf (kein eigener API-Aufruf). Checkmk nicht konfiguriert oder kein `selftest_repo` übergeben → `NotConfigured`; noch kein Lauf → `NotConfigured("noch nicht gelaufen")` (gleiche Behandlung wie "nicht konfiguriert", dafür liest `run_checks` jetzt den Text aus der Exception statt ihn fest zu verdrahten); Status `abweichend` oder `fehler` → Fehler (503 in `/health`). `build_checks(store, poller_status=None, selftest_repo=None)` ist rückwärtskompatibel (fehlender `selftest_repo` ergibt stets "nicht konfiguriert"). - `app/runtime.py`: `Runtime.selftest_repo`/`.selftest`/`.selftest_runner`, Task im Lifespan wie `retention_task` (Cancel in `finally`). `app/alerts/system.py` bekommt `SystemNotifier.selftest(ok, text)` (eigener Service-Name `Selbsttest` in `system_text.py`, Art `system`, wie die Checkmk-Ausfallmeldung). +- Zwei Lücken geschlossen: + - Sofort-Lauf bei Versionswechsel: `SelftestRunner.check_version_change()` fragt periodisch (Konstante `VERSION_CHECK_INTERVAL` 300 s) nur leichtgewichtig `GET /version` über `SelftestService.peek_version()` ab (kein voller Selbsttest) und vergleicht mit der zuletzt bekannten Version (einmalig aus `selftest_state` nachgeladen). Weicht sie ab, wird die neue Version sofort gemerkt (verhindert Endlosschleife bei hängendem/fehlschlagendem Lauf) und, außer beim allerersten Check überhaupt (keine bekannte Version), ein voller Selbsttest ausgelöst, gebremst durch `VERSION_CHANGE_COOLDOWN` (300 s, höchstens ein Wechsel-Lauf in diesem Abstand). Die `run()`-Schleife ruft `check_version_change()` in jedem Zyklus auf und schläft `min(CHECK_INTERVAL, VERSION_CHECK_INTERVAL)`; die Entprellung selbst steckt in der Methode, nicht in der Schleife. Deaktivierter Selbsttest (`enabled()` falsch) prüft gar nicht. + - Spaltenprüfung bei leerer Problemliste: `check_services`/`check_hosts` fragen bei leerer Problemliste zusätzlich über `CheckmkClient.sample_service()`/`sample_host()` ohne Zustandsfilter ab und werten nur das erste Element aus. **Annahme/Einschränkung**: Die Service-/Host-Collection der Checkmk-REST-API kennt (Stand 2.3) keinen Limit-/Pagination-Parameter für diese Abfrage; da keine Zustands- oder sonstige Einschränkung ohne Vorwissen möglich ist, wird ohne Filter abgefragt und ausschließlich das erste Element der Antwort geprüft (muss gegen 2.5 bestätigt werden, Risiko: Antwortgröße in großen Umgebungen, betrifft nur den Fall "keine Probleme vorhanden"). Gibt es gar kein Element (leeres Checkmk), meldet der Teilcheck `ok` mit Text "nicht prüfbar (keine Objekte)" statt eines Fehlers. - Settings `selftest.enabled` (Default an, schaltet nur den automatischen Zeitplan ab, ein manueller Lauf funktioniert immer) und `selftest.notify` (Default an) in neuer Section "Selbsttest" (`app/web/selftest_section.py`, `/settings/selftest`). - UI `/selftest` (`app/web/selftest_pages.py` + `selftest.html`): Tabelle der vier Teilchecks mit Ampel-Badge, Checkmk-Version, Zeitpunkt des letzten Laufs; Fallback-Hinweise stehen im Text des jeweiligen Teilchecks. Lesbar für alle angemeldeten Rollen, Button "Selbsttest jetzt ausführen" nur für Admins (CSRF über die globale FastAPI-Dependency, Rate-Limit 1 pro 30 s). Menüeinträge `/selftest` (alle) und `/settings/selftest` (Admins). - `deploy/checkmk/checkbot-local-check`: neuer Service `Checkbot_checkmk_api`, Detailtext `noch nicht gelaufen` wird wie `nicht konfiguriert` als WARN gewertet (`WARN_DETAILS`). diff --git a/app/checkmk.py b/app/checkmk.py index 4399a30..661e80c 100644 --- a/app/checkmk.py +++ b/app/checkmk.py @@ -298,6 +298,22 @@ class CheckmkClient: services = [] return rows, services + async def sample_service(self) -> dict | None: + # Die Service-Collection kennt (Stand 2.3) kein Limit/Pagination; um die Antwort klein zu + # halten, wird nur ohne Zustandsfilter abgefragt und davon ausschließlich das erste Element + # ausgewertet, statt die komplette Liste zu laden. + body = await self._request("POST", SERVICES_PATH, json={"columns": COLUMNS}) + rows = extract_extensions(body) + return rows[0] if rows else None + + async def sample_host(self) -> dict | None: + try: + body = await self._request("POST", HOSTS_PATH, json={"columns": HOST_COLUMNS}) + except CheckmkError: + body = await self._request("GET", HOSTS_PATH, params={"columns": ",".join(HOST_COLUMNS)}) + rows = extract_extensions(body) + return rows[0] if rows else None + async def problems(self) -> list[ServiceState]: return await self.problem_services() diff --git a/app/selftest/checks.py b/app/selftest/checks.py index 5da0530..8047cce 100644 --- a/app/selftest/checks.py +++ b/app/selftest/checks.py @@ -28,6 +28,21 @@ def _unreachable_or_unexpected(name: str, exc: CheckmkError) -> CheckResult: return CheckResult(name, STATUS_FAILED, f"Antwort unerwartet ({exc.brief})") +NOT_CHECKABLE_TEXT = "nicht prüfbar (keine Objekte)" + + +def _columns_result(name: str, rows: Sequence[dict], required: Sequence[str], optional: Sequence[str], + ok_text: str, sample: bool = False) -> CheckResult: + missing_required, missing_optional = _missing_columns(rows, required, optional) + if missing_required: + return CheckResult(name, STATUS_FAILED, f"Pflichtspalte(n) fehlen: {', '.join(missing_required)}") + if missing_optional: + prefix = "Spalten anhand Stichprobe geprüft; " if sample else "" + return CheckResult(name, STATUS_DEVIATED, + f"{prefix}optionale Spalte(n) fehlen, Fallback genutzt: {', '.join(missing_optional)}") + return CheckResult(name, STATUS_OK, ok_text) + + async def check_version(client: CheckmkClient) -> tuple[CheckResult, str]: try: info = await client.version() @@ -45,15 +60,17 @@ async def check_services(client: CheckmkClient) -> tuple[CheckResult, list[Servi rows, services = await client.selftest_services() except CheckmkError as exc: return _unreachable_or_unexpected(CHECK_SERVICES, exc), [] - missing_required, missing_optional = _missing_columns(rows, SERVICE_REQUIRED, SERVICE_OPTIONAL) - if missing_required: - return CheckResult(CHECK_SERVICES, STATUS_FAILED, - f"Pflichtspalte(n) fehlen: {', '.join(missing_required)}"), services - if missing_optional: - return CheckResult(CHECK_SERVICES, STATUS_DEVIATED, - f"optionale Spalte(n) fehlen, Fallback genutzt: {', '.join(missing_optional)}"), services - return CheckResult(CHECK_SERVICES, STATUS_OK, - f"{len(services)} Probleme gelesen, alle Spalten vorhanden"), services + if rows: + return _columns_result(CHECK_SERVICES, rows, SERVICE_REQUIRED, SERVICE_OPTIONAL, + f"{len(services)} Probleme gelesen, alle Spalten vorhanden"), services + try: + sample = await client.sample_service() + except CheckmkError as exc: + return _unreachable_or_unexpected(CHECK_SERVICES, exc), services + if sample is None: + return CheckResult(CHECK_SERVICES, STATUS_OK, NOT_CHECKABLE_TEXT), services + return _columns_result(CHECK_SERVICES, [sample], SERVICE_REQUIRED, SERVICE_OPTIONAL, + "0 Probleme gelesen, Spalten anhand Stichprobe geprüft", sample=True), services async def check_hosts(client: CheckmkClient) -> tuple[CheckResult, list[HostState]]: @@ -61,18 +78,32 @@ async def check_hosts(client: CheckmkClient) -> tuple[CheckResult, list[HostStat rows, hosts, mode = await client.selftest_hosts() except CheckmkError as exc: return _unreachable_or_unexpected(CHECK_HOSTS, exc), [] - missing_required, missing_optional = _missing_columns(rows, HOST_REQUIRED, HOST_OPTIONAL) - if missing_required: - return CheckResult(CHECK_HOSTS, STATUS_FAILED, - f"Pflichtspalte(n) fehlen: {', '.join(missing_required)}"), hosts notes = [] if mode == HOST_QUERY_GET: notes.append("GET-Fallback verwendet, POST wurde abgelehnt") + sample_rows, sample_used = rows, False + if not sample_rows: + try: + sample = await client.sample_host() + except CheckmkError as exc: + return _unreachable_or_unexpected(CHECK_HOSTS, exc), hosts + if sample is None: + notes.append(NOT_CHECKABLE_TEXT) + status = STATUS_DEVIATED if mode == HOST_QUERY_GET else STATUS_OK + return CheckResult(CHECK_HOSTS, status, "; ".join(notes)), hosts + sample_rows, sample_used = [sample], True + missing_required, missing_optional = _missing_columns(sample_rows, HOST_REQUIRED, HOST_OPTIONAL) + if missing_required: + return CheckResult(CHECK_HOSTS, STATUS_FAILED, + f"Pflichtspalte(n) fehlen: {', '.join(missing_required)}"), hosts if missing_optional: - notes.append(f"optionale Spalte(n) fehlen, Fallback genutzt: {', '.join(missing_optional)}") + prefix = "Spalten anhand Stichprobe geprüft; " if sample_used else "" + notes.append(f"{prefix}optionale Spalte(n) fehlen, Fallback genutzt: {', '.join(missing_optional)}") if notes: return CheckResult(CHECK_HOSTS, STATUS_DEVIATED, "; ".join(notes)), hosts - return CheckResult(CHECK_HOSTS, STATUS_OK, f"{len(hosts)} Probleme gelesen, alle Spalten vorhanden"), hosts + text = ("0 Probleme gelesen, Spalten anhand Stichprobe geprüft" if sample_used else + f"{len(hosts)} Probleme gelesen, alle Spalten vorhanden") + return CheckResult(CHECK_HOSTS, STATUS_OK, text), hosts async def check_downtimes(client: CheckmkClient, services: Sequence[ServiceState], diff --git a/app/selftest/constants.py b/app/selftest/constants.py index a97b82e..50336cc 100644 --- a/app/selftest/constants.py +++ b/app/selftest/constants.py @@ -20,6 +20,10 @@ START_DELAY = 20.0 CHECK_INTERVAL = 900.0 MANUAL_COOLDOWN_SECONDS = 30.0 +# Erkennung eines Versionswechsels per leichtem GET /version, unabhängig vom Tageslauf. +VERSION_CHECK_INTERVAL = 300.0 +VERSION_CHANGE_COOLDOWN = 300.0 + AUDIT_RUN = "selftest.run" AUDIT_TARGET = "selftest" SYSTEM_USER = "system" diff --git a/app/selftest/runner.py b/app/selftest/runner.py index 33dd8b7..69c7018 100644 --- a/app/selftest/runner.py +++ b/app/selftest/runner.py @@ -8,7 +8,7 @@ from .. import audit from ..alerts import SlidingWindowLimiter, SystemNotifier from ..db import Database from .constants import (AUDIT_RUN, AUDIT_TARGET, CHECK_INTERVAL, MANUAL_COOLDOWN_SECONDS, RUN_HOUR, START_DELAY, - SYSTEM_USER) + SYSTEM_USER, VERSION_CHANGE_COOLDOWN, VERSION_CHECK_INTERVAL) from .model import SelftestRun from .notify import message_for from .service import SelftestNotConfigured, SelftestService @@ -27,7 +27,10 @@ class SelftestRunner: enabled: Callable[[], bool], notify_enabled: Callable[[], bool], clock: Callable[[], datetime] = datetime.now, sleep: Callable[[float], Awaitable[None]] = asyncio.sleep, start_delay: float = START_DELAY, - interval: float = CHECK_INTERVAL, cooldown: float = MANUAL_COOLDOWN_SECONDS): + interval: float = CHECK_INTERVAL, cooldown: float = MANUAL_COOLDOWN_SECONDS, + time_fn: Callable[[], float] = time.time, + version_check_interval: float = VERSION_CHECK_INTERVAL, + version_change_cooldown: float = VERSION_CHANGE_COOLDOWN): self._db = db self._service = service self._notifier = notifier @@ -37,8 +40,16 @@ class SelftestRunner: self._sleep = sleep self._start_delay = start_delay self._interval = interval + self._time = time_fn + self._version_check_interval = version_check_interval + self._version_change_cooldown = version_change_cooldown self._manual_limiter = SlidingWindowLimiter(time.time, cooldown) self._last_attempt: float | None = None + # None = noch nicht bekannt (wird beim ersten Check aus selftest_state nachgeladen). + self._known_version: str | None = None + self._known_version_loaded = False + self._last_version_check: float | None = None + self._last_version_change_run: float | None = None def try_manual(self) -> bool: return self._manual_limiter.try_acquire(1) @@ -76,6 +87,38 @@ class SelftestRunner: log.exception("Selbsttest fehlgeschlagen") return True + async def check_version_change(self) -> bool: + if not self._enabled(): + return False + now = self._time() + if (self._last_version_check is not None + and now - self._last_version_check < self._version_check_interval): + return False + self._last_version_check = now + if not self._known_version_loaded: + record = await asyncio.to_thread(self._service.last_run) + self._known_version = (record.version if record else "") or None + self._known_version_loaded = True + version = await self._service.peek_version() + if not version or version == self._known_version: + return False + first_known = self._known_version is None + # Neue Version sofort merken, damit ein hängender/fehlschlagender Lauf nicht erneut auslöst. + self._known_version = version + if first_known: + return False + if (self._last_version_change_run is not None + and now - self._last_version_change_run < self._version_change_cooldown): + return False + self._last_version_change_run = now + try: + await self._execute(SYSTEM_USER) + except SelftestNotConfigured: + pass + except Exception: + log.exception("Selbsttest: Lauf nach Versionswechsel fehlgeschlagen") + return True + async def run(self) -> None: await self._sleep(self._start_delay) startup = True @@ -85,4 +128,8 @@ class SelftestRunner: except Exception: log.exception("Selbsttest: Prüfung fehlgeschlagen") startup = False - await self._sleep(self._interval) + try: + await self.check_version_change() + except Exception: + log.exception("Selbsttest: Versionsprüfung fehlgeschlagen") + await self._sleep(min(self._interval, self._version_check_interval)) diff --git a/app/selftest/service.py b/app/selftest/service.py index d9d3791..3f43226 100644 --- a/app/selftest/service.py +++ b/app/selftest/service.py @@ -24,6 +24,16 @@ class SelftestService: def last_run(self) -> SelftestRun | None: return self._repo.last_run() + async def peek_version(self) -> str | None: + client = self._client_factory(self._store) + if client is None: + return None + try: + _, version = await check_version(client) + finally: + await client.aclose() + return version or None + async def run(self) -> SelftestRun: client = self._client_factory(self._store) if client is None: diff --git a/tests/test_selftest.py b/tests/test_selftest.py index 4fe1fae..69c6ea4 100644 --- a/tests/test_selftest.py +++ b/tests/test_selftest.py @@ -1,3 +1,4 @@ +import json import os import httpx @@ -98,6 +99,56 @@ async def test_check_downtimes_falls_back_to_query_then_to_derived(): assert result.status == STATUS_DEVIATED and "abgeleitet" in result.text and "1 Downtime" in result.text +async def test_check_services_probes_sample_when_no_problems(): + def handler(request): + body = json.loads(request.content) + if "query" in body: + return httpx.Response(200, json={"value": []}) + return httpx.Response(200, json={"value": [service_row()]}) + + result, services = await check_services(make_client(handler)) + assert result.status == STATUS_OK and "Stichprobe" in result.text and services == [] + + +async def test_check_services_sample_detects_missing_column(): + def handler(request): + body = json.loads(request.content) + if "query" in body: + return httpx.Response(200, json={"value": []}) + return httpx.Response(200, json={"value": [ + {"extensions": {"host_name": "h", "description": "s", "state": 2}}]}) + + result, services = await check_services(make_client(handler)) + assert result.status == STATUS_DEVIATED and "last_state_change" in result.text + + +async def test_check_services_empty_checkmk_is_ok_not_checkable(): + def handler(request): + return httpx.Response(200, json={"value": []}) + + result, services = await check_services(make_client(handler)) + assert result.status == STATUS_OK and "nicht prüfbar" in result.text and services == [] + + +async def test_check_hosts_probes_sample_when_no_problems(): + def handler(request): + body = json.loads(request.content) + if "query" in body: + return httpx.Response(200, json={"value": []}) + return httpx.Response(200, json={"value": [host_row()]}) + + result, hosts = await check_hosts(make_client(handler)) + assert result.status == STATUS_OK and "Stichprobe" in result.text and hosts == [] + + +async def test_check_hosts_empty_checkmk_is_ok_not_checkable(): + def handler(request): + return httpx.Response(200, json={"value": []}) + + result, hosts = await check_hosts(make_client(handler)) + assert result.status == STATUS_OK and "nicht prüfbar" in result.text and hosts == [] + + def make_env(): db = Database(":memory:") store = SettingsStore(db, SecretBox(os.urandom(32))) diff --git a/tests/test_selftest_runner.py b/tests/test_selftest_runner.py index 06ea6ec..82f3b75 100644 --- a/tests/test_selftest_runner.py +++ b/tests/test_selftest_runner.py @@ -46,11 +46,13 @@ class FakeNotifier: class FakeService: - def __init__(self, runs=None, error=None): + def __init__(self, runs=None, error=None, versions=None): self.runs = list(runs or []) self.error = error self.record = None self.call_count = 0 + self.versions = list(versions or []) + self.peek_calls = 0 def last_run(self): return self.record @@ -63,6 +65,10 @@ class FakeService: self.record = run return run + async def peek_version(self): + self.peek_calls += 1 + return self.versions.pop(0) if self.versions else None + def run_with(ts, status, version_changed=False): return SelftestRun(ts, "2.5.0", [CheckResult(CHECK_VERSION, status, "x")], version_changed) @@ -140,3 +146,59 @@ async def test_notify_disabled_sends_nothing(): await runner.run_manual("admin1") await runner.run_manual("admin1") assert runner._notifier.calls == [] + + +def make_version_runner(service, time_fn, enabled=lambda: True): + db = Database(":memory:") + return SelftestRunner(db, service, FakeNotifier(), enabled, lambda: True, + clock=Moment(at(6, 10)), time_fn=time_fn, + version_check_interval=300.0, version_change_cooldown=300.0) + + +async def test_version_change_triggers_immediate_run(): + service = FakeService([run_with(1, STATUS_OK)], versions=["2.4.0", "2.5.0"]) + runner = make_version_runner(service, Moment(1000.0)) + # Erster Check lädt die bekannte Version (2.4.0), gilt nicht als Wechsel. + assert await runner.check_version_change() is False + assert service.call_count == 0 + + runner._time.value = 1301.0 + assert await runner.check_version_change() is True + assert service.call_count == 1 + + +async def test_version_change_first_run_ever_is_not_a_change(): + service = FakeService([run_with(1, STATUS_OK)], versions=["2.5.0"]) + runner = make_version_runner(service, Moment(1000.0)) + assert await runner.check_version_change() is False + assert service.call_count == 0 + + +async def test_version_change_is_debounced_between_checks(): + service = FakeService([], versions=["2.4.0", "2.5.0"]) + runner = make_version_runner(service, Moment(1000.0)) + assert await runner.check_version_change() is False + runner._time.value = 1100.0 + assert await runner.check_version_change() is False + assert service.peek_calls == 1 + + +async def test_version_change_cooldown_prevents_second_run(): + service = FakeService([run_with(1, STATUS_OK), run_with(2, STATUS_OK)], + versions=["2.4.0", "2.5.0", "2.6.0"]) + runner = make_version_runner(service, Moment(1000.0)) + await runner.check_version_change() + runner._time.value = 1301.0 + assert await runner.check_version_change() is True + assert service.call_count == 1 + + runner._time.value = 1602.0 + assert await runner.check_version_change() is True + assert service.call_count == 2 + + +async def test_version_change_disabled_runner_never_checks(): + service = FakeService([], versions=["2.4.0"]) + runner = make_version_runner(service, Moment(1000.0), enabled=lambda: False) + assert await runner.check_version_change() is False + assert service.peek_calls == 0