Skip to content

kind: run the rate limiters on Redis by default, as deployments do - #480

Merged
ac-pplx merged 1 commit into
ac/kind-fidelity-postgresfrom
ac/kind-fidelity-redis
Oct 15, 2026
Merged

ac-pplx merged 1 commit into
ac/kind-fidelity-postgresfrom
ac/kind-fidelity-redis

Conversation

@ac-pplx

@ac-pplx ac-pplx commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Stack: Part of the Kind fidelity stack, bottom to top: #485 → #480 → #472 → #484 → #507. Based on #485 (ac/kind-fidelity-postgres).
This PR is one commit, and that commit is the only thing to review.

What Kind now does like deployments

Kind already deployed space-redis in both bring-up paths, but no chart set SPACE_REDIS_URL. So the cluster-activation admission limiter and the per-client concurrency limiter ran on their fail-open misconfigured arms in every run. The one exception was local.rate_limiters_enforce_and_fail_open, which patched the URL into deploy/space-api itself. On main, with no URL rendered, every activation and per-client decision outside that scenario is accepted|admitted / misconfigured. On this round's bench, no decision was misconfigured. Each was one of three kinds: none (made against the store), a refusal a scenario provokes on purpose, or store_unavailable while a drill or the scenario had Redis down.

With this change Kind renders SPACE_REDIS_URL=redis://space-redis:6379 wherever the code reads it, in both bring-up paths (Tilt and bootstrap.sh render the same charts):

Service What reads it
space-api, space-admin-api AppState::from_pools_with_lifecycle builds both limiters above the --surface branch: ClusterSandboxAdmissionConfig::from_env and ClientConcurrencyConfig::from_env.
space-cluster-controller The public-operation dispatcher (NativePublicOperationExecutor::new) builds its own activation limiter from ClusterSandboxAdmissionConfig::from_env. It shares the same bucket key, so the URL must match the API's.
space-internal-api Not rendered. Nothing space-internal-api-svc links reads SPACE_REDIS_URL (the readers live only in crates/libs/api). See the README row.

Limiter configuration matches deployments:

  • Cluster-activation limiter: binary defaults (enabled, burst 60, refill 60/s).
  • Per-client concurrency: observe, pinned explicitly in the chart, because deployments pin it rather than inherit it.
  • SPACE_API_CLIENT_CONCURRENCY_KNOWN_CLIENTS: stays empty. Observe needs no allowlist to classify traffic: an unrecognised or missing X-Client-ID goes to the shared unspecified bucket, which is metered like any other (ClientKey::classify).

Code paths this exercises in every Kind run:

  • the Redis token-bucket EVALSHA on every cold create, restore and wake, in both the API and the controller's async dispatcher;
  • the per-client ZSET acquire/release on every ECW create and connect, including observe-mode would_reject metering;
  • deadpool reconnects and Lua script reloads after a store restart.

Redis placement and health: the store stays on the infra node. Its readiness probe is now redis-cli ping instead of a TCP check. Tilt now starts space-api after space-redis; bootstrap.sh already waited for its rollout before the API.

Rate-limiter scenario

local.rate_limiters_enforce_and_fail_open no longer writes the URL. It now:

  • checks that the API has a store;
  • shrinks the public API's caps, keeping the SPACE_CLUSTER_ACTIVATION_ADMISSION_BURST test hook, since a default burst of 60 would need 60+ VM boots to drain;
  • switches per-client concurrency to enforce;
  • scales Redis to zero for its fail-open half;
  • adds a recovery phase: once Redis is back, a connect burst must draw a per-client 429 again.

The controller's copy of the activation caps stays at the chart defaults. The token-bucket script refills by each caller's own burst and rate, so a native activation during the drain would top the bucket up. None runs then: the scenario sends only ECW requests, which the API admits itself, and the suite runs one scenario at a time. The scenario's comment says so.

