feat(alerts): renames carry their alerts — monitor and host renames move rules, windows and history (#5065)
#5065 "Renames carry their alerts". Alert rules, maintenance-window scopes and metric history key a thing by (source, name), so a rename used to leave the rules watching a name nothing reported under, and split the history. - core/renames.carry_rename moves AlertRule.resource_name, MaintenanceWindow.scope_resource, plugin_metrics and plugin_metrics_hourly to the new name in the caller's transaction. A host rename also carries its parts: host_agent "<host>:…" and docker "<host>/…" (HOST_SOURCES). Matching is exact (a prefix compare, not LIKE or a collation range). History left under the new name by something deleted is dropped first rather than merged in. - Web monitor edit, MCP update_monitor and web host edit call it; MCP answers with `carried` (it used to return a warning naming stranded rules). - A name another monitor of the same type, or another host, already has is refused on create and rename: two would share one history. - The rename is audited (monitor.updated / host.renamed) with what moved. Tests: an integration test renames a firing monitor through the web route and finds its rule still firing and cleared by a sample on the new name, its window and raw+hourly history moved and an orphan's history dropped; a host rename moves agent and container parts but not a longer-named host's or another source's; a guard that every host_agent resource is "<host>" or "<host>:…". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
"""Integration: the monitoring-config write tools against real Postgres.
|
||||
|
||||
A monitor is created on a host, edited (config merged, a rename warning for
|
||||
the rule that watches it by name), paused and deleted; a rule is created on
|
||||
A monitor is created on a host, edited (config merged, a rename that carries
|
||||
the rule watching it by name), paused and deleted; a rule is created on
|
||||
that monitor, edited, acknowledged once firing, and deleted; a maintenance
|
||||
window in progress is ended and an upcoming one cancelled. Every write leaves
|
||||
an audit row as "<user> via MCP".
|
||||
@@ -95,11 +95,15 @@ def test_monitor_rule_ack_and_maintenance_writes(app):
|
||||
row = await s.get(Monitor, made["monitor_id"])
|
||||
assert row.config["headers"] == {"Authorization": "Bearer s3cret"}
|
||||
|
||||
# A rename leaves the rule on the old name, and says so.
|
||||
# A rename takes the rule with it, and says what moved.
|
||||
renamed = await m.update_monitor(mon_name, name=f"{mon_name}-2")
|
||||
assert rule_name in renamed["warning"]
|
||||
await a.update_alert_rule(rule_name, resource=f"{mon_name}-2")
|
||||
assert "warning" not in await m.update_monitor(f"{mon_name}-2", check_interval_s=30)
|
||||
assert renamed["carried"]["from"] == mon_name and renamed["carried"]["rules"] == 1
|
||||
async with app.db_sessionmaker() as s:
|
||||
assert (await s.get(AlertRule, rule["rule_id"])).resource_name == f"{mon_name}-2"
|
||||
assert "carried" not in await m.update_monitor(f"{mon_name}-2", check_interval_s=30)
|
||||
# Another http monitor may not take the same name: they would share one history.
|
||||
with pytest.raises(ValueError, match="already named"):
|
||||
await m.create_monitor(f"{mon_name}-2", "http", "other.lan")
|
||||
assert (await m.set_monitor_enabled(f"{mon_name}-2", False))["enabled"] is False
|
||||
|
||||
# ── acknowledge ────────────────────────────────────────────────────
|
||||
|
||||
@@ -0,0 +1,165 @@
|
||||
"""Integration: a renamed monitor or host takes its alert rules, maintenance
|
||||
windows and metric history with it (#5065), through the web edit routes,
|
||||
against real Postgres."""
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import os
|
||||
import time
|
||||
import uuid
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
import pytest
|
||||
|
||||
pytestmark = pytest.mark.integration
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def app():
|
||||
if not os.environ.get("STEWARD_DATABASE_URL"):
|
||||
pytest.skip("needs Postgres")
|
||||
from steward.app import create_app
|
||||
return create_app(testing=False)
|
||||
|
||||
|
||||
async def _operator(app, tag):
|
||||
from steward.models.users import User, UserRole
|
||||
uid = str(uuid.uuid4())
|
||||
async with app.db_sessionmaker() as s:
|
||||
async with s.begin():
|
||||
s.add(User(id=uid, username=f"ren-{tag}", email=f"ren-{tag}@test",
|
||||
password_hash="x", role=UserRole.operator, is_active=True))
|
||||
client = app.test_client()
|
||||
async with client.session_transaction() as sess:
|
||||
sess.update(user_id=uid, user_role="operator", username=f"ren-{tag}",
|
||||
_csrf="test-token", login_at=time.time())
|
||||
return client, uid
|
||||
|
||||
|
||||
CSRF = {"X-CSRF-Token": "test-token"}
|
||||
|
||||
|
||||
def test_a_renamed_monitor_keeps_its_firing_rule_window_and_history(app):
|
||||
from sqlalchemy import func, select
|
||||
from steward.alerts import service as alert_svc
|
||||
from steward.core.alerts import record_metric
|
||||
from steward.models.alerts import AlertRule, AlertState, AlertStateEnum, MaintenanceWindow
|
||||
from steward.models.metrics import PluginMetric, PluginMetricHourly
|
||||
from steward.models.monitors import Monitor
|
||||
|
||||
async def _go():
|
||||
tag = uuid.uuid4().hex[:8]
|
||||
old, new = f"svc-{tag}", f"svc-{tag}-renamed"
|
||||
client, uid = await _operator(app, tag)
|
||||
mid = str(uuid.uuid4())
|
||||
now = datetime.now(timezone.utc)
|
||||
async with app.db_sessionmaker() as s:
|
||||
async with s.begin():
|
||||
s.add(Monitor(id=mid, name=old, type="tcp", target="10.9.9.9", config_json='{"port": 22}'))
|
||||
s.add(Monitor(id=str(uuid.uuid4()), name=f"other-{tag}", type="tcp", target="10.9.9.8",
|
||||
config_json='{"port": 22}'))
|
||||
# History under the old name, and an orphan's under the new one.
|
||||
s.add(PluginMetric(source_module="tcp", resource_name=old, metric_name="is_up", value=1))
|
||||
s.add(PluginMetricHourly(source_module="tcp", resource_name=old, metric_name="is_up",
|
||||
bucket=now.replace(minute=0, second=0, microsecond=0) - timedelta(days=3),
|
||||
value_avg=1, value_max=1, sample_count=60))
|
||||
s.add(PluginMetric(source_module="tcp", resource_name=new, metric_name="is_up", value=0,
|
||||
recorded_at=now - timedelta(days=30)))
|
||||
await s.flush()
|
||||
fields = await alert_svc.rule_fields(
|
||||
s, name=f"down {tag}", source_module="tcp", resource_name=old,
|
||||
metric_name="is_up", operator="<", threshold=1,
|
||||
consecutive_failures_required=1, consecutive_ok_required=1)
|
||||
rule = await alert_svc.add_rule(s, fields, created_by=uid)
|
||||
win = MaintenanceWindow(name=f"later {tag}", scope_source="tcp", scope_resource=old,
|
||||
created_by=uid, start_at=now + timedelta(days=1), end_at=now + timedelta(days=1, hours=1))
|
||||
s.add(win)
|
||||
rule_id, win_id = rule.id, win.id
|
||||
|
||||
async def sample(name, v):
|
||||
async with app.db_sessionmaker() as s:
|
||||
async with s.begin():
|
||||
await record_metric(s, "tcp", name, "is_up", v)
|
||||
|
||||
async def state():
|
||||
async with app.db_sessionmaker() as s:
|
||||
return (await s.execute(select(AlertState).where(AlertState.rule_id == rule_id))).scalar_one()
|
||||
|
||||
await sample(old, 0)
|
||||
assert (await state()).state == AlertStateEnum.firing
|
||||
|
||||
# Renaming to a name another tcp monitor has is refused, and moves nothing.
|
||||
resp = await client.post(f"/monitors/{mid}/edit", headers=CSRF, form={
|
||||
"name": f"other-{tag}", "target": "10.9.9.9", "port": "22"})
|
||||
assert resp.status_code == 400 and "already named" in await resp.get_data(as_text=True)
|
||||
|
||||
resp = await client.post(f"/monitors/{mid}/edit", headers=CSRF, form={
|
||||
"name": new, "target": "10.9.9.9", "port": "22"})
|
||||
assert resp.status_code == 302
|
||||
|
||||
async with app.db_sessionmaker() as s:
|
||||
assert (await s.get(AlertRule, rule_id)).resource_name == new
|
||||
assert (await s.get(MaintenanceWindow, win_id)).scope_resource == new
|
||||
raw = (await s.execute(select(PluginMetric.value, PluginMetric.resource_name).where(
|
||||
PluginMetric.source_module == "tcp",
|
||||
PluginMetric.resource_name.in_([old, new])))).all()
|
||||
hourly = (await s.execute(select(func.count()).select_from(PluginMetricHourly).where(
|
||||
PluginMetricHourly.source_module == "tcp",
|
||||
PluginMetricHourly.resource_name == new))).scalar_one()
|
||||
# Old samples (1 and the 0 that fired it) moved; the orphan's 30-day-old 0 was dropped.
|
||||
assert {r for _, r in raw} == {new} and sorted(v for v, _ in raw) == [0, 1]
|
||||
assert hourly == 1
|
||||
|
||||
# Still firing, and now watching the new name: one OK sample clears it.
|
||||
assert (await state()).state == AlertStateEnum.firing
|
||||
await sample(old, 1) # the old name is no one's now
|
||||
assert (await state()).state == AlertStateEnum.firing
|
||||
await sample(new, 1)
|
||||
assert (await state()).state == AlertStateEnum.inactive
|
||||
|
||||
asyncio.run(_go())
|
||||
|
||||
|
||||
def test_a_renamed_host_carries_agent_and_container_metrics_and_their_rules(app):
|
||||
from sqlalchemy import select
|
||||
from steward.alerts import service as alert_svc
|
||||
from steward.models.alerts import AlertRule
|
||||
from steward.models.hosts import Host
|
||||
from steward.models.metrics import PluginMetric
|
||||
|
||||
async def _go():
|
||||
tag = uuid.uuid4().hex[:8]
|
||||
old, new = f"h{tag}", f"h{tag}-new"
|
||||
client, uid = await _operator(app, tag)
|
||||
hid = str(uuid.uuid4())
|
||||
rows = [("host_agent", old), ("host_agent", f"{old}:/"), ("host_agent", f"{old}:core0"),
|
||||
("docker", f"{old}/nginx"),
|
||||
# Not this host's: a longer name sharing the prefix, and a different source.
|
||||
("host_agent", f"{old}x:/"), ("snmp", old)]
|
||||
async with app.db_sessionmaker() as s:
|
||||
async with s.begin():
|
||||
s.add(Host(id=hid, name=old, address="10.8.8.8"))
|
||||
s.add(Host(id=str(uuid.uuid4()), name=f"{old}x", address="10.8.8.9"))
|
||||
for src, res in rows:
|
||||
s.add(PluginMetric(source_module=src, resource_name=res, metric_name="disk_used_pct", value=1))
|
||||
await s.flush()
|
||||
fields = await alert_svc.rule_fields(
|
||||
s, name=f"disk {tag}", source_module="host_agent", resource_name=f"{old}:/",
|
||||
metric_name="disk_used_pct", operator=">", threshold=90,
|
||||
consecutive_failures_required=1, consecutive_ok_required=1)
|
||||
rule = await alert_svc.add_rule(s, fields, created_by=uid)
|
||||
rule_id = rule.id
|
||||
|
||||
resp = await client.post(f"/hosts/{hid}", headers=CSRF, form={"name": f"{old}x", "address": "10.8.8.8"})
|
||||
assert resp.status_code == 400
|
||||
|
||||
resp = await client.post(f"/hosts/{hid}", headers=CSRF, form={"name": new, "address": "10.8.8.8"})
|
||||
assert resp.status_code == 302
|
||||
async with app.db_sessionmaker() as s:
|
||||
got = set((await s.execute(select(PluginMetric.source_module, PluginMetric.resource_name).where(
|
||||
PluginMetric.resource_name.like(f"h{tag}%")))).all())
|
||||
assert (await s.get(AlertRule, rule_id)).resource_name == f"{new}:/"
|
||||
assert got == {("host_agent", new), ("host_agent", f"{new}:/"), ("host_agent", f"{new}:core0"),
|
||||
("docker", f"{new}/nginx"), ("host_agent", f"{old}x:/"), ("snmp", old)}
|
||||
|
||||
asyncio.run(_go())
|
||||
@@ -58,3 +58,23 @@ def test_expand_is_backcompat_with_old_agent_sample():
|
||||
n.startswith(("net_", "disk_read", "disk_write", "psi_", "temp_", "mem_cached"))
|
||||
for n in names
|
||||
)
|
||||
|
||||
|
||||
def test_every_resource_is_the_host_or_a_host_colon_part():
|
||||
"""A host rename carries "<host>" and "<host>:…" (steward/core/renames.py
|
||||
HOST_SOURCES); a resource named any other way would stay behind."""
|
||||
from steward.core.renames import HOST_SOURCES
|
||||
sample = {
|
||||
"ts": "x", "cpu_pct": 1.0, "cpu_cores": [1.0],
|
||||
"mem": {"total_bytes": 10, "available_bytes": 5, "cached_bytes": 1, "buffers_bytes": 1},
|
||||
"load": {"1m": 0.1}, "uptime_secs": 1,
|
||||
"storage": [{"mount": "/", "total_bytes": 10, "used_bytes": 5}],
|
||||
"net": [{"iface": "eth0", "rx_bps": 1.0, "tx_bps": 1.0}],
|
||||
"diskio": [{"device": "sda", "read_bps": 1.0, "write_bps": 1.0}],
|
||||
"temps": [{"label": "Package", "celsius": 40.0}],
|
||||
"psi": {"cpu_some_avg10": 0.0},
|
||||
}
|
||||
sep = dict(HOST_SOURCES)["host_agent"]
|
||||
resources = {r.resource_name for r in _expand_sample_to_metrics(sample, "web01", datetime.now(timezone.utc))}
|
||||
assert len(resources) > 5
|
||||
assert {r for r in resources if r != "web01" and not r.startswith("web01" + sep)} == set()
|
||||
|
||||
Reference in New Issue
Block a user