diff --git a/plugins/host_agent/cadence.py b/plugins/host_agent/cadence.py index 5d900e7..c7af7fc 100644 --- a/plugins/host_agent/cadence.py +++ b/plugins/host_agent/cadence.py @@ -29,9 +29,6 @@ WEEKDAYS = ("Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", SETTLE_LIMIT = timedelta(minutes=30) # An agent heard from this recently is posting. POSTING_WITHIN = timedelta(minutes=3) -# How a job failed for not coming back into service begins: its update (and -# reboot) went through, so once the host is back nothing is left failed. -NOT_BACK = "not back in service within" RUN_STATES = ("running", "done", "failed", "stopped") @@ -132,7 +129,7 @@ def next_move(run, plan, job, back: str | None, now: datetime) -> tuple[str, str if back is None: return "next", None if run.settle_since is not None and now - run.settle_since > SETTLE_LIMIT: - return "fail", f"{NOT_BACK} {int(SETTLE_LIMIT.total_seconds() // 60)} min: {back}" + return "fail", f"{maintenance.NOT_BACK} {int(SETTLE_LIMIT.total_seconds() // 60)} min: {back}" return "settling", back @@ -303,7 +300,9 @@ async def failed_hosts(session, now: datetime) -> dict[str, float]: figure an alert rule watches. A host with no job yet reads 0, so a rule on it reports rather than sitting at "no data". A later job that succeeds puts it back to 0, and so does a host that failed only for not - coming back into service in time, once it is back (#6105).""" + coming back into service in time, once it is back (#6105). That return + is kept on the job (came_back_at), so a later wobble in the host's + services doesn't flag it again (#6175).""" from sqlalchemy import select from steward.models.hosts import Host from .models import HostAgentRegistration, HostMaintenanceJob @@ -320,10 +319,11 @@ async def failed_hosts(session, now: datetime) -> dict[str, float]: if host.name not in latest or job.finished_at > latest[host.name][1].finished_at: latest[host.name] = (host, job) for name, (host, job) in latest.items(): - failed = job.state != "done" - if failed and (job.detail or "").startswith(NOT_BACK): + if (job.state == "failed" and job.came_back_at is None + and (job.detail or "").startswith(maintenance.NOT_BACK)): reg = (await session.execute(select(HostAgentRegistration).where( HostAgentRegistration.host_id == host.id))).scalar_one_or_none() - failed = await _back(session, host, job, reg, now) is not None - out[name] = 1.0 if failed else 0.0 + if await _back(session, host, job, reg, now) is None: + job.came_back_at = now + out[name] = 1.0 if maintenance.still_failed(job) else 0.0 return out diff --git a/plugins/host_agent/maintenance.py b/plugins/host_agent/maintenance.py index ab8fb04..a851be0 100644 --- a/plugins/host_agent/maintenance.py +++ b/plugins/host_agent/maintenance.py @@ -37,6 +37,10 @@ from datetime import datetime, timedelta, timezone ACTIVE = ("queued", "checking", "draining", "settling", "updating", "rebooting", "undraining", "recovering") FINISHED = ("done", "failed", "timed_out") +# How a job failed for not coming back into service begins (cadence.py): its +# update (and reboot) went through, so once the host is back nothing is left +# failed. +NOT_BACK = "not back in service within" # The helper action each handed state asks for. STEP_ACTION = {"checking": "check", "updating": "update", "draining": "drain", "rebooting": "reboot", "undraining": "undrain"} @@ -537,10 +541,26 @@ LABELS = { } +def came_back_late(job) -> bool: + """A job failed only for not coming back into service in time, whose + host has since been found back (#6175).""" + return (job.state == "failed" and job.came_back_at is not None + and (job.detail or "").startswith(NOT_BACK)) + + +def still_failed(job) -> bool: + """Whether a finished job still flags its host: it failed or timed out, + and isn't a late return whose host has since come back (#6175). The + maintenance_failed figure, the overview and the pages all ask this.""" + return job.state in ("failed", "timed_out") and not came_back_late(job) + + def summary(job) -> str: """One line on what the job did, for the panel.""" if job.state not in FINISHED: return LABELS[job.state] + if came_back_late(job): + return f"Back in service late, {job.came_back_at:%Y-%m-%d %H:%M}Z" parts = [] if job.docker_update: parts.append("Docker upgraded") diff --git a/plugins/host_agent/mcp_tools.py b/plugins/host_agent/mcp_tools.py index 46e8c83..82c8490 100644 --- a/plugins/host_agent/mcp_tools.py +++ b/plugins/host_agent/mcp_tools.py @@ -119,6 +119,7 @@ def _job_row(job, names: dict, now) -> dict: "swarm_node": job.node_id, "drained_by": names.get(job.drain_host_id, job.drain_host_id), "drained_at": iso(job.drained_at), + "came_back_at": iso(job.came_back_at), "docker_update": job.docker_update, "checked": job.checked, "detail": job.detail, @@ -205,7 +206,9 @@ async def get_maintenance_job(host: str = "", job: str = "", limit: int = 10) -> settling →] updating → [draining → settling →] [rebooting →] [undraining →] [recovering →] done, or failed / timed_out. recovering waits up to 10 min for those containers to run again; `detail` says why - a job failed, naming any that didn't. `summary` is the host page's line + a job failed, naming any that didn't. A job failed only for not coming + back into service in time has `came_back_at`, when its host was first + found back; from then it no longer flags the host. `summary` is the host page's line ("Done: 12 packages updated, rebooted"). `gives_up_at` is when an active job's current state times out. """ @@ -312,7 +315,8 @@ async def run_maintenance(host: str, reboot: str = "if_required") -> dict: async def _failed_jobs(db, now) -> list: """Each host's latest finished job, when it failed or timed out within - FAILED_JOB_DAYS (a later success clears it).""" + FAILED_JOB_DAYS and still flags the host (a later success clears it, and + so does a late return once the host is back, #6175).""" rows = (await db.execute( select(HostMaintenanceJob, Host.name).join(Host, Host.id == HostMaintenanceJob.host_id) .where(HostMaintenanceJob.state.in_(maintenance.FINISHED), @@ -321,7 +325,7 @@ async def _failed_jobs(db, now) -> list: latest = {} for j, name in rows: latest.setdefault(j.host_id, (j, name)) - return [(j, name) for j, name in latest.values() if j.state != "done"] + return [(j, name) for j, name in latest.values() if maintenance.still_failed(j)] async def mcp_overview(db) -> list[dict]: diff --git a/plugins/host_agent/migrations/versions/host_agent_011_came_back.py b/plugins/host_agent/migrations/versions/host_agent_011_came_back.py new file mode 100644 index 0000000..18a0d5d --- /dev/null +++ b/plugins/host_agent/migrations/versions/host_agent_011_came_back.py @@ -0,0 +1,23 @@ +# plugins/host_agent/migrations/versions/host_agent_011_came_back.py +"""host_agent: when a job failed for not coming back into service in time +first found its host back (#6175) + +Revision ID: host_agent_011_came_back +""" +from typing import Sequence, Union +from alembic import op +import sqlalchemy as sa + +revision: str = "host_agent_011_came_back" +down_revision: Union[str, None] = "host_agent_010_maint_check" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column("host_maintenance_jobs", + sa.Column("came_back_at", sa.DateTime(timezone=True), nullable=True)) + + +def downgrade() -> None: + op.drop_column("host_maintenance_jobs", "came_back_at") diff --git a/plugins/host_agent/models.py b/plugins/host_agent/models.py index edf9d8b..76b2dfb 100644 --- a/plugins/host_agent/models.py +++ b/plugins/host_agent/models.py @@ -121,6 +121,9 @@ class HostMaintenanceJob(Base): docker_update = Column(Boolean, nullable=True) checked = Column(JSON, nullable=True) drained_at = Column(DateTime(timezone=True), nullable=True) + # A job failed for not coming back into service in time: when its host + # was first found back (#6175). From then on the job no longer flags it. + came_back_at = Column(DateTime(timezone=True), nullable=True) class HostMaintenancePlan(Base): diff --git a/plugins/host_agent/routes.py b/plugins/host_agent/routes.py index 3bbbc0e..22b6374 100644 --- a/plugins/host_agent/routes.py +++ b/plugins/host_agent/routes.py @@ -1099,6 +1099,7 @@ async def host_panel(host_id: str): host=host, reg=reg, reporting=reporting, stale=stale, target=target, downtime=downtime, maint_job=maint_job, maint_blocker=maint_blocker, maint_summary=maintenance.summary(maint_job) if maint_job else None, + maint_failed=maint_job is not None and maintenance.still_failed(maint_job), ansible_available=has_capability("ansible.run_playbook"), managed_key_set=bool((ansible_cfg.get("ssh_public_key") or "").strip()), ) @@ -1448,7 +1449,7 @@ def _run_rows(run, jobs: dict, names: dict) -> list[dict]: job = jobs.get((run.id, host_id)) if job is not None: text = maintenance.summary(job) - bad = job.state in ("failed", "timed_out") + bad = maintenance.still_failed(job) busy = job.state in maintenance.ACTIVE elif run.state == "running" and i == run.position: text, bad, busy = "Starting", False, True diff --git a/plugins/host_agent/templates/panel.html b/plugins/host_agent/templates/panel.html index 7add40f..0cdd7eb 100644 --- a/plugins/host_agent/templates/panel.html +++ b/plugins/host_agent/templates/panel.html @@ -26,9 +26,9 @@ {# Steward's own maintenance job on this host (#5767): a running one first. #} {% if maint_job %} {% set running = maint_job.state not in ('done', 'failed', 'timed_out') %} -
+
Maintenance: {{ maint_summary }} - · {% if running %}since {{ maint_job.state_at|ago }}{% else %}{{ maint_job.finished_at|ago }}{% endif %}{% if maint_job.state in ('failed', 'timed_out') and maint_job.detail %} · {{ maint_job.detail|truncate(90) }}{% endif %} + · {% if running %}since {{ maint_job.state_at|ago }}{% else %}{{ maint_job.finished_at|ago }}{% endif %}{% if maint_failed and maint_job.detail %} · {{ maint_job.detail|truncate(90) }}{% endif %}
{% endif %} {% if reg.reboot_required %} diff --git a/steward/core/formatting.py b/steward/core/formatting.py index ce762c7..f6a467b 100644 --- a/steward/core/formatting.py +++ b/steward/core/formatting.py @@ -358,7 +358,7 @@ METRICS: dict[str, tuple[str, str]] = { "updates_pending": ("Updates waiting", "packages the host's package manager would upgrade, checked hourly"), "updates_security": ("Security updates waiting", "of the updates waiting, those from a security source"), "reboot_required": ("Reboot required", "1 when the host needs a reboot to finish an update, 0 when not"), - "maintenance_failed": ("Maintenance failed", "1 when the host's latest scheduled or manual update failed or timed out, or the host didn't come back into service and still isn't"), + "maintenance_failed": ("Maintenance failed", "1 when the host's latest scheduled or manual update failed or timed out, or the host didn't come back into service in time and hasn't been back since"), "temp_c_max": ("Hottest sensor", "the host's hottest real thermometer; inputs with nothing wired to them are left out"), "temp_c": ("Sensor temperature", "one temperature sensor (resource host:temp:label)"), "net_rx_bps": ("Network in", "bytes per second received, by the whole host, one interface or one container"), diff --git a/tests/integration/test_host_maintenance_cadence.py b/tests/integration/test_host_maintenance_cadence.py index 32956ed..4b09e9f 100644 --- a/tests/integration/test_host_maintenance_cadence.py +++ b/tests/integration/test_host_maintenance_cadence.py @@ -175,4 +175,18 @@ def test_a_host_that_came_back_late_reads_failed_only_until_it_is_back(): await _post(app, ta) assert (await _tick(app))[a.name] == 0.0 + # #6175: once back, a later wobble doesn't flag it again, and the + # overview agrees with the figure. + await _sql("UPDATE host_agent_registrations SET last_seen_at = now() - interval '1 hour' " + "WHERE host_id = :h") + assert (await _tick(app))[a.name] == 0.0 + mcp_tools = _ha("mcp_tools") + async with app.db_sessionmaker() as s: + (came_back,) = (await s.execute(text( + "SELECT came_back_at FROM host_maintenance_jobs WHERE host_id = :h"), + {"h": a.id})).one() + flagged = await mcp_tools._failed_jobs(s, datetime.now(timezone.utc)) + assert came_back is not None + assert a.name not in [name for _, name in flagged] + asyncio.run(_go()) diff --git a/tests/plugins/host_agent/test_maintenance.py b/tests/plugins/host_agent/test_maintenance.py index 0369c8a..042ec04 100644 --- a/tests/plugins/host_agent/test_maintenance.py +++ b/tests/plugins/host_agent/test_maintenance.py @@ -20,7 +20,8 @@ def _job(**kw): base = dict(id="job-1", host_id="h1", state="queued", state_at=T0, created_at=T0, reboot_policy="if_required", step_id=None, step_seen=False, node_id=None, drain_host_id=None, packages=None, reboot_needed=None, back_at=None, - detail=None, finished_at=None, drained_at=None, docker_update=None, checked=None) + detail=None, finished_at=None, drained_at=None, docker_update=None, checked=None, + came_back_at=None) base.update(kw) return SimpleNamespace(**base) @@ -269,3 +270,18 @@ def test_the_agent_reads_stewards_answer_when_asked(monkeypatch): answer: dict = {} assert a.post_payload("https://x", "tok", {}, answer) == (True, 200) assert answer["maintenance"] == [{"id": "job-1-reboot", "action": "reboot"}] + + +def test_a_late_return_stops_flagging_its_host_once_it_came_back(): + """#6175: one answer for the figure, the overview and the pages.""" + late = _job(state="failed", finished_at=T0, + detail=f"{m.NOT_BACK} 30 min: services below their replicas: portal_app") + assert m.still_failed(late) and m.summary(late) == "Failed" + late.came_back_at = T0 + timedelta(minutes=45) + assert not m.still_failed(late) + assert m.summary(late) == "Back in service late, 2026-10-14 23:45Z" + # An update that failed, or a job that timed out, flags its host whatever came back. + for job in (_job(state="failed", detail="dpkg was interrupted", came_back_at=T0), + _job(state="timed_out", detail="no answer within 60 min", came_back_at=T0)): + assert m.still_failed(job) + assert not m.still_failed(_job(state="done")) diff --git a/tests/plugins/host_agent/test_mcp_tools.py b/tests/plugins/host_agent/test_mcp_tools.py index 97e4713..6239fbf 100644 --- a/tests/plugins/host_agent/test_mcp_tools.py +++ b/tests/plugins/host_agent/test_mcp_tools.py @@ -34,7 +34,7 @@ def test_a_job_row_says_what_it_did_and_when_it_gives_up(): reboot_policy="if_required", packages=12, reboot_needed=True, back_at=None, node_id="n1", drain_host_id="h2", detail=None, created_by=None, run_id="r1", created_at=NOW - timedelta(minutes=20), finished_at=None, - drained_at=NOW - timedelta(minutes=11), docker_update=False, + drained_at=NOW - timedelta(minutes=11), docker_update=False, came_back_at=None, checked={"pending": 12, "docker_packages": [], "containers": []}) row = _job_row(job, {"h1": "web-1", "h2": "mgr-1"}, NOW) assert (row["host"], row["drained_by"], row["label"], row["active"]) == (