When a run that does not clean up is repaired, and by whom. The scenario's own teardown restores the env and scales Redis back; a restore that fails once is retried by the teardown. Beyond that:

  • Records. Before changing either, the run records the standing limiter env (JSON) and the Redis replica count as annotations on the public-API mutation Lease. A record is cleared only after its restore succeeded. A run whose restore keeps failing still releases the Lease, with the record left on it.
  • Who repairs. Whoever next acquires that Lease puts back what is recorded, before anything else. This happens on every acquisition: of a released Lease, an expired one, or one whose holder is proven dead (same user and host, process gone). The acquirers are the three scenarios that take the Lease, and the check below.
  • Before every scenario. space_smoke run reads that Lease before every scenario on the kind backend; a suite runs each scenario through it. If nothing is recorded, that one read is all it costs. If a fault is recorded, it acquires the Lease, repairs, prints REPAIRED: and runs the scenario. If a live holder has the fault up, it refuses to run (exit 2).
  • What is not covered. A killed run whose holder cannot be proven dead (another host or user) blocks the Lease until it expires (one hour). Scenarios refuse to run until then, rather than run against the fault.
  • Stale runners. Every write under the Lease (a restore, a Redis scale in either direction, setting or clearing a record) first renews the Lease in one patch guarded by uid, resourceVersion and holder. A runner that resumes after a successor took over its expired Lease fails at that patch, and leaves the successor's fault and records alone.

The generic Lease helper now runs a repair on every acquisition, which also closes the same gap for the worker-cordon Lease: a node a failed uncordon left cordoned is uncordoned by the next holder.

Also removed: kind_stack_ensure_redis, whose only caller script no longer exists.

This supersedes the opt-in direction of #36: Redis is now on by default. The rate-limiter scenario still needs a way to take the store away for its fail-open half, which #36's Lease-fenced store handling also addressed. Here it is the Lease-recorded scale-to-zero described above.

What stays different (README "Differences from the EKS deployment modules")

  • TLS to Redis: deployments use rediss://; Kind uses plaintext redis://. The workspace's Redis clients are built with tls-rustls-webpki-roots. With that feature, redis-rs builds its root store from webpki roots only and never loads native or custom roots, so a per-cluster CA would be refused without a code change. The #insecure URL fragment would be an environment-only switch. Kind also renders the URL as a literal, where the EKS module delivers it through the space-redis Secret.
  • SPACE_REDIS_URL on internal-api: deployments set it; Kind doesn't, because no code in that binary reads it.

infra/space/aws/reference/README.md said Kind sets the URL. That is true now, and the sentence names the services.

Validation

Commits

