fix(maintenance): a job given up after draining its node makes the node active again (#6387)
On 2026-10-25 the 09:00 job on VDNT-Docker02 drained the node, which ran Traefik and Steward. The 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, so the node stayed drained for hours. - 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. The outcome is kept in the new column ends_as, and the job finishes as that once the node is active again. If the undrain goes wrong too, detail says both. - The drain deadline is 30 minutes instead of 10, so a result held up while the proxy moves still lands. - Migration host_agent_012_ends_as. get_maintenance_job returns ends_as. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -214,8 +214,21 @@ transitions (model-free, unit-tested) and the database side.
|
|||||||
queued → [checking →] [draining → settling →] updating
|
queued → [checking →] [draining → settling →] updating
|
||||||
→ [draining → settling →] [rebooting →] [undraining →] [recovering →] done
|
→ [draining → settling →] [rebooting →] [undraining →] [recovering →] done
|
||||||
any state → failed (a step's result said so) | timed_out (its deadline passed)
|
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
|
**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
|
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
|
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
|
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
|
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.
|
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,
|
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
|
recovering 15 min. A minute tick (`host_agent_maintenance`) times out a job
|
||||||
past its deadline.
|
past its deadline.
|
||||||
|
|||||||
@@ -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
|
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
|
Steward's own waits, moved on by the minute tick. An agent older than
|
||||||
CHECK_AGENT gets no check: its job starts at updating.
|
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
|
from __future__ import annotations
|
||||||
|
|
||||||
@@ -49,7 +54,10 @@ STEP_ACTION = {"checking": "check", "updating": "update", "draining": "drain",
|
|||||||
DEADLINES = {
|
DEADLINES = {
|
||||||
"queued": timedelta(hours=1),
|
"queued": timedelta(hours=1),
|
||||||
"checking": timedelta(minutes=30), # two listings, the helper stops each at 10
|
"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
|
"settling": timedelta(minutes=15), # the tick moves it on at DRAIN_SETTLE
|
||||||
"updating": timedelta(minutes=90), # the helper itself stops at 60
|
"updating": timedelta(minutes=90), # the helper itself stops at 60
|
||||||
"rebooting": timedelta(minutes=30),
|
"rebooting": timedelta(minutes=30),
|
||||||
@@ -94,6 +102,25 @@ def _finish(job, state: str, now: datetime, detail: str | None = None) -> None:
|
|||||||
_enter(job, state, now)
|
_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:
|
def can_check(agent_version) -> bool:
|
||||||
"""Whether an agent this version hands the helper a check."""
|
"""Whether an agent this version hands the helper a check."""
|
||||||
try:
|
try:
|
||||||
@@ -209,8 +236,9 @@ def apply_result(job, event: dict, now: datetime) -> bool:
|
|||||||
what = STEP_ACTION[job.state]
|
what = STEP_ACTION[job.state]
|
||||||
detail = f"{what} failed: {event.get('detail') or 'no detail'}"
|
detail = f"{what} failed: {event.get('detail') or 'no detail'}"
|
||||||
if job.state == "undraining":
|
if job.state == "undraining":
|
||||||
detail += " (the node is still drained)"
|
_finish(job, "failed", now, _after_given_up(job, detail + " (the node is still drained)"))
|
||||||
_finish(job, "failed", now, detail)
|
else:
|
||||||
|
_give_up(job, "failed", now, detail)
|
||||||
return True
|
return True
|
||||||
if job.state == "checking":
|
if job.state == "checking":
|
||||||
_after_check(job, event, now)
|
_after_check(job, event, now)
|
||||||
@@ -223,6 +251,9 @@ def apply_result(job, event: dict, now: datetime) -> bool:
|
|||||||
job.drained_at = now
|
job.drained_at = now
|
||||||
_enter(job, "settling", now)
|
_enter(job, "settling", now)
|
||||||
elif job.state == "undraining":
|
elif job.state == "undraining":
|
||||||
|
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)
|
_recover(job, now)
|
||||||
else: # rebooting: the boot completes it, not this
|
else: # rebooting: the boot completes it, not this
|
||||||
return False
|
return False
|
||||||
@@ -284,10 +315,19 @@ def check_deadline(job, now: datetime) -> bool:
|
|||||||
"rebooting": "the host didn't come back",
|
"rebooting": "the host didn't come back",
|
||||||
"undraining": "the node wasn't made active again",
|
"undraining": "the node wasn't made active again",
|
||||||
"recovering": "its containers weren't seen again"}[job.state]
|
"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
|
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:
|
def window_end(job, now: datetime) -> datetime:
|
||||||
"""When the job's maintenance window should end: the longest the rest of
|
"""When the job's maintenance window should end: the longest the rest of
|
||||||
the job could take, or now once it has finished."""
|
the job could take, or now once it has finished."""
|
||||||
|
|||||||
@@ -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_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),
|
"came_back_at": iso(job.came_back_at),
|
||||||
|
"ends_as": job.ends_as,
|
||||||
"docker_update": job.docker_update,
|
"docker_update": job.docker_update,
|
||||||
"checked": job.checked,
|
"checked": job.checked,
|
||||||
"detail": job.detail,
|
"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
|
Docker, else before a reboot; `drained_at` is when, and its tasks get 5
|
||||||
min to move (settling). `state` moves queued → [checking →] [draining →
|
min to move (settling). `state` moves queued → [checking →] [draining →
|
||||||
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; 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
|
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
|
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
|
back into service in time has `came_back_at`, when its host was first
|
||||||
|
|||||||
@@ -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")
|
||||||
@@ -124,6 +124,9 @@ class HostMaintenanceJob(Base):
|
|||||||
# A job failed for not coming back into service in time: when its host
|
# 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.
|
# was first found back (#6175). From then on the job no longer flags it.
|
||||||
came_back_at = Column(DateTime(timezone=True), nullable=True)
|
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):
|
class HostMaintenancePlan(Base):
|
||||||
|
|||||||
@@ -21,7 +21,7 @@ def _job(**kw):
|
|||||||
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)
|
came_back_at=None, ends_as=None)
|
||||||
base.update(kw)
|
base.update(kw)
|
||||||
return SimpleNamespace(**base)
|
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"
|
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 ─────────────────────────────────────────────────────────
|
# ── the agent's side ─────────────────────────────────────────────────────────
|
||||||
|
|
||||||
def test_the_agent_leaves_each_handed_step_once(tmp_path):
|
def test_the_agent_leaves_each_handed_step_once(tmp_path):
|
||||||
|
|||||||
@@ -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",
|
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, came_back_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)
|
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"]) == (
|
||||||
"web-1", "mgr-1", "Rebooting", True)
|
"web-1", "mgr-1", "Rebooting", True)
|
||||||
|
|||||||
Reference in New Issue
Block a user