test+fix: harden probe-flake in validate-network-isolation

[jwbron]

Summary

• Bump _wait_for_probe_pod timeout from 30s → 75s in orchestrator/routes/deployment.py. The k3s integration cluster intermittently took longer than 30s to schedule the probe pod, so the route returned the probe_timeout shape (probe_id present, no result). 75s leaves headroom under the test's 90s HTTP timeout.
• In integration_tests/test_deployment_validation_logic.py::test_probe_runs_and_returns_expected_shape, handle the probe-timeout shape explicitly with pytest.fail(...) instead of letting data["result"] raise a bare KeyError: 'result'. Future flakes now name the probe, not look like a generic test bug.

Surfaced by #2688 comment. The same KeyError: 'result' was hitting unrelated branches today (e.g. egg/doc-update-populate-contract-errors).

Test plan

• Integration Tests job passes (was previously flaking on this exact test).
• Confirm test_probe_runs_and_returns_expected_shape still asserts the four gateway_reachable / internet_blocked / agent_pods_unreachable / orchestrator_api_reachable invariants on the happy path.

[egg-reviewer]

Review of PR #2699

Small, well-scoped flake fix. No blocking issues. One minor inaccuracy in an added comment and a couple of non-blocking observations.

Non-blocking

1. Comment mis-attributes the 90 s ceiling — orchestrator/routes/deployment.py:1270-1272

# 30s was too tight: probe-pod scheduling on the k3s integration
# cluster intermittently exceeded the deadline. 75s sits under the
# require_lifecycle_secret route's 90s HTTP-timeout ceiling.
pod = _wait_for_probe_pod(k8s, namespace, probe_id, timeout=75.0)

require_lifecycle_secret (orchestrator/lifecycle_auth.py) is purely a Bearer-token check — it imposes no HTTP timeout. The 90 s is the test client's timeout=90 parameter on the requests.post call in integration_tests/test_deployment_validation_logic.py:437. Naming the decorator in the comment is misleading; a future reader chasing the source of the 90 s ceiling will look in the wrong place.

Suggested rewrite:

# 30s was too tight: probe-pod scheduling on the k3s integration
# cluster intermittently exceeded the deadline. 75s leaves headroom
# under the integration test's 90s HTTP-client timeout
# (test_probe_runs_and_returns_expected_shape).

2. End-to-end latency budget is tight — same line

Worst-case wall clock when the pod runs to the deadline:

• _wait_for_probe_pod poll → up to 75 s
• _read_probe_log (urllib3 read-to-EOF on the pod log) → a few s
• _delete_probe_job (best-effort, propagation=Background) → ~1 s

That sits at roughly 80 s against the test's 90 s client timeout, leaving ~10 s headroom. If the underlying k3s flake gets worse (or log reads ever stall — _read_probe_log swallows exceptions but does not impose an inner timeout), the test will start failing on the requests-side ReadTimeout rather than the route's probe_timeout shape, and the new diagnostic branch in the test won't fire. Worth keeping in mind for the next bump: the route timeout and the client timeout need to be tuned as a pair.

3. pytest.fail vs pytest.skip — integration_tests/test_deployment_validation_logic.py:446-447

if "result" not in data:
    pytest.fail(f"probe pod did not complete before route timeout: {data!r}")

fail is the right call here — the PR explicitly wants probe-timeout occurrences to remain visible so the timeout can be retuned, not silenced. Flagging only so it's a deliberate choice, not an oversight: pytest.skip would silence ongoing flakes, which is not what this PR is going for. Leave as-is.

Verified, no concerns

• _wait_for_probe_pod has only one production caller (validate_network_isolation); the timeout bump has no other blast radius.
• Unit tests in orchestrator/tests/test_deployment_routes.py:834-897 mock _wait_for_probe_pod directly, so they're unaffected by the value change.
• The route's probe_timeout shape (line 1280) is unchanged — the new test branch correctly matches that shape (probe_id present, no result).
• _delete_probe_job runs in finally, so the bumped timeout cannot leak Jobs even on a slow pod.
• No new locking, no new state, no security-sensitive paths touched.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs