fix(maintenance): a late return, once back, stays back, and every reader agrees (#6175)
18032a4 re-checked a "not back in service" job every minute with no end,
so any later swarm wobble on the host (radarr's failed task during the
downloads move, say) flipped maintenance_failed back to 1: five false
firings and ten emails for Docker02 overnight on 10-19. And the overview's
finding never learned the rule at all, so it still said Docker02's
maintenance failed three days on.
The job now keeps came_back_at, set the first time its host is found back.
maintenance.still_failed(job) is the one answer: the figure, the overview,
the cadence page's run rows and the host panel all ask it. A failed update
or a timed-out job still flags its host until a job succeeds.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -29,9 +29,6 @@ WEEKDAYS = ("Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday",
|
|||||||
SETTLE_LIMIT = timedelta(minutes=30)
|
SETTLE_LIMIT = timedelta(minutes=30)
|
||||||
# An agent heard from this recently is posting.
|
# An agent heard from this recently is posting.
|
||||||
POSTING_WITHIN = timedelta(minutes=3)
|
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")
|
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:
|
if back is None:
|
||||||
return "next", None
|
return "next", None
|
||||||
if run.settle_since is not None and now - run.settle_since > SETTLE_LIMIT:
|
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
|
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
|
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
|
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
|
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 sqlalchemy import select
|
||||||
from steward.models.hosts import Host
|
from steward.models.hosts import Host
|
||||||
from .models import HostAgentRegistration, HostMaintenanceJob
|
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:
|
if host.name not in latest or job.finished_at > latest[host.name][1].finished_at:
|
||||||
latest[host.name] = (host, job)
|
latest[host.name] = (host, job)
|
||||||
for name, (host, job) in latest.items():
|
for name, (host, job) in latest.items():
|
||||||
failed = job.state != "done"
|
if (job.state == "failed" and job.came_back_at is None
|
||||||
if failed and (job.detail or "").startswith(NOT_BACK):
|
and (job.detail or "").startswith(maintenance.NOT_BACK)):
|
||||||
reg = (await session.execute(select(HostAgentRegistration).where(
|
reg = (await session.execute(select(HostAgentRegistration).where(
|
||||||
HostAgentRegistration.host_id == host.id))).scalar_one_or_none()
|
HostAgentRegistration.host_id == host.id))).scalar_one_or_none()
|
||||||
failed = await _back(session, host, job, reg, now) is not None
|
if await _back(session, host, job, reg, now) is None:
|
||||||
out[name] = 1.0 if failed else 0.0
|
job.came_back_at = now
|
||||||
|
out[name] = 1.0 if maintenance.still_failed(job) else 0.0
|
||||||
return out
|
return out
|
||||||
|
|||||||
@@ -37,6 +37,10 @@ from datetime import datetime, timedelta, timezone
|
|||||||
ACTIVE = ("queued", "checking", "draining", "settling", "updating", "rebooting",
|
ACTIVE = ("queued", "checking", "draining", "settling", "updating", "rebooting",
|
||||||
"undraining", "recovering")
|
"undraining", "recovering")
|
||||||
FINISHED = ("done", "failed", "timed_out")
|
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.
|
# The helper action each handed state asks for.
|
||||||
STEP_ACTION = {"checking": "check", "updating": "update", "draining": "drain",
|
STEP_ACTION = {"checking": "check", "updating": "update", "draining": "drain",
|
||||||
"rebooting": "reboot", "undraining": "undrain"}
|
"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:
|
def summary(job) -> str:
|
||||||
"""One line on what the job did, for the panel."""
|
"""One line on what the job did, for the panel."""
|
||||||
if job.state not in FINISHED:
|
if job.state not in FINISHED:
|
||||||
return LABELS[job.state]
|
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 = []
|
parts = []
|
||||||
if job.docker_update:
|
if job.docker_update:
|
||||||
parts.append("Docker upgraded")
|
parts.append("Docker upgraded")
|
||||||
|
|||||||
@@ -119,6 +119,7 @@ def _job_row(job, names: dict, now) -> dict:
|
|||||||
"swarm_node": job.node_id,
|
"swarm_node": job.node_id,
|
||||||
"drained_by": names.get(job.drain_host_id, job.drain_host_id),
|
"drained_by": names.get(job.drain_host_id, job.drain_host_id),
|
||||||
"drained_at": iso(job.drained_at),
|
"drained_at": iso(job.drained_at),
|
||||||
|
"came_back_at": iso(job.came_back_at),
|
||||||
"docker_update": job.docker_update,
|
"docker_update": job.docker_update,
|
||||||
"checked": job.checked,
|
"checked": job.checked,
|
||||||
"detail": job.detail,
|
"detail": job.detail,
|
||||||
@@ -205,7 +206,9 @@ async def get_maintenance_job(host: str = "", job: str = "", limit: int = 10) ->
|
|||||||
settling →] updating → [draining → settling →] [rebooting →]
|
settling →] updating → [draining → settling →] [rebooting →]
|
||||||
[undraining →] [recovering →] done, or failed / timed_out. recovering
|
[undraining →] [recovering →] done, or failed / timed_out. recovering
|
||||||
waits up to 10 min for those containers to run again; `detail` says why
|
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
|
("Done: 12 packages updated, rebooted"). `gives_up_at` is when an active
|
||||||
job's current state times out.
|
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:
|
async def _failed_jobs(db, now) -> list:
|
||||||
"""Each host's latest finished job, when it failed or timed out within
|
"""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(
|
rows = (await db.execute(
|
||||||
select(HostMaintenanceJob, Host.name).join(Host, Host.id == HostMaintenanceJob.host_id)
|
select(HostMaintenanceJob, Host.name).join(Host, Host.id == HostMaintenanceJob.host_id)
|
||||||
.where(HostMaintenanceJob.state.in_(maintenance.FINISHED),
|
.where(HostMaintenanceJob.state.in_(maintenance.FINISHED),
|
||||||
@@ -321,7 +325,7 @@ async def _failed_jobs(db, now) -> list:
|
|||||||
latest = {}
|
latest = {}
|
||||||
for j, name in rows:
|
for j, name in rows:
|
||||||
latest.setdefault(j.host_id, (j, name))
|
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]:
|
async def mcp_overview(db) -> list[dict]:
|
||||||
|
|||||||
@@ -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")
|
||||||
@@ -121,6 +121,9 @@ class HostMaintenanceJob(Base):
|
|||||||
docker_update = Column(Boolean, nullable=True)
|
docker_update = Column(Boolean, nullable=True)
|
||||||
checked = Column(JSON, nullable=True)
|
checked = Column(JSON, nullable=True)
|
||||||
drained_at = Column(DateTime(timezone=True), 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):
|
class HostMaintenancePlan(Base):
|
||||||
|
|||||||
@@ -1099,6 +1099,7 @@ async def host_panel(host_id: str):
|
|||||||
host=host, reg=reg, reporting=reporting, stale=stale, target=target, downtime=downtime,
|
host=host, reg=reg, reporting=reporting, stale=stale, target=target, downtime=downtime,
|
||||||
maint_job=maint_job, maint_blocker=maint_blocker,
|
maint_job=maint_job, maint_blocker=maint_blocker,
|
||||||
maint_summary=maintenance.summary(maint_job) if maint_job else None,
|
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"),
|
ansible_available=has_capability("ansible.run_playbook"),
|
||||||
managed_key_set=bool((ansible_cfg.get("ssh_public_key") or "").strip()),
|
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))
|
job = jobs.get((run.id, host_id))
|
||||||
if job is not None:
|
if job is not None:
|
||||||
text = maintenance.summary(job)
|
text = maintenance.summary(job)
|
||||||
bad = job.state in ("failed", "timed_out")
|
bad = maintenance.still_failed(job)
|
||||||
busy = job.state in maintenance.ACTIVE
|
busy = job.state in maintenance.ACTIVE
|
||||||
elif run.state == "running" and i == run.position:
|
elif run.state == "running" and i == run.position:
|
||||||
text, bad, busy = "Starting", False, True
|
text, bad, busy = "Starting", False, True
|
||||||
|
|||||||
@@ -26,9 +26,9 @@
|
|||||||
{# Steward's own maintenance job on this host (#5767): a running one first. #}
|
{# Steward's own maintenance job on this host (#5767): a running one first. #}
|
||||||
{% if maint_job %}
|
{% if maint_job %}
|
||||||
{% set running = maint_job.state not in ('done', 'failed', 'timed_out') %}
|
{% set running = maint_job.state not in ('done', 'failed', 'timed_out') %}
|
||||||
<div class="{{ 'tone-warn' if running or maint_job.state != 'done' else '' }}" title="{{ maint_job.detail or '' }}">
|
<div class="{{ 'tone-warn' if running or maint_failed else '' }}" title="{{ maint_job.detail or '' }}">
|
||||||
<a href="/plugins/host_agent/maintenance" style="color:inherit;">Maintenance</a>: {{ maint_summary }}
|
<a href="/plugins/host_agent/maintenance" style="color:inherit;">Maintenance</a>: {{ maint_summary }}
|
||||||
<span style="color:var(--text-muted);">· {% 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 %}</span>
|
<span style="color:var(--text-muted);">· {% 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 %}</span>
|
||||||
</div>
|
</div>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
{% if reg.reboot_required %}
|
{% if reg.reboot_required %}
|
||||||
|
|||||||
@@ -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_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"),
|
"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"),
|
"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_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)"),
|
"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"),
|
"net_rx_bps": ("Network in", "bytes per second received, by the whole host, one interface or one container"),
|
||||||
|
|||||||
@@ -175,4 +175,18 @@ def test_a_host_that_came_back_late_reads_failed_only_until_it_is_back():
|
|||||||
await _post(app, ta)
|
await _post(app, ta)
|
||||||
assert (await _tick(app))[a.name] == 0.0
|
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())
|
asyncio.run(_go())
|
||||||
|
|||||||
@@ -20,7 +20,8 @@ def _job(**kw):
|
|||||||
base = dict(id="job-1", host_id="h1", state="queued", state_at=T0, created_at=T0,
|
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,
|
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,
|
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)
|
base.update(kw)
|
||||||
return SimpleNamespace(**base)
|
return SimpleNamespace(**base)
|
||||||
|
|
||||||
@@ -269,3 +270,18 @@ def test_the_agent_reads_stewards_answer_when_asked(monkeypatch):
|
|||||||
answer: dict = {}
|
answer: dict = {}
|
||||||
assert a.post_payload("https://x", "tok", {}, answer) == (True, 200)
|
assert a.post_payload("https://x", "tok", {}, answer) == (True, 200)
|
||||||
assert answer["maintenance"] == [{"id": "job-1-reboot", "action": "reboot"}]
|
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"))
|
||||||
|
|||||||
@@ -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,
|
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",
|
node_id="n1", drain_host_id="h2", detail=None, created_by=None, run_id="r1",
|
||||||
created_at=NOW - timedelta(minutes=20), finished_at=None,
|
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": []})
|
checked={"pending": 12, "docker_packages": [], "containers": []})
|
||||||
row = _job_row(job, {"h1": "web-1", "h2": "mgr-1"}, NOW)
|
row = _job_row(job, {"h1": "web-1", "h2": "mgr-1"}, NOW)
|
||||||
assert (row["host"], row["drained_by"], row["label"], row["active"]) == (
|
assert (row["host"], row["drained_by"], row["label"], row["active"]) == (
|
||||||
|
|||||||
Reference in New Issue
Block a user