diff --git a/docs/core/mcp.md b/docs/core/mcp.md index e8852f8..872d19c 100644 --- a/docs/core/mcp.md +++ b/docs/core/mcp.md @@ -76,6 +76,7 @@ Each role includes the tools of the roles above it in this list. | Monitors | `list_monitors`, `get_monitor` | | Alerts | `list_alerts`, `get_alert_history`, `list_alert_rules`, `get_alert_rule_options`, `list_maintenance_windows` | | Metric anomalies | `list_anomalies` (metrics that left their normal for the hour, to review first), `get_anomaly` (one anomaly with each occurrence against what is usual at that hour, the metric around it, what else happened on its host — alerts, Ansible runs, container deaths and starts, maintenance, reboots, other anomalies — and how earlier ones on the same metric were judged), `get_metric_normal` (the learned normal slot by slot, and the level that would be flagged), `get_anomaly_settings` (the review window and each floor) | +| Thresholds | `get_thresholds` (the warn and crit cutoffs CPU, memory, disk, load and temperature are graded by on the pages) | | Automation | `list_playbooks`, `get_playbook`, `list_inventory`, `list_ansible_runs`, `get_ansible_run`, `wait_for_run`, `list_schedules` | | Audit | `list_audit_events` | | Timeline | `get_timeline` (everything that happened, newest first, in one list: alerts firing and clearing, playbook runs and what they changed, maintenance, reboots, deploys and crash-loops, reviewed anomalies, changes recorded by hand, changes to Steward's settings; by host, area or kind, and kept after the records it came from age out) | @@ -102,7 +103,8 @@ Each role includes the tools of the roles above it in this list. ### Admin: destructive and remediation `set_anomaly_settings` (the anomaly review window and each normal's floor, as -Settings → Thresholds sets them), `set_alert_action` (the playbook a rule runs when it fires), `delete_monitor`, +Settings → Thresholds sets them), `set_thresholds` (the warn and crit cutoffs host +readings are graded by, as the same page sets them), `set_alert_action` (the playbook a rule runs when it fires), `delete_monitor`, `delete_alert_rule`, `prune_docker`, `run_maintenance` (update a host now, rebooting only if the update needs it, as the host page's Update now does). diff --git a/plugins/host_agent/agent.py b/plugins/host_agent/agent.py index bbb6c5b..58570be 100644 --- a/plugins/host_agent/agent.py +++ b/plugins/host_agent/agent.py @@ -22,7 +22,7 @@ from collections import deque from concurrent.futures import ThreadPoolExecutor, wait as wait_futures from datetime import datetime, timezone -AGENT_VERSION = "1.26.1" +AGENT_VERSION = "1.27.0" # Default path to the local Docker Engine socket. Overridable via the # `docker_socket` config key; collection is silently skipped if it's absent or @@ -480,7 +480,9 @@ def nfs_delays(cur: dict, prev: dict, dt: float) -> list: def collect_temps() -> list: - """Return [{label, celsius}] from /sys/class/hwmon (best-effort, may be empty).""" + """Return [{label, celsius, chip}] from /sys/class/hwmon (best-effort, may + be empty). `chip` is the hwmon driver's name (coretemp, nvme, nct6798…), + which says what a bare label like "Sensor 2" is measuring.""" out: list = [] try: chips = os.listdir(HWMON_DIR) @@ -511,7 +513,7 @@ def collect_temps() -> list: except OSError: pass name = label or (f"{chip}_{idx}" if chip else idx) - out.append({"label": name, "celsius": round(milli / 1000.0, 1)}) + out.append({"label": name, "celsius": round(milli / 1000.0, 1), "chip": chip}) return out diff --git a/plugins/host_agent/routes.py b/plugins/host_agent/routes.py index 04b7469..e50cc48 100644 --- a/plugins/host_agent/routes.py +++ b/plugins/host_agent/routes.py @@ -124,13 +124,15 @@ def is_thermometer(host_name: str, label: str, celsius: float) -> bool: def hottest_sensor(host_name: str, temps: dict[str, float]) -> list[str]: """Which sensor sets a host's temperature, as an alert on it says: - ["Package id 0 at 86°C"], or [] with no thermometer among `temps`.""" + ["CPU · Package id 0 at 86°C"], or [] with no thermometer among `temps`. + The chip leads the label when the host's agent has said which it is.""" from steward.core.formatting import metric_value real = {label: c for label, c in temps.items() if is_thermometer(host_name, label, c)} if not real: return [] + from .sensors import sensor_name label = max(real, key=real.get) - return [f"{label} at {metric_value('temp_c', real[label])}"] + return [f"{sensor_name(host_name, label)} at {metric_value('temp_c', real[label])}"] async def name_hottest_sensor(session, resource: str) -> list[str]: @@ -374,6 +376,8 @@ def _expand_sample_to_metrics( # counts only readings that can be real (#5759), from sensors that behave # like thermometers (#5783). max_temp: float | None = None + from .sensors import note_chips + note_chips(host_name, sample.get("temps")) for t in sample.get("temps") or []: c = t.get("celsius") if c is None: diff --git a/plugins/host_agent/sensors.py b/plugins/host_agent/sensors.py index 031f176..8c0737a 100644 --- a/plugins/host_agent/sensors.py +++ b/plugins/host_agent/sensors.py @@ -53,6 +53,51 @@ def unwired(host_name: str) -> dict[str, str]: return _unwired.get(host_name, {}) +# {host name: {sensor label: hwmon chip}}, as the host's latest sample said. +# A sensor's metric is keyed by its label alone; the chip only names it. +_chips: dict[str, dict[str, str]] = {} + +# What each hwmon driver measures, in the words a page uses. A driver not +# listed is shown by its own name. +_CHIP_WORDS = ( + (re.compile(r"^(coretemp|k10temp|zenpower|cpu_thermal)$"), "CPU"), + (re.compile(r"^nvme$"), "NVMe"), + (re.compile(r"^drivetemp$"), "Drive"), + (re.compile(r"^(nct\d+|it\d+|w83\w+|f71\w+)$"), "Board"), + (re.compile(r"^pch_\w+$"), "Chipset"), + (re.compile(r"^(amdgpu|radeon|nouveau|i915|xe)$"), "GPU"), + (re.compile(r"^(spd5118|jc42)$"), "Memory"), + (re.compile(r"^iwlwifi"), "Wi-Fi"), + (re.compile(r"^acpitz$"), "ACPI"), +) + + +def note_chips(host_name: str, temps) -> None: + """Remember which chip each of a host's sensors is on, from a sample's + temps. An agent that doesn't say leaves what is known as it is.""" + seen = {str(t["label"]): str(t["chip"]) for t in temps or [] if t.get("chip")} + if seen: + _chips[host_name] = seen + + +def chip_word(chip: str) -> str: + """What a hwmon chip measures ("nvme" → "NVMe"), or its own name.""" + for pattern, word in _CHIP_WORDS: + if pattern.match(chip): + return word + return chip + + +def sensor_name(host_name: str, label: str) -> str: + """A sensor as a page names it: its chip, then its label, as in + "NVMe · Sensor 2". The label alone when the chip isn't known or the + label already carries it ("nvme_temp1").""" + chip = _chips.get(host_name, {}).get(label) + if not chip or label.lower().startswith(chip.lower()): + return label + return f"{chip_word(chip)} · {label}" + + def reference_label(labels) -> str | None: """The sensor that reads the host's CPU, or None when it has none.""" labels = sorted(labels) diff --git a/steward/mcp/tools/__init__.py b/steward/mcp/tools/__init__.py index a6544ac..5a90810 100644 --- a/steward/mcp/tools/__init__.py +++ b/steward/mcp/tools/__init__.py @@ -13,10 +13,10 @@ each argument's empty value means, and what comes back. from __future__ import annotations from . import (alerts, anomalies, audit, automation, backups, hosts, metrics, monitors, overview, session, - tickets, timeline) + thresholds, tickets, timeline) -_MODULES = (session, overview, hosts, monitors, alerts, anomalies, metrics, audit, timeline, tickets, - automation, backups) +_MODULES = (session, overview, hosts, monitors, alerts, anomalies, thresholds, metrics, audit, timeline, + tickets, automation, backups) def register_all(register_tool) -> None: diff --git a/steward/mcp/tools/thresholds.py b/steward/mcp/tools/thresholds.py new file mode 100644 index 0000000..376a95f --- /dev/null +++ b/steward/mcp/tools/thresholds.py @@ -0,0 +1,94 @@ +"""The cutoffs a host's readings are graded by, as Settings → Thresholds sets +them: amber past `warn`, red past `crit`.""" +from __future__ import annotations + +from steward.core.settings import DEFAULTS, get_all_settings, set_setting, to_thresholds_cfg +from steward.mcp._context import mcp_app +from steward.models.users import UserRole + +from ._audit import audit_mcp +from ._shape import db_session + +# Each graded reading, its unit, and what it measures. +READINGS = { + "cpu": ("%", "CPU use"), + "mem": ("%", "memory used"), + "disk": ("%", "fullest mount's space used"), + "load": ("%", "1-minute load per core"), + "temp": ("°C", "hottest real thermometer"), +} + + +def _view(settings: dict) -> dict: + cfg = to_thresholds_cfg(settings) + return {kind: {"warn": cfg[kind]["warn"], "crit": cfg[kind]["crit"], "unit": unit, "measures": what, + "default": {"warn": DEFAULTS[f"thresholds.{kind}_warn"], + "crit": DEFAULTS[f"thresholds.{kind}_crit"]}} + for kind, (unit, what) in READINGS.items()} + + +async def get_thresholds() -> dict: + """The cutoffs host readings are graded by, as Settings → Thresholds + sets them: per reading (cpu, mem, disk, load, temp), `warn` (amber past + it) and `crit` (red past it) in `unit`, with Steward's default for each. + The host pages, the Overview and get_overview grade by these. Alert + rules have their own cutoffs (list_alert_rules); these don't fire + anything. set_thresholds (admin) changes them. + """ + async with db_session() as db: + return _view(await get_all_settings(db)) + + +async def set_thresholds(cutoffs: dict[str, dict[str, float]]) -> dict: + """Change the cutoffs host readings are graded by, as Settings → + Thresholds does. Takes effect on the next page load; audited. + + cutoffs: {reading: {"warn": n, "crit": n}} for any of get_thresholds' + readings, e.g. {"temp": {"crit": 90}}. Either of warn and crit may be + left out and keeps its value; warn must stay below crit. A reading not + named is left as it is. Returns the cutoffs as they now stand. + """ + unknown = sorted(set(cutoffs or {}) - set(READINGS)) + if unknown: + raise ValueError(f"No reading named {', '.join(unknown)}; they are {', '.join(READINGS)}.") + if not cutoffs: + raise ValueError("Nothing to change: pass cutoffs, e.g. {\"temp\": {\"crit\": 90}}.") + async with db_session() as db: + current = _view(await get_all_settings(db)) + changes: dict = {} + for kind, given in cutoffs.items(): + extra = sorted(set(given or {}) - {"warn", "crit"}) + if extra or not given: + raise ValueError(f"{kind} takes warn and/or crit, not {', '.join(extra) or 'nothing'}.") + new = {} + for edge in ("warn", "crit"): + if edge not in given: + new[edge] = current[kind][edge] + continue + try: + value = float(given[edge]) + except (TypeError, ValueError): + raise ValueError(f"{kind} {edge} must be a number.") from None + if not value > 0: + raise ValueError(f"{kind} {edge} must be more than 0.") + new[edge] = int(value) if value.is_integer() else value + changes[f"thresholds.{kind}_{edge}"] = new[edge] + if new["warn"] >= new["crit"]: + raise ValueError(f"{kind} warn ({new['warn']}) must be below its crit ({new['crit']}).") + app = mcp_app() + async with db_session() as db: + async with db.begin(): + before = await get_all_settings(db) + for key, value in changes.items(): + await set_setting(db, key, value) + fresh = await get_all_settings(db) + app.config["THRESHOLDS"] = to_thresholds_cfg(fresh) + await audit_mcp("settings.saved", "settings", "thresholds", + {"section": "thresholds", + "changes": {k: {"from": before.get(k), "to": v} for k, v in changes.items()}}) + return _view(fresh) + + +def register(register_tool) -> None: + register_tool(get_thresholds, role=UserRole.viewer, read_only=True) + register_tool(set_thresholds, role=UserRole.admin, read_only=False) diff --git a/tests/mcp/test_thresholds.py b/tests/mcp/test_thresholds.py new file mode 100644 index 0000000..d244614 --- /dev/null +++ b/tests/mcp/test_thresholds.py @@ -0,0 +1,83 @@ +"""get_thresholds / set_thresholds: the cutoffs host readings are graded by, +read and changed as Settings → Thresholds does.""" +from __future__ import annotations + +from contextlib import asynccontextmanager +from types import SimpleNamespace as NS + +import pytest + +from steward.mcp.tools import thresholds +from steward.models.users import UserRole + + +@pytest.fixture +def store(monkeypatch): + """The settings table, the app and the audit log, in memory.""" + saved: dict = {"thresholds.temp_crit": 85} + app = NS(config={}) + audited: list = [] + + class _Db: + @asynccontextmanager + async def begin(self): + yield + + @asynccontextmanager + async def db_session(): + yield _Db() + + async def get_all_settings(_db): + return dict(saved) + + async def set_setting(_db, key, value): + saved[key] = value + + async def audit_mcp(*args): + audited.append(args) + + monkeypatch.setattr(thresholds, "db_session", db_session) + monkeypatch.setattr(thresholds, "get_all_settings", get_all_settings) + monkeypatch.setattr(thresholds, "set_setting", set_setting) + monkeypatch.setattr(thresholds, "mcp_app", lambda: app) + monkeypatch.setattr(thresholds, "audit_mcp", audit_mcp) + return NS(saved=saved, app=app, audited=audited) + + +async def test_the_cutoffs_read_with_their_units_and_defaults(store): + got = await thresholds.get_thresholds() + assert set(got) == {"cpu", "mem", "disk", "load", "temp"} + assert got["temp"] == {"warn": 70, "crit": 85, "unit": "°C", "measures": "hottest real thermometer", + "default": {"warn": 70, "crit": 85}} + + +async def test_raising_one_cutoff_keeps_the_other_and_grades_by_it_at_once(store): + got = await thresholds.set_thresholds({"temp": {"crit": 90}}) + assert got["temp"]["crit"] == 90 and got["temp"]["warn"] == 70 + assert store.saved == {"thresholds.temp_crit": 90} # warn isn't written + assert store.app.config["THRESHOLDS"]["temp"]["crit"] == 90 + (_action, _kind, _id, detail), = store.audited + assert detail["changes"] == {"thresholds.temp_crit": {"from": 85, "to": 90}} + + +@pytest.mark.parametrize("cutoffs, refusal", [ + ({}, "Nothing to change"), + ({"fan": {"crit": 1}}, "No reading named fan"), + ({"temp": {"high": 90}}, "warn and/or crit"), + ({"temp": {}}, "warn and/or crit"), + ({"temp": {"crit": "hot"}}, "must be a number"), + ({"temp": {"crit": 0}}, "more than 0"), + ({"temp": {"crit": 60}}, r"warn \(70\) must be below its crit \(60\)"), +]) +async def test_refusals_change_nothing(store, cutoffs, refusal): + with pytest.raises(ValueError, match=refusal): + await thresholds.set_thresholds(cutoffs) + assert store.saved == {"thresholds.temp_crit": 85} and not store.audited + + +def test_the_tools_are_registered_with_their_roles(app): + mcp = app.mcp_server + for name, role, read_only in (("get_thresholds", UserRole.viewer, True), + ("set_thresholds", UserRole.admin, False)): + assert mcp.tool_roles[name] is role, name + assert mcp._tool_manager.get_tool(name).annotations.read_only_hint is read_only, name diff --git a/tests/plugins/host_agent/test_collectors.py b/tests/plugins/host_agent/test_collectors.py index 40551b7..ae7d0d1 100644 --- a/tests/plugins/host_agent/test_collectors.py +++ b/tests/plugins/host_agent/test_collectors.py @@ -146,3 +146,19 @@ def test_build_sample_first_call_has_no_rates_but_keeps_state(monkeypatch): assert s1["net"] == [] and s1["diskio"] == [] # no prior counters yet assert state["mono"] is not None assert state["net"] == {"eth0": (1000, 500)} + + +def test_collect_temps_says_which_chip_each_sensor_is_on(tmp_path, monkeypatch): + nvme = tmp_path / "hwmon1" + nvme.mkdir() + (nvme / "name").write_text("nvme\n") + (nvme / "temp3_input").write_text("49850\n") + (nvme / "temp3_label").write_text("Sensor 2\n") + bare = tmp_path / "hwmon2" + bare.mkdir() + (bare / "name").write_text("acpitz\n") + (bare / "temp1_input").write_text("27800\n") + monkeypatch.setattr(agent, "HWMON_DIR", str(tmp_path)) + got = sorted(agent.collect_temps(), key=lambda t: t["label"]) + assert got == [{"label": "Sensor 2", "celsius": 49.9, "chip": "nvme"}, + {"label": "acpitz_temp1", "celsius": 27.8, "chip": "acpitz"}] diff --git a/tests/plugins/host_agent/test_expand_metrics.py b/tests/plugins/host_agent/test_expand_metrics.py index 3d5a08d..39fbc27 100644 --- a/tests/plugins/host_agent/test_expand_metrics.py +++ b/tests/plugins/host_agent/test_expand_metrics.py @@ -114,6 +114,24 @@ def test_the_heat_alert_names_the_sensor_that_set_it(): assert hottest_sensor("box", {"AUXTIN0": 110.0}) == [] +def test_the_hottest_sensor_is_named_by_its_chip_once_the_agent_says_it(): + """An NVMe drive's "Sensor 2" alone doesn't say it's the drive.""" + from plugins.host_agent.routes import hottest_sensor + sample = {"ts": "2026-06-16T00:00:00+00:00", + "temps": [{"label": "Sensor 2", "celsius": 49.0, "chip": "nvme"}, + {"label": "Package id 0", "celsius": 41.0, "chip": "coretemp"}, + {"label": "nct6798_temp9", "celsius": 30.0, "chip": "nct6798"}, + {"label": "fan_ic", "celsius": 35.0, "chip": "mystery9"}]} + idx = _index(_expand_sample_to_metrics(sample, "chipbox", datetime.now(timezone.utc))) + assert idx[("temp_c", "chipbox:temp:Sensor 2")] == 49.0 # keyed by label alone + temps = {"Sensor 2": 49.0, "Package id 0": 41.0} + assert hottest_sensor("chipbox", temps) == ["NVMe · Sensor 2 at 49°C"] + assert hottest_sensor("chipbox", {"Package id 0": 41.0}) == ["CPU · Package id 0 at 41°C"] + assert hottest_sensor("chipbox", {"nct6798_temp9": 30.0}) == ["nct6798_temp9 at 30°C"] + assert hottest_sensor("chipbox", {"fan_ic": 35.0}) == ["mystery9 · fan_ic at 35°C"] + assert hottest_sensor("otherbox", temps) == ["Sensor 2 at 49°C"] # an agent that never said + + def test_a_wired_board_input_still_counts(): """#5759: below 100 °C a board input is a real reading.""" from plugins.host_agent.routes import counts_as_hottest