What Commit Base (main)
Live bench, drills and scenario runs 8936b98 0f1d3be (#486)
Rust artifacts on that bench, and PR Kind CI d06d56e, GitHub's merge of 8936b98 into 0f1d3be, same tree as 8936b98 0f1d3be
Final head f1e7708, the same commit rebased 9254f09 (#470)

The rebase onto 9254f09 was clean. The upstream commits in between (#470, #312, #165 and three docs commits) change no file this PR changes. They do touch other Kind and harness files (host-path checks, the template-build containerd check). On f1e7708 I reran the harness unit tests, and the PR's Kind CI ran the full typed suite on it (table below).

Bench

A fresh Kind cluster on one Linux/KVM devbox, with CI's settings:

  • SPACE_KIND_TELEMETRY_MODE=otel and DATA_PLANE_URL=http://127.0.0.1:8090;
  • the admin token file and cold-receipts directory created as in kind-ci.yml;
  • SPACE_KIND_RUST_ARTIFACTS=prebuilt, using the Rust artifact image that this PR's CI producer built for d06d56e. I loaded it with rust-artifact-handoff.py load, which checks the archive digest and the image ID. The bring-up ran from a checkout of d06d56e.

Before the suite, a health gate: Tilt quiet, no ReplicaSet younger than 180 s, every node Ready and published, then a 120 s settle.

Environment notes:

  • The devbox's shared home volume was full (other work). So the per-node containerd stores went on the Docker host's tmpfs (SPACE_KIND_DATA_ROOT). A Docker host overlay directory does not work there: kubeadm's control plane never started on it.
  • Six stores (7–8 GB each) do not fit the 31 GB tmpfs. So for this bench only, an uncommitted edit to the cluster template dropped the containerd mount for the control-plane node and one builder, and their stores lived in Kind's default /var volume. Nothing the scenarios exercise depends on where containerd keeps its images.
  • One bring-up attempt ran out of space and was torn down. I then pruned the build cache, dangling images and unused tagged images (31 GB freed) and rebuilt from scratch.

Provenance:

  • Every API, admin-API and controller pod reported version d06d56ee2da8.
  • The live pod specs of space-api, space-admin-api and space-cluster-controller carried SPACE_REDIS_URL=redis://space-redis:6379; space-internal-api carried none. The API ran per-client concurrency in observe.
  • The set of pod image IDs was the same before and after all runs.
Run Result
bash scripts/tests/smoke-kind-e2e.sh on the fresh cluster PASS, 265 s (build cache warm from the attempt that ran out of space)
local.rate_limiters_enforce_and_fail_open ×3 PASS ×3 (69, 70, 71 s; recovery bursts 1, 2, 2); after each, the bench's env, Redis and Lease were back at standing
local.node_group_boundary_schedules (takes the Lease) PASS, 46 s
local.node_selector_extras_render_and_enforce (takes the Lease) PASS, 71 s
ECW: ecw.command_round_trip, ecw.sandbox_info (the drill (d) suite), ecw.filesystem_round_trip, ecw.pause_connect PASS
ECW: local.telemetry_reaches_local_backends FAIL once (Loki held no log line for the sandbox yet), PASS on rerun; see below
native: ecw.command_round_trip, native_contract.token_scope, native.resident_pause PASS
PR Kind CI on d06d56e (4 shards, full typed suite) ECW 78/78, native 24/24; no REPAIRED: line in any shard
PR Kind CI on the final head f1e7708 (GitHub's merge onto 9254f09) ECW 78/78, native 24/24; no REPAIRED: line in any shard
uv run pytest space_smoke/tests -q 543 passed on 8936b98; 544 passed on f1e7708

Fault drills (disposable, against the live bench)

"Standing" below means: the public API's limiter env back to the chart's values (SPACE_API_CLIENT_CONCURRENCY_MODE=observe, the enable flag empty, the cap variables unset), Redis at 1/1, and the Lease released with no records. Commands:

  • space_smoke run <scenario> with CI's environment;
  • the suite in (d) is space_smoke.suite.run_scenarios, the runner behind run --all, over two named scenarios;
  • process groups come from setsid.
Drill What I did Outcome
(a) SIGKILL mid-fault Started the limiter scenario. Once it reached its fail-open phase with Redis at 0, kill -9 -- -<pgid>. Left behind: the smoke caps and enforce mode, Redis 0/0, and the Lease held by the dead pid with both records.
(d) suite after (a) Ran the suite over ecw.command_round_trip and ecw.sandbox_info. The first scenario's process printed REPAIRED:: it took the Lease from the provably dead holder and restored Redis and the env. Then both scenarios passed. Standing afterwards.
(b) restore fails once A kubectl wrapper on PATH failed the first set env that wrote SPACE_API_CLIENT_CONCURRENCY_MODE=observe. The scenario phase failed (infra error). Teardown retried the restore, which succeeded, and the bench was standing. A single failure never leaves a record.
(b') restore fails twice The same wrapper, failing that call twice. This run's preflight create timed out first (see below). Its teardown's restore then failed. It released the Lease with the env record still on it, and the bench stayed on the smoke caps. The next holder, another limiter run, printed REPAIRED:. It recorded the true standing env (observe, not enforce) as its baseline, and passed. Standing afterwards.
(c) stale runner after takeover Run A reached its fault and was stopped (SIGSTOP); I back-dated the Lease two hours. Run B then took the expired Lease, repaired A's records, and raised its own fault; I stopped B there. Then I resumed A (SIGCONT). A failed at its next write. It refused to scale Redis, restore the env, and release the Lease ("without the caller's live mutation Lease"; "owned by another caller"). B's env, Redis 0/0, holder and records were byte-identical before and after A ran. B, resumed, passed. Standing afterwards.
(e) live holder Ran ecw.sandbox_info while a limiter run held its fault. It refused to run (exit 2, "refusing to run while the public API mutation Lease records a limiter fault"). The limiter run then passed. Standing afterwards.

Failures seen, and their classification

  • Preflight timeout in drill (b'). The preflight create got no response within 90 s. The API pod it landed on had received SIGTERM 1.5 s earlier, at the end of the enforce rollout. wait_public_api_rollout and the /readyz wait return before the old pods have drained. This PR changes neither. It did not recur in the other limiter runs here or in CI. I classify it ENVIRONMENTAL (a slow bench under memory pressure from the tmpfs stores), with the race noted as a possible pre-existing harness weakness. It is not fixed here.
  • local.telemetry_reaches_local_backends. Loki had not yet ingested the sandbox's log line. It passed on rerun 40 s later; this PR changes nothing in its path. ENVIRONMENTAL.

Review follow-up

Second review: the infra/space/cluster installer reuses these charts but provisions no Redis, so it would have rendered the Kind endpoint with no store behind it. Fixed: its defaults keep redis.url empty for both charts, and an empty URL renders no SPACE_REDIS_URL, so the installer renders exactly as it did before this PR. values.example.yaml shows how to name the caller's store, and templates/validate.yaml refuses the Kind endpoint and API and controller URLs that differ. Kind's own renders are unchanged.

Restack onto #485: no conflicts; the commit's content is unchanged.

Earlier rounds:

  1. A record left on a released Lease was never repaired. Fixed: every acquisition repairs, including of a released Lease, and a record is cleared only after its restore succeeded. I kept "release with the record on it" rather than "refuse to release": holding the Lease would block all three Lease scenarios for its full hour, whereas any next acquirer, or the per-scenario check, repairs at once. Drills (b'), (a) and (d).
  2. Stale cleanup could overwrite a successor. Fixed: restores, Redis scaling in both directions, and record writes first renew the Lease in one patch guarded by uid, resourceVersion and holder. Drill (c).
  3. Killed-run repair was not a prerequisite for other scenarios. Fixed: space_smoke run checks the Lease before every kind-backend scenario. It repairs under acquired ownership, including from a provably dead holder, or refuses to run. Drills (d) and (e).
  4. New harness tests broke the test policy. Fixed: removed; the paths they covered were exercised by the live drills above.
  5. The controller shares the activation bucket. Documented in the scenario, not overridden: the scenario sends only ECW requests and the suite is serial, so no native activation can refill the bucket during the drain. Overriding the controller would add a second rollout and a second record to recover.
  6. Line-number citations in chart comments. Fixed: symbols only, here too.
  7. Evidence bases. Fixed: each run's commit and base are stated above.
  8. "Restores everything it changes even if killed" claimed too much. Fixed: the section above says when a killed or failed run is repaired and by whom, and the case it does not cover.

Rollback

Reverting removes SPACE_REDIS_URL from the Kind charts, and both limiters fail open as they did before. Redis holds only short-lived limiter keys, so there is nothing to clean up. The infra/space/cluster installer is unchanged either way: its defaults keep the URL empty.


This change and its validation were produced with an AI coding assistant (Claude Code) working unattended; the operator reviews before it leaves draft.

🤖 Generated with Claude Code

@ac-pplx
ac-pplx force-pushed the ac/kind-fidelity-redis branch 3 times, most recently from f1e7708 to 9a508e3 Compare October 14, 2026 23:35
@ac-pplx
ac-pplx changed the base branch from main to ac/kind-fidelity-postgres October 14, 2026 23:35
@ac-pplx
ac-pplx force-pushed the ac/kind-fidelity-redis branch from 9a508e3 to a629528 Compare October 15, 2026 00:42
@ac-pplx
ac-pplx force-pushed the ac/kind-fidelity-redis branch from a629528 to 409f142 Compare October 15, 2026 14:26
@ac-pplx
ac-pplx marked this pull request as ready for review October 15, 2026 16:54
@ac-pplx
ac-pplx added this pull request to stack #533 October 15, 2026 17:00

@nickdujay-pplx nickdujay-pplx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 409f142 against #485. The Kind wiring and limiter recovery flow look reasonable, and all four Kind CI shards passed. Requesting one small correction for the other consumer of these charts: the caller-provided-cluster installer now silently inherits a Redis endpoint it does not provision. Details inline.

I reproduced the before/after with the documented umbrella-chart example values: the parent renders no SPACE_REDIS_URL; this head renders redis://space-redis:6379 on API, admin API and cluster-controller, with no Redis Service. Please keep the Kind default while making the umbrella's Redis configuration intentional.

Kind deployed space-redis in both bring-up paths, but no chart set
SPACE_REDIS_URL, so the cluster activation limiter and the per-client
concurrency limiter ran on their fail-open (`misconfigured`) arms in every
run except the one rate-limiter scenario, which patched the URL into
space-api itself.

The charts now render SPACE_REDIS_URL=redis://space-redis:6379 where the
code reads it: space-api and space-admin-api (both limiters are built above
the --surface branch, in AppState::from_pools_with_lifecycle) and
space-cluster-controller (NativePublicOperationExecutor builds its own
activation limiter from ClusterSandboxAdmissionConfig::from_env). The
internal-api binary links no reader, so it gets nothing. The activation
limiter runs at the binary defaults and per-client concurrency runs in
`observe`, which deployments pin; observe needs no allowlist, since
unrecognised ids are metered in the shared `unspecified` bucket.

The infra/space/cluster installer reuses these charts but provisions no
Redis, so its defaults keep redis.url empty for both, and an empty URL now
renders no SPACE_REDIS_URL: an install that names no store renders as
before, with both limiters off. Its values.example.yaml shows how to name
the caller's store, and templates/validate.yaml refuses the Kind endpoint
and API and controller URLs that differ (they share the activation bucket).

Redis stays plaintext: the workspace's Redis clients are built with
`tls-rustls-webpki-roots`, so they trust only webpki roots and would refuse
a per-cluster CA, and the `#insecure` fragment would be an environment-only
switch. The README's Differences table records that and the internal-api
omission. Redis gets a PING readiness probe, and Tilt now starts space-api
after it (bootstrap.sh already waited for its rollout before the API).

local.rate_limiters_enforce_and_fail_open no longer writes the URL. It
checks the API has one, shrinks the public API's caps (keeping the burst
test hook, since the default burst of 60 would take 60+ VM boots to drain)
and switches per-client concurrency to enforce, scales Redis to zero for
the fail-open half, and adds a recovery phase: once Redis is back, a
connect burst draws a per-client 429 again. The controller's copy of the
activation caps is left at the defaults, and the scenario says why: it
sends only ECW requests, which the API admits itself.

The scenario's faults are recoverable when its own cleanup does not run or
fails:
- The public-API mutation Lease records the standing limiter env and the
  Redis replica count before either changes. A record is cleared only once
  its restore succeeded.
- Every acquisition of that Lease, including of a released one, puts back
  whatever is still recorded. A Lease with a repair can also be taken over
  from a holder proven dead (same user and host, pid gone), so a killed
  run does not block the next one for the Lease's hour.
- Every write under the Lease first renews it in one patch guarded by uid,
  resourceVersion and holder, which also sets or clears the records. A
  runner resumed after a successor took over the Lease fails there instead
  of undoing the successor's fault or erasing its records.
- Before every scenario on the kind backend, `space_smoke run` reads the
  Lease. If a fault is recorded, it acquires the Lease to repair it, or
  refuses to run while a live holder has it up.
The worker-cordon Lease's repair also runs on a released Lease now.

Also removes kind_stack_ensure_redis, whose only caller script is gone, and
corrects the reference README's sentence about how Kind sets the URL.

@nickdujay-pplx nickdujay-pplx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed ed4554e against the previously reviewed head. My Redis installer blocker is resolved: the shared cluster chart defaults omit SPACE_REDIS_URL on API, admin API, and controller, preserving operation without a Redis store. Callers can supply matching external URLs; mismatched URLs and the absent Kind store are rejected.

Validation: all 45 existing chart tests and 11 disposable render checks passed. The Kind API/controller default renders are semantically identical to the previously reviewed head. This follow-up is limited to Helm defaults, validation, and conditional environment rendering; I found no further correctness, deployment, or rollback blocker.

Pull CI is green. The four Kind smoke shards and Rust CodeQL are still running at review time; their completion remains part of the merge checks.

@ac-pplx
ac-pplx added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit 0681e84 Oct 15, 2026
29 checks passed
@ac-pplx
ac-pplx deleted the ac/kind-fidelity-redis branch October 15, 2026 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants