diff --git a/docs/RUNBOOK.md b/docs/RUNBOOK.md index 5a5cd98..a64f122 100644 --- a/docs/RUNBOOK.md +++ b/docs/RUNBOOK.md @@ -16,12 +16,13 @@ because that's the order you meet them in. cd ~/git/relops-bootstrap/orchestrator HOSTS=~/Desktop/wave.txt # one short hostname per line, '#' comments ok +uv run reprovision group-parity # ← do the hosts get prod's profiles? uv run reprovision batch $HOSTS --action mint -j3 # SecureToken (idempotent, often a no-op) uv run reprovision batch $HOSTS --action os-update -j3 # in-place upgrade to target OS # ⏳ wait ~35 min: ~14GB download, then startosinstall, then a reboot uv run reprovision batch $HOSTS --action preflight --allow-sip-enabled -j3 -uv run reprovision batch $HOSTS --action add-to-group --quarantine-on-register -j2 -# ⏳ blocks ~30 min per host: this is the bootstrap, and the watch that holds each host +uv run reprovision batch $HOSTS --action add-to-group --quarantine-on-register +# ⏳ two phases, automatically: a paced SimpleMDM add, then every watcher at once (~30 min) uv run reprovision batch $HOSTS --action validate -j3 # ← the gate. Do not skip. uv run reprovision unquarantine macmini-m4-XXX # only what validate passed ``` @@ -55,24 +56,48 @@ bound by a different resource. | `mint` | 3–4 | SSH | fast; often a no-op when the token already exists | | `os-update` | **irrelevant** ⚠️ | the mirror | **launch-and-return** — `-j` paces only the *launches*. The **hosts-file length is the real concurrency.** 33 hosts = 33 simultaneous 14GB pulls | | `preflight` | 3–4 | SSH | read-only | -| `add-to-group` | **2** 🚨 | **SimpleMDM API** | 3 API calls per host. At `-j12` the 429 retry budget blew and hosts failed | +| `add-to-group` | **2** 🚨 | **SimpleMDM API** | 3 API calls per host. At `-j12` the 429 retry budget blew and hosts failed. Clamped to `REPROVISION_SIMPLEMDM_MAX_CONCURRENT` (2) regardless of `-j` | +| `quarantine-on-register` | all of them | local process count | an idle poll loop; `-j` does not apply | | `validate` | 3 | SSH | read-only | | `provision` | 3 | MDC1 imaging throughput | matches the runner's `RUNNER_MAX_CONCURRENT` | -### 🪤 The `-j` trap that bit us hardest +### 🪤 The `-j` trap that bit us hardest — now fixed -`add-to-group --quarantine-on-register` couples a **SimpleMDM-bound action** to a -**30-minute Taskcluster-bound wait**, so one `-j` has to serve both. Raise it for wall-clock -and you hammer SimpleMDM; lower it for SimpleMDM and 33 hosts serialize into ~5½ hours. +`add-to-group --quarantine-on-register` used to couple a **SimpleMDM-bound action** to a +**30-minute Taskcluster-bound wait**, so one `-j` had to serve both. Raise it for wall-clock and +you hammered SimpleMDM; lower it for SimpleMDM and 33 hosts serialized into ~5½ hours. + +**It now splits itself into two phases**, so the command below is all you run: + +```bash +uv run reprovision batch $HOSTS --action add-to-group --quarantine-on-register +``` + +Phase 1 adds hosts at the SimpleMDM cap (2), clamping `-j` with a warning if you asked for more. +Phase 2 then watches every host at once, with the bootstrap-spanning budget passed explicitly — +no `REPROVISION_QUARANTINE_ON_REGISTER_MAX_WAIT_SECONDS` export needed. Both phases log under one +batch directory, and a `Ctrl-C` writes `added.txt` naming every host that is bootstrapping +unwatched, plus the command to re-attach: + +```bash +uv run reprovision batch ~/.local/state/reprovision/batch-/added.txt \ + --action quarantine-on-register +``` + +A host whose add *failed* is still watched — the add can succeed while the follow-up `push_apps` +429s, which is exactly what happened on 2026-08-14. + +
+The old manual workaround, for reference At `-j12` on 2026-08-14 it looked like 5 hosts failed. What actually happened: **12 hosts had already been added to the group** (the add succeeded, the follow-up `push_apps` 429'd), so killing the batch orphaned 12 live bootstraps with no watcher. They'd have gone into production unvalidated. -**Workaround until this is fixed** — split it along the resource boundary: +Split it along the resource boundary by hand: ```bash # 1. the SimpleMDM half, gently @@ -90,6 +115,8 @@ grep '^macmini' $HOSTS | xargs -P 33 -I{} \ > (~30 min). Too short and the watch expires before there's anything to quarantine — and the > host goes live unheld, i.e. exactly the failure the flag exists to prevent. +
+ --- ## 🔬 Never trust `ok`. Verify the thing itself. @@ -259,6 +286,30 @@ looping over many hosts by hand. - ⚠️ **Membership does not prove the PKG was pushed.** The already-a-member path skips `push_apps`, so a host added by hand in the UI can sit in the group with nothing installed. `add-to-group` warns when the payload is missing. +- 🧬 **Check profile parity before a wave**, not after the first host hangs: + + ```bash + uv run reprovision group-parity # the whole bootstrap group + uv run reprovision group-parity --host macmini-m4-241 # one host + ``` + + Read-only, API-only, no SSH unless you pass `--host`. It builds a baseline from the profiles + *every* sampled production device shares, then names anything a target device lacks — with the + m4-214 story attached for the profiles that caused it. + + It also runs a second, **peer-wise membership pass**: a group that two thirds of these devices + are in but some are not means those devices were *moved* rather than added. This catches strictly + more than the profile diff, because the groups a mis-clicked host loses are mostly app-bearing — + on 2026-08-19 three hosts were missing `Relops Public SSH Key`, `Sudoers`, `Enable SSH` and + `DEP Enrollment`, i.e. no admin key, no passwordless sudo and no sshd after their next wipe. Two + of them were also in the prod group, so they still received the profiles and a profile-only diff + said nothing. + + It compares **effective per-device** profile sets, deliberately. Diffing the assignment groups + themselves reports Skip Setup Assistant and the FDA SSH Keygen Wrapper as missing from the + bootstrap group — true, and irrelevant: its devices get both from the additive DEP Enrollment + group. Don't "simplify" it back to a group-level diff; it would cry wolf on the exact pair from + the postmortem. --- diff --git a/orchestrator/README.md b/orchestrator/README.md index 67269c9..4e272b8 100644 --- a/orchestrator/README.md +++ b/orchestrator/README.md @@ -202,10 +202,11 @@ second trip to the rack, and **SIP stays enabled** — no Recovery visit require ```bash HOSTS=~/Desktop/wave.txt +reprovision group-parity # ⓪ profile parity, before anything reprovision batch $HOSTS --action mint -j3 # ① SecureToken (idempotent) reprovision batch $HOSTS --action os-update -j3 # ② in-place upgrade, ~35 min reprovision batch $HOSTS --action preflight --allow-sip-enabled -j3 # ③ read-only gate -reprovision batch $HOSTS --action add-to-group --quarantine-on-register -j2 # ④ the GO +reprovision batch $HOSTS --action add-to-group --quarantine-on-register # ④ the GO (2 phases) reprovision batch $HOSTS --action validate -j3 # ⑤ fitness — do not skip reprovision unquarantine macmini-m4-XXX # ⑥ release what passed ``` @@ -293,12 +294,14 @@ hosts offline simultaneously on the 2026-05-12 batch. > - **`os-update` ignores `-j` entirely.** It's launch-and-return (~10s/host), so `-j` paces only > the launches while the ~14GB download runs detached. **The hosts-file length is the real > concurrency.** 33 hosts means 33 simultaneous pulls no matter what you pass. -> - **`add-to-group` wants `-j2`.** It makes 3 SimpleMDM calls per host, and the API's rate -> limiter is unforgiving. At `-j12` the 429 retry budget was exhausted in ~64s. +> - **`add-to-group` is capped at 2 for you.** It makes 3 SimpleMDM calls per host and the API's +> rate limiter is unforgiving — at `-j12` the 429 retry budget was exhausted in ~64s. A larger +> `-j` is clamped, with a warning. +> - **`add-to-group --quarantine-on-register` runs as two phases**, because a SimpleMDM-bound add +> and a 30-minute Taskcluster-bound watch cannot share one `-j`. The add is paced; the watchers +> all start at once. > -> The full table, plus why `add-to-group --quarantine-on-register` couples two different -> bottlenecks to one `-j` and how to split them: -> [Runbook → Concurrency](../docs/RUNBOOK.md#concurrency). +> The full table: [Runbook → Concurrency](../docs/RUNBOOK.md#concurrency). > 🔬 **`ok` means the command succeeded, not that the work happened.** For `os-update`, `ok` > means *the upgrade launched* — verify with `pgrep -x curl` plus growth of @@ -358,6 +361,7 @@ direct `REPROVISION_*` value always wins over its `_REF`. | `reprovision run ` | Full pipeline, **including an EACS wipe**. `--unquarantine` returns it to service at the end. | | `reprovision provision ` | Fresh DEP host → prod. **No wipe in this path.** `--no-wait` stops after the BST escrow. | | `reprovision preflight ` | Read-only readiness check (OS version, SIP, SecureToken, BST). Needs no SimpleMDM/TC credential. | +| 🧬 `reprovision group-parity` | Read-only: do a group's hosts get the profiles a working production host gets? Builds a baseline from the profiles every sampled prod device shares and names what a target lacks. Compares **effective per-device** sets, not assignment groups — the bootstrap group carries neither m4-214 profile yet its devices hold both, via the additive DEP Enrollment group. Also flags **membership outliers** — a device missing a group two thirds of its peers are in was moved, not added, and the groups it lost are mostly app-bearing (admin key, sudo, sshd), which a profile diff cannot see. `--host` checks one box; needs only the SimpleMDM API key otherwise. | | 🚀 `reprovision add-to-group ` | **ADD** the host to the SimpleMDM bootstrap group — the action that triggers the whole bootstrap. Additive only, never a move. Idempotent. Refuses production groups. `--quarantine-on-register` starts the registration watch here, where it belongs. | | ✅ `reprovision validate ` | Read-only **fitness** check on a bootstrapped host — display mode (60Hz), last puppet run, worker. **Run this before every unquarantine.** Exit 2 = not bootstrapped yet, exit 1 = bootstrapped but UNFIT. | | `reprovision quarantine-on-register ` | Watch for a fresh worker to register, then quarantine it on sight. | @@ -387,7 +391,9 @@ skipped separately from failed, which is what makes "38 ok, 15 not ready, 2 brok | `-j` / `REPROVISION_BATCH_MAX_CONCURRENT` | `3` | | `REPROVISION_PREFLIGHT_SSHD_WAIT_SECONDS` | `60` | | `REPROVISION_QUARANTINE_ON_REGISTER_POLL_SECONDS` | `5` (this interval *is* the exposure window) | -| `REPROVISION_QUARANTINE_ON_REGISTER_MAX_WAIT_SECONDS` | `900` — ⚠️ sized for a watch started *after* bootstrap. Driving the watch by hand from group-add needs **5400**, or it expires before there is anything to quarantine | +| `REPROVISION_QUARANTINE_ON_REGISTER_MAX_WAIT_SECONDS` | `900` — sized for a watch started *after* bootstrap. Every path that starts the watch earlier now passes a bootstrap-spanning budget explicitly (`--max-wait-seconds`), so you should not need to export this | +| `REPROVISION_SIMPLEMDM_MAX_CONCURRENT` | `2` — fan-out cap for SimpleMDM-bound batch work, independent of `-j` | +| `REPROVISION_REFERENCE_GROUP_ID` | `2017918` — the production group `group-parity` measures against (read-only) | | `REPROVISION_VALIDATE_EXPECTED_REFRESH_HZ` | `60.0` — matches what mozharness itself enforces, so `validate` agrees with CI rather than inventing a second standard | | `REPROVISION_BOOTSTRAP_GROUP_ID` | `2417981` (`gecko-t-osx-1500-m4-bootstrap`). Production groups are separately blocked in `clients.simplemdm.PROTECTED_GROUP_IDS`, which this **cannot** override | | `REPROVISION_BOOTSTRAP_PKG_MAX_WAIT_SECONDS` | `300` — how long to wait for the PKG before calling it a group problem | diff --git a/orchestrator/orchestrator/batch.py b/orchestrator/orchestrator/batch.py index 083067b..fb1336d 100644 --- a/orchestrator/orchestrator/batch.py +++ b/orchestrator/orchestrator/batch.py @@ -45,7 +45,15 @@ EXIT_FAILED = 1 EXIT_NOT_READY = 2 -ACTIONS = ("preflight", "mint", "os-update", "add-to-group", "validate", "provision") +ACTIONS = ( + "preflight", + "mint", + "os-update", + "add-to-group", + "quarantine-on-register", + "validate", + "provision", +) @dataclass @@ -112,6 +120,7 @@ def _child_cmd( allow_sip_enabled: bool, wait: bool, quarantine_on_register: bool = False, + watch_max_wait_seconds: int = 0, ) -> list[str]: exe = _reprovision_exe() if action == "preflight": @@ -131,6 +140,15 @@ def _child_cmd( # 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 == "quarantine-on-register": + # Pure Taskcluster polling: no SSH, no SimpleMDM, no gate flags. The budget is passed + # explicitly because the default is sized for a watch that starts AFTER bootstrap; a watch + # started at group-add has to span the whole thing, and getting that wrong is not a + # harmless timeout — the host goes live UNHELD, the exact failure the watch prevents. + cmd = [exe, "quarantine-on-register", host] + if watch_max_wait_seconds: + cmd += ["--max-wait-seconds", str(watch_max_wait_seconds)] + return cmd 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 @@ -252,6 +270,141 @@ def _mmss(seconds: float) -> str: return f"{s // 60}:{s % 60:02d}" +def _write_roster(log_dir: Path, hosts: list[str]) -> Path: + """Record which hosts are (or may be) in the group, so the watch phase is re-runnable. + + A file rather than a printed list: the resume command is then a real command an operator can + paste at 6pm without re-deriving 49 hostnames from scrollback. + """ + roster = log_dir / "added.txt" + roster.write_text( + "# hosts added to the bootstrap group by this batch — they are bootstrapping NOW.\n" + "# Re-attach watchers: reprovision batch --action quarantine-on-register\n" + + "".join(f"{h}\n" for h in hosts) + ) + return roster + + +def _add_to_group_then_watch( + hosts: list[str], + *, + concurrency: int = 0, + expected_os: str = "", + allow_sip_enabled: bool = False, + per_host_timeout: int = 0, + dry_run: bool = False, +) -> int: + """Run the SimpleMDM add and the Taskcluster registration watch as two separate phases. + + `add-to-group --quarantine-on-register` couples an action bound by the SimpleMDM API (three + calls per host, no SSH to pace it) to a ~30-minute wait bound by Taskcluster. One + `--concurrency` cannot serve both: raise it for wall-clock and you hammer SimpleMDM, lower it + for SimpleMDM and 33 hosts serialise into five and a half hours. + + On 2026-08-14 at `-j12` this looked like 5 hosts failing. What actually happened: **12 hosts + had already been added to the group** — the POST succeeded and the follow-up `push_apps` + 429'd — so killing the batch orphaned 12 live, autonomous bootstraps with no watcher. They + would have registered and started claiming production work unvalidated. The runbook has + carried a two-command workaround for this ever since; this is that workaround, built in. + + Phase 1 is clamped to `simplemdm_max_concurrent`, independent of `--concurrency`. Phase 2 runs + every host at once: a watcher is an idle poll loop, so the binding resource is neither the API + nor the network but local process count. + """ + s = get_settings() + asked = concurrency or s.batch_max_concurrent + add_j = min(asked, s.simplemdm_max_concurrent) + watch_budget = s.bootstrap_max_wait_seconds + s.quarantine_on_register_max_wait_seconds + + ui.step("BATCH", f"add-to-group then watch × {len(hosts)} host(s), in two phases") + ui.info( + f"phase 1: add to the group, {add_j} at a time (SimpleMDM-bound) · " + f"phase 2: watch for registration, all {len(hosts)} at once (Taskcluster-bound)" + ) + if asked > add_j: + ui.warn( + f"-j {asked} ignored for the add phase: it is SimpleMDM-bound, clamped to {add_j}. " + "At -j12 the 429 retry budget blew and left 12 hosts added-but-unwatched " + "(2026-08-14). Raise REPROVISION_SIMPLEMDM_MAX_CONCURRENT if you really mean it." + ) + + # One directory for both phases, resolved before either starts: an interrupt during phase 1 + # still has to be able to write the resume roster somewhere findable. + root = None if dry_run else _log_dir() + + added: dict[str, HostResult] = {} + try: + add_failed = run_batch( + hosts, + action="add-to-group", + concurrency=add_j, + expected_os=expected_os, + allow_sip_enabled=allow_sip_enabled, + quarantine_on_register=False, + per_host_timeout=per_host_timeout, + dry_run=dry_run, + results_out=added, + log_dir=None if root is None else root / "add", + watch_follows=True, + ) + except KeyboardInterrupt: + _report_orphans(hosts, added, root) + raise + + # Watch every host that MIGHT now be in the group — "ok" and "failed" alike. A failed add is + # precisely the 2026-08-14 case: the add landed and the push 429'd, so the host is in the group + # and bootstrapping while the batch called it a failure. Only "skipped" (exit 2 — e.g. no SSH, + # so its serial was never read) means it was definitely never added. Over-watching wastes an + # idle poll loop; under-watching puts an unvalidated host into production. + watch = list(hosts) if dry_run else [ + h for h in hosts if h in added and added[h].state != "skipped" + ] + if not watch: + ui.warn("no host was added to the group — nothing to watch") + return add_failed + + if root is not None: + ui.info(f"roster: {_write_roster(root, watch)}") + + try: + watch_failed = run_batch( + watch, + action="quarantine-on-register", + concurrency=len(watch), + per_host_timeout=per_host_timeout or watch_budget + 300, + dry_run=dry_run, + watch_max_wait_seconds=watch_budget, + log_dir=None if root is None else root / "watch", + ) + except KeyboardInterrupt: + _report_orphans(watch, added, root) + raise + + return add_failed + watch_failed + + +def _report_orphans( + hosts: list[str], added: dict[str, HostResult], log_dir: Path | None +) -> None: + """On interrupt, say which hosts are bootstrapping unwatched — and how to re-attach. + + Silence here is what turned a Ctrl-C into 12 unheld production workers. + """ + maybe = [h for h in hosts if h not in added or added[h].state != "skipped"] + if not maybe: + return + ui.err( + f"INTERRUPTED with {len(maybe)} host(s) possibly already in the bootstrap group. The " + "bootstrap is autonomous: they will finish, register, and claim production work with " + "nothing holding them." + ) + if log_dir is None: + ui.err("re-attach watchers now: reprovision batch --action quarantine-on-register") + return + roster = _write_roster(log_dir, maybe) + ui.err(f"re-attach watchers now: reprovision batch {roster} --action quarantine-on-register") + + def run_batch( hosts: list[str], *, @@ -263,6 +416,10 @@ def run_batch( quarantine_on_register: bool = False, per_host_timeout: int = 0, dry_run: bool = False, + watch_max_wait_seconds: int = 0, + results_out: dict[str, HostResult] | None = None, + log_dir: Path | None = None, + watch_follows: bool = False, ) -> int: """Drive `action` across `hosts`. Returns the number of hosts that FAILED (not skipped). @@ -272,9 +429,26 @@ def run_batch( if action not in ACTIONS: raise ReprovisionError(f"unknown batch action {action!r} — one of {', '.join(ACTIONS)}") + # One --concurrency cannot serve both halves of this action; split it. See the function. + if action == "add-to-group" and quarantine_on_register: + return _add_to_group_then_watch( + hosts, + concurrency=concurrency, + expected_os=expected_os, + allow_sip_enabled=allow_sip_enabled, + per_host_timeout=per_host_timeout, + dry_run=dry_run, + ) + s = get_settings() concurrency = concurrency or s.batch_max_concurrent expected_os = expected_os or s.provision_expected_os + if action == "quarantine-on-register" and not watch_max_wait_seconds: + # The child's own default (900s) assumes the watch starts once bootstrap has finished. + # Reached through a batch it never does — the operator is attaching watchers to hosts that + # are mid-bootstrap — so size it for the whole thing here instead of making the runbook + # tell people to export REPROVISION_QUARANTINE_ON_REGISTER_MAX_WAIT_SECONDS=5400. + watch_max_wait_seconds = s.bootstrap_max_wait_seconds + s.quarantine_on_register_max_wait_seconds def _cmd_for(host: str) -> list[str]: return _child_cmd( host, @@ -283,6 +457,7 @@ def _cmd_for(host: str) -> list[str]: allow_sip_enabled=allow_sip_enabled, wait=wait, quarantine_on_register=quarantine_on_register, + watch_max_wait_seconds=watch_max_wait_seconds, ) # A provision blocks on the bootstrap sentinel, so give the child the sentinel budget plus @@ -297,6 +472,10 @@ 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 == "quarantine-on-register": + # Must outlast the child's own watch budget, or the batch kills the watcher and the + # host it was holding goes live unheld. + per_host_timeout = watch_max_wait_seconds + 300 elif action == "validate": per_host_timeout = 300 elif action == "add-to-group": @@ -318,6 +497,8 @@ def _cmd_for(host: str) -> list[str]: # passed, when mint is handed neither flag and checks neither thing. if action == "validate": gate_note = "read-only fitness check on already-bootstrapped hosts" + elif action == "quarantine-on-register": + gate_note = "Taskcluster-bound watch only — no SimpleMDM call, no SSH, no OS/SIP gate" 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": @@ -333,14 +514,23 @@ def _cmd_for(host: str) -> list[str]: 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") + if action == "quarantine-on-register": ui.info( - "watching for registration and quarantining on sight (blocks for the whole bootstrap)" - if quarantine_on_register - else "NOT quarantining: the bootstrap is autonomous, so these hosts will go live and " - "claim production work unvalidated — pass --quarantine-on-register for fresh hardware" + f"watch budget {_mmss(watch_max_wait_seconds)} per host — sized to span a whole " + "bootstrap, not just the registration gap" ) + if action == "add-to-group": + ui.info("ADD only, never a move; already-member hosts are skipped — then wait for the pkg to land") + if quarantine_on_register: + ui.info("watching for registration and quarantining on sight (blocks for the whole bootstrap)") + elif watch_follows: + # Don't cry wolf: the operator DID ask to hold these hosts, and phase 2 does it. + ui.info("watchers are attached in phase 2, immediately after this phase completes") + else: + ui.info( + "NOT quarantining: the bootstrap is autonomous, so these hosts will go live and " + "claim production work unvalidated — pass --quarantine-on-register for fresh hardware" + ) if action == "provision": ui.info( "hosts will be quarantined on registration" @@ -354,7 +544,10 @@ def _cmd_for(host: str) -> list[str]: ui.wire(" ".join(_cmd_for(host))) return 0 - log_dir = _log_dir() + # An explicit dir lets a multi-phase run keep every phase under one batch directory, so the + # operator has a single place to look and the resume roster sits beside the logs. + log_dir = log_dir or _log_dir() + log_dir.mkdir(parents=True, exist_ok=True) ui.info(f"logs: {log_dir}") # Resolve secrets once, here, before any child starts. See _prewarm_secret_env: N children @@ -363,7 +556,10 @@ def _cmd_for(host: str) -> list[str]: if secret_env: ui.info(f"pre-warmed {len(secret_env)} credential(s) — children won't call 1Password") - results: dict[str, HostResult] = {} + # The caller's dict when given one, populated as each host completes rather than at the end: + # _add_to_group_then_watch needs to know which hosts are already in the group even if the + # operator Ctrl-Cs mid-phase, because those hosts keep bootstrapping either way. + results: dict[str, HostResult] = results_out if results_out is not None else {} started = time.monotonic() done = 0 total = len(hosts) diff --git a/orchestrator/orchestrator/cli.py b/orchestrator/orchestrator/cli.py index 15cd545..8c4ca18 100644 --- a/orchestrator/orchestrator/cli.py +++ b/orchestrator/orchestrator/cli.py @@ -74,14 +74,28 @@ def provision( @_app.command() -def quarantine_on_register(hostname: str) -> None: +def quarantine_on_register( + hostname: str, + max_wait_seconds: int = typer.Option( + 0, + "--max-wait-seconds", + help="Watch budget. Default (900s) assumes bootstrap is already finished. Starting the " + "watch at group-add needs ~30 min of budget — pass it explicitly rather than exporting " + "REPROVISION_QUARANTINE_ON_REGISTER_MAX_WAIT_SECONDS.", + ), +) -> None: """Wait for a fresh worker to appear in Taskcluster, then quarantine it on sight. A worker that isn't registered yet can't be quarantined (`quarantineWorker` 404s), so this watches for it. Narrows the window between registration and the first claimed task to seconds — it does not eliminate it. Use standalone for hosts already mid-bootstrap. + + A budget that expires before the worker registers is not a harmless timeout: the host then + goes live UNHELD, which is the exact failure this command exists to prevent. """ - workflow.step_quarantine_on_register(workflow.resolve_offline(hostname)) + workflow.step_quarantine_on_register( + workflow.resolve_offline(hostname), max_wait_seconds=max_wait_seconds or None + ) @_app.command() @@ -122,7 +136,8 @@ def batch( action: str = typer.Option( "provision", "--action", - help="What to run per host: preflight | mint | os-update | add-to-group | validate | provision.", + help="What to run per host: preflight | mint | os-update | add-to-group | " + "quarantine-on-register | validate | provision.", ), concurrency: int = typer.Option( 0, "--concurrency", "-j", help="How many hosts in flight (default 3 — MDC1 throughput, not CPU)." @@ -240,6 +255,42 @@ def add_to_group( ) +@_app.command() +def group_parity( + group_id: int = typer.Option( + 0, "--group-id", help="Group to check (default: settings.bootstrap_group_id)." + ), + reference_group_id: int = typer.Option( + 0, "--reference-group-id", help="Group to measure against (default: settings.reference_group_id)." + ), + reference_sample: int = typer.Option( + 0, "--reference-sample", help="Reference devices to intersect for the baseline (default 5)." + ), + max_devices: int = typer.Option( + 0, "--max-devices", help="Check only the first N devices of the group (default: all)." + ), + host: str = typer.Option( + "", "--host", help="Check one host instead of the whole group (needs SSH, to read its serial)." + ), +) -> None: + """Do this group's hosts get the profiles a working production host gets? (read-only) + + Run this BEFORE a wave. A group that receives freshly-erased hosts must carry Skip Setup + Assistant and the FDA SSH Keygen Wrapper or every host hangs at the Wi-Fi pane — and that + failure presents as "Safari automation is broken", not as a missing profile (m4-214). + + Compares effective per-device profile sets, so it does not flag profiles that reach the hosts + by another additive path. Needs only the SimpleMDM API key; writes nothing. + """ + workflow.step_group_parity( + group_id=group_id or None, + reference_group_id=reference_group_id or None, + reference_sample=reference_sample or None, + max_devices=max_devices, + hostname=host or None, + ) + + @_app.command() def validate( hostname: str, diff --git a/orchestrator/orchestrator/clients/simplemdm.py b/orchestrator/orchestrator/clients/simplemdm.py index d44d9aa..ead5a1e 100644 --- a/orchestrator/orchestrator/clients/simplemdm.py +++ b/orchestrator/orchestrator/clients/simplemdm.py @@ -108,6 +108,56 @@ def assignment_group_device_ids(group_id: int) -> list[int]: return [int(d["id"]) for d in rel.get("devices", {}).get("data", [])] +def _paginated(path: str, *, limit: int = 100) -> list[dict]: + """Every page of a SimpleMDM list endpoint, following has_more/starting_after. + + Not optional for correctness: /custom_configuration_profiles returns has_more=True at the + default page size on this account, so a single unpaginated GET silently reports a partial + profile set — and a parity check built on a partial set reports missing profiles that are + simply on page two. + """ + out: list[dict] = [] + params: dict[str, object] = {"limit": limit} + while True: + page_json = _request("GET", path, params=params).json() + page = page_json.get("data", []) + out += page + if not page_json.get("has_more") or not page: + return out + params["starting_after"] = page[-1]["id"] + + +def device_profiles(device_id: int) -> dict[int, str]: + """Configuration profiles SimpleMDM considers assigned to this device, {id: name}. + + This is the *effective* set, and that is the whole point. Profiles reach a device by several + independent paths — assignment groups, device groups — and only this endpoint composes them. + Comparing assignment groups to each other instead is actively misleading: the bootstrap group + is NOT attached to "Skip Setup Assistant - All Screens" or the FDA "SSH Keygen Wrapper", yet + its devices hold both, because a DEP arrival is still in the additive DEP Enrollment group. + A group-level diff therefore flags exactly the two profiles from the m4-214 postmortem as + missing when they are in fact delivered — the one false alarm this check cannot afford. + + NB: assignment-group records carry no `profiles` relationship at all (verified 2026-08-19: + the keys are apps / device_groups / devices / media). The link lives on the profile side, as + `relationships.groups`. + """ + return { + int(p["id"]): p.get("attributes", {}).get("name", f"profile {p['id']}") + for p in _paginated(f"/devices/{device_id}/profiles") + } + + +def assignment_groups() -> list[dict]: + """Every assignment group, with its device membership. One paginated sweep. + + Cheaper and more useful than asking per device: a device record does not list the assignment + groups it belongs to, so the only way to answer "what groups is this host in?" is to invert + this. 47 groups on this account as of 2026-08-19. + """ + return _paginated("/assignment_groups") + + def add_device_to_assignment_group(group_id: int, device_id: int) -> None: """ADD a device to an assignment group. Purely additive — never moves or unassigns. diff --git a/orchestrator/orchestrator/config.py b/orchestrator/orchestrator/config.py index 8b84318..70434ce 100644 --- a/orchestrator/orchestrator/config.py +++ b/orchestrator/orchestrator/config.py @@ -101,6 +101,24 @@ class Settings(BaseSettings): # override. bootstrap_group_id: int = Field(default=2417981) + # The group `group-parity` measures against: a live production group whose devices are known + # to work. Read-only here — PROTECTED_GROUP_IDS blocks WRITES to 2017918, and reading it is + # precisely how we learn what a working host is supposed to have. + reference_group_id: int = Field(default=2017918) + # How many reference-group devices to sample for the baseline. The baseline is the + # INTERSECTION of their profile sets, so a handful is enough and more only costs API calls: + # one atypical prod host can't drag a profile into the baseline, and a profile every sampled + # host has is one the fleet genuinely standardises on. + group_parity_reference_sample: int = Field(default=5) + + # Fan-out cap for SimpleMDM-bound batch work, independent of --concurrency. `add-to-group` + # makes three API calls per host with no SSH to pace it, so the API is the binding resource, + # not MDC1 throughput. At -j12 on 2026-08-14 the 429 retry budget blew: 5 hosts reported + # failure while 12 had ALREADY been added to the group (the add succeeded, the follow-up + # push_apps 429'd), so killing the batch orphaned 12 live bootstraps with no watcher and they + # would have gone into production unvalidated. + simplemdm_max_concurrent: int = Field(default=2) + # `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 diff --git a/orchestrator/orchestrator/workflow.py b/orchestrator/orchestrator/workflow.py index ffd148c..d72cdf8 100644 --- a/orchestrator/orchestrator/workflow.py +++ b/orchestrator/orchestrator/workflow.py @@ -360,6 +360,230 @@ def step_add_to_group( ) +# Profiles whose absence has already cost real debugging time. Matched on a name SUBSTRING, not +# an id, so the warning survives a profile being rebuilt (which changes its id) or renamed around +# the stem. Anything missing from the baseline is reported; these get the story attached. +_LOAD_BEARING_PROFILES: tuple[tuple[str, str], ...] = ( + ( + "Skip Setup Assistant", + "a freshly-erased host then stops on the 'Select your Wi-Fi network' pane, which nothing " + "ever dismisses on an ethernet-only DC machine — and it presents as 'Safari automation is " + "broken', not as a stuck Setup Assistant (m4-214, cost most of a day)", + ), + ( + "SSH Keygen Wrapper", + "Full Disk Access for the SSH keygen wrapper — lost in the same m4-214 incident", + ), + ( + "CI Worker Support Binaries", + "the PPPC profile granting system-level TCC to the worker support binaries. Under SIP a " + "profile is the ONLY way those grants can land, so a SIP-on host without it diverges from " + "the prod fleet silently", + ), +) + + +# What share of a group's devices must belong to some OTHER group before a device that doesn't is +# treated as an outlier rather than as normal variation. Two thirds: high enough that a legitimate +# split (half a wave earmarked for a different role) isn't flagged, low enough to catch the single +# mis-clicked host among 40. +_MEMBERSHIP_QUORUM = 2 / 3 + + +def _membership_outliers( + gid: int, target_ids: list[int] +) -> dict[int, tuple[str, list[int]]]: + """Devices missing a group that almost all of their peers are in — i.e. moved, not added. + + Profile parity alone under-reports this. A device record does not list its assignment groups, + so this inverts the group->devices relationship once and asks the question peer-wise, with no + reference group involved: 39 of 40 hosts are in DEP Enrollment, so the 40th is the anomaly. + + Catches strictly more than the profile diff, because the groups a mis-clicked host loses are + not only profile-bearing. Found live on 2026-08-19: one bootstrap-group device was in that + group ALONE, having lost DEP Enrollment (Skip Setup Assistant, FDA), Relops Public SSH Key, + Sudoers and Enable SSH. Without the admin key the orchestrator cannot reach it at all — every + step is SSH — so the host is unprovisionable, and no profile-only check would say why. + """ + quorum = len(target_ids) * _MEMBERSHIP_QUORUM + out: dict[int, tuple[str, list[int]]] = {} + for group in simplemdm.assignment_groups(): + other = int(group["id"]) + if other == gid: + continue + ids = {int(d["id"]) for d in group.get("relationships", {}).get("devices", {}).get("data", [])} + if len([d for d in target_ids if d in ids]) < quorum: + continue + missing = [d for d in target_ids if d not in ids] + if missing: + out[other] = (group.get("attributes", {}).get("name", "?"), missing) + return out + + +def _device_label(device_id: int) -> str: + """id + serial + name. A bare device id is not identifying enough to act on. + + A fresh DEP arrival is named "Mac mini", so on 2026-08-19 a device id alone was mistaken for + macmini-m4-214 on nothing more than a matching enrollment date. The serial is what an operator + can actually search for in the SimpleMDM UI. + """ + try: + a = simplemdm.get_device(device_id).get("attributes", {}) + except ReprovisionError: + return f"device {device_id}" + name, serial = a.get("name") or "?", a.get("serial_number") or "?" + return f"device {device_id} (serial {serial}, named {name!r})" + + +def step_group_parity( + *, + group_id: int | None = None, + reference_group_id: int | None = None, + reference_sample: int | None = None, + max_devices: int = 0, + hostname: str | None = None, +) -> None: + """Do the hosts in the bootstrap group get the profiles a working prod host gets? + + Read-only, API-only: no SSH, no writes, safe on a live fleet. Nothing else in the toolchain + answers this, and it is the one question the m4-214 incident turned on — a host missing Skip + Setup Assistant and the FDA SSH Keygen Wrapper hung at first boot and presented as a Safari + fault. The postmortem's advice was to diff `profiles show -type configuration` against a + known-good host by hand, which needs SSH to a box that by definition may not be reachable yet. + This asks SimpleMDM instead, before a wave starts. + + The baseline is the INTERSECTION of the profile sets of several sampled reference-group + devices, not one sampled host: an intersection can't be skewed by a single atypical prod box, + and a profile that every sampled host carries is one the fleet genuinely standardises on. + + Deliberately compares EFFECTIVE per-device sets rather than the groups themselves — see + simplemdm.device_profiles. A group-level diff flags the two m4-214 profiles as missing from + the bootstrap group, which is true and irrelevant: its devices receive both from the additive + DEP Enrollment group. Crying wolf on that exact pair would train the operator to ignore this. + + Raises ReprovisionError (exit 1) when target devices lack baseline profiles. + """ + s = get_settings() + gid = group_id or s.bootstrap_group_id + ref_gid = reference_group_id or s.reference_group_id + n_sample = reference_sample or s.group_parity_reference_sample + + ui.step("GROUP PARITY", "do these hosts get the profiles a working prod host gets? (read-only)") + + if gid == ref_gid: + raise ReprovisionError( + f"group and reference group are both {gid} — nothing to compare. Pass " + "--reference-group-id to measure against a different group." + ) + + ref_name = simplemdm.get_assignment_group(ref_gid).get("attributes", {}).get("name", "?") + ref_devices = simplemdm.assignment_group_device_ids(ref_gid)[:n_sample] + if not ref_devices: + raise ReprovisionError( + f"reference group {ref_gid} ({ref_name}) has no devices — nothing to build a baseline " + "from. Point --reference-group-id at a populated production group." + ) + + baseline: dict[int, str] | None = None + for did in ref_devices: + profiles = simplemdm.device_profiles(did) + baseline = profiles if baseline is None else {i: n for i, n in baseline.items() if i in profiles} + assert baseline is not None + ui.info( + f"baseline: {len(baseline)} profile(s) common to {len(ref_devices)} device(s) " + f"in {ref_gid} ({ref_name})" + ) + if not baseline: + raise ReprovisionError( + f"the {len(ref_devices)} sampled devices in {ref_gid} share no profiles at all — that " + "group is too heterogeneous to be a baseline. Sample fewer, or pick another group." + ) + + # Targets: one named host, or the group's own membership. + if hostname: + device = _resolve_mdm_device(resolve_offline(hostname)) + targets = [(hostname, int(device["id"]))] + ui.info(f"checking {hostname} (device {targets[0][1]})") + else: + name = simplemdm.get_assignment_group(gid).get("attributes", {}).get("name", "?") + ids = simplemdm.assignment_group_device_ids(gid) + if not ids: + raise ReprovisionError(f"group {gid} ({name}) has no devices to check") + if max_devices: + ids = ids[:max_devices] + # A DEP arrival is named "Mac mini" in SimpleMDM, so device ids are the only stable label + # here. Not worth a GET per device to print a name they mostly don't have yet. + # Labelled lazily: naming 40 devices up front costs 40 GETs to print ids the operator + # mostly doesn't need. Only devices with a gap get resolved to a serial. + targets = [(f"device {i}", i) for i in ids] + ui.info(f"checking {len(targets)} device(s) in {gid} ({name})") + + # profile id -> (name, devices lacking it) + gaps: dict[int, tuple[str, list[str]]] = {} + for label, did in targets: + have = simplemdm.device_profiles(did) + for pid, pname in baseline.items(): + if pid not in have: + gaps.setdefault(pid, (pname, []))[1].append(label) + + total = len(targets) + if gaps: + ui.warn(f"{len(gaps)} profile(s) missing from at least one device") + else: + ui.ok(f"every checked device has all {len(baseline)} baseline profile(s)") + + # Second, independent question: is any device missing a GROUP its peers are all in? Catches + # the mis-clicked move, including the app-bearing groups a profile diff cannot see. + outliers = _membership_outliers(gid, [did for _label, did in targets]) if not hostname else {} + if outliers: + ui.warn(f"{len(outliers)} group(s) that most of these devices are in, some are not") + elif not hostname: + ui.ok("group membership is consistent across the group") + + if not gaps and not outliers: + ui.ok("parity: no gap against the prod fleet") + return + + sections: list[str] = [] + + if gaps: + lines = [] + for pid, (pname, lacking) in sorted(gaps.items(), key=lambda kv: -len(kv[1][1])): + why = next((story for stem, story in _LOAD_BEARING_PROFILES if stem in pname), "") + line = f"{pname} (profile {pid}) — missing on {len(lacking)}/{total} device(s)" + if len(lacking) <= 3: + line += "\n " + "\n ".join( + _device_label(d) for _l, d in targets if _l in lacking + ) + if why: + line += f"\n ^ {why}" + lines.append(line) + sections.append( + f"profile parity gap against {ref_gid} ({ref_name}):\n - " + "\n - ".join(lines) + ) + + if outliers: + lines = [] + for other, (oname, missing) in sorted(outliers.items(), key=lambda kv: -len(kv[1][1])): + lines.append( + f"{oname} ({other}) — {total - len(missing)}/{total} of these devices are in it, " + f"{len(missing)} are not:\n " + + "\n ".join(_device_label(d) for d in missing[:5]) + ) + sections.append( + "group membership outliers — these look MOVED rather than ADDED:\n - " + + "\n - ".join(lines) + ) + + raise ReprovisionError( + "\n\n".join(sections) + + "\n\nFix by attaching the profile to the group in SimpleMDM (a profile reaches devices " + "via its own `groups` relationship), or by ADDING the device to the groups it is missing. " + "Never MOVE it — a move strips the source group's profiles and apps, which is how this " + "state arises in the first place." + ) + + 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? diff --git a/orchestrator/tests/test_batch_phases.py b/orchestrator/tests/test_batch_phases.py new file mode 100644 index 0000000..7b39332 --- /dev/null +++ b/orchestrator/tests/test_batch_phases.py @@ -0,0 +1,184 @@ +"""`batch --action add-to-group --quarantine-on-register` must not be one `-j` serving two resources. + +The add is bound by the SimpleMDM API (three calls per host, no SSH to pace it); the watch is a +~30-minute wait bound by Taskcluster. Sharing one concurrency knob between them is what produced +the worst near-miss of wave 1: at `-j12` on 2026-08-14 five hosts *reported* failure while twelve +had already been added to the group, so killing the batch orphaned twelve autonomous bootstraps +with nothing watching them. They would have gone into production unvalidated. + +These tests pin the properties that stop that recurring, not the implementation: +concurrency is clamped per resource, a *failed* add is still watched, and an interrupt leaves a +resume roster on disk. +""" + +from __future__ import annotations + +import subprocess +import threading +from pathlib import Path +from unittest.mock import patch + +import pytest + +from orchestrator import batch +from orchestrator.config import get_settings + + +@pytest.fixture(autouse=True) +def _no_prewarm(): + with patch("orchestrator.batch._prewarm_secret_env", return_value={}): + yield + + +HOSTS = [f"macmini-m4-2{n:02d}" for n in range(40, 52)] # 12 hosts — the -j12 run + + +class _Recorder: + """Stands in for subprocess.run, recording every child invocation by phase.""" + + def __init__(self, codes: dict[tuple[str, str], int] | None = None): + self.codes = codes or {} + self.calls: list[dict] = [] + self.peak: dict[str, int] = {} + self._in_flight: dict[str, int] = {} + self._lock = threading.Lock() + + def __call__(self, cmd, stdout=None, stderr=None, timeout=None, check=False, env=None): + action = cmd[1] + host = next(c for c in cmd if c.startswith("macmini-")) + with self._lock: + self._in_flight[action] = self._in_flight.get(action, 0) + 1 + self.peak[action] = max(self.peak.get(action, 0), self._in_flight[action]) + self.calls.append({"action": action, "host": host, "cmd": cmd, "timeout": timeout}) + try: + threading.Event().wait(0.01) + finally: + with self._lock: + self._in_flight[action] -= 1 + return subprocess.CompletedProcess( + args=cmd, returncode=self.codes.get((action, host), 0), stdout=None, stderr=None + ) + + def hosts_for(self, action: str) -> list[str]: + return [c["host"] for c in self.calls if c["action"] == action] + + +def _run(rec: _Recorder, tmp_path: Path, hosts=None, **kw): + with patch("orchestrator.batch.subprocess.run", side_effect=rec), \ + patch("orchestrator.batch._log_dir", return_value=tmp_path), \ + patch("orchestrator.batch.ui.batch_summary"): + return batch.run_batch( + hosts or HOSTS, action="add-to-group", quarantine_on_register=True, **kw + ) + + +def test_the_coupled_invocation_runs_as_two_distinct_phases(tmp_path): + rec = _Recorder() + _run(rec, tmp_path) + # Sorted: both phases run concurrently, so completion order is not deterministic. + assert sorted(rec.hosts_for("add-to-group")) == sorted(HOSTS) + assert sorted(rec.hosts_for("quarantine-on-register")) == sorted(HOSTS) + # and the add child is no longer asked to do the watching itself + for call in rec.calls: + if call["action"] == "add-to-group": + assert "--quarantine-on-register" not in call["cmd"] + + +def test_the_add_phase_is_clamped_to_the_simplemdm_cap_whatever_j_says(tmp_path): + """-j is the knob operators reach for to speed up the watch. It must not reach SimpleMDM.""" + rec = _Recorder() + _run(rec, tmp_path, concurrency=12) + assert rec.peak["add-to-group"] <= get_settings().simplemdm_max_concurrent + + +def test_the_watch_phase_is_not_throttled_by_the_simplemdm_cap(tmp_path): + """A watcher is an idle poll loop; serialising 12 of them at -j2 costs hours for nothing.""" + rec = _Recorder() + _run(rec, tmp_path, concurrency=12) + assert rec.peak["quarantine-on-register"] > get_settings().simplemdm_max_concurrent + + +def test_a_host_whose_add_FAILED_is_still_watched(tmp_path): + """The 2026-08-14 case exactly: the POST landed, the follow-up push_apps 429'd. + + The host is in the group and bootstrapping regardless of what the batch called it, so not + watching it is how an unvalidated worker reaches production. + """ + rec = _Recorder(codes={("add-to-group", "macmini-m4-241"): 1}) + _run(rec, tmp_path) + assert "macmini-m4-241" in rec.hosts_for("quarantine-on-register") + + +def test_a_host_that_was_never_ready_is_not_watched(tmp_path): + """Exit 2 means the add never happened (no SSH, so its serial was never read).""" + rec = _Recorder(codes={("add-to-group", "macmini-m4-241"): batch.EXIT_NOT_READY}) + _run(rec, tmp_path) + assert "macmini-m4-241" not in rec.hosts_for("quarantine-on-register") + + +def test_the_watch_child_gets_a_bootstrap_spanning_budget_explicitly(tmp_path): + """The child's 900s default assumes bootstrap already finished; here it hasn't even started. + + Passed as an argument rather than left to REPROVISION_QUARANTINE_ON_REGISTER_MAX_WAIT_SECONDS, + because a budget that expires before the worker registers puts the host live UNHELD. + """ + rec = _Recorder() + _run(rec, tmp_path) + s = get_settings() + watch = next(c for c in rec.calls if c["action"] == "quarantine-on-register") + assert "--max-wait-seconds" in watch["cmd"] + budget = int(watch["cmd"][watch["cmd"].index("--max-wait-seconds") + 1]) + assert budget >= s.bootstrap_max_wait_seconds + # the subprocess timeout must outlast the child's own budget, or the batch kills the watcher + assert watch["timeout"] > budget + + +def test_an_interrupt_mid_add_leaves_a_resume_roster_on_disk(tmp_path): + """Silence on Ctrl-C is what turned an interrupt into twelve unheld production workers.""" + rec = _Recorder() + calls: list[str] = [] + + def _boom(cmd, **kw): + host = next(c for c in cmd if c.startswith("macmini-")) + calls.append(host) + if len(calls) > 2: + raise KeyboardInterrupt + return rec(cmd, **kw) + + errs: list[str] = [] + with patch("orchestrator.batch.subprocess.run", side_effect=_boom), \ + patch("orchestrator.batch._log_dir", return_value=tmp_path), \ + patch("orchestrator.batch.ui.batch_summary"), \ + patch("orchestrator.batch.ui.err", side_effect=errs.append), \ + pytest.raises(KeyboardInterrupt): + batch.run_batch(HOSTS, action="add-to-group", quarantine_on_register=True, concurrency=2) + + roster = tmp_path / "added.txt" + assert roster.exists(), "no resume roster written — the operator has no way back" + listed = [ln for ln in roster.read_text().splitlines() if not ln.startswith("#")] + assert listed, "roster names no hosts" + assert any("quarantine-on-register" in e for e in errs), "no resume command surfaced" + + +def test_each_phase_keeps_its_own_logs_under_one_batch_directory(tmp_path): + # Both phases run the same hostnames; sharing one flat directory would have phase 2 overwrite + # phase 1's log and lose the record of what the add actually did. + rec = _Recorder() + _run(rec, tmp_path, hosts=HOSTS[:2]) + assert (tmp_path / "add" / "macmini-m4-240.log").exists() + assert (tmp_path / "watch" / "macmini-m4-240.log").exists() + + +def test_dry_run_shows_both_phases_and_touches_nothing(tmp_path): + wires: list[str] = [] + with patch("orchestrator.batch.subprocess.run") as run, \ + patch("orchestrator.batch._log_dir") as log_dir, \ + patch("orchestrator.batch.ui.batch_summary"), \ + patch("orchestrator.batch.ui.wire", side_effect=wires.append): + assert batch.run_batch( + HOSTS[:2], action="add-to-group", quarantine_on_register=True, dry_run=True + ) == 0 + run.assert_not_called() + log_dir.assert_not_called() + joined = "\n".join(wires) + assert "add-to-group" in joined and "quarantine-on-register" in joined diff --git a/orchestrator/tests/test_group_parity.py b/orchestrator/tests/test_group_parity.py new file mode 100644 index 0000000..bb13ee1 --- /dev/null +++ b/orchestrator/tests/test_group_parity.py @@ -0,0 +1,270 @@ +"""`reprovision group-parity` — do a group's hosts get the profiles a working prod host gets? + +This is the check the m4-214 incident (2026-08-12) needed and nobody had. A host missing +"Skip Setup Assistant - All Screens" and the FDA "SSH Keygen Wrapper" came up on the Wi-Fi pane +at first boot, and because a modal Setup Assistant holds focus it presented as *Safari automation +is broken* — hours went into Safari versions, TCC grants and puppet ordering before the actual +cause surfaced. The postmortem's advice was to diff `profiles show -type configuration` against a +known-good host by hand, which needs SSH to a box that by definition may not be reachable. + +The load-bearing design decision, pinned below: compare EFFECTIVE per-device profile sets, never +assignment groups to each other. Verified against the live account on 2026-08-19 — the bootstrap +group is not attached to either m4-214 profile, yet its devices hold both, because a DEP arrival +is still in the additive DEP Enrollment group. A group-level diff reports exactly those two as +missing when they are in fact delivered, which would train an operator to ignore this check. +""" + +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from orchestrator import workflow +from orchestrator.errors import ReprovisionError + +BOOTSTRAP_GID = 2417981 +PROD_GID = 2017918 + +SKIP_SETUP = (197155, "System Settings - Skip Setup Assistant - All Screens") +FDA_KEYGEN = (197148, "System Settings - Full Disk Access - SSH Keygen Wrapper") +PPPC = (222765, "TCC - CI Worker Support Binaries (PPPC)") +COMMON = (161696, "System Settings - Desktop & Dock - Disable Click to Show Desktop") + + +_GROUP_NAMES = { + PROD_GID: "gecko-t-osx-1500-m4", + BOOTSTRAP_GID: "gecko-t-osx-1500-m4-bootstrap", +} + + +def _group(gid): + return {"id": gid, "attributes": {"name": _GROUP_NAMES.get(gid, f"group-{gid}")}} + + +def _assignment_group(gid, name, device_ids): + return { + "id": gid, + "attributes": {"name": name}, + "relationships": {"devices": {"data": [{"id": d} for d in device_ids]}}, + } + + +def _parity(*, ref_devices, target_devices, profiles, hostname=None, all_groups=None, **kw): + """Drive step_group_parity against a fake fleet. `profiles` maps device id -> {pid: name}. + + `all_groups` is the account-wide assignment-group list the membership pass inverts; default + empty, so profile-only tests stay about profiles. + """ + def _ids(gid): + return ref_devices if gid == PROD_GID else target_devices + + def _dev(did): + return {"id": did, "attributes": {"name": "Mac mini", "serial_number": f"SER{did}"}} + + with patch("orchestrator.workflow.simplemdm.get_assignment_group", side_effect=_group), \ + patch("orchestrator.workflow.simplemdm.assignment_group_device_ids", side_effect=_ids), \ + patch("orchestrator.workflow.simplemdm.assignment_groups", return_value=all_groups or []), \ + patch("orchestrator.workflow.simplemdm.get_device", side_effect=_dev), \ + patch("orchestrator.workflow.simplemdm.device_profiles", + side_effect=lambda did: dict(profiles[did])) as dp: + workflow.step_group_parity( + group_id=BOOTSTRAP_GID, reference_group_id=PROD_GID, hostname=hostname, **kw + ) + return dp + + +# --- the baseline --- + + +def test_the_baseline_is_the_intersection_not_one_sampled_host(): + """One atypical prod box must not be able to drag a profile into the baseline. + + A union would flag every target host for a profile that only one reference host happens to + carry; a single sample makes the whole check depend on which device the API listed first. + """ + profiles = { + 1: dict([COMMON, SKIP_SETUP]), # reference hosts... + 2: dict([COMMON]), # ...disagree about SKIP_SETUP + 9: dict([COMMON]), # target lacks it too — but it isn't in the baseline + } + _parity(ref_devices=[1, 2], target_devices=[9], profiles=profiles) # must not raise + + +def test_a_profile_every_reference_host_has_and_the_target_lacks_is_a_gap(): + profiles = { + 1: dict([COMMON, PPPC]), + 2: dict([COMMON, PPPC]), + 9: dict([COMMON]), + } + with pytest.raises(ReprovisionError, match="CI Worker Support Binaries"): + _parity(ref_devices=[1, 2], target_devices=[9], profiles=profiles) + + +def test_full_parity_passes_quietly(): + profiles = {1: dict([COMMON, PPPC]), 9: dict([COMMON, PPPC])} + _parity(ref_devices=[1], target_devices=[9], profiles=profiles) + + +# --- the false-alarm this check cannot afford --- + + +def test_a_profile_delivered_by_another_additive_path_is_NOT_flagged(): + """The bootstrap group carries neither m4-214 profile; its devices get both from DEP Enrollment. + + Because the comparison is per-device and effective, that is correctly a non-event. If this ever + starts failing, someone has reimplemented the check against assignment-group attachment and it + will cry wolf on the exact pair from the postmortem. + """ + profiles = { + 1: dict([COMMON, SKIP_SETUP, FDA_KEYGEN]), + 9: dict([COMMON, SKIP_SETUP, FDA_KEYGEN]), # via DEP Enrollment, not the bootstrap group + } + _parity(ref_devices=[1], target_devices=[9], profiles=profiles) + + +def test_it_reads_the_effective_set_of_every_target_device(): + profiles = {1: dict([COMMON]), 9: dict([COMMON]), 10: dict([COMMON]), 11: dict([COMMON])} + dp = _parity(ref_devices=[1], target_devices=[9, 10, 11], profiles=profiles) + assert {c.args[0] for c in dp.call_args_list} == {1, 9, 10, 11} + + +# --- the report --- + + +def test_a_gap_reports_how_many_devices_are_affected(): + profiles = { + 1: dict([COMMON, PPPC]), + 9: dict([COMMON]), + 10: dict([COMMON, PPPC]), + 11: dict([COMMON]), + } + with pytest.raises(ReprovisionError, match=r"missing on 2/3 device"): + _parity(ref_devices=[1], target_devices=[9, 10, 11], profiles=profiles) + + +@pytest.mark.parametrize( + "profile,expected", + [ + (SKIP_SETUP, "Wi-Fi network"), # the symptom, not the cause — that's how you meet it + (FDA_KEYGEN, "m4-214"), + (PPPC, "SIP"), + ], +) +def test_a_known_load_bearing_profile_carries_its_story(profile, expected): + """A bare profile name doesn't tell an operator why they should care, or what it'll look like.""" + profiles = {1: dict([COMMON, profile]), 9: dict([COMMON])} + with pytest.raises(ReprovisionError, match=expected): + _parity(ref_devices=[1], target_devices=[9], profiles=profiles) + + +def test_the_fix_it_suggests_never_says_move(): + """A move strips the source group's profiles — the m4-214 root cause. It must not be advice.""" + profiles = {1: dict([COMMON, PPPC]), 9: dict([COMMON])} + with pytest.raises(ReprovisionError) as e: + _parity(ref_devices=[1], target_devices=[9], profiles=profiles) + assert "ADDING" in str(e.value) + assert "Never MOVE" in str(e.value) + + +# --- guards --- + + +def test_max_devices_caps_the_work(): + profiles = {1: dict([COMMON]), 9: dict([COMMON]), 10: dict([COMMON])} + dp = _parity(ref_devices=[1], target_devices=[9, 10], profiles=profiles, max_devices=1) + assert {c.args[0] for c in dp.call_args_list} == {1, 9} + + +def test_comparing_a_group_against_itself_is_refused(): + with pytest.raises(ReprovisionError, match="nothing to compare"): + workflow.step_group_parity(group_id=PROD_GID, reference_group_id=PROD_GID) + + +def test_an_empty_reference_group_is_refused(): + with pytest.raises(ReprovisionError, match="no devices"): + _parity(ref_devices=[], target_devices=[9], profiles={9: dict([COMMON])}) + + +def test_a_reference_group_with_nothing_in_common_is_refused(): + """An intersection of zero isn't "parity achieved" — it means the baseline is meaningless.""" + profiles = {1: dict([COMMON]), 2: dict([PPPC]), 9: {}} + with pytest.raises(ReprovisionError, match="share no profiles"): + _parity(ref_devices=[1, 2], target_devices=[9], profiles=profiles) + + +def test_single_host_mode_resolves_the_device_by_serial(): + """A DEP arrival is named "Mac mini" in SimpleMDM, so the serial is the only join key.""" + profiles = {1: dict([COMMON, PPPC]), 555: dict([COMMON])} + with patch("orchestrator.workflow.ssh.platform_serial", return_value="W4LT930Y9Q"), \ + patch("orchestrator.workflow.simplemdm.find_device_by_serial", return_value={"id": 555}), \ + pytest.raises(ReprovisionError, match="CI Worker Support Binaries"): + _parity( + ref_devices=[1], target_devices=[], profiles=profiles, hostname="macmini-m4-241" + ) + + +# --- group membership outliers: the mis-clicked MOVE, which a profile diff under-reports --- + +DEP_GID = 2017921 +SSHKEY_GID = 1514391 + + +def _clean(pids=(COMMON,)): + return dict(pids) + + +def test_a_device_missing_a_group_all_its_peers_are_in_is_flagged(): + """Found live 2026-08-19: one device sat in the bootstrap group ALONE. + + It had lost DEP Enrollment, Relops Public SSH Key, Sudoers and Enable SSH — so it was missing + the admin key too, and every orchestrator step is SSH. A profile-only check reported two + missing profiles and could not say the host was unreachable by design. + """ + targets = [9, 10, 11, 12] + profiles = {1: _clean(), **{d: _clean() for d in targets}} + groups = [ + _assignment_group(DEP_GID, "DEP Enrollment", [9, 10, 11]), # 12 is missing + _assignment_group(SSHKEY_GID, "Relops Public SSH Key", [9, 10, 11]), + ] + with pytest.raises(ReprovisionError) as e: + _parity(ref_devices=[1], target_devices=targets, profiles=profiles, all_groups=groups) + msg = str(e.value) + assert "DEP Enrollment" in msg and "Relops Public SSH Key" in msg + assert "MOVED rather than ADDED" in msg + assert "SER12" in msg, "the outlier must be identified by serial, not just an id" + + +def test_a_group_only_a_minority_share_is_not_an_outlier(): + """Half a wave earmarked for a different role is normal variation, not a mis-click.""" + targets = [9, 10, 11, 12] + profiles = {1: _clean(), **{d: _clean() for d in targets}} + groups = [_assignment_group(4242, "tart-vm-hosts", [9, 10])] # 50% < quorum + _parity(ref_devices=[1], target_devices=targets, profiles=profiles, all_groups=groups) + + +def test_the_group_being_checked_is_not_reported_against_itself(): + targets = [9, 10, 11] + profiles = {1: _clean(), **{d: _clean() for d in targets}} + groups = [_assignment_group(BOOTSTRAP_GID, "gecko-t-osx-1500-m4-bootstrap", targets)] + _parity(ref_devices=[1], target_devices=targets, profiles=profiles, all_groups=groups) + + +def test_consistent_membership_and_full_profiles_passes(): + targets = [9, 10, 11] + profiles = {1: _clean(), **{d: _clean() for d in targets}} + groups = [_assignment_group(DEP_GID, "DEP Enrollment", targets)] + _parity(ref_devices=[1], target_devices=targets, profiles=profiles, all_groups=groups) + + +def test_single_host_mode_skips_the_membership_pass(): + """One host has no peers to be an outlier against; the comparison would be meaningless.""" + profiles = {1: _clean(), 555: _clean()} + with patch("orchestrator.workflow.ssh.platform_serial", return_value="W4LT930Y9Q"), \ + patch("orchestrator.workflow.simplemdm.find_device_by_serial", return_value={"id": 555}), \ + patch("orchestrator.workflow.simplemdm.assignment_groups") as ag: + _parity( + ref_devices=[1], target_devices=[], profiles=profiles, hostname="macmini-m4-241", + all_groups=[_assignment_group(DEP_GID, "DEP Enrollment", [1])], + ) + ag.assert_not_called()