Skip to content

Commit b108180

Browse files
dshoen619claudeZivxx
authored
test(opal-server): git leak/resilience test environment (PR1) (#922)
* feat(opal-server): gated /internal git-fetcher cache stats endpoint Add an off-by-default diagnostics endpoint so tests can observe the in-memory GitPolicyFetcher cache sizes (repo_locks/repos/repos_last_fetched) and process RSS that the upcoming memory-leak fix eliminates. - debug_stats.py: read-only git_fetcher_cache_stats() helper + a register_internal_stats_route() registrar that mounts GET /internal/git-fetcher-cache-stats only when enabled. - config.py: new OPAL_DEBUG_INTERNAL_STATS flag, default False. - server.py: register the route, gated by the flag, beside /healthcheck. No production behavior change when the flag is off (the default). Also ignore .claude/ so private planning artifacts are never committed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): add OPAL git leak/resilience test bed A self-contained docker-compose stack (opal-server x2 workers + Redis + Postgres broadcaster + Gitea) plus a pytest harness that reproduces, as tests that fail on master, the git-fetcher memory leak, the offline-repo hang, the slow serial boot, and the broadcaster-disconnect gap. These become the regression gates for the follow-up fixes. - seed/: idempotent Gitea seeding sidecar (N policy repos) + Dockerfile. - docker-compose.yml: 4-service stack, opal-server built from the repo's own docker/Dockerfile (server target), scopes on, Postgres broadcaster. - helpers.py / conftest.py: HTTP + infra helpers and stack fixtures. - test_leak.py / test_resilience.py / test_boot.py: the flagship tests. - README.md: how to run and expected fail-on-master behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): add GiteaAdmin and make_repo_unreachable helpers Complete the helpers.py surface promised in the plan's file-structure table. Both are now functional and used, not dead code: - make_repo_unreachable(name): returns a git URL on a routable-but-dead TEST-NET-1 host (RFC 5737). test_offline_repo now uses it instead of an inlined literal. - GiteaAdmin: host-side Gitea admin client (list_repos / repo_exists / create_repo / delete_repo), exposed as the `gitea_admin` pytest fixture for tests that need to inspect or stage repos beyond the seed sidecar. Gitea is published on host port 13000 (uncommon, to avoid the usual :3000 clash) so GiteaAdmin can reach it; opal_server and the seed sidecar still use the internal http://gitea:3000. README updated with the helper and port notes. Verified live: GiteaAdmin lists the seeded repos and round-trips create/exists/delete against Gitea over the published port. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): correct postgres-bounce framing (passes on master) Verified test_server_recovers_after_postgres_bounce against the stack: it PASSES on master (~14-19s). On a broadcaster drop the affected worker triggers a graceful shutdown, gunicorn respawns it, and the sibling worker keeps serving HTTP, so the surface recovers within the window — recovery happens via gunicorn's in-container worker supervision, not an external supervisor and not an in-process reconnect. Reframe #5 as a recovery guard (not a known-broken case) in the docstring and README; the prior "FAILS on master / needs external supervisor" wording was wrong. PER-15065's in-process reconnect would avoid the worker churn but recovery already holds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(git-leak): apply black/isort/docformatter (pre-commit) Run the repo's pinned pre-commit formatters (black 23.1.0, isort 5.12.0, docformatter 1.7.5) over the PR1 files to satisfy the pre-commit CI check. Formatting only — no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: scope root pytest collection to packages/ (exclude git-leak bed) The CI `build` job runs `pytest` from the repo root with no path, which recursed into app-tests/git-leak/ and ran the flagship tests — these are designed to FAIL on master (they are the regression gates for PR2-PR5), so they broke the build job. Set `testpaths = packages` so the rootdir run collects only the unit tests under packages/ (matching master's effective behavior, since app-tests/ had no pytest files before). testpaths only applies when pytest is invoked from the rootdir with no args, so `cd app-tests/git-leak && pytest` still collects and runs the test bed. Verified both: root run -> packages only; subdir run -> all 5 flagship tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): address Copilot review feedback - stats(): sample the /internal endpoint several times and merge per key with max(). The caches are per-process on a multi-worker server, so a single read can hit an empty (non-leader) worker — max-merge avoids both false negatives (missing a populated leader) and false positives (an `== 0` drain assertion passing only because it hit an empty worker). - test_leak: assert the initial-load `_wait_until` succeeded before deleting / before taking baseline, so the tests can't pass vacuously when load never completed. - refresh_all(): correct the misleading comment — a 404 is a no-op, there is no client-side fallback. - conftest: skip the suite cleanly if docker is unavailable (defense in depth; it's already excluded from the default pytest run via testpaths). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): isolate scopes per test and fix false repeat-sync gate Two review findings on the regression gates: - Cross-test contamination: clone paths are keyed by repo URL (source_id = sha256(url)+branch-shard), not scope_id, so scopes from different tests that point at the same seeded repo share one GitPolicyFetcher cache entry. With a session-scoped stack and no teardown, a leftover boot-*/stable-* scope kept those entries alive and would make test_churn's `repos == 0` drain assertion fail on fixed code. OpalServerClient now tracks created scopes and the opal fixture deletes them on teardown (best-effort drain wait, swallows errors so master — where delete never purges — doesn't fail the passing test). - False gate: test_repeat_sync_does_not_grow re-syncs identical scopes, which a path-keyed cache can't grow even on master, so it could never be the leak gate it claimed. Reframed as an honest idempotency guard (passes on master) that points at test_churn_releases_caches as the real leak gate; README's "Expected on master" reclassifies it alongside the postgres-bounce guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): make the regression gates trustworthy (address PR review) Addresses the CHANGES_REQUESTED review on PR #922. Root cause behind most findings: a fresh scope's first sync takes the _clone() branch, which only fills GitPolicyFetcher.repo_locks; repos/repos_last_fetched are filled on a *second* sync. So the load gates on `repos` hung, and the 2-worker per-process caches made `== 0` drain assertions unsafe. Blocking: - Single worker (UVICORN_NUM_WORKERS=1): deterministic per-process cache reads; removes the false `== 0` drain class. - Load gate (CRITICAL + Zivxx HIGH): _load_scopes gates on repo_locks then refresh_all() to force the second sync, so repos/repos_last_fetched are actually populated before any drain/purge assertion. - compose() surfaces captured stdout/stderr on failure. - Seed completeness asserted in conftest; seed script isolates per-repo failures and exits non-zero with a count. Secondary: - test_repeat_sync asserts an RSS bound (count can't grow for any impl); churn asserts all three caches drain + a loose RSS backstop. - blackhole socat sidecar replaces TEST-NET-1 (deterministic hang); offline test saturates the fetch executor with 40 hung clones and recovers via OpalServerClient.hard_reset() (stop -> redis FLUSHALL -> start). - Per-test clean slate deletes all server scopes (fixes orphan-scope leak). - Postgres-bounce proves broadcast recovery (PUT post-bounce, assert sync) and uses `up -d --wait`. - Remove dead 404 branch in refresh_all; boot clock starts at restart; gitea-admin `|| true` -> "already exists"-only guard; README reworded. opal-server: - /internal stats route now takes the JWTAuthenticator dependency (protected when JWT on, no-op in the test bed); unit test asserts enforcement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(git-leak): apply black/isort/docformatter (pre-commit) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): tighten stat polling and pin test-bed images (PR review) - snapshot a single /internal stats read per poll in the churn-drain and boot gates (consistent multi-key observation; fewer HTTP round-trips) - document why a 200 from the healthy scope can't be a masked default bundle, and why the stats route is intentionally a sync def - pin alpine/socat and the seed image's pip deps for reproducibility Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): isolate offline-hang healthy probe to a never-cloned repo Ziv (PR review, round 2) caught that the offline-resilience gate can false-PASS in the full-suite run. The "healthy" scope pointed at policy-repo-0000 (list_seeded_repos(1)[0]), but on-disk clones are keyed by URL-hash and survive compose restart/stop/start (opal_server mounts no volume at /opal; only `down -v` wipes them). test_boot/test_leak run first (alphabetical) and already clone every seeded repo, so the healthy scope hit the existing clone via _discover_repository, skipped _clone(), and served 200 without ever touching the saturated fetch executor — the gate that must FAIL on this branch (no PR3 timeout) passed. Fix: seed a reserved repo (policy-repo-healthy-probe) outside the numeric policy-repo-NNNN range that no boot/leak test enumerates, and point the healthy probe at it. A never-cloned repo forces a genuine fresh clone through the starved pool, so the gate fails correctly. The seed- completeness check in conftest now covers the reserved repo too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): harden harness teardown and tighten assertions (PR review) Follow-up to PR review of the git-leak/resilience test bed: - hard_reset: always restart opal_server + wait_healthy via a finally, so a failed redis FLUSHALL can't leave the server stopped (which would fail every later session-scoped test and, running in a test finally, mask the result). - delete_all_scopes drain: a transient /internal read error no longer counts as a successful drain (was `except: return`); keep polling to the deadline so a not-yet-drained cache can't leak into the next test once PR2 lands. - use stats(samples=1) for the zero-waiting drain/empty polls (the peak-merge only matters for load assertions; this also drops 3x HTTP per poll). - resilience: narrow broad `except Exception` to requests.RequestException (and RuntimeError for wait_healthy timeout) so harness bugs surface instead of masquerading as "never served"/"never recovered". - resilience: collapse `assert opal.stats()` + redundant re-read into one read. - debug_stats_test: assert rss_kb > 0 on Linux (was `>= 0`, which passed for the wrong reason where /proc is absent and RSS reads fall back to 0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): make PR4/PR5 tests genuine gates (address PR review) Zeev's gate-coverage review found only 2 of 5 flagship tests (churn #1, offline #4) were genuine fail-now / pass-after gates. Lift the other three: - #5 broadcaster: run 2 workers (OPAL_TEST_WORKERS) so the Postgres backbone is actually fanned out cross-worker (references/debug-pubsub.md §3-4), and assert the gunicorn worker PIDs are unchanged across a transient bounce -- the in-place-reconnect signal that distinguishes #915 (PER-15065) from a plain worker respawn. Prove recovery via a servable post-bounce scope, not /internal cache counts (per-process, non-deterministic on 2 workers). - #3 boot: key completion on "all scopes served" (GET /scopes/{id}/policy == 200) instead of repo_locks (set at fetch start, so it undercounts the final clone); document the PR4 tight-BOOT_TARGET_SECONDS carry-forward. - #2 repeat-sync: rename to test_repeat_sync_rss_stays_bounded and drop the tautological len(repos) assertion; RSS is the sole (load-bearing) gate. Adds worker_pids() (/proc-based, matched host-side so the scan can't count its own sh -c wrapper) and the opal_multiworker fixture (recreate to 2 workers, restore to 1 on teardown). Validated live (--boot-scopes=50): #2/#3 pass, #5 passes (worker PIDs held across the bounce, post-bounce scope served), #1/#4 fail for the right reason (PR2/PR3 not landed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): address PR review round 3 (fixture robustness + cleanup) Zeev's latest inline batch: - pytest.ini: self-root the suite (app-tests/git-leak/pytest.ini) so `cd app-tests/git-leak && pytest --boot-scopes=N` is deterministic across pytest versions/cwd. (The documented command already works -- testpaths only applies when pytest runs from the rootdir -- but this makes it explicit.) - compose(): add a subprocess timeout (default 1200s) so a wedged up/wait/build fails fast instead of hanging session-scoped fixture setup to the CI job limit (pytest-timeout does not cover fixture setup). - delete_all_scopes(): cut the drain wait 20s -> 3s; on master the caches can't purge (the leak this gates), so the old wait burned ~40s of dead time per test across setup+teardown. - seed_gitea.py: inject push creds scheme-agnostically (urllib.parse) instead of string-replacing "http://"; drop the unused /seed-output token artifact and the seed-output volume (host uses basic auth, never the token). The order-dependent bounce signal (test_resilience.py) was already fixed in 8e24cb0 (asserts GET /scopes/post-bounce/policy == 200, not a delta on a shared process-global counter). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): harden gates per multi-agent review (H1 + M1-M6) Addresses the HIGH and all MEDIUM findings from the opal-development / python-pro / backend-architect microreview: - H1 (#5 vacuous pass): the broadcaster gate now positively verifies a disconnect+reconnect actually happened, not just that no respawn occurred. New helpers.broadcaster_connect_count() counts the reconnecting broadcaster's "listener connected to channel" log line; the test asserts it increased across the bounce (paired with worker PIDs unchanged = in-place, not respawn). - M1 (#4): move the 40 executor-saturating PUTs inside the try/finally so a PUT failure still runs hard_reset() instead of leaking hung clone threads into the session stack. - M2 (#1/#2): _wait_until now treats a transient requests error from opal.stats() as "not yet" and retries, instead of ERRORing the test. - M3 (#3): measure a deterministic pure-cold boot (--force-recreate wipes the ephemeral FS -> preload cold-clones all N from Redis) instead of a nondeterministic warm/cold mix, so PR4's tight BOOT_TARGET_SECONDS can gate. - M4: verify the single-worker invariant -- opal_multiworker teardown asserts the stack is back to 1 worker, and the opal fixture asserts single-worker at setup, so a botched restore fails loudly instead of silently breaking cache gates. - M5 (#4): correct the reserved-probe comment (serving shares the fetch executor, so a shared repo would be starved on serve too; the probe additionally exercises the clone). - M6: gitea-admin retries on "database is locked" (CLI mutating live SQLite); rewritten as a `|` literal block so the create call stays on one line. Validated live (--boot-scopes=20): #2/#3 pass, #5 passes (reconnect count increased across the bounce + PIDs unchanged + scope served), #1/#4 fail for the right reason. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(git-leak): add PR2 over-purge/update-path gates + during-outage publish guard (review items 1-4) Coverage additions from the test-coverage review pass: 1. Unit tests for the server.py wiring: build the real OpalServer app and assert /internal/git-fetcher-cache-stats is mounted iff DEBUG_INTERNAL_STATS is on — removing the register call from _init_fast_api_app now fails the unit suite. 2. test_shared_repo_survives_sibling_scope_delete: deleting one of two scopes sharing a repo URL must not purge the URL-keyed cache entry or break the survivor (guards PR2 against over-purging; churn only covers the all-scopes-gone direction). 3. test_scope_repoint_releases_old_repo_cache: PUT /scopes is create-or-update, so re-pointing a scope orphans the old URL's cache entries — a leak path churn (delete-only) never takes. Red until PR2's purge covers updates too. 4. Postgres-bounce test: bounce_postgres gained a during= callback (with a finally that restores Postgres even if it raises) and the test now PUTs a scope mid-outage, asserting (d) it becomes servable after recovery — its sync trigger must be buffered/replayed across the gap, not silently dropped. Uses a distinct seeded repo from (c) so a stale on-disk clone can't satisfy it vacuously. Validated live (--boot-scopes=2): #2 and the bounce test pass; #3 fails for the right reason (caches stuck at 2/2/2 after the delete — PR2 not landed). Unit suite: 52 passed; pre-commit clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: zivxx <zivxx1@gmail.com>
1 parent 9e241f5 commit b108180

