pools: stop a stale worker_pool label counting as pool membership

[rcurranmoz]

The report

Pinning gecko-3-b-osx-arm64 to Monitored Pools shows 9 workers. The pool has 6:

Surface	Source	Applies the membership rule?	Reports
Pinned-pool card running/N	sync/load_sampler.py → PoolLoadSample.capacity	no	9
/fleet?worker_pool=…&view=table	api/workers.py list_workers	yes	6
Pool Health "Total"	api/fleet.py pool_health	yes	6

Live TC agrees with the table — 6 workers (m4-120, 123, 124, 193, 195, 196), all active within 15 minutes, none quarantined.

The three phantom workers

worker_pool is sticky by design: sync/puppet.py clears only puppet_role when a host leaves inventory.d ("worker_pool is set by several syncs"), and the TC/MDM/sheets syncs only ever backfill the label. So a host pulled from a pool keeps that pool's name forever.

ronin_puppet 290b3075 (2025-09-17, "m4 arm builder pool") deleted inventory.d/macmini-m2.yaml, which held exactly three gecko-3-b-osx-arm64 targets: macmini-m2-40, -41, -42. They're still SimpleMDM-managed in Defective / Spares, so prune_decommissioned correctly keeps their rows — with the dead label. 6 + 3 = 9.

This was fixed once already. bf141e8 (2026-07-15) names this very pool — "stale labels (e.g. Defective/Spares M2s tagged gecko-3-b-osx-arm64) inflated pool totals" — and added Worker.in_excluded_mdm_group plus the filter in pool_health and list_workers. It never touched load_sampler.py, which had shipped five weeks earlier (fa53ce2) and is the only worker count in the app that reads worker_pool with no filter at all.

The fix

One rule, one place: Worker.counts_toward_pool (excluded MDM group + retired pool), with models.exclude_non_pool_members() as its SQL form. Then:

• load_sampler — capacity and running. The reported bug. running could also be inflated by a stale RUNNING task state on an excluded host.
• fleet_summary.by_pool and fleet_showcase's per-pool rollup (feeds the migration axes, the over-provisioned / backed-up lists, the VM pool list). Both bucket a non-member as "unknown" rather than dropping it, so by_pool still sums to the fleet size and scale.workers stays the true fleet count. This follows the DECOMMISSIONED_POOLS precedent already in fleet_summary.
• pool_health and list_workers now call the rule instead of reimplementing it. Behaviour unchanged.

Deliberately not changed: consolidation_analysis keeps counting every host — it's the r8 retirement analysis, and surfacing defective/spare hosts under the pool they came from is the point. Commented in place.

Not affected: reprovision._pool_plan enumerates by tc_worker_pool_id (live TC registration), not the sticky label, so the destructive path could never be pointed at a phantom host.

Tests — the repo's first backend tests, plus a CI job

Nothing in CI executed backend/ before, which is why both misses of this rule went unnoticed. backend/tests/test_pool_membership.py (16 tests) covers the rule's truth table, the sampler's numbers, and agreement between the property and its SQL form.

Proven to fail on the pre-fix code — reverting just the counts_toward_pool guard in pool_worker_counts:

FAILED tests/test_pool_membership.py::test_sampler_capacity_ignores_stale_labels
FAILED tests/test_pool_membership.py::test_sampler_running_ignores_stale_labels
FAILED tests/test_pool_membership.py::test_sampler_reports_zero_not_missing_for_a_fully_excluded_pool
3 failed, 13 passed

assert capacity[POOL] == 6 fails with the real prod number, 9.

Dependency-light on purpose (pure functions + in-memory SQLite, no Postgres, no network), so the gate is fast and can't fail for environmental reasons. requirements-dev.txt includes the pinned requirements.txt verbatim rather than re-resolving requirements.in, so tests run against prod's exact versions; a fresh resolve drifted 8 packages to newer releases.

Verification

• pytest -q → 16 passed; 3 fail on the pre-fix code (above).
• ruff check per changed file is byte-identical in finding count to main — zero new lint findings; backend/tests/ is clean.
• requirements-dev.txt structurally validated for --require-hashes: 50 requirements, all ==-pinned, all hashed; the only additions over the runtime lock are pytest, iniconfig, pluggy, packaging, pygments.
• Not verified locally: the pip install --require-hashes step itself. No Docker daemon available this session, and a macOS pip --dry-run can't see the linux pins (it rejected greenlet==3.5.4, which is already in main's requirements.txt and installs in the image job every PR). The new backend (pytest) job is the real check.

