lease gate: stop denying every seat launch on a 2s probe budget
The activation-capability probe spawns the whole Node CLI rather than exec'ing a binary. Measured 3.0-3.7s on an idle 4-core VM and 3.55-3.61s on web1, against `node -e 0` at 0.05s. The budget was 2.0s, so the probe timed out on every call on both hosts. The gate is fail-closed, and an expiry is indistinguishable from "no capability", so every fleet seat launch was denied with a version-skew message telling the operator to "upgrade both as one unit" -- advice that cannot fix a timeout. This is why web1 shows roster seats with no live sessions. Raise the budget to 20s, well clear of the measured range, and add MOSAIC_LEASE_VERSION_PROBE_TIMEOUT_SECONDS for slower hosts. Unusable override values fall back to the default rather than removing the bound. Also fix _resolve_probe_command ignoring the environ it is handed: shutil.which was called without path=, so it read the ambient PATH. That made test_returns_none_when_mosaic_is_not_resolvable_on_path pass only because the 2.0s budget expired first -- right answer, wrong reason, and it masked the timeout defect. The suite's runtime drops from 2.0s to 0.002s, which is that accidental timeout leaving. Verified: 18/18 version_coupling_unittest (new tests red against the old gate: 2 failures + 1 error), tsc build clean, test-start-agent-session.sh and test-fleet-units.sh rc=0. invariant_r_unittest fails identically with and without this change (pinned pi 0.84.1 vs installed 0.84.2).
This commit is contained in:
@@ -62,7 +62,20 @@ EXPECTED_ACTIVATION_CAPABILITY: Final[ActivationCapability] = {
|
||||
# capability as compact JSON.
|
||||
LEASE_CAPABILITY_PROBE_COMMAND: Final = "__lease-capability"
|
||||
|
||||
PROBE_TIMEOUT_SECONDS: Final = 2.0
|
||||
# Running the probe boots the whole Node CLI; it does not merely exec a binary.
|
||||
# Measured: 3.0-3.7 s on an idle 4-core VM and 3.55-3.61 s on web1, against
|
||||
# `node -e 0` at 0.05 s. The former 2.0 s budget therefore expired on every
|
||||
# call on both hosts. Because the probe is fail-closed, an expiry is
|
||||
# indistinguishable from "no capability", so every seat launch was denied with
|
||||
# a version-skew message that no upgrade could fix. Sized well above the
|
||||
# measured range: the gate still fails closed, it just no longer fails closed
|
||||
# on a stopwatch.
|
||||
PROBE_TIMEOUT_SECONDS: Final = 20.0
|
||||
|
||||
# Override hook: seconds to wait for the probe, for hosts slow or loaded enough
|
||||
# that even the default is tight. Non-numeric or non-positive values are
|
||||
# ignored in favour of the default rather than disabling the bound.
|
||||
PROBE_TIMEOUT_OVERRIDE_VAR: Final = "MOSAIC_LEASE_VERSION_PROBE_TIMEOUT_SECONDS"
|
||||
|
||||
# Override hook: a full shell-style command line (parsed with `shlex.split`)
|
||||
# to run INSTEAD of resolving `mosaic` on PATH and appending the probe
|
||||
@@ -83,12 +96,28 @@ class VersionCouplingError(Exception):
|
||||
silent pass, and never let its absence be treated as compatible."""
|
||||
|
||||
|
||||
def _resolve_probe_timeout(environ: Mapping[str, str]) -> float:
|
||||
raw = environ.get(PROBE_TIMEOUT_OVERRIDE_VAR)
|
||||
if not raw:
|
||||
return PROBE_TIMEOUT_SECONDS
|
||||
try:
|
||||
seconds = float(raw)
|
||||
except ValueError:
|
||||
return PROBE_TIMEOUT_SECONDS
|
||||
if seconds <= 0 or seconds != seconds or seconds == float("inf"):
|
||||
return PROBE_TIMEOUT_SECONDS
|
||||
return seconds
|
||||
|
||||
|
||||
def _resolve_probe_command(environ: Mapping[str, str]) -> list[str] | None:
|
||||
override = environ.get(MOSAIC_COMMAND_OVERRIDE_VAR)
|
||||
if override:
|
||||
parsed = shlex.split(override)
|
||||
return parsed or None
|
||||
resolved = shutil.which("mosaic")
|
||||
# Resolve against the caller's PATH, not the ambient process one. The
|
||||
# function is handed an `environ` and honoured it only for the override
|
||||
# var, so a caller passing an explicit PATH was silently ignored here.
|
||||
resolved = shutil.which("mosaic", path=environ.get("PATH"))
|
||||
if resolved is None:
|
||||
return None
|
||||
return [resolved, LEASE_CAPABILITY_PROBE_COMMAND]
|
||||
@@ -116,7 +145,7 @@ def default_probe_activation_capability(
|
||||
command,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=PROBE_TIMEOUT_SECONDS,
|
||||
timeout=_resolve_probe_timeout(source_environment),
|
||||
check=False,
|
||||
)
|
||||
except (OSError, subprocess.TimeoutExpired, ValueError):
|
||||
|
||||
@@ -187,6 +187,43 @@ class ProbeActivationCapabilityTest(unittest.TestCase):
|
||||
)
|
||||
self.assertIsNone(result)
|
||||
|
||||
def test_probe_budget_exceeds_real_cli_boot_time(self) -> None:
|
||||
# Regression: the budget was 2.0 s while the probe boots the whole Node
|
||||
# CLI, measured at 3.0-3.7 s on an idle VM and 3.55-3.61 s on web1. The
|
||||
# gate is fail-closed, so an expiry is indistinguishable from "no
|
||||
# capability" and every seat launch was denied with a version-skew
|
||||
# message no upgrade could fix. Guards the ordering, not a stopwatch.
|
||||
self.assertGreater(VERSION_GATE.PROBE_TIMEOUT_SECONDS, 10.0)
|
||||
|
||||
def test_probe_timeout_override_is_honoured_and_rejects_unusable_values(self) -> None:
|
||||
self.assertEqual(VERSION_GATE._resolve_probe_timeout({}), VERSION_GATE.PROBE_TIMEOUT_SECONDS)
|
||||
self.assertEqual(
|
||||
VERSION_GATE._resolve_probe_timeout(
|
||||
{VERSION_GATE.PROBE_TIMEOUT_OVERRIDE_VAR: "45"}
|
||||
),
|
||||
45.0,
|
||||
)
|
||||
# An unusable override must fall back to the default rather than
|
||||
# removing the bound: no override may make the gate wait forever.
|
||||
for unusable in ("", "abc", "0", "-5", "nan", "inf"):
|
||||
with self.subTest(value=unusable):
|
||||
self.assertEqual(
|
||||
VERSION_GATE._resolve_probe_timeout(
|
||||
{VERSION_GATE.PROBE_TIMEOUT_OVERRIDE_VAR: unusable}
|
||||
),
|
||||
VERSION_GATE.PROBE_TIMEOUT_SECONDS,
|
||||
)
|
||||
|
||||
def test_command_resolution_honours_the_supplied_path(self) -> None:
|
||||
# Regression: `shutil.which` was called without `path=`, so it read the
|
||||
# ambient process PATH and silently ignored the environ handed in. On a
|
||||
# host with `mosaic` installed that made the not-resolvable case pass
|
||||
# only because the 2.0 s budget expired first — right answer, wrong
|
||||
# reason, and it masked the timeout defect above.
|
||||
self.assertIsNone(
|
||||
VERSION_GATE._resolve_probe_command({"PATH": "/nonexistent-bin-dir-for-869-c4-test"})
|
||||
)
|
||||
|
||||
|
||||
class LaunchRuntimeVersionCouplingSeamTest(unittest.TestCase):
|
||||
"""End-to-end (still fully faked) coverage of the seam as wired into
|
||||
|
||||
Reference in New Issue
Block a user