17 files changed

Lines changed: 1706 additions & 0 deletions

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,3 +137,6 @@ dmypy.json
137137
*.iml
138138

139139
.DS_Store
140+
141+
# Private Claude Code working artifacts (plans/specs) — never commit
142+
.claude/

app-tests/git-leak/README.md

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# OPAL git-leak / resilience test bed
2+
3+
Reproduces (as failing tests) the four issues fixed by PR2–PR5: memory leak,
4+
offline-repo hang, slow serial boot, broadcaster no-reconnect.
5+
6+
Every assertion is driven through `GET /internal/git-fetcher-cache-stats`, which
7+
**this PR (PR1) adds** — it does not exist on `master`. So the suite runs against
8+
*this branch*: the leak/offline tests fail here *until PR2/PR3 land*, then go
9+
green. Run against true `master` they would all error at setup on the missing
10+
endpoint, not "fail for the targeted bug."
11+
12+
## Stack
13+
- `opal_server` (single worker, scopes on, Postgres broadcaster, built from `docker/Dockerfile`)
14+
- `redis`, `postgres`, `gitea` (+ one-shot `gitea-admin` and `seed` sidecars)
15+
- `blackhole` (alpine/socat: accepts TCP then never answers — the offline repo)
16+
17+
Only `opal_server` (`:7002`) and `gitea` (`:13000` on the host) are published;
18+
Postgres and `blackhole` are internal to the compose network.
19+
20+
## Helpers (`helpers.py`)
21+
- `OpalServerClient` — drive opal over HTTP (`stats`, `put_scope`, `delete_scope`,
22+
`refresh_all`, `get_scope_policy`, `list_scope_ids`, `delete_all_scopes`).
23+
- `GiteaAdmin` — host-side Gitea admin client (`list_repos`, `repo_exists`,
24+
`create_repo`, `delete_repo`); also exposed as the `gitea_admin` pytest fixture.
25+
- `make_repo_unreachable(name)` — git URL on the `blackhole` sidecar (completes
26+
the TCP handshake, never answers) so the clone hangs for the offline-repo test.
27+
- `bounce_postgres(down_seconds, during=None)` — stop Postgres, optionally run a
28+
callback while it is down (the bounce test publishes a scope mid-outage), then
29+
`up -d --wait` it back to simulate a broadcaster outage and await readiness
30+
before the recovery poll.
31+
32+
## Run
33+
```bash
34+
cd app-tests/git-leak
35+
python -m pytest -v --boot-scopes=50 # full set
36+
python -m pytest test_leak.py -v --boot-scopes=20 # just the leak gates
37+
```
38+
Useful flags: `--boot-scopes=N` (any N), `--keep-stack` (skip teardown),
39+
env `BOOT_TARGET_SECONDS=120` (tighten the boot gate).
40+
41+
## Expected behavior
42+
43+
Gate-coverage matrix (what each flagship test actually does):
44+
45+
| Test | Role | Behaviour here |
46+
|---|---|---|
47+
| `test_churn_releases_caches` | **gate (PR2)** | FAILS without the PR2 leak fix — delete leaves the caches populated; flips green when PR2 lands |
48+
| `test_scope_repoint_releases_old_repo_cache` | **gate (PR2, update path)** | FAILS without PR2 — re-pointing a scope to a new URL orphans the old URL's cache entries; stays red after PR2 unless its purge also covers scope *updates*, not just deletes |
49+
| `test_shared_repo_survives_sibling_scope_delete` | **over-purge guard (PR2)** | PASSES here (nothing purges on master); once PR2 lands it guards against purging a URL-keyed entry that a surviving sibling scope still references |
50+
| `test_offline_repo_does_not_block_healthy_scopes` | **gate (PR3)** | FAILS without the PR3 fetch timeout — 40 hung clones starve the executor so a healthy scope never serves; flips green when PR3 lands |
51+
| `test_boot_loads_all_scopes` | **baseline → gate (PR4)** | PASSES with the loose default target; set `BOOT_TARGET_SECONDS` low (plan: 120 @ 50) on PR4 to gate the parallel-boot fix |
52+
| `test_repeat_sync_rss_stays_bounded` | **RSS guard** | PASSES; an RSS-budget guard against per-sync allocation leaks (the cache *count* can't grow for any impl, so there is no count assertion — see below) |
53+
| `test_server_recovers_after_postgres_bounce` | **guard (PER-15065 + gap publishes)** | PASSES on this branch (which has #915); guards the in-place broadcaster reconnect and that a scope PUT *during* the outage is buffered/replayed, not dropped |
54+
55+
Notes on the guards:
56+
- `test_repeat_sync_rss_stays_bounded` — clone paths are keyed by the repo URL,
57+
so re-syncing identical scopes reuses cache entries and the cache *counts*
58+
can't grow for any implementation; the load-bearing assertion is therefore on
59+
RSS only (a `len(repos)` check would be tautological and is intentionally
60+
omitted), guarding against a regression that leaks per-sync allocations.
61+
- `test_server_recovers_after_postgres_bounce` — runs **2 workers** so the
62+
Postgres backbone is actually exercised (cross-worker fan-out needs >=2
63+
workers; a single worker fans out in-process and never touches the backbone).
64+
Across a transient bounce it asserts the gunicorn **worker PIDs are unchanged**
65+
— proving #915's reconnecting broadcaster recovered the reader *in place*
66+
rather than gunicorn respawning a graceful-shutdown worker (the pre-fix
67+
behaviour) — that a scope PUT after the bounce becomes servable, proving
68+
the broadcast/sync path recovered (not just HTTP), and that a scope PUT
69+
*during* the outage becomes servable too: its sync trigger rides the
70+
git-webhook topic, which the reconnecting broadcaster buffers and replays on
71+
reconnect (and which #933's publish freeze exempts on master), so a 201
72+
acknowledged mid-gap must never be silently dropped.
73+
- `test_shared_repo_survives_sibling_scope_delete` — the caches are keyed by
74+
repo URL, not scope id, so it green-guards PR2 against purging an entry that
75+
another live scope still references (churn only covers the all-scopes-gone
76+
direction).
77+
78+
## Requires
79+
Docker + docker compose v2, plus host Python with `pytest pytest-timeout requests GitPython`.
80+
81+
## Notes
82+
- Auth is disabled in the stack: `OPAL_AUTH_PUBLIC_KEY` is left unset so the JWT
83+
verifier is disabled and the harness can call scope routes without minting JWTs.
84+
Local test bed only; never a production setting. (The `/internal` endpoint is
85+
registered with the same `JWTAuthenticator` dependency as the other routes, so
86+
it is protected when JWT verification is enabled and open only here.)
87+
- The server runs a **single** uvicorn worker. The `GitPolicyFetcher` caches read
88+
by `/internal/git-fetcher-cache-stats` are per-process, so a multi-worker stack
89+
would make a round-robin read miss the worker that fetched and let a `== 0`
90+
drain assertion pass falsely. One worker makes every cache read deterministic;
91+
the leak/boot/offline bugs all reproduce single-worker.
92+
- First-sync of a fresh scope takes the clone path, which fills only `repo_locks`;
93+
`repos` / `repos_last_fetched` are filled by the discover/fetch path on a second
94+
sync, so the load helpers issue a `refresh_all()` before asserting on `repos`.

app-tests/git-leak/conftest.py

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
import os
2+
import shutil
3+
4+
import pytest
5+
from helpers import (
6+
HEALTHY_PROBE_REPO,
7+
GiteaAdmin,
8+
OpalServerClient,
9+
compose,
10+
list_seeded_repos,
11+
worker_pids,
12+
)
13+
14+
15+
def pytest_addoption(parser):
16+
parser.addoption(
17+
"--boot-scopes",
18+
action="store",
19+
default="50",
20+
help="number of repos to seed/boot (default 50)",
21+
)
22+
parser.addoption(
23+
"--keep-stack",
24+
action="store_true",
25+
default=False,
26+
help="do not tear the compose stack down after the run",
27+
)
28+
29+
30+
@pytest.fixture(scope="session")
31+
def repo_count(request) -> int:
32+
return int(request.config.getoption("--boot-scopes"))
33+
34+
35+
@pytest.fixture(scope="session")
36+
def stack(request, repo_count):
37+
# Defense-in-depth: this docker-compose suite is already excluded from the
38+
# repo's default `pytest` run via `testpaths = packages` in pytest.ini, so
39+
# the unit-test CI matrix never collects it. If it is ever collected in an
40+
# environment without docker, skip cleanly instead of erroring.
41+
if shutil.which("docker") is None:
42+
pytest.skip("docker (compose) is required for the git-leak test bed")
43+
os.environ["REPO_COUNT"] = str(repo_count)
44+
# build + start infra; seed runs to completion then exits
45+
compose("up", "-d", "--build")
46+
# block until seeding sidecar has finished creating repos. compose() raises
47+
# (with output) if the seed container exited non-zero, so a hard seed
48+
# failure surfaces here rather than as a confusing later test failure.
49+
compose("wait", "seed")
50+
# Verify the seed actually produced all N repos before any test runs: a
51+
# partial seed would otherwise look like a server bug when the load gate
52+
# can't reach N. Fail loudly with the gap.
53+
# include the reserved probe repo the resilience test relies on, so a
54+
# partial seed of it is caught here too rather than as a later test failure
55+
expected = set(list_seeded_repos(repo_count)) | {HEALTHY_PROBE_REPO}
56+
present = set(GiteaAdmin().list_repos())
57+
missing = expected - present
58+
assert not missing, (
59+
f"seed incomplete: {len(missing)}/{repo_count} repos missing "
60+
f"(e.g. {sorted(missing)[:5]})"
61+
)
62+
client = OpalServerClient()
63+
client.wait_healthy()
64+
yield client
65+
if not request.config.getoption("--keep-stack"):
66+
compose("down", "-v")
67+
68+
69+
@pytest.fixture()
70+
def opal(stack) -> OpalServerClient:
71+
# The compose stack is session-scoped (one server for the whole run), but
72+
# scopes must not leak between tests: clone paths are keyed by repo URL, so
73+
# a scope left behind by one test shares a cache entry with any later test
74+
# using the same seeded repo and would pollute its drain assertions.
75+
#
76+
# Delete every scope the *server* currently knows (not just this client's
77+
# tracked set) at setup, so a scope orphaned by a prior failed test can't
78+
# contaminate this one; then again on teardown.
79+
stack.delete_all_scopes()
80+
# Guard the single-worker invariant the cache gates depend on: if a prior
81+
# opal_multiworker teardown failed to restore 1 worker, the per-process cache
82+
# reads would be nondeterministic here (a `== 0` drain could false-pass).
83+
# Fail loudly and ordering-independently rather than silently mis-measure.
84+
assert (
85+
len(worker_pids()) == 1
86+
), f"expected a single-worker stack, found workers {sorted(worker_pids())}"
87+
yield stack
88+
stack.delete_all_scopes()
89+
90+
91+
@pytest.fixture()
92+
def opal_multiworker(stack) -> OpalServerClient:
93+
"""opal_server reconfigured to 2 gunicorn workers, for the broadcaster
94+
test.
95+
96+
The session stack is single-worker (the right call for the per-
97+
process cache drain assertions), but the Postgres broadcaster's
98+
cross-worker fan-out — the reason it is in this compose file at all
99+
— is only exercised with >=2 workers (references/debug-pubsub.md
100+
§3-4). This force-recreates opal_server with 2 workers for one test,
101+
then restores the single-worker stack on teardown so the cache tests
102+
keep their determinism. Each side starts from a clean slate: the
103+
recreate wipes the container's on-disk clones, and clearing scopes
104+
stops a leftover scope (whose clone is URL-keyed) from being re-
105+
cloned on boot.
106+
"""
107+
os.environ["OPAL_TEST_WORKERS"] = "2"
108+
try:
109+
# --no-deps: don't bounce redis/postgres/gitea; --force-recreate: apply
110+
# the new worker count. No --wait (opal_server has no compose
111+
# healthcheck) — wait_healthy() polls the HTTP surface instead.
112+
compose("up", "-d", "--no-deps", "--force-recreate", "opal_server")
113+
stack.wait_healthy()
114+
stack.delete_all_scopes()
115+
yield stack
116+
finally:
117+
os.environ["OPAL_TEST_WORKERS"] = "1"
118+
compose("up", "-d", "--no-deps", "--force-recreate", "opal_server")
119+
stack.wait_healthy()
120+
stack.delete_all_scopes()
121+
# Verify the restore actually reduced the stack back to one worker. If it
122+
# did not (a botched recreate), fail loudly here rather than leave a
123+
# 2-worker stack that would silently break later single-worker cache
124+
# gates' determinism.
125+
assert (
126+
len(worker_pids()) == 1
127+
), f"opal_multiworker teardown left workers {sorted(worker_pids())}, expected 1"
128+
129+
130+
@pytest.fixture(scope="session")
131+
def gitea_admin(stack) -> GiteaAdmin:
132+
"""Host-side Gitea admin client (depends on `stack` so Gitea is up)."""
133+
return GiteaAdmin()
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
name: opal-git-leak-test
2+
3+
services:
4+
redis:
5+
image: redis:7-alpine
6+
healthcheck:
7+
test: ["CMD", "redis-cli", "ping"]
8+
interval: 2s
9+
timeout: 3s
10+
retries: 30
11+
12+
postgres:
13+
image: postgres:16-alpine
14+
environment:
15+
POSTGRES_USER: opal
16+
POSTGRES_PASSWORD: opal
17+
POSTGRES_DB: opal
18+
# not published to the host: only opal_server reaches it over the compose
19+
# network, and bounce_postgres() uses `docker compose stop/start`. Publishing
20+
# 5432 would collide with any Postgres already running on the host.
21+
healthcheck:
22+
test: ["CMD-SHELL", "pg_isready -U opal"]
23+
interval: 2s
24+
timeout: 3s
25+
retries: 30
26+
27+
gitea:
28+
image: gitea/gitea:1.21
29+
environment:
30+
GITEA__security__INSTALL_LOCK: "true"
31+
GITEA__server__ROOT_URL: "http://gitea:3000/"
32+
GITEA__database__DB_TYPE: "sqlite3"
33+
# published on 13000 (not 3000) for the host-side GiteaAdmin helper; the
34+
# uncommon port avoids the usual :3000 clash. opal_server and the seed
35+
# sidecar still reach it over the compose network via http://gitea:3000.
36+
ports:
37+
- "13000:3000"
38+
volumes:
39+
- gitea-data:/data
40+
healthcheck:
41+
test: ["CMD-SHELL", "wget -qO- http://localhost:3000/api/v1/version || exit 1"]
42+
interval: 3s
43+
timeout: 5s
44+
retries: 40
45+
46+
gitea-admin:
47+
# creates the admin user once gitea is healthy
48+
image: gitea/gitea:1.21
49+
depends_on:
50+
gitea:
51+
condition: service_healthy
52+
user: git
53+
entrypoint: ["/bin/sh", "-c"]
54+
# Tolerate the idempotent "already exists" case, and RETRY on "database is
55+
# locked": this CLI mutates the same SQLite file the live gitea server holds
56+
# open, so it can lose a lock race and fail transiently. Any other failure
57+
# aborts so `seed` (which depends on this completing) doesn't run against a
58+
# Gitea with no admin user and fail with a confusing 401.
59+
# `|` (literal) block, not `>` (folded): the `gitea admin user create` call is
60+
# kept on ONE line so YAML can't fold a newline into the middle of its args
61+
# (that would run `--email ...` as its own command -> exit 127).
62+
command:
63+
- |
64+
for attempt in 1 2 3 4 5 6; do
65+
out=$$(gitea admin user create --username opaladmin --password opaladmin --email admin@example.com --admin --must-change-password=false --config /data/gitea/conf/app.ini 2>&1)
66+
rc=$$?
67+
echo "$$out"
68+
if [ $$rc -eq 0 ] || echo "$$out" | grep -qi "already exist"; then exit 0; fi
69+
if echo "$$out" | grep -qi "database is locked"; then echo "gitea db locked; retry $$attempt"; sleep 2; continue; fi
70+
exit $$rc
71+
done
72+
echo "gitea admin create failed after retries"
73+
exit 1
74+
volumes:
75+
- gitea-data:/data
76+
restart: "no"
77+
78+
blackhole:
79+
# Accepts the TCP handshake then never answers — a clone connects and
80+
# blocks reading the git smart-HTTP response, holding the fetch executor.
81+
# Deterministic, unlike a TEST-NET-1 address which many networks reject
82+
# fast with ICMP-unreachable (so the clone would fail fast, not hang).
83+
image: alpine/socat:1.8.0.3
84+
command: ["TCP-LISTEN:80,fork,reuseaddr", "SYSTEM:sleep 3600"]
85+
86+
seed:
87+
build: ./seed
88+
depends_on:
89+
gitea:
90+
condition: service_healthy
91+
gitea-admin:
92+
condition: service_completed_successfully
93+
environment:
94+
GITEA_URL: "http://gitea:3000"
95+
GITEA_ADMIN_USER: "opaladmin"
96+
GITEA_ADMIN_PASSWORD: "opaladmin"
97+
REPO_COUNT: "${REPO_COUNT:-50}"
98+
restart: "no"
99+
100+
opal_server:
101+
build:
102+
context: ../..
103+
dockerfile: docker/Dockerfile
104+
target: server
105+
environment:
106+
# Default single worker: the GitPolicyFetcher caches read by
107+
# /internal/git-fetcher-cache-stats are per-process, so with >1 worker a
108+
# round-robin read can miss the worker that fetched and make a `== 0`
109+
# drain assertion pass falsely. One worker makes every cache read
110+
# deterministic. The leak/boot/offline bugs all reproduce single-worker.
111+
# The postgres-bounce test (test_resilience.py) overrides this to 2 via
112+
# OPAL_TEST_WORKERS for its own container, because cross-worker fan-out
113+
# over the Postgres backbone only happens with >=2 workers
114+
# (references/debug-pubsub.md §3-4) — a single worker can't tell #915's
115+
# in-place broadcaster reconnect from a plain worker respawn.
116+
UVICORN_NUM_WORKERS: "${OPAL_TEST_WORKERS:-1}"
117+
OPAL_SCOPES: "1"
118+
OPAL_REDIS_URL: "redis://redis:6379"
119+
OPAL_BROADCAST_URI: "postgres://opal:opal@postgres:5432/opal"
120+
# Make the broadcaster reconnect fast + deterministic for the postgres-
121+
# bounce test (defaults are 30s max backoff / 2s settle). Harmless to the
122+
# single-worker tests, which don't exercise cross-worker fan-out.
123+
OPAL_BROADCAST_RECONNECT_BACKOFF_MAX_SECONDS: "2"
124+
OPAL_BROADCAST_RESYNC_SETTLE_SECONDS: "2"
125+
OPAL_BASE_DIR: "/opal"
126+
OPAL_POLICY_REFRESH_INTERVAL: "0"
127+
OPAL_DEBUG_INTERNAL_STATS: "1"
128+
# OPAL_AUTH_PUBLIC_KEY is intentionally left unset: with no public key the
129+
# JWT verifier is disabled, so the harness can call scope routes without
130+
# minting JWTs. Local test bed only; never a production setting.
131+
OPAL_LOG_FORMAT_INCLUDE_PID: "true"
132+
ports:
133+
- "7002:7002"
134+
depends_on:
135+
redis:
136+
condition: service_healthy
137+
postgres:
138+
condition: service_healthy
139+
140+
volumes:
141+
gitea-data:

0 commit comments

Comments
 (0)