diff --git a/orchestrator/orchestrator/workflow.py b/orchestrator/orchestrator/workflow.py index 17d333f..769f773 100644 --- a/orchestrator/orchestrator/workflow.py +++ b/orchestrator/orchestrator/workflow.py @@ -44,9 +44,17 @@ class HostContext: ) -_PROD_POOL_BY_ROLE = { - "gecko_t_osx_1500_m4": "releng-hardware/gecko-t-osx-1500-m4", - "gecko_t_osx_1400_r8": "releng-hardware/gecko-t-osx-1400-r8", +# Every prod pool a role's workers can be registered in, primary LAST (it is the fallback +# for a host registered nowhere). A pool missing here is not a lookup miss but a safety +# hole: resolve() calls such a host unregistered, and the reprovision flow then skips +# quarantine + drain and asks the wrong pool whether it is busy -- which 404s as "idle". +# The macOS 26 minis run the 1500 role but register in gecko-t-osx-2600-m4. +_PROD_POOLS_BY_ROLE = { + "gecko_t_osx_1500_m4": ( + "releng-hardware/gecko-t-osx-2600-m4", + "releng-hardware/gecko-t-osx-1500-m4", + ), + "gecko_t_osx_1400_r8": ("releng-hardware/gecko-t-osx-1400-r8",), } @@ -58,10 +66,10 @@ def candidate_pools(role: str) -> list[str]: alone can't disambiguate — callers probe these in order against TC. Shared by resolve() and the fresh-host quarantine so the two can't drift onto different pool names. """ - base_pool = _PROD_POOL_BY_ROLE.get(role) - if not base_pool: + prod_pools = _PROD_POOLS_BY_ROLE.get(role) + if not prod_pools: raise ValueError(f"no worker pool mapping for role '{role}'") - return [f"{base_pool}-staging", base_pool] + return [f"{p}-staging" for p in prod_pools] + list(prod_pools) def resolve(hostname: str) -> HostContext: @@ -1176,6 +1184,29 @@ def step_wipe(ctx: HostContext) -> None: f"{ctx.hostname} is still running a task — NOT wiping. Quarantine + drain first " f"(`reprovision quarantine`, `reprovision drain`) or wait for the task to finish." ) + # An unregistered host was never quarantined or drained, and the busy check above asked + # a pool the worker isn't in -- a 404 there reads as idle. That is only safe if nothing is + # running a worker: a live generic-worker means it IS registered, in a pool this + # orchestrator doesn't know (how the macOS 26 minis would have been wiped mid-task). + if not ctx.registered: + ui.wire( + f"ssh admin@{ctx.hostname} pgrep generic-worker (unregistered — confirm no live worker)" + ) + cp = ssh.run( + ctx.fqdn, "/usr/bin/pgrep -f /usr/local/bin/generic-worker", check=False + ) + if cp.returncode != 1: + state = ( + "generic-worker is running on it" + if cp.returncode == 0 + else f"its worker state couldn't be checked (pgrep exit {cp.returncode})" + ) + raise ReprovisionError( + f"{ctx.hostname} is not registered in any known pool " + f"({', '.join(candidate_pools(ctx.role))}) but {state} — refusing to wipe. " + "It is probably registered in a pool missing from _PROD_POOLS_BY_ROLE; " + "add it, then retry." + ) ui.ok("no task in flight") # Record the current enrolled_at so wait_for_reenroll can detect a *fresh* enrollment # (status alone is unreliable: it stays "enrolled" until the erase actually executes). diff --git a/orchestrator/tests/test_mint.py b/orchestrator/tests/test_mint.py index 4bb6c45..9359e8b 100644 --- a/orchestrator/tests/test_mint.py +++ b/orchestrator/tests/test_mint.py @@ -263,6 +263,87 @@ def test_wipe_proceeds_with_escrowed_bst(): wipe.assert_called_once() +def _unregistered_ctx(): + ctx = _ctx() + ctx.registered = False + return ctx + + +def _ssh_with_pgrep(pgrep_rc: int): + """BST check reports escrowed; the unregistered-host pgrep returns `pgrep_rc`.""" + + def fake(_host, cmd, **_k): + cp = MagicMock() + if "pgrep" in cmd: + cp.returncode, cp.stdout = pgrep_rc, b"" + else: + cp.returncode, cp.stdout = ( + 0, + b"profiles: Bootstrap Token escrowed to server: YES", + ) + return cp + + return patch("orchestrator.workflow.ssh.run", side_effect=fake) + + +@pytest.mark.parametrize( + ("pgrep_rc", "why"), + [(0, "generic-worker is running"), (255, "couldn't be checked")], +) +def test_wipe_refuses_an_unregistered_host_with_a_live_worker(pgrep_rc, why): + """Unregistered means no quarantine/drain ran and the busy check asked a pool the worker + isn't in (404 = idle). A running generic-worker means it IS registered somewhere unknown; + so does being unable to tell. Either way: do not EACS.""" + with ( + patch("orchestrator.workflow.ssh.forget_host_key"), + _ssh_with_pgrep(pgrep_rc), + patch( + "orchestrator.workflow.taskcluster.is_currently_busy", return_value=False + ), + patch("orchestrator.workflow.simplemdm.wipe") as wipe, + ): + with pytest.raises(ReprovisionError, match=why): + workflow.step_wipe(_unregistered_ctx()) + wipe.assert_not_called() + + +def test_wipe_proceeds_for_an_unregistered_host_with_no_worker_running(): + """A genuinely fresh host (no generic-worker yet) must still be wipeable.""" + with ( + patch("orchestrator.workflow.ssh.forget_host_key"), + _ssh_with_pgrep(1), + patch( + "orchestrator.workflow.taskcluster.is_currently_busy", return_value=False + ), + patch( + "orchestrator.workflow.simplemdm.get_device", + return_value={"attributes": {"enrolled_at": "x"}}, + ), + patch("orchestrator.workflow.simplemdm.wipe") as wipe, + ): + workflow.step_wipe(_unregistered_ctx()) + wipe.assert_called_once() + + +def test_wipe_does_not_pgrep_a_registered_host(): + """Registered hosts were quarantined and drained; the extra check is for the unregistered path.""" + with ( + _ssh_with_pgrep(0) as run, + patch("orchestrator.workflow.ssh.forget_host_key"), + patch( + "orchestrator.workflow.taskcluster.is_currently_busy", return_value=False + ), + patch( + "orchestrator.workflow.simplemdm.get_device", + return_value={"attributes": {"enrolled_at": "x"}}, + ), + patch("orchestrator.workflow.simplemdm.wipe") as wipe, + ): + workflow.step_wipe(_ctx()) + wipe.assert_called_once() + assert not any("pgrep" in c.args[1] for c in run.call_args_list) + + # --- reprovision() sequence --- diff --git a/orchestrator/tests/test_provision.py b/orchestrator/tests/test_provision.py index d90bd41..9672531 100644 --- a/orchestrator/tests/test_provision.py +++ b/orchestrator/tests/test_provision.py @@ -353,7 +353,41 @@ def test_provision_no_wait_skips_the_pkg_gate_too(): def test_candidate_pools_probes_staging_before_prod(): # The role backs both; staging must be tried first or a staging worker gets quarantined in # the wrong pool (the 404 that PR #35 fixed). - assert workflow.candidate_pools("gecko_t_osx_1500_m4") == [f"{POOL}-staging", POOL] + pools = workflow.candidate_pools("gecko_t_osx_1500_m4") + assert pools.index(f"{POOL}-staging") < pools.index(POOL) + assert all(p.endswith("-staging") for p in pools[: len(pools) // 2]) + + +def test_candidate_pools_covers_the_macos26_pool_and_keeps_1500_as_fallback(): + """The macOS 26 minis run the 1500 role but register in gecko-t-osx-2600-m4. Missing it made + resolve() call them unregistered, which skips quarantine + drain before an EACS.""" + pools = workflow.candidate_pools("gecko_t_osx_1500_m4") + assert "releng-hardware/gecko-t-osx-2600-m4" in pools + assert ( + pools[-1] == POOL + ) # resolve() falls back to the last entry for an unregistered host + + +def test_resolve_finds_a_macos26_host_in_the_2600_pool(): + from taskcluster.exceptions import TaskclusterRestFailure + + def get_worker(pool, _group, _worker): + if pool == "releng-hardware/gecko-t-osx-2600-m4": + return {"workerId": "macmini-m4-130"} + e = TaskclusterRestFailure("not found", None) + e.status_code = 404 + raise e + + with ( + patch("orchestrator.clients.taskcluster.get_worker", side_effect=get_worker), + patch( + "orchestrator.workflow.simplemdm.find_device_by_name", + return_value={"id": 1}, + ), + ): + ctx = workflow.resolve("macmini-m4-130") + assert ctx.registered is True + assert ctx.worker_pool_id == "releng-hardware/gecko-t-osx-2600-m4" def test_candidate_pools_rejects_an_unmapped_role():