diff --git a/orchestrator/orchestrator/batch.py b/orchestrator/orchestrator/batch.py index 063d3c3..8c560b6 100644 --- a/orchestrator/orchestrator/batch.py +++ b/orchestrator/orchestrator/batch.py @@ -45,7 +45,7 @@ EXIT_FAILED = 1 EXIT_NOT_READY = 2 -ACTIONS = ("preflight", "mint", "os-update", "add-to-group", "provision") +ACTIONS = ("preflight", "mint", "os-update", "add-to-group", "validate", "provision") @dataclass @@ -127,6 +127,10 @@ def _child_cmd( # No gate flags: mint runs BEFORE the Recovery trip and the OS update, because Recovery # needs a volume owner and mint is what creates one. Gating it would deadlock the order. return [exe, "mint", host] + elif action == "validate": + # Read-only fitness check; no gate flags, and it inspects the RUNNING host rather than + # gating on a target version, so --expected-os doesn't apply. + return [exe, "validate", host] elif action == "add-to-group": # No gate flags: nothing here inspects the OS version or SIP state. It does need SSH, # though — the host's serial is the only join key to its SimpleMDM device record, since a @@ -242,6 +246,8 @@ def _cmd_for(host: str) -> list[str]: # Launch-and-return: staging the script plus the started-cleanly check. The ~14GB # download runs detached and outlives this call by design. per_host_timeout = 600 + elif action == "validate": + per_host_timeout = 300 elif action == "add-to-group": # Three SimpleMDM calls plus one SSH round-trip for the serial; seconds, bar the 429 # backoff. With --quarantine-on-register it instead blocks for the whole bootstrap, @@ -259,7 +265,9 @@ def _cmd_for(host: str) -> list[str]: # "gate: macOS 15.3 · SIP must be disabled" on every action was actively misleading: on a # `--action mint` run over SIP-on hosts it read as though SIP state had been validated and # passed, when mint is handed neither flag and checks neither thing. - if action in ("mint", "add-to-group"): + if action == "validate": + gate_note = "read-only fitness check on already-bootstrapped hosts" + elif action in ("mint", "add-to-group"): gate_note = f"no OS/SIP gate — {action} doesn't inspect the running OS" elif action == "os-update": gate_note = f"target macOS {expected_os} · SIP not checked" @@ -272,6 +280,8 @@ def _cmd_for(host: str) -> list[str]: ui.info("--no-wait: mint + escrow only; sweep sentinels afterwards") if action == "os-update": ui.info("launches the upgrade and returns; hosts reboot on their own — sweep with --action preflight after") + if action == "validate": + ui.info("exit 2 = not bootstrapped yet (skipped) · exit 1 = bootstrapped but UNFIT to take work") if action == "add-to-group": ui.info("ADD only, never a move; already-member hosts are skipped — then wait for the pkg to land") ui.info( diff --git a/orchestrator/orchestrator/cli.py b/orchestrator/orchestrator/cli.py index e26dfd0..15cd545 100644 --- a/orchestrator/orchestrator/cli.py +++ b/orchestrator/orchestrator/cli.py @@ -122,7 +122,7 @@ def batch( action: str = typer.Option( "provision", "--action", - help="What to run per host: preflight | mint | os-update | add-to-group | provision.", + help="What to run per host: preflight | mint | os-update | add-to-group | validate | provision.", ), concurrency: int = typer.Option( 0, "--concurrency", "-j", help="How many hosts in flight (default 3 — MDC1 throughput, not CPU)." @@ -240,6 +240,26 @@ def add_to_group( ) +@_app.command() +def validate( + hostname: str, + expected_refresh_hz: float = typer.Option( + 0.0, + "--expected-refresh-hz", + help="Required display refresh rate (default: settings.validate_expected_refresh_hz, 60).", + ), +) -> None: + """Read-only fitness check on a bootstrapped host — run this before unquarantining it. + + Checks the things that can be perfect everywhere else and still fail every task: the display + mode (a KVM at 75Hz makes mozharness halt before any test runs — see m4-242, 15 tasks lost), + the last puppet run, and the worker. Exits 2 if the host hasn't bootstrapped yet, 1 if unfit. + """ + workflow.step_validate( + workflow.resolve_offline(hostname), expected_refresh_hz=expected_refresh_hz or None + ) + + @_app.command() def wait_bootstrap_pkg(hostname: str) -> None: """Confirm the signed bootstrap pkg landed — i.e. the host is in the bootstrap group. diff --git a/orchestrator/orchestrator/clients/ssh.py b/orchestrator/orchestrator/clients/ssh.py index 57378d1..b404184 100644 --- a/orchestrator/orchestrator/clients/ssh.py +++ b/orchestrator/orchestrator/clients/ssh.py @@ -215,6 +215,45 @@ def secure_token_status(hostname: str) -> str: return cp.stdout.decode(errors="replace").strip() +def display_mode(hostname: str, *, session_user: str = "cltbld") -> tuple[float, int, int] | None: + """(refresh_hz, width, height) of the main display, or None if it can't be read. + + Must run INSIDE the logged-in GUI session. Over a plain SSH login as admin, CoreGraphics has no + window server to talk to and answers `refresh=0.0, 0x0` — not an error, just zeros, which would + sail past a naive check. So we hop into the console user's session with `launchctl asuser`, the + same technique the Safari automation uses. + + Returns None rather than raising when the session isn't there (no such user yet, nobody logged + in, PyObjC missing). Callers must treat None as "unknown", never as "fine": on a host that has + not bootstrapped, `cltbld` doesn't exist at all. + + Reads what mozharness reads, so this agrees with what CI will decide about the host. + """ + cp = run( + hostname, + "uid=$(id -u " + shlex.quote(session_user) + " 2>/dev/null) || exit 9; " + '[ -n "$uid" ] || exit 9; ' + "sudo launchctl asuser \"$uid\" /usr/local/bin/python3 -c " + "'import Quartz;" + "d=Quartz.CGMainDisplayID();m=Quartz.CGDisplayCopyDisplayMode(d);" + 'print("%.2f %d %d" % (Quartz.CGDisplayModeGetRefreshRate(m),' + "Quartz.CGDisplayModeGetPixelWidth(m),Quartz.CGDisplayModeGetPixelHeight(m)))'", + check=False, + ) + for line in reversed(cp.stdout.decode(errors="replace").strip().splitlines()): + parts = line.split() + if len(parts) == 3: + try: + hz, w, h = float(parts[0]), int(parts[1]), int(parts[2]) + except ValueError: + continue + # All-zero means we asked from outside the session; that's "unknown", not 0Hz. + if (hz, w, h) == (0.0, 0, 0): + return None + return hz, w, h + return None + + def platform_serial(hostname: str) -> str: """Hardware serial number, or '' if unreachable. diff --git a/orchestrator/orchestrator/config.py b/orchestrator/orchestrator/config.py index b65d8d1..8b84318 100644 --- a/orchestrator/orchestrator/config.py +++ b/orchestrator/orchestrator/config.py @@ -101,6 +101,14 @@ class Settings(BaseSettings): # override. bootstrap_group_id: int = Field(default=2417981) + # `validate` refuses a host whose main display isn't at this refresh rate. 60.0 because that is + # what mozharness's own pre-test check enforces — matching it means validate agrees with what CI + # will decide, rather than inventing a second standard. A KVM presenting 75Hz made m4-242 fail + # 15 consecutive production tasks in ~43s each (2026-08-14) without ever running a test. + # Resolution is deliberately NOT gated: it varies legitimately across the fleet, so validate + # reports it and only hard-fails on the thing CI actually rejects. + validate_expected_refresh_hz: float = Field(default=60.0) + # Hostname -> puppet role mapping # Loaded from a per-fleet JSON/YAML file outside this code so it can be edited # without redeploying the CLI. For now we default to pattern-matching on hostname. diff --git a/orchestrator/orchestrator/workflow.py b/orchestrator/orchestrator/workflow.py index 90fc9c9..ffd148c 100644 --- a/orchestrator/orchestrator/workflow.py +++ b/orchestrator/orchestrator/workflow.py @@ -360,6 +360,83 @@ def step_add_to_group( ) +def step_validate(ctx: HostContext, *, expected_refresh_hz: float | None = None) -> None: + """Read-only fitness check on a bootstrapped host: is it actually able to run tasks? + + This fills the gap the quarantine message already promises. `--quarantine-on-register` holds a + fresh host "pending validation", but nothing validated anything — so the only way a host proved + itself unfit was by failing real work. macmini-m4-242 destroyed 15 production tasks (mochitest, + jsreftest, web-platform-tests) at ~43s each before anyone looked, because its KVM presented + 1280x1024@75Hz and mozharness fatally halts a pre-test refresh-rate check at anything but 60Hz. + Every other signal on that host was perfect: puppet green, sentinel present, worker up, disk + fine, semaphores byte-identical to a working host. + + Deliberately runs AFTER bootstrap, not as part of preflight: reading the display needs the + logged-in cltbld session, and on a fresh host cltbld does not exist until puppet creates it. A + preflight version would silently pass exactly the hosts it was meant to catch. + + Read-only — safe on live workers. Raises NotReadyError (exit 2, "skipped") when the host hasn't + bootstrapped yet, and ReprovisionError (exit 1) when it has and is unfit. + """ + s = get_settings() + want_hz = expected_refresh_hz or s.validate_expected_refresh_hz + ui.step("VALIDATE", "is this host fit to take work? (read-only)") + + if not ssh.file_exists(ctx.fqdn, SENTINEL): + raise NotReadyError( + f"{ctx.hostname}: {SENTINEL} missing — host hasn't finished bootstrapping, nothing to " + "validate yet" + ) + + problems: list[str] = [] + + # The display check first: it's the one that passes every other signal and still eats tasks. + ui.wire(f"ssh admin@{ctx.hostname} launchctl asuser $(id -u cltbld) … CGDisplayModeGetRefreshRate") + mode = ssh.display_mode(ctx.fqdn) + if mode is None: + # Unknown, not fine. A host whose GUI session we can't reach can't run tests either. + problems.append( + "couldn't read the display mode — no cltbld GUI session? Without it mozharness's " + "pre-test refresh-rate check can't pass either" + ) + else: + hz, w, h = mode + if abs(hz - want_hz) < 0.5: + ui.ok(f"display {w}x{h} @ {hz:.2f}Hz") + else: + problems.append( + f"display is {w}x{h} @ {hz:.2f}Hz, expected {want_hz:.2f}Hz — mozharness halts every " + "task on this before running a single test. Usually the KVM isn't set correctly." + ) + + puppet_ok = ssh.run( + ctx.fqdn, + "sudo grep -o '\"success\": [a-z]*' /opt/puppet_environments/last_run_metadata.json " + "2>/dev/null | head -1 | awk '{print $2}'", + check=False, + ).stdout.decode(errors="replace").strip() + if puppet_ok == "true": + ui.ok("last puppet run succeeded") + else: + problems.append(f"last puppet run reported success={puppet_ok or 'unknown'}") + + worker_up = ssh.run( + ctx.fqdn, "pgrep -f 'start-worker ' >/dev/null && echo up || echo down", check=False + ).stdout.decode(errors="replace").strip() + if worker_up == "up": + ui.ok("generic-worker is running") + else: + # Not fatal on its own: these hosts reboot between tasks, so a down worker can just mean + # we caught it mid-cycle. Report it without failing the host on timing alone. + ui.warn("generic-worker isn't running right now (may be mid-reboot between tasks)") + + if problems: + raise ReprovisionError( + f"{ctx.hostname} is NOT fit to take work:\n - " + "\n - ".join(problems) + ) + ui.ok(f"{ctx.hostname} looks fit — safe to unquarantine") + + def step_wait_for_bootstrap_pkg(ctx: HostContext) -> None: """Confirm the bootstrap pkg actually landed, before committing to the long sentinel wait.