Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions integration_tests/test_deployment_validation_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def _post(
*,
secret: str,
body: dict,
timeout: int = 60,
timeout: int = 90,
) -> requests.Response:
return requests.post(
f"{orchestrator_url}{path}",
Expand Down Expand Up @@ -682,10 +682,11 @@ class TestProbeJobCleanup:
"""Belt-and-braces: no orphan probe Jobs after the route returns.

The route uses a ``try/finally`` to call ``_delete_probe_job`` and
sets ``ttlSecondsAfterFinished: 0`` on the Job. With #2646 fixed
the probe actually launches, so this assertion catches a cleanup
regression where the finally path failed to delete the Job (or the
ttl-after-finished GC failed to fire).
sets ``ttlSecondsAfterFinished: 30`` on the Job as a backstop. With
#2646 fixed the probe actually launches, so this assertion catches
a cleanup regression where the finally path failed to delete the
Job (or the ttl-after-finished GC failed to fire within the 30s +
grace window).
"""

def test_no_orphan_probe_jobs_after_call(
Expand All @@ -710,9 +711,12 @@ def test_no_orphan_probe_jobs_after_call(
timeout=90,
)

# Allow up to a few seconds for ttlSecondsAfterFinished=0 to
# reap any Job that did launch — the API call returned, but
# the controller may not have run the GC pass yet.
# The route's try/finally _delete_probe_job is the primary
# cleanup path, so the Job should be gone within milliseconds
# of the route returning. The 15s deadline is deliberately
# below ttlSecondsAfterFinished=30: a finally-path regression
# must surface as a test failure here, not get masked by the
# TTL controller sweeping the orphan inside the poll window.
deadline = time.time() + 15
leftover: list[str] = []
while time.time() < deadline:
Expand Down Expand Up @@ -742,5 +746,5 @@ def test_no_orphan_probe_jobs_after_call(
time.sleep(1)
pytest.fail(
f"probe Jobs still present after route returned (route should "
f"clean up via finally + ttlSecondsAfterFinished=0): {leftover}"
f"clean up via finally + ttlSecondsAfterFinished=30): {leftover}"
)
7 changes: 4 additions & 3 deletions orchestrator/mcp_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -1111,9 +1111,10 @@ def _is_timeout_error(exc: BaseException) -> bool:
"Spawn a throwaway probe Job in the egg-agents namespace to verify "
"Calico NetworkPolicy enforcement. Returns a structured "
"{gateway_reachable, internet_blocked, agent_pods_unreachable, "
"orchestrator_api_reachable} result. The Job self-deletes on exit "
"(ttlSecondsAfterFinished=0). Only available on the Kubernetes "
"runtime and on CNIs that enforce NetworkPolicies."
"orchestrator_api_reachable} result. The route deletes the Job "
"in a try/finally; ttlSecondsAfterFinished=30 is the backstop. "
"Only available on the Kubernetes runtime and on CNIs that "
"enforce NetworkPolicies."
),
"inputSchema": {
"type": "object",
Expand Down
13 changes: 12 additions & 1 deletion orchestrator/routes/deployment.py
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,18 @@ def _build_probe_job_manifest(
},
},
"spec": {
"ttlSecondsAfterFinished": 0,
# 0 raced with _wait_for_probe_pod's 1s poll: the Job could
# complete and be GC'd by the TTL-after-finished controller
# before the next poll observed the pod's terminal phase,
# leaving the wait loop scanning an empty list until its 75s
# ceiling (seen as the bimodal ~10s-or-75s distribution in
# https://github.com/jwbron/egg/actions/runs/25817353877).
# 30s guarantees the wait loop sees Succeeded/Failed and
# _read_probe_log still has a pod to read from; the route's
# try/finally _delete_probe_job remains the primary cleanup
# path, so this only extends lifetime when the route crashed
# before reaching finally.
"ttlSecondsAfterFinished": 30,
"activeDeadlineSeconds": 30,
"backoffLimit": 0,
"template": {
Expand Down
7 changes: 6 additions & 1 deletion orchestrator/tests/test_deployment_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,12 @@ def test_probe_manifest_has_expected_labels_and_safety(self):
assert labels["egg.agent.role"] == "coder"

spec = manifest["spec"]
assert spec["ttlSecondsAfterFinished"] == 0
# 30s (not 0): _wait_for_probe_pod polls at 1Hz, and ttl=0
# raced the poll cadence — Job/pod could be GC'd before the
# terminal-phase observation. 30s keeps the pod observable for
# the wait loop and the subsequent _read_probe_log. The route's
# try/finally is still the primary cleanup path.
assert spec["ttlSecondsAfterFinished"] == 30
assert spec["activeDeadlineSeconds"] == 30
assert spec["backoffLimit"] == 0

Expand Down
Loading