Deploy note

PoolLoadSample rows are history and aren't rewritten, so the card keeps showing 9 until the next sampler tick after deploy, and its 24h sparkline stays inflated for about a day. Pool Health and the fleet table are correct immediately.

Follow-ups for a human

1. Add backend (pytest) to the branch ruleset's required checks — it's blocking-capable but not yet required, so today it can't stop a merge. Same gap #118 left for the terraform job.
2. The second commit untracks the 15 committed __pycache__/*.pyc files (already in .gitignore; they dirty the tree on any local python run, which a pytest gate makes routine).
3. The root cause is upstream of Hangar: three hosts that left a pool a year ago still carry its Worker_Config / pool label in SimpleMDM. Clearing it there would fix the data as well as the display.

🤖 Generated with Claude Code

[rcurranmoz]

Docker verification against real Postgres (pre-merge)

CI proved the unit tests pass. This closes the two things CI can't reach: the SQL half of the rule on Postgres rather than SQLite, and the actual PoolLoadSample row the pinned card reads.

Setup: postgres:16-alpine + the production image built from this branch (docker build --file=backend/Dockerfile, linux/amd64), seeded with the real host set — 6 live members plus macmini-m2-40/41/42 labelled gecko-3-b-osx-arm64 and in Defective / Spares. The app's startup then ran real syncs against live sources (puppet 421, windows-inventory 172, taskcluster 879), so the 6 m4 hosts carry genuine live TC state and 593 real fleet hosts are in the table alongside the fixture.

Control — origin/main, same seeded database

rows in DB labelled gecko-3-b-osx-arm64: 9
  load_sampler capacity      = 9   <-- what the pinned card renders
  /api/workers total         = 6
  /api/fleet/pools total     = 6
  /fleet/summary by_pool     = 9  (unknown bucket: None)

DISTINCT VALUES ACROSS SURFACES: [6, 9]  <-- the bug

Reproduces the report exactly: card 9, table 6.

This branch, over HTTP through the built image

GET /api/workers?worker_pool=gecko-3-b-osx-arm64
  total: 6
  hosts: macmini-m4-120, -123, -124, -193, -195, -196

GET /api/fleet/pools    → gecko-3-b-osx-arm64 total: 6 | production: 6
GET /api/fleet/summary  → gecko-3-b-osx-arm64: 6
GET /api/fleet/showcase → scale.workers: 596 (full fleet, unchanged)

The row the card actually reads, written by the real load_sampler.run_sync

load_sampler.run_sync sampled 56 pools

 pool                | capacity | running | pending | ts
 gecko-3-b-osx-arm64 |        6 |       6 |      29 | 2026-09-16 14:39:11

capacity = 6. Pre-fix this row was the 9.

The phantoms are still there — the 6 isn't a deletion artifact

 macmini-m2-40 | gecko-3-b-osx-arm64 | ["Defective / Spares"] | (no puppet_role)
 macmini-m2-41 | gecko-3-b-osx-arm64 | ["Defective / Spares"] | (no puppet_role)
 macmini-m2-42 | gecko-3-b-osx-arm64 | ["Defective / Spares"] | (no puppet_role)
 macmini-m4-120 | gecko-3-b-osx-arm64 | ["Mac Production"] | gecko_3_b_osx_arm64
 ... 5 more m4 ...
 count = 9

All nine rows present and labelled; the excluded three are simply no longer counted. prune_decommissioned correctly ran 0 removals (it refuses to act without a recent SimpleMDM sync, which this environment has no key for).

Also confirmed

• exclude_non_pool_members on Postgres keeps exactly the 6 the property keeps — the NOT LIKE '%' || group || '%' form behaves the same as under SQLite, so the unit test isn't SQLite-specific.
• The backend (pytest) CI step reproduces locally in python:3.11-slim linux/amd64: hash-pinned install of all 50 packages, 16 passed.
• The production image builds clean on this branch.