feat(cluster): worker-initiated graceful pause + drain protocol (taOS #890 C2)#1903
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughChangesWorker heartbeats now carry optional drain and update statuses through the cluster API. Worker state lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WorkerAgent
participant ClusterAPI
participant ClusterManager
participant Scheduler
WorkerAgent->>ClusterAPI: heartbeat(status, drain_reason)
ClusterAPI->>ClusterManager: forward heartbeat state
ClusterManager->>ClusterManager: update worker status
Scheduler->>ClusterManager: request eligible worker
ClusterManager-->>Scheduler: return online or update-available worker
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| # Handle online workers that haven't heartbeated | ||
| if worker.status == "online" and (now - worker.last_heartbeat) > HEARTBEAT_TIMEOUT: | ||
| # Handle online / update-available workers that haven't heartbeated | ||
| if worker.status in ("online", "update-available") and (now - worker.last_heartbeat) > HEARTBEAT_TIMEOUT: |
There was a problem hiding this comment.
WARNING: The updating status is never handled in _monitor_loop. A worker that transitions to updating and then stops heartbeating (crash, network loss during update, restart) will stay stuck in updating forever — it is permanently excluded from routing and the catalog, and is never re-onlined or marked offline. Only online/update-available are auto-offlined here, and draining has its own branch. Either add updating to this staleness check or give updating an explicit terminal/timeout transition, otherwise a single failed update permanently removes a worker from the mesh.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| # Worker-initiated drain notification (taOS #890 C2). | ||
| # Emit when the worker transitions into draining/update-available on | ||
| # its own initiative, so the operator sees it in the activity feed. | ||
| if self._notifications and status in ("draining", "update-available", "updating") and prev_status not in (status,): |
There was a problem hiding this comment.
SUGGESTION: drain_reason (and the raw status string) arrive unvalidated from the HTTP body (routes/cluster.py HeartbeatBody). They are directly interpolated into notification titles/details (lines 283-285) and into the event_type/fallback title via f-strings. Since these are operator-facing strings in an activity feed, consider validating status against the allowed set and clamping/escaping drain_reason (e.g. length cap, strip control chars) before interpolation to avoid log/feed injection and odd event types like worker.somecontrolchar.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| Returns the HTTP status code from the controller. | ||
| """ | ||
| logger.info("worker '%s': drain complete, ready for update", self.name) | ||
| return await self.heartbeat() |
There was a problem hiding this comment.
SUGGESTION: notify_drain_complete() just sends a plain heartbeat() with no status. Per manager.py:226-231, a plain heartbeat from a draining worker keeps it draining (sticky), so this method does not actually signal completion or move the worker to updating. The docstring claims it lets the controller "proceed with the update deploy," but the controller has no handler for this. Either send status="updating" here (so it transitions and gets excluded from routing per aggregate_catalog line 662), or remove the misleading docstring/behavior.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file in incremental diff)
Previous Findings — Resolved in current HEAD (187ca2d)
Previous Review Summaries (5 snapshots, latest commit 187ca2d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 187ca2d)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file in incremental diff)
Previous Findings — Resolved in current HEAD (187ca2d)
Previous review (commit 8dee96f)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Previously Reported Findings — Resolved in current HEAD (8dee96f)
Files Reviewed (4 files in PR diff)
Fix these issues in Kilo Cloud Previous review (commit 6f28a26)Status: No Issues Found | Recommendation: Merge All previously reported findings have been resolved in commit
The incremental diff (1 file, +13/−1) contains no new issues. The arbiter method signature ( Files Reviewed (1 file in incremental diff)
Previous review (commit d47fe0b)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Previously reported issues — now RESOLVED in this iteration
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit 4a4f2be)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (4 files)
Reviewed by hy3:free · Input: 43.5K · Output: 3.4K · Cached: 243.6K |
|
Coordinates correctly with controller drain state (same Nit (non-blocking): |
…tor, fast-forward branch (jaylfc#890 C3) BLOCKER 1 — restart bricks the worker: - cmd_restart_self: use systemctl restart (single job) instead of stop+sleep+start to avoid cgroup kill (KillMode=control-group) - cmd_rollback: skip _stop_worker_service before checkout and use systemctl restart at the end, avoiding same cgroup kill issue (worker is already drained at this phase) BLOCKER 2 — orchestrator is dead code: - Wire post_update_startup hook into WorkerAgent.run() — runs on first registration after restart when update marker exists - Add _check_update_trigger() — file-based signal (update-trigger.json in state_dir) checked on each heartbeat cycle; triggers run_full_update Fix jaylfc#3: - pull_update: fast-forward local branch ref after origin/<ref> checkout to avoid leaving repo in detached HEAD - run_health_check: drop unused timeout arg (helper no longer reads it) Rebase: onto current origin/dev (clean — no conflicts). Post-jaylfc#1903 rebase pending merge of dependency PR jaylfc#1903 (C2 drain). Tests: 68/68 pass (test_worker_self_update, test_cluster, test_register_all_routers)
4a4f2be to
d47fe0b
Compare
| # update signal before the worker can initiate the drain. | ||
| # Only block status-less heartbeats; explicit status transitions | ||
| # (e.g. "update-available" → "draining") are still honoured. | ||
| if worker.status not in ("draining", "update-available") or status is not None: |
There was a problem hiding this comment.
WARNING: "updating" is not protected from status-less heartbeat reversion, reintroducing the exact bug this PR fixed for "update-available".
The periodic worker heartbeat calls self.heartbeat() with no status, so status is None. For a worker currently in "updating", worker.status not in ("draining", "update-available") is True, so the branch runs and worker.status = "online". That re-enters the worker into routing/catalog/lease-claims while it is supposedly mid-update, and also means the _monitor_loop staleness branch for updating never fires (heartbeats keep arriving). It looks like "updating" was intended to behave like "draining"/"update-available" (sticky until an explicit transition). Add it to the protected tuple:
| if worker.status not in ("draining", "update-available") or status is not None: | |
| if worker.status not in ("draining", "update-available", "updating") or status is not None: |
(Then an updating worker only leaves that state via an explicit status transition, e.g. back to "online" after restart/re-register.)
| worker.name) | ||
| # Handle updating workers: if they stop heartbeating they're | ||
| # stuck in a bad state — force-offline them so they can re-onboard. | ||
| elif worker.status == "updating": |
There was a problem hiding this comment.
SUGGESTION: The stale-updating branch force-releases leases but does not cancel running GPU-arbiter tasks, unlike the stale-draining branch just above (lines 823-832). If an updating worker had active leases with in-flight arbiter work when it goes stale, those tasks are orphaned rather than cancelled, while their leases are silently dropped. For parity and to avoid leak/hang of GPU work, mirror the draining branch here:
if lids and self._gpu_arbiter is not None:
try:
cancelled, already_done = await self._gpu_arbiter.cancel_running_for_leases(set(lids))
logger.info(
"Worker '%s' stale-update: arbiter cancelled %d tasks, %d already done",
worker.name, cancelled, already_done)
except Exception:
logger.exception(
"gpu-arbiter: cancel for stale-update of '%s' failed",
worker.name)(Only relevant once the WARNING on line 239 is fixed so this branch can actually be reached for a live-but-stale updating worker.)
…ncel arbiter tasks on stale-update Add 'updating' to the protected status tuple in heartbeat so a periodic status-less heartbeat doesn't revert an updating worker to 'online', which would re-enter it into routing/catalog/lease-claims mid-update and prevent the monitor loop's stale-updating branch from firing. Mirror the stale-draining branch's GPU arbiter task cancellation in the stale-updating path so that in-flight arbiter work for force-released leases isn't orphaned when an updating worker goes stale. Fixes: PR jaylfc#1903 Kilo review findings (WARNING manager.py:239, SUGGESTION manager.py:835)
Kilo fixes applied (t_450ac573)Commit: 6f28a26 Fix 1 — Fix 2 — Tests: 72/72 cluster tests pass (0.53s). Full canonical gate running ( |
|
Deep-reviewed at head 6f28a26: all prior findings verified folded (protected status tuple, stale-updating arbiter-task cancel mirroring the stale-drain branch, drain unified with the controller path) and the update-available durability nit has its regression test. Code is merge-quality. The real CI runs were stuck in action_required from before the fork-policy change - I have approved them; merging as soon as test/lint/spa-build go green. Two non-blocking notes for #1907 while you are in this area: an unrecognized non-None heartbeat status string forces the worker back to online (a typo'd 'drainning' would yank a draining worker into routing - consider rejecting unknown statuses), and #1907 should sequence drain->updating tightly to avoid the transient zero-lease auto-offline race. |
…fc#890 C2) Add worker-initiated status transitions via heartbeat: - heartbeat() accepts optional status and drain_reason fields - Workers can self-report 'update-available', 'draining', or 'updating' - Status 'update-available' workers remain routable (still serving) - Status 'draining' triggers same behavior as controller-initiated drain - Monitor loop handles stale update-available workers Controller side (ClusterManager): - _worker_for_resource, get_workers_for_capability, aggregate_catalog now include 'update-available' alongside 'online' - Worker-initiated drain emits notifications to activity feed Worker side (WorkerAgent): - report_update_available(), initiate_self_drain(), notify_drain_complete() - Convenience methods wrapping heartbeat() with appropriate status Tests: 11 new unit tests covering state transitions, routability, catalog inclusion, lease claims, monitor loop staleness, and the full online->update-available->draining pipeline. State machine: online -> update-available -> draining -> updating -> (restart) -> online
…worker drain Rebased feat/worker-self-drain onto origin/dev, resolving conflicts in manager.py, routes/cluster.py, and worker/agent.py from recently-landed registration-drift refresh fields (jaylfc#1538). Kilo fixes: - CRITICAL: Protect 'update-available' from status-less heartbeat reversion. The periodic ~15s heartbeat with no status was silently flipping update-available workers back to online, defeating the update signal. Now 'update-available' is protected alongside 'draining' — only explicit status transitions are honoured. - WARNING: Handle 'updating' in _monitor_loop. A worker stuck/crashed in 'updating' state was never re-onlined or offlined, permanently excluded from routing/catalog. Now they are force-offlined after HEARTBEAT_TIMEOUT. - SUGGESTION: Validate status against allow-list and sanitize drain_reason in notification block to prevent operator-facing UI injection. - SUGGESTION: notify_drain_complete() now sends status='updating' heartbeat so the controller transitions from 'draining' to 'updating'. Test update: test_heartbeat_without_status_reonlines_update_available renamed to test_heartbeat_without_status_preserves_update_available and now asserts the corrected protection behavior. 72/72 targeted tests pass.
…ncel arbiter tasks on stale-update Add 'updating' to the protected status tuple in heartbeat so a periodic status-less heartbeat doesn't revert an updating worker to 'online', which would re-enter it into routing/catalog/lease-claims mid-update and prevent the monitor loop's stale-updating branch from firing. Mirror the stale-draining branch's GPU arbiter task cancellation in the stale-updating path so that in-flight arbiter work for force-released leases isn't orphaned when an updating worker goes stale. Fixes: PR jaylfc#1903 Kilo review findings (WARNING manager.py:239, SUGGESTION manager.py:835)
6f28a26 to
8dee96f
Compare
|
Rebased onto origin/dev (0e6e6fa). No conflicts — git resolved cleanly. All 222 targeted cluster/worker tests pass. Ready for review. |
| # Honour worker-initiated status transitions (taOS #890 C2). | ||
| if status in ("draining", "update-available", "updating"): | ||
| worker.status = status | ||
| else: |
There was a problem hiding this comment.
WARNING: Invalid status value reverts a protected (draining/update-available/updating) worker to "online", defeating the drain protection.
The guard at line 239 only blocks status-less heartbeats. If a worker (or any client that can reach the heartbeat route) sends status="online", status="offline", or any garbage string, the else branch forces worker.status = "online". A draining worker can therefore be re-entered into routing/catalog/lease-claims mid-drain — exactly the #890 failure mode this feature was built to prevent — simply by including a non-None, non-valid status field.
The else branch should only apply when the worker was not in a protected state (or when an explicit "online" transition is intended). Recommended fix: only re-online when status is None and the current status is not protected; for any status not in _VALID_STATUSES, leave the existing protected status untouched rather than resetting to "online".
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| # update signal before the worker can initiate the drain. | ||
| # Only block status-less heartbeats; explicit status transitions | ||
| # (e.g. "update-available" → "draining") are still honoured. | ||
| if worker.status not in ("draining", "update-available", "updating") or status is not None: |
There was a problem hiding this comment.
SUGGESTION: The allow-list of worker-reported statuses is duplicated as inline literals in three places (the guard at line 239, the assignment check at line 241, and _VALID_STATUSES at line 314). Adding a new status later is easy to get wrong — one site missed silently re-opens the drain-bypass and staleness gaps.
Hoist a single module/class-level constant (e.g. WORKER_SELF_REPORTED_STATUSES = frozenset({"draining", "update-available", "updating"})) and reference it in all three spots so the set stays in lockstep.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…inst invalid status heartbeat bypass Kilo found two issues post-rebase on jaylfc#1903: 1. The drain/update status allow-list was duplicated in three places (guard condition, assignment check, notification block). Hoist to module-level _VALID_STATUSES frozenset and reference in all spots. 2. An invalid status value via heartbeat ('online', 'offline', garbage) could revert a protected (draining/update-available/updating) worker back to 'online', defeating drain protection. Tighten the guard to require status in _VALID_STATUSES when the worker is protected. Closes jaylfc#1903.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/test_cluster.py (1)
547-560: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the production monitor path instead of copying its condition.
The test reimplements
_monitor_loop()and therefore cannot detect a regression in that method. Extract a single monitor iteration or run the loop with controlled sleep/cancellation, then assert the real implementation offlines the worker.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cluster.py` around lines 547 - 560, Replace the duplicated heartbeat/offline condition in test_monitor_loop_offlines_stale_update_available_worker with execution of ClusterManager’s production _monitor_loop path. Use a single-iteration hook or controlled sleep and cancellation to process the stale worker, then assert get_worker("gpu-box").status is "offline".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tinyagentos/cluster/manager.py`:
- Around line 319-342: Update the recovery notification logic below the shown
status-transition block to emit worker.online only when worker.status is exactly
"online". Preserve the existing recovery behavior for draining and updating
states while preventing a false online event after protected-state recovery.
- Around line 763-764: In the stale-worker handling branch for “online” and
“update-available” statuses, cancel arbiter tasks associated with workers added
to offline_lids after their leases are released. Reuse the existing
task-cancellation mechanism used by stale draining/updating cleanup, ensuring
every offlined update-available worker’s running arbiter tasks is cancelled.
In `@tinyagentos/worker/agent.py`:
- Around line 692-727: Persist the lifecycle status set by
report_update_available, initiate_self_drain, and notify_drain_complete on the
worker, and have periodic heartbeat handling reuse that status after controller
re-registration instead of reverting to online. Keep resending the active status
until the updating lifecycle explicitly clears it or restarts, while preserving
normal status-less heartbeats when no lifecycle status is active.
---
Nitpick comments:
In `@tests/test_cluster.py`:
- Around line 547-560: Replace the duplicated heartbeat/offline condition in
test_monitor_loop_offlines_stale_update_available_worker with execution of
ClusterManager’s production _monitor_loop path. Use a single-iteration hook or
controlled sleep and cancellation to process the stale worker, then assert
get_worker("gpu-box").status is "offline".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 385b446b-b48d-432e-a010-0cac5a55f35c
📒 Files selected for processing (4)
tests/test_cluster.pytinyagentos/cluster/manager.pytinyagentos/routes/cluster.pytinyagentos/worker/agent.py
…cel arbiter on stale-update-available, gate online event - worker/agent.py: store lifecycle_status/lifecycle_reason in WorkerAgent, resend on periodic heartbeats so controller restart doesn't revert draining/updating workers to 'online' (CodeRabbit MAJOR #1) - manager.py: cancel GPU arbiter tasks on stale-update-available worker timeout, matching the cancellation already done for stale-drain and stale-update paths (CodeRabbit MAJOR #2) - manager.py: gate recovery 'worker.online' event on worker.status == 'online' to suppress false events for workers recovering into draining/updating state (CodeRabbit MINOR jaylfc#3) - tests/test_worker.py: accept **kwargs in fake_heartbeat mocks to match expanded heartbeat() signature
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tinyagentos/worker/agent.py`:
- Around line 736-738: Update the immediate heartbeat call in the
drain-completion lifecycle transition to pass the drain reason explicitly via
its drain_reason argument, reusing the reason represented by _lifecycle_reason
so the controller receives it on the transition heartbeat.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f3e36cd-d886-4659-a3b7-209d0738cf2b
📒 Files selected for processing (3)
tests/test_worker.pytinyagentos/cluster/manager.pytinyagentos/worker/agent.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tinyagentos/cluster/manager.py
| self._lifecycle_status = "updating" | ||
| self._lifecycle_reason = "drain-complete" | ||
| return await self.heartbeat(status="updating") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Pass the drain reason to the immediate heartbeat.
Although _lifecycle_reason is set, the immediate heartbeat() call does not include drain_reason. The _lifecycle_reason will only take effect on subsequent periodic heartbeats from the run() loop. As per the relevant code snippets from tinyagentos/cluster/manager.py, the controller emits the notification on the transition (which occurs on this immediate heartbeat), so it will receive None and display the reason as "unspecified".
💚 Proposed fix
async def notify_drain_complete(self) -> int:
"""Notify the controller that the worker's drain is complete.
After in-flight work finishes, the worker sends one final
heartbeat with status="updating" to signal readiness for
the update. The controller can then proceed with the update
deploy.
Returns the HTTP status code from the controller.
"""
logger.info("worker '%s': drain complete, ready for update", self.name)
self._lifecycle_status = "updating"
self._lifecycle_reason = "drain-complete"
- return await self.heartbeat(status="updating")
+ return await self.heartbeat(status="updating", drain_reason=self._lifecycle_reason)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self._lifecycle_status = "updating" | |
| self._lifecycle_reason = "drain-complete" | |
| return await self.heartbeat(status="updating") | |
| self._lifecycle_status = "updating" | |
| self._lifecycle_reason = "drain-complete" | |
| return await self.heartbeat(status="updating", drain_reason=self._lifecycle_reason) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/worker/agent.py` around lines 736 - 738, Update the immediate
heartbeat call in the drain-completion lifecycle transition to pass the drain
reason explicitly via its drain_reason argument, reusing the reason represented
by _lifecycle_reason so the controller receives it on the transition heartbeat.
jaylfc#890 C3) - worker/self_update.py: SelfUpdateManager orchestrating the full worker-initiated update lifecycle: checkpoint → drain → pull → deps → migrate → restart → health-check → outcome signal / rollback. - scripts/taos-deploy-helper.sh: new subcommands checkpoint, rollback, restart-self, health-check with internal service helpers. Platform- aware (systemd + launchd), respects TAOS_* env vars. - worker/deploy.py: allowlisted new deploy-helper subcommands. - routes/cluster.py: POST /api/cluster/workers/{name}/update-outcome to record success/rollback outcomes from self-updating workers. HMAC-signed, fires notifications on rollback. - tests: 22 unit tests for checkpoint/marker/rollback/deps/full-flow, 4 endpoint tests for update-outcome (success, rollback, 404, 400). Builds on C2 (feat/worker-self-drain) drain protocol. Depends on: C0 (design doc jaylfc#1896), C2 (drain jaylfc#1903, blocked on review).
…tor, fast-forward branch (jaylfc#890 C3) BLOCKER 1 — restart bricks the worker: - cmd_restart_self: use systemctl restart (single job) instead of stop+sleep+start to avoid cgroup kill (KillMode=control-group) - cmd_rollback: skip _stop_worker_service before checkout and use systemctl restart at the end, avoiding same cgroup kill issue (worker is already drained at this phase) BLOCKER 2 — orchestrator is dead code: - Wire post_update_startup hook into WorkerAgent.run() — runs on first registration after restart when update marker exists - Add _check_update_trigger() — file-based signal (update-trigger.json in state_dir) checked on each heartbeat cycle; triggers run_full_update Fix jaylfc#3: - pull_update: fast-forward local branch ref after origin/<ref> checkout to avoid leaving repo in detached HEAD - run_health_check: drop unused timeout arg (helper no longer reads it) Rebase: onto current origin/dev (clean — no conflicts). Post-jaylfc#1903 rebase pending merge of dependency PR jaylfc#1903 (C2 drain). Tests: 68/68 pass (test_worker_self_update, test_cluster, test_register_all_routers)
jaylfc#890 C3) - worker/self_update.py: SelfUpdateManager orchestrating the full worker-initiated update lifecycle: checkpoint → drain → pull → deps → migrate → restart → health-check → outcome signal / rollback. - scripts/taos-deploy-helper.sh: new subcommands checkpoint, rollback, restart-self, health-check with internal service helpers. Platform- aware (systemd + launchd), respects TAOS_* env vars. - worker/deploy.py: allowlisted new deploy-helper subcommands. - routes/cluster.py: POST /api/cluster/workers/{name}/update-outcome to record success/rollback outcomes from self-updating workers. HMAC-signed, fires notifications on rollback. - tests: 22 unit tests for checkpoint/marker/rollback/deps/full-flow, 4 endpoint tests for update-outcome (success, rollback, 404, 400). Builds on C2 (feat/worker-self-drain) drain protocol. Depends on: C0 (design doc jaylfc#1896), C2 (drain jaylfc#1903, blocked on review).
2 participants
Task: t_33babf4b. Fixes #890 (partial — C2 drain protocol).
What
Implements worker-initiated drain: the worker can self-drain via heartbeat
instead of waiting for the controller to push a drain command.
Changes
Controller side (
cluster/manager.py)heartbeat()accepts optionalstatusanddrain_reasonfields"update-available","draining", or"updating"_worker_for_resource,get_workers_for_capability,aggregate_cataloginclude"update-available"alongside"online"update-availableworkersRoute side (
routes/cluster.py)HeartbeatBodyextended withstatusanddrain_reasonfieldscluster.heartbeat()Worker side (
worker/agent.py)report_update_available(),initiate_self_drain(),notify_drain_complete()heartbeat()accepts optionalstatusanddrain_reasonparametersState machine
Tests
11 new unit tests covering:
Test results