hermes: make pool lease recovery and release isolation safe
Independent review t_5975c06a blocked this branch on a P1: a Kanban write that
failed while a lease expired left a `lease_failed` row that was invisible to
every pass, immortal to garbage collection, and fatal to the coordinator. It
poisoned `reconcile()` forever with a conflicting-duplicate primary key,
produced a spurious capability `block_task` from `dispatch()`, and -- because
startup maintenance ran unguarded before the port bound, against a store on a
PVC -- crash-looped the coordinator with no automatic recovery.
`lease_failed` is now a retryable state that every maintenance pass drains, and
a row only reaches a terminal state on authoritative evidence about its exact
Kanban run, so nothing is collected before its outcome is known and nothing is
silently dropped. Each row, task, and board is processed in isolation, and a
coordinator-side fault is never converted into a Kanban mutation. Startup runs
through the same guarded cycle as the steady-state loop.
The wire protocol and the durable store are now separate modules, and the
maintenance passes moved out of the coordinator, so each file stays under the
managed line ceiling with room for the recovery logic.
Also closes three consequential handoff risks the same review raised:
* mediator-N pinned itself hard to worker-N while sharing a ReadWriteOnce
claim, so a drain or preemption that moved only the lower-priority worker
deadlocked the ordinal on Multi-Attach until an operator deleted a Pod. The
shared workspace is now ReadWriteMany (as the hermes-chat tenant workspaces
already are on the same class), colocation is a preference, and the mediator
shares the worker's preemption priority, so each Pod reschedules on its own.
* the broker permits only branch creation, so a retry that added commits could
never submit and the run's work was discarded with the failure. Submission
now targets a fresh attempt- or content-scoped ref in the same reviewed
namespace -- never an update -- and is idempotent under replay. A refused
submission downgrades the result and says why instead of unwinding the run.
* the provider CLIs were reinstalled into an emptyDir on every Pod start inside
the 10m Flux health window for the whole hermes app. They now install once
per pinned version onto a durable volume, re-verified against the real
binaries and time-bounded, and the best-effort pool no longer gates the
health of the app its dependents wait on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-17 16:31:15 +00:00
|
|
|
"""Placement and rollout-cost contracts for the distributed worker pool.
|
|
|
|
|
|
|
|
|
|
The independent review found that mediator-N pinned itself hard to worker-N while
|
|
|
|
|
sharing a ReadWriteOnce claim with it, so a drain or preemption that moved only
|
|
|
|
|
the worker deadlocked the ordinal on Multi-Attach until an operator deleted a
|
|
|
|
|
Pod. It also found that the provider-CLI install was redone on every Pod start
|
|
|
|
|
inside the 10m Flux health window for the whole ``hermes`` app.
|
|
|
|
|
"""
|
|
|
|
|
|
|
|
|
|
from __future__ import annotations
|
|
|
|
|
|
|
|
|
|
import json
|
2026-09-13 15:04:01 -05:00
|
|
|
import subprocess
|
|
|
|
|
import sys
|
hermes: make pool lease recovery and release isolation safe
Independent review t_5975c06a blocked this branch on a P1: a Kanban write that
failed while a lease expired left a `lease_failed` row that was invisible to
every pass, immortal to garbage collection, and fatal to the coordinator. It
poisoned `reconcile()` forever with a conflicting-duplicate primary key,
produced a spurious capability `block_task` from `dispatch()`, and -- because
startup maintenance ran unguarded before the port bound, against a store on a
PVC -- crash-looped the coordinator with no automatic recovery.
`lease_failed` is now a retryable state that every maintenance pass drains, and
a row only reaches a terminal state on authoritative evidence about its exact
Kanban run, so nothing is collected before its outcome is known and nothing is
silently dropped. Each row, task, and board is processed in isolation, and a
coordinator-side fault is never converted into a Kanban mutation. Startup runs
through the same guarded cycle as the steady-state loop.
The wire protocol and the durable store are now separate modules, and the
maintenance passes moved out of the coordinator, so each file stays under the
managed line ceiling with room for the recovery logic.
Also closes three consequential handoff risks the same review raised:
* mediator-N pinned itself hard to worker-N while sharing a ReadWriteOnce
claim, so a drain or preemption that moved only the lower-priority worker
deadlocked the ordinal on Multi-Attach until an operator deleted a Pod. The
shared workspace is now ReadWriteMany (as the hermes-chat tenant workspaces
already are on the same class), colocation is a preference, and the mediator
shares the worker's preemption priority, so each Pod reschedules on its own.
* the broker permits only branch creation, so a retry that added commits could
never submit and the run's work was discarded with the failure. Submission
now targets a fresh attempt- or content-scoped ref in the same reviewed
namespace -- never an update -- and is idempotent under replay. A refused
submission downgrades the result and says why instead of unwinding the run.
* the provider CLIs were reinstalled into an emptyDir on every Pod start inside
the 10m Flux health window for the whole hermes app. They now install once
per pinned version onto a durable volume, re-verified against the real
binaries and time-bounded, and the best-effort pool no longer gates the
health of the app its dependents wait on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-17 16:31:15 +00:00
|
|
|
|
|
|
|
|
import yaml
|
|
|
|
|
|
|
|
|
|
from testing.tests.test_hermes_execution_pool_manifests import ( # noqa: E402
|
|
|
|
|
FLUX,
|
|
|
|
|
HERMES,
|
|
|
|
|
documents,
|
|
|
|
|
mediators,
|
|
|
|
|
node,
|
|
|
|
|
placeable,
|
|
|
|
|
worker,
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
SCRIPTS = HERMES / "scripts"
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_the_shared_workspace_claim_cannot_deadlock_on_multi_attach():
|
|
|
|
|
claims = {
|
|
|
|
|
item["metadata"]["name"]: item["spec"]["accessModes"]
|
|
|
|
|
for item in worker()["spec"]["volumeClaimTemplates"]
|
|
|
|
|
}
|
|
|
|
|
assert claims["workspace"] == ["ReadWriteMany"]
|
|
|
|
|
assert claims["tools"] == ["ReadWriteOnce"]
|
|
|
|
|
assert claims["provider-access"] == ["ReadWriteOnce"]
|
|
|
|
|
|
|
|
|
|
# Every claim the mediator co-mounts with the worker must be multi-attachable.
|
|
|
|
|
for ordinal, deployment in enumerate(mediators()):
|
|
|
|
|
shared = {
|
|
|
|
|
volume["persistentVolumeClaim"]["claimName"]
|
|
|
|
|
for volume in deployment["spec"]["template"]["spec"]["volumes"]
|
|
|
|
|
if "persistentVolumeClaim" in volume
|
|
|
|
|
}
|
|
|
|
|
assert f"workspace-hermes-execution-worker-{ordinal}" in shared
|
|
|
|
|
for name in shared:
|
|
|
|
|
if name.startswith("workspace-hermes-execution-worker-"):
|
|
|
|
|
assert claims["workspace"] == ["ReadWriteMany"]
|
|
|
|
|
else:
|
|
|
|
|
assert name == f"hermes-execution-mediator-state-{ordinal}"
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_mediator_placement_is_a_preference_and_shares_the_worker_priority():
|
|
|
|
|
stateful = worker()["spec"]["template"]["spec"]
|
|
|
|
|
worker_terms = stateful["affinity"]["nodeAffinity"][
|
|
|
|
|
"requiredDuringSchedulingIgnoredDuringExecution"
|
|
|
|
|
]["nodeSelectorTerms"]
|
|
|
|
|
for ordinal, deployment in enumerate(mediators()):
|
|
|
|
|
pod = deployment["spec"]["template"]["spec"]
|
|
|
|
|
assert pod["priorityClassName"] == stateful["priorityClassName"] == "scavenger"
|
|
|
|
|
assert deployment["spec"]["strategy"]["type"] == "Recreate"
|
|
|
|
|
affinity = pod["affinity"]
|
|
|
|
|
# No hard podAffinity: the mediator must never be unschedulable purely
|
|
|
|
|
# because its worker is Pending or placed on a different node.
|
|
|
|
|
assert "requiredDuringSchedulingIgnoredDuringExecution" not in affinity["podAffinity"]
|
|
|
|
|
preferred = affinity["podAffinity"][
|
|
|
|
|
"preferredDuringSchedulingIgnoredDuringExecution"
|
|
|
|
|
]
|
|
|
|
|
assert preferred[0]["podAffinityTerm"]["labelSelector"]["matchLabels"] == {
|
|
|
|
|
"statefulset.kubernetes.io/pod-name": f"hermes-execution-worker-{ordinal}"
|
|
|
|
|
}
|
|
|
|
|
assert affinity["nodeAffinity"][
|
|
|
|
|
"requiredDuringSchedulingIgnoredDuringExecution"
|
|
|
|
|
]["nodeSelectorTerms"] == worker_terms
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_a_preemption_that_moves_the_worker_leaves_both_pods_schedulable():
|
|
|
|
|
"""The exact drain/pressure sequence that used to need a manual Pod delete."""
|
|
|
|
|
nodes = [node(f"titan-{index:02d}") for index in (5, 6, 7, 8)]
|
|
|
|
|
stateful = worker()["spec"]["template"]["spec"]
|
|
|
|
|
mediator = mediators()[1]["spec"]["template"]["spec"]
|
|
|
|
|
|
|
|
|
|
# Steady state: worker-1 and mediator-1 are colocated on titan-05.
|
|
|
|
|
assert "titan-05" in placeable(stateful, nodes)
|
|
|
|
|
assert "titan-05" in placeable(mediator, nodes)
|
|
|
|
|
|
|
|
|
|
# titan-05 comes under pressure and evicts the scavenger-priority worker,
|
|
|
|
|
# which reschedules elsewhere while the mediator has not moved yet.
|
|
|
|
|
survivors = placeable(stateful, nodes, occupied=("titan-05",))
|
|
|
|
|
assert survivors and "titan-05" not in survivors
|
|
|
|
|
|
|
|
|
|
# The mediator still serves from titan-05 -- the shared claim is RWX, so the
|
|
|
|
|
# rescheduled worker does not block on Multi-Attach.
|
|
|
|
|
assert "titan-05" in placeable(mediator, nodes)
|
|
|
|
|
|
|
|
|
|
# And when titan-05 itself is drained the mediator follows independently,
|
|
|
|
|
# because nothing pins it to a Pod that may still be Pending.
|
|
|
|
|
drained = [
|
|
|
|
|
node(item["name"], cordoned=item["name"] == "titan-05") for item in nodes
|
|
|
|
|
]
|
|
|
|
|
rescheduled = placeable(mediator, drained)
|
|
|
|
|
assert rescheduled and "titan-05" not in rescheduled
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_three_workers_still_spread_across_distinct_eligible_nodes():
|
|
|
|
|
stateful = worker()["spec"]["template"]["spec"]
|
|
|
|
|
nodes = [node(f"titan-{index:02d}") for index in (5, 6, 7)] + [
|
|
|
|
|
node("titan-13"), node("titan-22"),
|
|
|
|
|
]
|
|
|
|
|
occupied: list[str] = []
|
|
|
|
|
for _replica in range(3):
|
|
|
|
|
allowed = placeable(stateful, nodes, occupied=tuple(occupied))
|
|
|
|
|
assert allowed, f"replica {len(occupied)} is unschedulable"
|
|
|
|
|
occupied.append(allowed[0])
|
|
|
|
|
assert sorted(occupied) == ["titan-05", "titan-06", "titan-07"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_provider_clis_install_once_onto_a_durable_verified_volume():
|
|
|
|
|
stateful = worker()
|
|
|
|
|
pod = stateful["spec"]["template"]["spec"]
|
|
|
|
|
claims = {item["metadata"]["name"] for item in stateful["spec"]["volumeClaimTemplates"]}
|
|
|
|
|
assert "tools" in claims
|
|
|
|
|
ephemeral = {
|
|
|
|
|
volume["name"] for volume in pod["volumes"] if "emptyDir" in volume
|
|
|
|
|
}
|
|
|
|
|
assert "tools" not in ephemeral, "the CLI cache must survive a Pod restart"
|
|
|
|
|
|
|
|
|
|
install = next(
|
|
|
|
|
item for item in pod["initContainers"] if item["name"] == "install-provider-clis"
|
|
|
|
|
)
|
|
|
|
|
script = install["args"][0]
|
|
|
|
|
# The marker alone is not trusted: a pruned or partial cache reinstalls.
|
|
|
|
|
assert 'if [ ! -f "${marker}" ] || [ ! -x "${tools}/bin/codex" ]' in script
|
|
|
|
|
assert 'rm -f "${marker}"' in script
|
|
|
|
|
# And the install itself is bounded, so a stalled registry fails fast.
|
|
|
|
|
assert "timeout 900 npm install" in script
|
|
|
|
|
assert "--fetch-timeout=120000" in script
|
|
|
|
|
assert script.count("npm install") == 1
|
|
|
|
|
|
|
|
|
|
worker_mount = next(
|
|
|
|
|
item for item in pod["containers"][0]["volumeMounts"]
|
|
|
|
|
if item["name"] == "tools"
|
|
|
|
|
)
|
|
|
|
|
assert worker_mount["readOnly"] is True
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_flux_health_for_hermes_does_not_wait_on_the_best_effort_pool():
|
|
|
|
|
kustomization = documents(FLUX / "hermes/kustomization.yaml")[0]
|
|
|
|
|
gated = {
|
|
|
|
|
(item["kind"], item["name"])
|
|
|
|
|
for item in kustomization["spec"]["healthChecks"]
|
|
|
|
|
}
|
|
|
|
|
assert ("StatefulSet", "hermes-execution-worker") not in gated
|
|
|
|
|
assert not any(name.startswith("hermes-execution-") for _kind, name in gated)
|
|
|
|
|
# The rest of the app's health contract is untouched, so its dependents keep
|
|
|
|
|
# the same guarantees they had before the pool existed.
|
|
|
|
|
assert ("Deployment", "hermes-agent") in gated
|
|
|
|
|
assert ("Deployment", "hermes-switchyard") in gated
|
|
|
|
|
assert kustomization["spec"]["timeout"] == "10m"
|
|
|
|
|
|
|
|
|
|
for name in ("hermes-chat", "hermes-observer-bindings"):
|
|
|
|
|
dependent = documents(FLUX / name / "kustomization.yaml")[0]
|
|
|
|
|
assert {item["name"] for item in dependent["spec"]["dependsOn"]} >= {"hermes"}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_the_pool_still_reports_its_own_readiness():
|
|
|
|
|
pod = worker()["spec"]["template"]["spec"]
|
|
|
|
|
assert pod["containers"][0]["readinessProbe"]["exec"]["command"][-1].startswith(
|
|
|
|
|
"test -w /workspace"
|
|
|
|
|
)
|
|
|
|
|
for deployment in mediators():
|
|
|
|
|
container = deployment["spec"]["template"]["spec"]["containers"][0]
|
|
|
|
|
assert container["readinessProbe"]["httpGet"] == {
|
|
|
|
|
"path": "/ready", "port": "mediator"
|
|
|
|
|
}
|
|
|
|
|
assert container["startupProbe"]["failureThreshold"] == 60
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_the_rendered_pool_configmap_carries_every_mounted_module():
|
|
|
|
|
kustomization = yaml.safe_load((HERMES / "kustomization.yaml").read_text())
|
|
|
|
|
generator = next(
|
|
|
|
|
item for item in kustomization["configMapGenerator"]
|
|
|
|
|
if item["name"] == "hermes-execution-pool"
|
|
|
|
|
)
|
|
|
|
|
keys = {entry.split("=", 1)[0] for entry in generator["files"]}
|
|
|
|
|
assert {
|
|
|
|
|
"execution_pool_store.py",
|
|
|
|
|
"execution_pool_maintenance.py",
|
|
|
|
|
"execution_pool_coordinator.py",
|
|
|
|
|
"execution_pool_server.py",
|
|
|
|
|
} <= keys
|
|
|
|
|
modules = sorted(
|
|
|
|
|
path.name for path in SCRIPTS.glob("execution_pool_*.py")
|
|
|
|
|
)
|
|
|
|
|
assert set(modules) <= keys, "a pool module is missing from its own mount"
|
|
|
|
|
assert json.dumps(sorted(keys))
|
2026-09-13 15:04:01 -05:00
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_rendered_pool_configmap_imports_its_own_route_dependencies(tmp_path):
|
|
|
|
|
"""Workers import routing code only from their mounted ConfigMap payload."""
|
|
|
|
|
rendered = subprocess.run(
|
|
|
|
|
["kustomize", "build", str(HERMES)],
|
|
|
|
|
check=True,
|
|
|
|
|
capture_output=True,
|
|
|
|
|
text=True,
|
|
|
|
|
)
|
|
|
|
|
configmap = next(
|
|
|
|
|
item
|
|
|
|
|
for item in yaml.safe_load_all(rendered.stdout)
|
|
|
|
|
if item
|
|
|
|
|
and item.get("kind") == "ConfigMap"
|
|
|
|
|
and item.get("metadata", {}).get("name", "").startswith(
|
|
|
|
|
"hermes-execution-pool-"
|
|
|
|
|
)
|
|
|
|
|
)
|
|
|
|
|
for name, content in configmap["data"].items():
|
|
|
|
|
if name.endswith(".py"):
|
|
|
|
|
(tmp_path / name).write_text(content, encoding="utf-8")
|
|
|
|
|
|
|
|
|
|
result = subprocess.run(
|
|
|
|
|
[
|
|
|
|
|
sys.executable,
|
|
|
|
|
"-I",
|
|
|
|
|
"-c",
|
|
|
|
|
f"import sys; sys.path.insert(0, {str(tmp_path)!r}); import cli_lane_routing",
|
|
|
|
|
],
|
|
|
|
|
cwd=tmp_path,
|
|
|
|
|
capture_output=True,
|
|
|
|
|
text=True,
|
|
|
|
|
)
|
|
|
|
|
assert result.returncode == 0, result.stderr
|
|
|
|
|
|
|
|
|
|
pod = worker()["spec"]["template"]["spec"]
|
|
|
|
|
container = pod["containers"][0]
|
|
|
|
|
assert {
|
|
|
|
|
"name": "routing-catalog",
|
|
|
|
|
"mountPath": "/routing-catalog",
|
|
|
|
|
"readOnly": True,
|
|
|
|
|
} in container["volumeMounts"]
|
|
|
|
|
volume = next(item for item in pod["volumes"] if item["name"] == "routing-catalog")
|
|
|
|
|
assert volume["persistentVolumeClaim"] == {
|
|
|
|
|
"claimName": "hermes-routing-catalog",
|
|
|
|
|
"readOnly": True,
|
|
|
|
|
}
|