fix(mcp): the overview drops an Ansible failure a later run put right (#5842)
get_overview listed every failed or interrupted run of the last 24 hours, so a dry run that failed, was fixed and re-run green kept warning for a day. A failure now drops out once a later success of the same playbook, source and hosts is at least as real: an applied success clears either kind of failure, a dry-run success only a dry-run failure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -179,6 +179,24 @@ def agent_findings(hosts, agent: dict, thresholds: dict,
|
|||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
def _run_key(r) -> tuple:
|
||||||
|
return (r.source_name, r.playbook_path, r.inventory_scope)
|
||||||
|
|
||||||
|
|
||||||
|
def _dry(r) -> bool:
|
||||||
|
return bool((r.params or {}).get("check"))
|
||||||
|
|
||||||
|
|
||||||
|
def still_failing(failed, succeeded) -> list:
|
||||||
|
"""The failed runs nothing has since put right: a later success of the
|
||||||
|
same playbook from the same source on the same hosts clears one, as long
|
||||||
|
as it was at least as real (a dry run's success clears only a dry run's
|
||||||
|
failure)."""
|
||||||
|
return [f for f in failed if not any(
|
||||||
|
_run_key(s) == _run_key(f) and s.started_at > f.started_at and (_dry(f) or not _dry(s))
|
||||||
|
for s in succeeded)]
|
||||||
|
|
||||||
|
|
||||||
def run_findings(runs) -> list[dict]:
|
def run_findings(runs) -> list[dict]:
|
||||||
out = []
|
out = []
|
||||||
for r in runs:
|
for r in runs:
|
||||||
@@ -292,6 +310,13 @@ async def get_overview() -> dict:
|
|||||||
.order_by(AnsibleRun.started_at.desc())
|
.order_by(AnsibleRun.started_at.desc())
|
||||||
.limit(20)
|
.limit(20)
|
||||||
)).scalars())
|
)).scalars())
|
||||||
|
if runs:
|
||||||
|
runs = still_failing(runs, (await db.execute(
|
||||||
|
select(AnsibleRun)
|
||||||
|
.where(AnsibleRun.status == AnsibleRunStatus.success,
|
||||||
|
AnsibleRun.started_at > min(r.started_at for r in runs),
|
||||||
|
AnsibleRun.playbook_path.in_({r.playbook_path for r in runs}))
|
||||||
|
)).scalars())
|
||||||
windows = await alert_svc.list_maintenance_windows(db)
|
windows = await alert_svc.list_maintenance_windows(db)
|
||||||
open_anomalies = await anom.open_anomalies(db)
|
open_anomalies = await anom.open_anomalies(db)
|
||||||
to_review = await anom.to_review_count(db)
|
to_review = await anom.to_review_count(db)
|
||||||
|
|||||||
@@ -195,6 +195,25 @@ def test_run_findings_quote_the_failures():
|
|||||||
assert f["kind"] == "ansible_run_failed" and "apt lock held" in f["summary"]
|
assert f["kind"] == "ansible_run_failed" and "apt lock held" in f["summary"]
|
||||||
|
|
||||||
|
|
||||||
|
def _run(status, ago, *, check=False, scope="steward:target:util", playbook="upkeep/x.yml"):
|
||||||
|
return NS(id=f"{status}-{ago}", source_name="steward-local", playbook_path=playbook,
|
||||||
|
inventory_scope=scope, status=status, started_at=NOW - timedelta(minutes=ago),
|
||||||
|
params={"check": check})
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_failed_run_put_right_since_is_not_a_finding():
|
||||||
|
F, S = AnsibleRunStatus.failed, AnsibleRunStatus.success
|
||||||
|
dry_fail, real_fail = _run(F, 60, check=True), _run(F, 50)
|
||||||
|
# A dry run fixed and re-run dry: the dry failure is cleared, the real one is not.
|
||||||
|
assert overview.still_failing([dry_fail, real_fail], [_run(S, 55, check=True)]) == [real_fail]
|
||||||
|
# A dry success after a real failure doesn't clear it; a real success does.
|
||||||
|
assert overview.still_failing([real_fail], [_run(S, 40, check=True)]) == [real_fail]
|
||||||
|
assert overview.still_failing([real_fail], [_run(S, 40)]) == []
|
||||||
|
# A success elsewhere, or before the failure, clears nothing.
|
||||||
|
assert overview.still_failing([real_fail], [_run(S, 40, scope="steward:target:other"),
|
||||||
|
_run(S, 40, playbook="upkeep/y.yml"), _run(S, 70)]) == [real_fail]
|
||||||
|
|
||||||
|
|
||||||
def test_rank_puts_critical_first_then_oldest():
|
def test_rank_puts_critical_first_then_oldest():
|
||||||
fs = [overview.finding("warning", "k", "w-old", "", since=NOW - timedelta(days=1)),
|
fs = [overview.finding("warning", "k", "w-old", "", since=NOW - timedelta(days=1)),
|
||||||
overview.finding("critical", "k", "c-new", "", since=NOW - timedelta(minutes=1)),
|
overview.finding("critical", "k", "c-new", "", since=NOW - timedelta(minutes=1)),
|
||||||
|
|||||||
Reference in New Issue
Block a user