diff --git a/docs/plugins/host-agent-design.md b/docs/plugins/host-agent-design.md index 9039657..3e4cd0b 100644 --- a/docs/plugins/host-agent-design.md +++ b/docs/plugins/host-agent-design.md @@ -214,8 +214,21 @@ transitions (model-free, unit-tested) and the database side. queued → [checking →] [draining → settling →] updating → [draining → settling →] [rebooting →] [undraining →] [recovering →] done any state → failed (a step's result said so) | timed_out (its deadline passed) +draining / settling / updating / rebooting, once drained → undraining → failed | timed_out ``` +**A job never ends leaving its drain behind (#6387).** On 2026-10-25 a job +drained VDNT-Docker02, which ran Traefik and Steward. Its agent's result +reaches Steward through Traefik, and Traefik took 11.5 minutes to start on +Docker03 (its image had to be pulled). The job gave up at the 10-minute drain +deadline and finished `timed_out` where it stood, leaving the node drained +for hours. Now a job that fails or times out while its node may be drained +(the drain was handed out, or went through and wasn't undone) goes to +**undraining** first, with the outcome kept in `ends_as`, and finishes as that +once the node is active again. If the undrain goes wrong too, `detail` says +both. The drain deadline is 30 minutes, so a result held up by the proxy +moving still lands. + **Docker upgraded, drained first (1.24.0, #6098).** On 2026-10-18 an update on VDNT-Docker02 restarted the Docker daemon six seconds in, before the drain that came only with the reboot: every container on the node stopped, and @@ -251,7 +264,7 @@ check, and its job runs as before. by hostname in the Docker plugin's swarm nodes; a manager drains itself, a worker is drained by a manager whose agent has the helper. With no such manager the job is refused rather than rebooting an undrained node. -- **Deadlines** (`DEADLINES`): queued 1 h, checking 30 min, draining 10 min, +- **Deadlines** (`DEADLINES`): queued 1 h, checking 30 min, draining 30 min, settling 15 min, updating 90 min, rebooting 30 min, undraining 10 min, recovering 15 min. A minute tick (`host_agent_maintenance`) times out a job past its deadline. diff --git a/plugins/host_agent/maintenance.py b/plugins/host_agent/maintenance.py index a582cfb..f76b838 100644 --- a/plugins/host_agent/maintenance.py +++ b/plugins/host_agent/maintenance.py @@ -29,6 +29,11 @@ failed / timed_out. A node is drained at most once a job: before the update when Docker is upgraded, else before the reboot. settling and recovering are Steward's own waits, moved on by the minute tick. An agent older than CHECK_AGENT gets no check: its job starts at updating. + +A job that fails or is given up while its node may be drained makes the node +active again first: it goes to undraining with the outcome kept in +`ends_as`, and finishes as that once the undrain is through. A job never ends +leaving behind a drain it made. """ from __future__ import annotations @@ -49,7 +54,10 @@ STEP_ACTION = {"checking": "check", "updating": "update", "draining": "drain", DEADLINES = { "queued": timedelta(hours=1), "checking": timedelta(minutes=30), # two listings, the helper stops each at 10 - "draining": timedelta(minutes=10), + # The drain's result comes back the way the drain can take down: draining + # the node that runs the proxy (or Steward) moves them, and the result + # waits until they run elsewhere, image pull included. + "draining": timedelta(minutes=30), "settling": timedelta(minutes=15), # the tick moves it on at DRAIN_SETTLE "updating": timedelta(minutes=90), # the helper itself stops at 60 "rebooting": timedelta(minutes=30), @@ -94,6 +102,25 @@ def _finish(job, state: str, now: datetime, detail: str | None = None) -> None: _enter(job, state, now) +def may_be_drained(job) -> bool: + """Whether the job may have left its node drained: its drain was handed + out (a result may just not have come back), or went through and the node + hasn't been made active again.""" + return drains(job) and (job.state == "draining" or + (job.drained_at is not None and job.state in ("settling", "updating", "rebooting"))) + + +def _give_up(job, state: str, now: datetime, detail: str) -> None: + """End the job as `state` (failed or timed_out), making its node active + again first when the job may have drained it.""" + if may_be_drained(job): + job.detail = detail[:DETAIL_MAX] + job.ends_as = state + _enter(job, "undraining", now) + else: + _finish(job, state, now, detail) + + def can_check(agent_version) -> bool: """Whether an agent this version hands the helper a check.""" try: @@ -209,8 +236,9 @@ def apply_result(job, event: dict, now: datetime) -> bool: what = STEP_ACTION[job.state] detail = f"{what} failed: {event.get('detail') or 'no detail'}" if job.state == "undraining": - detail += " (the node is still drained)" - _finish(job, "failed", now, detail) + _finish(job, "failed", now, _after_given_up(job, detail + " (the node is still drained)")) + else: + _give_up(job, "failed", now, detail) return True if job.state == "checking": _after_check(job, event, now) @@ -223,7 +251,10 @@ def apply_result(job, event: dict, now: datetime) -> bool: job.drained_at = now _enter(job, "settling", now) elif job.state == "undraining": - _recover(job, now) + if job.ends_as: # given up: the node is active again, the job ends as it was + _enter(job, job.ends_as, now) + else: + _recover(job, now) else: # rebooting: the boot completes it, not this return False return True @@ -284,10 +315,19 @@ def check_deadline(job, now: datetime) -> bool: "rebooting": "the host didn't come back", "undraining": "the node wasn't made active again", "recovering": "its containers weren't seen again"}[job.state] - _finish(job, "timed_out", now, f"{waiting} within {int(limit.total_seconds() // 60)} min") + detail = f"{waiting} within {int(limit.total_seconds() // 60)} min" + if job.state == "undraining": + _finish(job, job.ends_as or "timed_out", now, _after_given_up(job, detail)) + else: + _give_up(job, "timed_out", now, detail) return True +def _after_given_up(job, detail: str) -> str: + """What an undrain going wrong adds to why a given-up job ended.""" + return f"{job.detail}; then {detail}" if job.ends_as and job.detail else detail + + def window_end(job, now: datetime) -> datetime: """When the job's maintenance window should end: the longest the rest of the job could take, or now once it has finished.""" diff --git a/plugins/host_agent/mcp_tools.py b/plugins/host_agent/mcp_tools.py index 94ea921..fee94b7 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: "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), + "ends_as": job.ends_as, "docker_update": job.docker_update, "checked": job.checked, "detail": job.detail, @@ -203,7 +204,9 @@ async def get_maintenance_job(host: str = "", job: str = "", limit: int = 10) -> Docker, else before a reboot; `drained_at` is when, and its tasks get 5 min to move (settling). `state` moves queued → [checking →] [draining → settling →] updating → [draining → settling →] [rebooting →] - [undraining →] [recovering →] done, or failed / timed_out. recovering + [undraining →] [recovering →] done, or failed / timed_out; a job given + up while its node may be drained goes through undraining first, and + `ends_as` is what it then finishes as. recovering waits up to 10 min for those containers to run again; `detail` says why 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 diff --git a/plugins/host_agent/migrations/versions/host_agent_012_ends_as.py b/plugins/host_agent/migrations/versions/host_agent_012_ends_as.py new file mode 100644 index 0000000..ee46ec1 --- /dev/null +++ b/plugins/host_agent/migrations/versions/host_agent_012_ends_as.py @@ -0,0 +1,22 @@ +# plugins/host_agent/migrations/versions/host_agent_012_ends_as.py +"""host_agent: what a job given up while its node may be drained finishes as, +once the node is active again + +Revision ID: host_agent_012_ends_as +""" +from typing import Sequence, Union +from alembic import op +import sqlalchemy as sa + +revision: str = "host_agent_012_ends_as" +down_revision: Union[str, None] = "host_agent_011_came_back" +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("ends_as", sa.String(16), nullable=True)) + + +def downgrade() -> None: + op.drop_column("host_maintenance_jobs", "ends_as") diff --git a/plugins/host_agent/models.py b/plugins/host_agent/models.py index 76b2dfb..9bc3f55 100644 --- a/plugins/host_agent/models.py +++ b/plugins/host_agent/models.py @@ -124,6 +124,9 @@ class HostMaintenanceJob(Base): # 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) + # A job given up (failed or timed_out) while its node may be drained makes + # the node active again first; ends_as is what it finishes as after that. + ends_as = Column(String(16), nullable=True) class HostMaintenancePlan(Base): diff --git a/tests/plugins/host_agent/test_maintenance.py b/tests/plugins/host_agent/test_maintenance.py index 042ec04..11d5c6b 100644 --- a/tests/plugins/host_agent/test_maintenance.py +++ b/tests/plugins/host_agent/test_maintenance.py @@ -21,7 +21,7 @@ def _job(**kw): 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, - came_back_at=None) + came_back_at=None, ends_as=None) base.update(kw) return SimpleNamespace(**base) @@ -239,6 +239,73 @@ def test_the_new_states_are_given_up_too(state, minutes): assert m.check_deadline(job, T0 + timedelta(minutes=minutes)) and job.state == "timed_out" +def _drained_job(**kw): + return _job(node_id="n" * 25, drain_host_id="mgr", docker_update=True, **kw) + + +def test_a_drain_whose_result_is_late_still_gets_its_time(): + # Draining the node that runs the proxy moves it, and the drain's result + # waits for it to run again elsewhere. + job = _drained_job(state="draining", step_id="job-1-drain") + assert not m.check_deadline(job, T0 + timedelta(minutes=29)) + m.apply_result(job, _result(job, "done", ok=True), T0 + timedelta(minutes=12)) + assert job.state == "settling" + + +def test_a_drain_given_up_makes_the_node_active_again_before_the_job_ends(): + job = _drained_job(state="draining", step_id="job-1-drain") + assert m.check_deadline(job, T0 + timedelta(minutes=31)) + assert job.state == "undraining" and job.finished_at is None + assert m.executor(job) == "mgr" + assert m.handoff(job) == {"id": "job-1-undrain", "action": "undrain", "node": "n" * 25} + late = {"kind": "maintenance", "id": "job-1-drain", "state": "done", "ok": True} + assert not m.apply_result(job, late, T0 + timedelta(minutes=32)) # the drain's late result + t1 = T0 + timedelta(minutes=33) + m.apply_result(job, _result(job, "done", ok=True), t1) + assert (job.state, job.finished_at) == ("timed_out", t1) + assert job.detail == "the node wasn't drained within 30 min" + assert m.still_failed(job) + + +@pytest.mark.parametrize("state, step, minutes", [ + ("settling", None, 16), ("updating", "job-1-update", 91), ("rebooting", "job-1-reboot", 31), +]) +def test_a_job_given_up_after_its_drain_makes_the_node_active_again(state, step, minutes): + job = _drained_job(state=state, step_id=step, drained_at=T0) + assert m.check_deadline(job, T0 + timedelta(minutes=minutes)) + assert job.state == "undraining" and job.ends_as == "timed_out" + + +def test_a_failed_step_after_the_drain_makes_the_node_active_again_and_fails(): + job = _drained_job(state="updating", step_id="job-1-update", drained_at=T0) + m.apply_result(job, _result(job, "failed", ok=False, detail="dpkg lock"), T0) + assert job.state == "undraining" + m.apply_result(job, _result(job, "done", ok=True), T0) + assert job.state == "failed" and job.detail == "update failed: dpkg lock" + + +def test_an_undrain_that_goes_wrong_after_giving_up_says_both(): + job = _drained_job(state="draining", step_id="job-1-drain") + m.check_deadline(job, T0 + timedelta(minutes=31)) + assert m.check_deadline(job, T0 + timedelta(minutes=42)) + assert job.state == "timed_out" + assert job.detail == ("the node wasn't drained within 30 min; " + "then the node wasn't made active again within 10 min") + job = _drained_job(state="updating", step_id="job-1-update", drained_at=T0) + m.apply_result(job, _result(job, "failed", ok=False, detail="dpkg lock"), T0) + m.apply_result(job, _result(job, "failed", ok=False, detail="not a manager"), T0) + assert job.state == "failed" + assert job.detail == ("update failed: dpkg lock; then undrain failed: not a manager " + "(the node is still drained)") + + +def test_a_job_that_never_drained_ends_where_it_is_given_up(): + job = _job(state="updating", step_id="job-1-update", node_id="n" * 25, drain_host_id="mgr") + assert m.check_deadline(job, T0 + timedelta(minutes=91)) and job.state == "timed_out" + kept = _job(state="draining", step_id="job-1-drain", node_id="n" * 25) # kept drained: no drainer + assert m.check_deadline(kept, T0 + timedelta(minutes=31)) and kept.state == "timed_out" + + # ── the agent's side ───────────────────────────────────────────────────────── def test_the_agent_leaves_each_handed_step_once(tmp_path): diff --git a/tests/plugins/host_agent/test_mcp_tools.py b/tests/plugins/host_agent/test_mcp_tools.py index 6239fbf..3ede835 100644 --- a/tests/plugins/host_agent/test_mcp_tools.py +++ b/tests/plugins/host_agent/test_mcp_tools.py @@ -35,7 +35,7 @@ def test_a_job_row_says_what_it_did_and_when_it_gives_up(): 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, came_back_at=None, - checked={"pending": 12, "docker_packages": [], "containers": []}) + ends_as=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"]) == ( "web-1", "mgr-1", "Rebooting", True)