diff --git a/Tiltfile b/Tiltfile index 60df93b8ad..63adb85e42 100644 --- a/Tiltfile +++ b/Tiltfile @@ -513,10 +513,12 @@ k8s_resource( # API server. # Per-sandbox pods need the shim wired into containerd before the first create -# lands. +# lands, and the rate limiters' store should answer before the first create +# reaches them (they would admit it open, not fail it). SPACE_API_DEPS = ["space-postgres", "label-kind-workers", "label-kind-infra", "advertise-kind-worker-network-slots", "install-space-shim", - "internal-rpc-tls", "space-sandbox-network-policy", "space-db-migrate"] + "internal-rpc-tls", "space-sandbox-network-policy", "space-db-migrate", + "space-redis"] # No port_forward: localhost:8080 is the cluster's host port mapping onto the # space-api-entry NodePort (kind-cluster.yaml.tmpl), which spreads connections # across the autoscaled replicas as a deployment's load balancer does. @@ -644,9 +646,9 @@ k8s_resource( ) # Redis: shared store for the two API rate limiters (cluster activation -# admission + per-client concurrency). Public docker.io image pulled directly by -# the kind nodes (no `kind load`). Idle unless SPACE_REDIS_URL points at it; the -# rate-limiter smoke (scripts/tests/smoke-kind-rate-limiters.sh) wires it up. +# admission + per-client concurrency), which space-api, space-admin-api and +# space-cluster-controller reach through SPACE_REDIS_URL. Public docker.io image +# pulled directly by the kind nodes (no `kind load`). space-api waits for it. k8s_yaml(helm_release("space", "infra/space/kind/charts/space-redis-local")) k8s_resource( "space-redis", diff --git a/infra/space/aws/reference/README.md b/infra/space/aws/reference/README.md index d0c99cff97..e31d75bebe 100644 --- a/infra/space/aws/reference/README.md +++ b/infra/space/aws/reference/README.md @@ -158,8 +158,9 @@ retagged image. They run in the `space` namespace, and each writes a Secret a SPACE deployment can read. `space-postgres` holds `POSTGRES_USER`, `POSTGRES_PASSWORD`, `POSTGRES_DB` and `POSTGRES_CONNECTION_STRING`, the same Secret and keys SPACE's services read in the Kind harness. `space-valkey` -holds `SPACE_REDIS_URL`, the variable SPACE's API reads its rate-limiter store -from; Kind sets that variable directly rather than from a Secret. +holds `SPACE_REDIS_URL`, the variable SPACE's API and cluster-controller read +their rate-limiter store from; the Kind charts render that variable as a +literal on the API, admin API and cluster-controller rather than from a Secret. Terraform generates both passwords and keeps them in `addons/` state. Valkey holds SPACE's rate-limiter counters, a guardrail rather than a source diff --git a/infra/space/cluster/chart/templates/validate.yaml b/infra/space/cluster/chart/templates/validate.yaml index 26fe42e177..2c00506e64 100644 --- a/infra/space/cluster/chart/templates/validate.yaml +++ b/infra/space/cluster/chart/templates/validate.yaml @@ -120,6 +120,15 @@ rejects the Kind value itself. {{- if or (not $cc.snapshotEndpoint) (eq (toString $cc.snapshotEndpoint) "http://object-store:8333") -}} {{- $fail = append $fail "space-cluster-controller-local.snapshotEndpoint is required (the Kind object store is not installed)" -}} {{- end -}} +{{- /* The rate limiters' Redis is optional; this installer provides none. */ -}} +{{- $apiRedis := toString (($api.redis | default dict).url | default "") -}} +{{- $ccRedis := toString (($cc.redis | default dict).url | default "") -}} +{{- if or (contains "space-redis" $apiRedis) (contains "space-redis" $ccRedis) -}} +{{- $fail = append $fail "redis.url must not name the Kind store space-redis (this installer does not install Redis): leave it empty or name the caller's store" -}} +{{- end -}} +{{- if ne $apiRedis $ccRedis -}} +{{- $fail = append $fail "space-api-local.redis.url and space-cluster-controller-local.redis.url must be the same: both consume the cluster activation bucket" -}} +{{- end -}} {{- end -}} {{- /* Node services: the object store. */ -}} diff --git a/infra/space/cluster/chart/values.yaml b/infra/space/cluster/chart/values.yaml index 61412a2245..5732e265b9 100644 --- a/infra/space/cluster/chart/values.yaml +++ b/infra/space/cluster/chart/values.yaml @@ -28,6 +28,18 @@ space-api-local: entry: type: ClusterIP ecwHosts: "" + # This installer provisions no Redis, so the rate limiters' store is off + # unless the caller names one (values.example.yaml): an unset URL renders no + # SPACE_REDIS_URL and both limiters fail open, as they did before Kind turned + # its store on. templates/validate.yaml refuses the Kind endpoint and a + # URL that differs from space-cluster-controller-local's. + redis: + url: "" + +space-cluster-controller-local: + # Shares the API's cluster-activation bucket, so it names the same store. + redis: + url: "" space-data-plane-local: entry: diff --git a/infra/space/cluster/values.example.yaml b/infra/space/cluster/values.example.yaml index 96454f48c1..604fdcf9bd 100644 --- a/infra/space/cluster/values.example.yaml +++ b/infra/space/cluster/values.example.yaml @@ -50,6 +50,11 @@ space-api-local: dataPlanePublicUrl: "" entry: type: ClusterIP + # Optional: the rate limiters' Redis, which this installer does not provide. + # Leave it unset to run without rate limiting; set the same URL under + # space-cluster-controller-local, which shares the activation bucket. + # redis: + # url: rediss://redis.example.com:6379 space-internal-api-local: clusterId: *clusterId @@ -84,6 +89,8 @@ space-cluster-controller-local: snapshotRegion: us-east-1 snapshotBucket: snapshots snapshotCredentialsSecretName: space-object-store + # redis: + # url: rediss://redis.example.com:6379 # the same URL as space-api-local space-sandbox-network-policy: # Every private range sandboxes must not reach: pod, Service and node CIDRs diff --git a/infra/space/kind/README.md b/infra/space/kind/README.md index ec7cf04eb2..badd3d8dc6 100644 --- a/infra/space/kind/README.md +++ b/infra/space/kind/README.md @@ -995,6 +995,7 @@ kind cluster: space-local (kind-cluster.yaml.tmpl — 6 nodes) ├── worker3 [pool=infra, space.pplx.net/infra=true] │ ├── space-postgres (StatefulSet; PostgreSQL 17 serving TLS only, non-superuser master user) │ ├── space-object-store (versitygw S3 gateway + bucket-init Job) +│ ├── space-redis (Deployment; the rate limiters' store, which api, admin-api and cluster-controller reach through SPACE_REDIS_URL) │ ├── space-api (Deployment, 2–3 replicas by HPA; SPACE_K8S_CONTROL_PLANE=true) │ ├── space-admin-api (Deployment; the same binary on the dedicated admin surface) │ ├── space-internal-api (Deployment; ClusterApi — node registration + template releases) @@ -1402,6 +1403,8 @@ asks of the hosting deployment. | Server logs lock waits; the application roles' `lock_timeout` is off (`SPACE_DB_APP_LOCK_TIMEOUT` `0`) | `log_lock_waits = on`; `SPACE_DB_APP_LOCK_TIMEOUT` is unset, which leaves `lock_timeout` off the same way. Other server parameters are the image's defaults rather than a managed parameter group | | The migration Job fails fast: one attempt, no restarts, its deadline passed to the migrator | The same `space-db-migrate` Job shape (`backoffLimit: 0`, `restartPolicy: Never`, `activeDeadlineSeconds` = `SPACE_DB_MIGRATE_JOB_DEADLINE_SECONDS`), bootstrap-roles → migrate → bootstrap-roles. `bootstrap.sh` stops on a failed Job with its logs; rerunning it replaces the Job | | S3 for snapshots + template artifacts | In-cluster versitygw (real S3 protocol, sigv4, path-style) | +| Rate limiters (cluster activation admission at the binary defaults, per-client concurrency in `observe`) on a Redis-compatible store reached over TLS (`rediss://`), its URL delivered through the `space-redis` Secret (`app_deploy.tf`) | the same limiters, modes and defaults on the in-cluster `space-redis` Deployment (one in-memory replica on the infra node), reached over plaintext `redis://space-redis:6379` rendered as a literal: the workspace's Redis clients trust only the webpki roots (`tls-rustls-webpki-roots` in `Cargo.toml`), so they would refuse a CA `internal-rpc-tls.sh` could issue, and the `#insecure` URL fragment would be an environment-only switch. `local.rate_limiters_enforce_and_fail_open` shrinks the caps and scales the store to zero for its fail-open half | +| `SPACE_REDIS_URL` on api, admin-api, cluster-controller and internal-api | on api, admin-api and cluster-controller only: the limiters that read it live in `crates/libs/api` (api and admin-api build both, the cluster-controller's public-operation dispatcher builds the activation limiter), and nothing `space-internal-api-svc` links reads it | | `space-credmgr-authority` Deployment whose platform KEK comes from an external intermediate-key broker, with TLS from its own certificate (`credmgr_authority_tls.tf`; the direct KMS backend is gone, `credmgr_authority_kms.tf`) | two replicas on the same broker path against the in-repo broker fixture (`space-kind-key-broker`), which derives keys instead of protecting them; TLS from a self-signed certificate `internal-rpc-tls.sh` issues per cluster | | Internal RPC CA and listener leaves issued by Terraform and synced by External Secrets (`internal_rpc_tls.tf`) | the same Secrets issued per cluster by `scripts/internal-rpc-tls.sh`; the ingress activation leaf names `space-api` rather than `space-ingress-activation` | | volume-manager credentials via credmgr STS/IRSA | static `SPACE_OBJECT_STORE_STATIC_*` creds (the documented local escape hatch) | diff --git a/infra/space/kind/charts/space-api-local/templates/deployment.yaml b/infra/space/kind/charts/space-api-local/templates/deployment.yaml index 6989fb45ca..79a2af3f4b 100644 --- a/infra/space/kind/charts/space-api-local/templates/deployment.yaml +++ b/infra/space/kind/charts/space-api-local/templates/deployment.yaml @@ -197,6 +197,11 @@ spec: value: {{ .Values.clusterActivationAdmission.enabled | quote }} - name: SPACE_API_CLIENT_CONCURRENCY_MODE value: {{ .Values.clientConcurrency.mode | quote }} + {{- if .Values.redis.url }} + # Both limiters' shared store; see `redis` in values.yaml. + - name: SPACE_REDIS_URL + value: {{ .Values.redis.url | quote }} + {{- end }} {{- if .Values.eppo.secretName }} - name: EPPO_SDK_KEY valueFrom: diff --git a/infra/space/kind/charts/space-api-local/values.yaml b/infra/space/kind/charts/space-api-local/values.yaml index adb0eca720..41e7bb442e 100644 --- a/infra/space/kind/charts/space-api-local/values.yaml +++ b/infra/space/kind/charts/space-api-local/values.yaml @@ -170,10 +170,34 @@ dataPlanePublicUrl: "" # default the binary ships. Writing a value here would be a copy of a Rust # constant with nothing linking the two, and would silently override it the day # it changes. The client-concurrency mode accepts `off` as the hard off. +# +# The one exception is the client-concurrency mode, which deployments pin to +# `observe` rather than inherit: it is their configured value, not a copy of +# the binary's default, and it stays `observe` if that default ever moves. +# Observe needs no allowlist to classify traffic: an id outside +# SPACE_API_CLIENT_CONCURRENCY_KNOWN_CLIENTS, or none, lands in the shared +# `unspecified` bucket, which is metered like any other +# (`ClientKey::classify` in crates/libs/api/src/client_concurrency), so the +# allowlist stays at its empty default. clusterActivationAdmission: enabled: "" clientConcurrency: - mode: "" + mode: "observe" + +# The store both limiters keep their state in: the space-redis Deployment +# (charts/space-redis-local). Each limiter reads SPACE_REDIS_URL when the +# process builds its state, above the `--surface` branch +# (`AppState::from_pools_with_lifecycle`), so the admin API reads it too: +# `ClusterSandboxAdmissionConfig::from_env` and +# `ClientConcurrencyConfig::from_env`. Without it both fail open as +# `misconfigured`. Plaintext `redis://`, unlike a deployment's +# `rediss://`: the clients trust only the webpki roots (the +# `tls-rustls-webpki-roots` feature in the workspace Cargo.toml), so they +# would refuse a per-cluster CA. Must equal `redis.url` in the +# cluster-controller chart, whose copy of the activation limiter shares the +# bucket. +redis: + url: "redis://space-redis:6379" eppo: secretName: "" diff --git a/infra/space/kind/charts/space-cluster-controller-local/templates/deployment.yaml b/infra/space/kind/charts/space-cluster-controller-local/templates/deployment.yaml index 69b4eea8c7..de3c6e9281 100644 --- a/infra/space/kind/charts/space-cluster-controller-local/templates/deployment.yaml +++ b/infra/space/kind/charts/space-cluster-controller-local/templates/deployment.yaml @@ -180,6 +180,11 @@ spec: # `api.clusterActivationAdmission.enabled`. - name: SPACE_CLUSTER_ACTIVATION_ADMISSION_ENABLED value: {{ .Values.clusterActivationAdmission.enabled | quote }} + {{- if .Values.redis.url }} + # The limiter's shared store; see `redis` in values.yaml. + - name: SPACE_REDIS_URL + value: {{ .Values.redis.url | quote }} + {{- end }} {{- range $name, $reconciler := .Values.reconcilers }} {{- range $key, $value := (default (dict) $reconciler.env) }} {{- if ne $key "SPACE_SNAPSHOT_RETENTION_SWEEPER_MIN_AGE_SECS" }} diff --git a/infra/space/kind/charts/space-cluster-controller-local/values.yaml b/infra/space/kind/charts/space-cluster-controller-local/values.yaml index 0a33de2e0c..90f7a10d44 100644 --- a/infra/space/kind/charts/space-cluster-controller-local/values.yaml +++ b/infra/space/kind/charts/space-cluster-controller-local/values.yaml @@ -18,6 +18,16 @@ clusterId: "kind-local" clusterActivationAdmission: enabled: "" +# The activation limiter's store. The public-operation dispatcher +# (`NativePublicOperationExecutor::new`) builds its limiter from +# `ClusterSandboxAdmissionConfig::from_env`, which reads SPACE_REDIS_URL; +# without it this copy admits every queued activation as `misconfigured`. +# **Must equal `redis.url` in `charts/space-api-local/values.yaml`**: both +# copies consume from one bucket key, so a second store would double the +# cluster's rate. This process runs no per-client concurrency limiter. +redis: + url: "redis://space-redis:6379" + # Telemetry defaults come from the space-kind-telemetry library. telemetry: {} diff --git a/infra/space/kind/charts/space-redis-local/templates/deployment.yaml b/infra/space/kind/charts/space-redis-local/templates/deployment.yaml index 42c3cbc9de..9637247ec3 100644 --- a/infra/space/kind/charts/space-redis-local/templates/deployment.yaml +++ b/infra/space/kind/charts/space-redis-local/templates/deployment.yaml @@ -48,9 +48,12 @@ spec: - name: redis containerPort: {{ .Values.service.port }} protocol: TCP + # Ready once the server answers PING, not merely once its port + # accepts connections: the limiters fail open on any store error, so + # a ready endpoint should be one that serves their commands. readinessProbe: - tcpSocket: - port: redis + exec: + command: ["redis-cli", "-p", "{{ .Values.service.port }}", "ping"] initialDelaySeconds: 2 periodSeconds: 5 livenessProbe: diff --git a/infra/space/kind/charts/space-redis-local/values.yaml b/infra/space/kind/charts/space-redis-local/values.yaml index b05104a2f3..bf60e07813 100644 --- a/infra/space/kind/charts/space-redis-local/values.yaml +++ b/infra/space/kind/charts/space-redis-local/values.yaml @@ -1,7 +1,7 @@ # Redis for Kind's cluster activation and per-client concurrency limiters. -# Both use SPACE_REDIS_URL. This chart provides a local store for -# scripts/tests/smoke-kind-rate-limiters.sh; without a configured URL the -# limiters fail open and this pod idles. +# space-api, space-admin-api and space-cluster-controller reach it through +# SPACE_REDIS_URL (their charts' `redis.url`), as deployed services reach +# theirs, so every Kind run exercises both limiters against a real store. # # Single-node, in-memory only (no RDB/AOF persistence): the limiter state is a # guardrail, not a source of truth, and a wiped store just refills the buckets. @@ -14,7 +14,7 @@ image: datadogTeam: team-space service: - # DNS name the api-server reaches via SPACE_REDIS_URL=redis://space-redis:6379. + # DNS name in the API and cluster-controller charts' `redis.url`. name: space-redis port: 6379 diff --git a/infra/space/kind/scripts/bootstrap.sh b/infra/space/kind/scripts/bootstrap.sh index 571f8954ad..46974e82c3 100755 --- a/infra/space/kind/scripts/bootstrap.sh +++ b/infra/space/kind/scripts/bootstrap.sh @@ -374,8 +374,9 @@ log "waiting for object-store Deployment to be ready" kubectl --context "$KCTX" -n "$NAMESPACE" rollout status deploy/space-object-store --timeout="${TIMEOUT}s" # Redis — shared store for the API rate limiters (cluster activation admission -# + per-client concurrency). Idle unless SPACE_REDIS_URL points at it; the -# rate-limiter smoke (scripts/tests/smoke-kind-rate-limiters.sh) wires that up. +# + per-client concurrency), which space-api, space-admin-api and +# space-cluster-controller reach through SPACE_REDIS_URL. Ready before any of +# them starts, so the first create meets the limiters, not their fail-open arm. helm_apply space "$KIND_DIR/charts/space-redis-local" log "waiting for redis Deployment to be ready" kubectl --context "$KCTX" -n "$NAMESPACE" rollout status deploy/space-redis --timeout="${TIMEOUT}s" diff --git a/scripts/tests/lib/bootstrap-kind-stack.sh b/scripts/tests/lib/bootstrap-kind-stack.sh index 17be5b190d..2199aa612f 100755 --- a/scripts/tests/lib/bootstrap-kind-stack.sh +++ b/scripts/tests/lib/bootstrap-kind-stack.sh @@ -24,9 +24,8 @@ # consuming the timeout; a Tilt bound to a deleted cluster is restarted. # # Public entry points: `bootstrap_kind_stack`, `kind_stack_prepare_cluster`, plus -# `kind_stack_sweep_stale_node_tombstones`, `kind_stack_psql`, and -# `kind_stack_ensure_redis` for suites that have to repeat a bring-up step in -# their own setup or cleanup. +# `kind_stack_sweep_stale_node_tombstones` and `kind_stack_psql` for suites that +# have to repeat a bring-up step in their own setup or cleanup. # # Inputs (env vars, all optional with defaults set by the caller): # API_URL — health-check URL (e.g. http://localhost:8080) @@ -664,37 +663,6 @@ kind_stack_psql() { psql -U "$KIND_STACK_PG_USER" -d "$KIND_STACK_PG_DB" -tA -v ON_ERROR_STOP=1 "$@" } -# Redis, the store behind the two API rate limiters. Part of the harness -# (`bootstrap.sh` / the Tiltfile), so this renders the chart only when it is -# missing — which keeps the normal path from re-applying it while still letting a -# suite run against a stack that predates the chart. It always scales to 1 and -# waits, so it also recovers a prior run that died with Redis scaled to zero. -# -# Here rather than in each suite because it is bring-up, not assertion. Two -# suites had the same twelve lines when it was extracted; one of them -# (`smoke-kind-node-selector-extras.sh`) dropped its Redis setup later in the same -# change, once the token bucket turned out not to affect either of its phases — so -# `smoke-kind-rate-limiters.sh` is the only caller today. -kind_stack_ensure_redis() { - local deploy="${1:-space-redis}" - # Declared here rather than in the bring-up loop above, because a suite - # running with SKIP_BOOTSTRAP=1 reaches this function and never that loop. - if ! command -v helm >/dev/null 2>&1; then - echo "ERROR: helm is required to ensure the Redis chart is applied" >&2 - return 1 - fi - # Targeting flags last, per the repository's kubectl convention. None of - # these is an `exec`, where the flags would have to precede `--`. - local target=(-n "${NAMESPACE:-space}" --context "kind-${KIND_CLUSTER}") - if ! kubectl get "deploy/$deploy" "${target[@]}" >/dev/null 2>&1; then - helm template space "$REPO_ROOT/infra/space/kind/charts/space-redis-local" \ - --namespace "${NAMESPACE:-space}" \ - | kubectl apply -f - "${target[@]}" >/dev/null - fi - kubectl scale "deploy/$deploy" --replicas=1 "${target[@]}" >/dev/null 2>&1 || true - kubectl rollout status "deploy/$deploy" --timeout=120s "${target[@]}" >/dev/null -} - # Wait for `space-api` to report `/readyz=200`, up to a caller-chosen bound. # 0 when it did, 1 on timeout — the caller decides whether that is fatal. # diff --git a/space_smoke/__main__.py b/space_smoke/__main__.py index 14aff54f17..402a1461c5 100644 --- a/space_smoke/__main__.py +++ b/space_smoke/__main__.py @@ -13,7 +13,7 @@ from space_smoke.registry import Scenario from space_smoke.runtime import SmokeConfigurationError, execute_scenario from space_smoke.scenarios import REGISTRY -from space_smoke.services import MissingToolError, SmokeServices +from space_smoke.services import MissingToolError, SmokeServices, repair_orphaned_kind_faults from space_smoke.sharding import select_shard from space_smoke.suite import ( RESULT_PREFIX, @@ -91,6 +91,13 @@ def _run(scenario_id: str) -> int: except SmokeConfigurationError as error: print(f"ERROR: {error}", file=sys.stderr) return 2 + # Outside the scenario's budget: the repair may wait out two rollouts. + try: + if repair_orphaned_kind_faults(os.environ): + print("REPAIRED: a limiter fault an earlier run left recorded", flush=True) + except Exception as error: # noqa: BLE001 - a fault left up makes any verdict untrustworthy + print(f"ERROR: {type(error).__name__}: {error}", file=sys.stderr) + return 2 result = execute_scenario( title=scenario.title, body=lambda context: scenario.run(context, services), diff --git a/space_smoke/adapters.py b/space_smoke/adapters.py index e2b865ab69..e38563c370 100644 --- a/space_smoke/adapters.py +++ b/space_smoke/adapters.py @@ -3,6 +3,7 @@ from __future__ import annotations import base64 +import contextlib import hashlib import importlib import ipaddress @@ -217,6 +218,68 @@ def _lease_document(output: str) -> dict[str, object]: return document +def _lease_annotations(document: Mapping[str, object]) -> Mapping[str, object]: + metadata = document.get("metadata") + annotations = metadata.get("annotations") if isinstance(metadata, dict) else None + return annotations if isinstance(annotations, dict) else {} + + +def _require_limiter_env_keys(values: Mapping[str, object]) -> None: + from space_smoke.rate_limiters import LIMITER_ENV_KEYS + + unknown = sorted(set(values) - set(LIMITER_ENV_KEYS)) + if unknown: + raise ValueError(f"refusing non-limiter public API env keys: {unknown}") + + +def _recorded_limiter_env(recorded: str) -> dict[str, str | None]: + """The standing limiter env a Lease recorded, refused unless every entry is restorable.""" + + try: + standing = json.loads(recorded) + except json.JSONDecodeError as error: + raise SmokeConfigurationError("the recorded limiter env is not JSON") from error + if not isinstance(standing, dict) or not all( + value is None or (isinstance(value, str) and (not value or _LIMITER_ENV_VALUE.fullmatch(value))) + for value in standing.values() + ): + raise SmokeConfigurationError("the recorded limiter env holds a value a restore cannot write") + _require_limiter_env_keys(standing) + return standing + + +def _holder_process_gone(holder: object) -> bool: + """True only for a holder that was a process of this user on this host and has exited. + + A smoke killed mid-fault leaves its Lease live for the full duration; a + successor that can prove the holder dead need not wait that out. Any doubt + (another host or user, a malformed identity, a live or reused pid) is + treated as alive. + """ + + if not isinstance(holder, str): + return False + fields = dict(part.partition("=")[::2] for part in holder.split(";")) + pid = fields.get("pid", "") + if ( + fields.get("uid") != str(os.getuid()) + or fields.get("host") != socket.gethostname() + or not pid.isdigit() + ): + return False + try: + os.kill(int(pid), 0) + except ProcessLookupError: + return True + except OSError: + return False + return False + + +def _json_pointer_token(key: str) -> str: + return key.replace("~", "~0").replace("/", "~1") + + def _lease_parts( document: Mapping[str, object], ) -> tuple[dict[str, object], dict[str, object]]: @@ -329,9 +392,20 @@ def _sql_identifier_ish(value: str) -> str: BUILDER_ROUTING_LABEL = "space.pplx.net/routing-smoke" OCI_BUILD_CACHE_DIR = "/space/cache/firecracker/builds/oci" CONTAINERD_BUILDS_NAMESPACE = "space-builds" -# The worker-cordon Lease records which node its holder cordoned, so a takeover -# of an expired Lease can uncordon it. Nothing in SPACE itself ever uncordons. +# The worker-cordon Lease records which node its holder cordoned, so the next +# holder can uncordon it if that holder never did. Nothing in SPACE itself ever +# uncordons. CORDONED_NODE_ANNOTATION = "space.pplx.net/smoke-cordoned-node" +# The public-API mutation Lease records what the rate-limiter scenario is about +# to change before it changes it: the limiter env the public API Deployment +# held (JSON, null for unset) and the Redis replica count it scales down from. +# A record is cleared only once its restore succeeded. The next holder of the +# Lease puts back whatever is still recorded, and every Kind scenario run first +# makes sure nothing is (`repair_orphaned_limiter_faults`), so a run killed or +# failing mid-fault does not leave later scenarios on smoke-sized caps or +# without the limiters' store. +LIMITER_ENV_ANNOTATION = "space.pplx.net/smoke-limiter-env" +REDIS_REPLICAS_ANNOTATION = "space.pplx.net/smoke-redis-replicas" DRAIN_KEEP_ANNOTATION = "space.pplx.net/drain-keep" NODE_GROUP_BOUNDARY_LABEL = "node-restriction.kubernetes.io/space-node-group" NODE_GROUP_GATE_ENV = "SPACE_SANDBOX_NODE_GROUP_PLACEMENT" @@ -668,6 +742,7 @@ def __init__( self.environment = dict(environment or os.environ) self._retention_lease_ownership: _LeaseOwnership | None = None self._cordon_lease_ownership: _LeaseOwnership | None = None + self._public_api_lease_ownership: _LeaseOwnership | None = None # The public API autoscaler's bounds before `scale_public_api` pinned # them: (name, uid, minReplicas, maxReplicas). self._public_api_autoscaler_bounds: tuple[str, str, int, int] | None = None @@ -1058,19 +1133,27 @@ def _acquire_named_mutation_lease( name: str, subject: str, *, - on_expired_takeover: Callable[[Mapping[str, object], _LeaseOwnership], None] - | None = None, + repair: Callable[[Mapping[str, object], _LeaseOwnership], None] | None = None, ) -> _LeaseOwnership: """Create or take over `name`; refuse while another live holder exists. - `on_expired_takeover` runs with the dead holder's document and the new - ownership only AFTER the resourceVersion-guarded patch has succeeded. - Two contenders can both read the same expired Lease; running repairs - before the patch would let the loser undo a fault the winner has since - started. The callback must fence its own writes on `ownership`. + `repair` runs with the previous document and the new ownership on every + acquisition of an existing Lease, released or expired, and only AFTER + the resourceVersion-guarded patch has succeeded. A holder whose cleanup + failed releases with its records still on the Lease, so a released + Lease is no proof that nothing is left to undo. Two contenders can both + read the same Lease; running repairs before the patch would let the + loser undo a fault the winner has since started. The callback must + fence its own writes on `ownership`. A Lease with a repair may also be + taken over from a holder `_holder_process_gone` proves dead, since the + repair undoes what that holder left. If the repair fails, the Lease is + released with its records for the next holder to retry. """ - holder = f"uid={os.getuid()};pid={os.getpid()};token={secrets.token_hex(16)}" + holder = ( + f"uid={os.getuid()};pid={os.getpid()};host={socket.gethostname()};" + f"token={secrets.token_hex(16)}" + ) acquired_at = datetime.now(UTC) spec = { "holderIdentity": holder, @@ -1119,22 +1202,25 @@ def _acquire_named_mutation_lease( ) if acquired_at < _parse_kube_timestamp(renewed) + timedelta( seconds=duration - ): + ) and not (repair is not None and _holder_process_gone(current_holder)): raise SmokeConfigurationError( f"another smoke holds the {subject} Lease on this kind " f"cluster (owner {current_holder})" ) previous = lease lease = self._replace_lease_spec(name, metadata, spec, subject=subject) - took_over = bool(current_holder) and on_expired_takeover is not None metadata, _ = _lease_parts(lease) ownership = _LeaseOwnership( name=name, uid=metadata["uid"], holder=holder, subject=subject ) - if observed.returncode == 0 and took_over: - assert on_expired_takeover is not None - on_expired_takeover(previous, ownership) + if observed.returncode == 0 and repair is not None: + try: + repair(previous, ownership) + except BaseException: + with contextlib.suppress(Exception): + self._release_lease(ownership) + raise return ownership def _release_lease(self, ownership: _LeaseOwnership) -> None: @@ -1158,7 +1244,9 @@ def _release_lease(self, ownership: _LeaseOwnership) -> None: ownership.name, metadata, released, subject=ownership.subject ) - def _assert_lease_owned(self, ownership: _LeaseOwnership, action: str) -> None: + def _assert_lease_owned( + self, ownership: _LeaseOwnership, action: str + ) -> tuple[dict[str, object], dict[str, object]]: current = self.kubectl("get", "lease", ownership.name, "-o", "json") if current.returncode != 0: raise SmokeConfigurationError( @@ -1182,6 +1270,59 @@ def _assert_lease_owned(self, ownership: _LeaseOwnership, action: str) -> None: f"refusing {action} without the caller's live mutation Lease " f"({ownership.subject})" ) + return metadata, spec + + def _renew_lease( + self, + ownership: _LeaseOwnership, + action: str, + annotations: Mapping[str, str | None] | None = None, + ) -> None: + """Renew our live Lease and set or clear `annotations` in one guarded patch. + + Callers renew right before each write a successor could own. A runner + stopped past its Lease's expiry may resume after a successor took over + and repaired its records; the patch tests uid, resourceVersion and + holder, so that runner fails here instead of undoing the successor's + fault or erasing its records, and a renewed Lease cannot expire under + the write that follows. + """ + + metadata, _ = self._assert_lease_owned(ownership, action) + existing = metadata.get("annotations") + existing = existing if isinstance(existing, dict) else None + patch: list[dict[str, object]] = [ + {"op": "test", "path": "/metadata/uid", "value": metadata["uid"]}, + { + "op": "test", + "path": "/metadata/resourceVersion", + "value": metadata["resourceVersion"], + }, + {"op": "test", "path": "/spec/holderIdentity", "value": ownership.holder}, + { + "op": "add", + "path": "/spec/renewTime", + "value": _kube_timestamp(datetime.now(UTC)), + }, + ] + for key, value in (annotations or {}).items(): + path = f"/metadata/annotations/{_json_pointer_token(key)}" + if value is None: + if existing is not None and key in existing: + patch.append({"op": "remove", "path": path}) + continue + if existing is None: + patch.append({"op": "add", "path": "/metadata/annotations", "value": {}}) + existing = {} + patch.append({"op": "add", "path": path, "value": value}) + result = self.kubectl( + "patch", "lease", ownership.name, "--type=json", "--patch", json.dumps(patch) + ) + if result.returncode != 0: + raise SmokeConfigurationError( + f"{ownership.subject} Lease changed under {action}; refusing to write: " + f"{result.detail() or ''}" + ) def _annotate_lease(self, name: str, key: str, value: str | None) -> None: assignment = f"{key}-" if value is None else f"{key}={value}" @@ -1494,11 +1635,11 @@ def acquire_worker_cordon_mutation(self) -> Callable[[], None]: def repair_orphaned_cordon( previous: Mapping[str, object], ownership: _LeaseOwnership ) -> None: - # The dead holder recorded the node it cordoned; nothing in SPACE - # ever uncordons, so undo it. This runs only after we own the - # Lease, and re-checks that ownership right before the write, so a - # contender that lost the takeover can never uncordon the winner's - # fault. + # A previous holder that died, or whose uncordon failed, left the + # node it cordoned recorded; nothing in SPACE ever uncordons, so + # undo it. This runs only after we own the Lease, and re-checks + # that ownership right before the write, so a contender that lost + # the takeover can never uncordon the winner's fault. metadata = previous.get("metadata") annotations = ( metadata.get("annotations") if isinstance(metadata, dict) else None @@ -1517,7 +1658,7 @@ def repair_orphaned_cordon( self._annotate_lease(name, CORDONED_NODE_ANNOTATION, None) ownership = self._acquire_named_mutation_lease( - name, "worker cordon mutation", on_expired_takeover=repair_orphaned_cordon + name, "worker cordon mutation", repair=repair_orphaned_cordon ) self._cordon_lease_ownership = ownership @@ -3790,12 +3931,106 @@ def delete_template_as_admin(self, alias: str) -> str | None: # --- rate limiters ----------------------------------------------------- - def acquire_public_api_mutation(self) -> Callable[[], None]: + def _public_api_lease_name(self) -> str: context_hash = hashlib.sha256(self.config.context.encode()).hexdigest()[:16] + return f"space-smoke-public-api-{context_hash}" + + def acquire_public_api_mutation(self) -> Callable[[], None]: ownership = self._acquire_named_mutation_lease( - f"space-smoke-public-api-{context_hash}", "public API deployment mutation" + self._public_api_lease_name(), + "public API deployment mutation", + repair=self._repair_recorded_limiter_faults, ) - return lambda: self._release_lease(ownership) + self._public_api_lease_ownership = ownership + + def release() -> None: + self._release_lease(ownership) + if self._public_api_lease_ownership is ownership: + self._public_api_lease_ownership = None + + return release + + def _repair_recorded_limiter_faults( + self, previous: Mapping[str, object], ownership: _LeaseOwnership + ) -> None: + # A previous holder recorded what it changed before changing it, and + # clears a record only after the restore succeeded. Every write here + # renews the Lease first, so a contender that lost the acquisition + # never undoes the winner's fault. + annotations = _lease_annotations(previous) + recorded_replicas = annotations.get(REDIS_REPLICAS_ANNOTATION) + if isinstance(recorded_replicas, str) and recorded_replicas: + if not recorded_replicas.isdigit(): + raise SmokeConfigurationError("the recorded Redis replica count is not a count") + self._renew_lease(ownership, "an orphaned Redis scale-down repair") + self._scale_redis_deployment(max(int(recorded_replicas), 1)) + self.wait_redis_rollout(timeout=120) + self._renew_lease( + ownership, "clearing the Redis record", {REDIS_REPLICAS_ANNOTATION: None} + ) + recorded_env = annotations.get(LIMITER_ENV_ANNOTATION) + if isinstance(recorded_env, str) and recorded_env: + standing = _recorded_limiter_env(recorded_env) + self._renew_lease(ownership, "an orphaned limiter env repair") + self._apply_public_api_env( + standing, "restore the limiter env a previous smoke Lease holder left" + ) + self._renew_lease( + ownership, "clearing the limiter env record", {LIMITER_ENV_ANNOTATION: None} + ) + + def repair_orphaned_limiter_faults(self) -> bool: + """Undo a limiter fault an earlier run left recorded, or refuse to proceed. + + Every Kind scenario runs this first: a run killed while Redis was down + or the caps were shrunk leaves its records on the public-API mutation + Lease, and only the three scenarios that take that Lease would repair + them. One Lease read when nothing is recorded. A recorded fault is + repaired by acquiring the Lease, which is possible once it is released, + expired or held by a holder proven dead; a live holder is a smoke + mid-fault, and the caller must not run against it. True if a repair ran. + """ + + contexts = self.executor.run(["kubectl", "config", "get-contexts", "-o", "name"], timeout=15) + if contexts.returncode == 0 and self.config.context not in contexts.stdout.split(): + # No cluster yet: the run bootstraps a fresh one, which has no faults. + return False + observed = self.kubectl("get", "lease", self._public_api_lease_name(), "-o", "json", timeout=15) + if observed.returncode != 0: + if "notfound" in observed.detail().lower(): + return False + raise SmokeConfigurationError( + "could not inspect the public API mutation Lease for an orphaned limiter fault: " + f"{observed.detail() or ''}" + ) + document = _lease_document(observed.stdout) + annotations = _lease_annotations(document) + if not any(annotations.get(key) for key in (LIMITER_ENV_ANNOTATION, REDIS_REPLICAS_ANNOTATION)): + return False + try: + release = self.acquire_public_api_mutation() + except SmokeConfigurationError as error: + _, spec = _lease_parts(document) + raise SmokeConfigurationError( + "refusing to run while the public API mutation Lease records a limiter fault " + f"(holder {spec.get('holderIdentity') or ''}): {error}" + ) from error + release() + return True + + def _held_public_api_lease(self, action: str) -> _LeaseOwnership: + ownership = self._public_api_lease_ownership + if ownership is None: + raise SmokeConfigurationError( + f"refusing {action} without the caller's live mutation Lease" + ) + return ownership + + def _lease_annotation(self, ownership: _LeaseOwnership, key: str) -> str | None: + current = self.kubectl("get", "lease", ownership.name, "-o", "json") + _require_ok(current, f"read the {ownership.subject} Lease") + value = _lease_annotations(_lease_document(current.stdout)).get(key) + return value if isinstance(value, str) and value else None def _public_api_container(self, name: str) -> str: container = _require_ok( @@ -3819,11 +4054,7 @@ def public_api_env(self, keys: Sequence[str]) -> dict[str, str | None]: return self._deployment_env_literals(self._public_api_deployment(), keys) def _apply_public_api_env(self, values: Mapping[str, str | None], action: str) -> None: - from space_smoke.rate_limiters import LIMITER_ENV_KEYS - - unknown = sorted(set(values) - set(LIMITER_ENV_KEYS)) - if unknown: - raise ValueError(f"refusing non-limiter public API env keys: {unknown}") + _require_limiter_env_keys(values) self._require_isolated_kind("public API env override") assignments = [ f"{key}-" if value is None else f"{key}={value}" for key, value in values.items() @@ -3839,13 +4070,40 @@ def _apply_public_api_env(self, values: Mapping[str, str | None], action: str) - self.wait_public_api_rollout(timeout=240) def set_public_api_env(self, overrides: Mapping[str, str]) -> None: + from space_smoke.rate_limiters import LIMITER_ENV_KEYS + for key, value in overrides.items(): if not _LIMITER_ENV_VALUE.fullmatch(value): raise ValueError(f"refusing limiter env value {value!r} for {key}") + _require_limiter_env_keys(overrides) + action = "a public API limiter override" + ownership = self._held_public_api_lease(action) + # Record first, and only the first time: a run that dies between here + # and the write leaves a record of values still standing, which the + # next holder restores to no effect, and a second override in the same + # run must not record the first one's values as standing. + record: dict[str, str | None] = {} + if self._lease_annotation(ownership, LIMITER_ENV_ANNOTATION) is None: + standing = self.public_api_env(LIMITER_ENV_KEYS) + record[LIMITER_ENV_ANNOTATION] = json.dumps(standing, sort_keys=True) + self._renew_lease(ownership, action, record) self._apply_public_api_env(overrides, "set the public API limiter env") def restore_public_api_env(self, standing: Mapping[str, str | None]) -> None: + """Put the standing env back, then clear its record; both only while the Lease is ours. + + A runner that lost its Lease refuses: the successor that took it over + has already restored the record, and may have a fault of its own up. + A failed restore keeps the record for the next holder. + """ + + action = "restoring the public API limiter env" + ownership = self._held_public_api_lease(action) + self._renew_lease(ownership, action) self._apply_public_api_env(standing, "restore the public API limiter env") + self._renew_lease( + ownership, "clearing the limiter env record", {LIMITER_ENV_ANNOTATION: None} + ) def wait_public_api_ready(self, *, timeout: float) -> None: """`/readyz`, not `/healthz`: readiness gates the Service on the reflector's initial LIST.""" @@ -3873,22 +4131,42 @@ def redis_replicas(self) -> int: return int(previous) if previous.isdigit() else 1 def scale_redis(self, replicas: int) -> int: + """Scale the limiters' store, only while the public-API Lease is ours. + + Every Kind API, admin API and cluster-controller replica runs its + limiters against this store, so scaling it to zero is recorded on the + Lease first, where the next holder scales it back if this run does + not. A runner that lost its Lease refuses either way, so it cannot + undo or erase a successor's fault. + """ + self._require_isolated_kind("Redis scaling") if replicas < 0 or replicas > 1: raise ValueError(f"refusing to scale Redis to {replicas} replicas") - previous = _require_ok( - self.kubectl( - "get", "deploy", KIND_REDIS_DEPLOYMENT, "-o", "jsonpath={.spec.replicas}", timeout=15 - ), - "read Redis replicas", - ).strip() + action = f"scaling Redis to {replicas}" + ownership = self._held_public_api_lease(action) + previous = self.redis_replicas() + if replicas == 0: + record: dict[str, str | None] = {} + if self._lease_annotation(ownership, REDIS_REPLICAS_ANNOTATION) is None: + record[REDIS_REPLICAS_ANNOTATION] = str(previous) + self._renew_lease(ownership, action, record) + self._scale_redis_deployment(0) + else: + self._renew_lease(ownership, action) + self._scale_redis_deployment(replicas) + self._renew_lease( + ownership, "clearing the Redis record", {REDIS_REPLICAS_ANNOTATION: None} + ) + return previous + + def _scale_redis_deployment(self, replicas: int) -> None: _require_ok( self.kubectl( "scale", f"deploy/{KIND_REDIS_DEPLOYMENT}", f"--replicas={replicas}", timeout=30 ), f"scale Redis to {replicas}", ) - return int(previous) if previous.isdigit() else 1 def wait_redis_unreachable(self, *, timeout: float) -> None: """Pod gone, endpoints empty, then a settle so the API's pooled connection dies.""" diff --git a/space_smoke/rate_limiters.py b/space_smoke/rate_limiters.py index b12d01cc71..58758a6718 100644 --- a/space_smoke/rate_limiters.py +++ b/space_smoke/rate_limiters.py @@ -1,16 +1,24 @@ """The two Redis-backed API rate limiters, proven end to end on a real cluster. -`smoke-kind-rate-limiters.sh` pointed the kind `space-api` at Redis, enabled -both limiters with low caps in enforce mode, and drove real HTTP traffic at -them: sequential creates until the cluster activation token bucket answered +Kind runs both limiters against its Redis by default, as deployments do: the +charts render `SPACE_REDIS_URL` on the API, admin API and cluster-controller, +with the activation limiter at the binary defaults and per-client concurrency +in `observe`. This scenario shrinks the caps and switches per-client +concurrency to enforce on the public API, then drives real HTTP traffic at +them: sequential creates until the cluster activation token bucket answers 429 `Server rate limit:`, concurrent creates and connects for one -`X-Client-ID` until the per-client concurrency limiter answered 429 -`Per-client rate limit:`, then Redis scaled to zero to show both limiters -admit when their store is unreachable. This module is that suite in typed -form: the Deployment env override is a named, captured-and-restored, kind-only -operation behind one cluster Lease; the Redis fault has its compensation -armed before dispatch; every sandbox the probes admit carries run metadata -the teardown kills by. The control surface is [`RateLimiterControl`]; the raw +`X-Client-ID` until the per-client concurrency limiter answers 429 +`Per-client rate limit:`, Redis scaled to zero to show both limiters admit +when their store is unreachable, and Redis back to show they enforce again. + +The Deployment env override is a named, captured-and-restored, kind-only +operation behind one cluster Lease, which records the standing env and the +Redis replica count before either changes and clears each record only once +its restore succeeded. If this run is killed or a restore fails, the next +Kind scenario puts back what is recorded before it starts (see +`KindHarness.repair_orphaned_limiter_faults`). The Redis fault has its +compensation armed before dispatch; every sandbox the probes admit carries +run metadata the teardown kills by. The control surface is [`RateLimiterControl`]; the raw requests go through [`RateLimitProbe`] so a 429's `Retry-After` survives. """ @@ -37,9 +45,10 @@ PER_CLIENT_LIMIT_MESSAGE = "Per-client rate limit:" # The env this scenario may write on the public API Deployment, and nothing -# else. `enable` refuses anything outside it before kubectl runs. +# else. `enable` refuses anything outside it before kubectl runs. The store's +# URL is not among them: the chart renders it, and the fail-open half takes +# the store itself away instead. LIMITER_ENV_KEYS: tuple[str, ...] = ( - "SPACE_REDIS_URL", "SPACE_CLUSTER_ACTIVATION_ADMISSION_ENABLED", "SPACE_CLUSTER_ACTIVATION_ADMISSION_BURST", "SPACE_CLUSTER_ACTIVATION_ADMISSION_REFILL_RPS", @@ -47,7 +56,7 @@ "SPACE_API_CLIENT_CONCURRENCY_MAX_PER_CLIENT", "SPACE_API_CLIENT_CONCURRENCY_KNOWN_CLIENTS", ) -KIND_REDIS_URL = "redis://space-redis:6379" +REDIS_URL_ENV = "SPACE_REDIS_URL" def is_limiter_rejection(outcome: LimiterProbeOutcome, message: str) -> bool: @@ -118,12 +127,13 @@ def enable_limiters( cluster_refill_rps: str = "0.1", rollout_timeout: float = 240, ) -> None: - """Point the API at Redis and turn both limiters on in enforce mode. + """Shrink both limiters' caps and put per-client concurrency in enforce mode. The standing values are captured first and restored at scope end (or eagerly by `restore_limiters`), so a chart or operator value comes back rather than being deleted. The Lease serializes cooperating - smokes on the same Deployment. + smokes on the same Deployment and records the standing values for a + successor if this run dies. """ if not (1 <= per_client_cap <= 8 and 1 <= cluster_burst <= 8): @@ -132,6 +142,12 @@ def enable_limiters( self._template = template release = self._control.acquire_public_api_mutation() self._release_lease = self._scope.defer("release the public API mutation lease", release) + store = self._control.public_api_env((REDIS_URL_ENV,))[REDIS_URL_ENV] + self._fixture.verify( + bool(store), + f"the public API has no {REDIS_URL_ENV}; both limiters would fail open, not enforce", + ) + self._fixture.record("limiter_store", store) standing = self._control.public_api_env(LIMITER_ENV_KEYS) self._fixture.record("limiter_env_before", dict(standing)) @@ -142,9 +158,17 @@ def restore() -> None: self._restore_env = self._scope.defer( "restore the public API limiter env", restore, critical=True ) + # Only the public API's copy of the activation caps shrinks. The + # cluster-controller's `NativePublicOperationExecutor` consumes the + # same bucket at the chart's defaults, and 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 here: this + # scenario sends only ECW requests, which the API admits itself, and + # the suite runs one scenario at a time. Overriding the controller + # too would add a second Deployment rollout and a second record to + # restore for a race the suite cannot produce. self._control.set_public_api_env( { - "SPACE_REDIS_URL": KIND_REDIS_URL, "SPACE_CLUSTER_ACTIVATION_ADMISSION_ENABLED": "true", "SPACE_CLUSTER_ACTIVATION_ADMISSION_BURST": str(cluster_burst), "SPACE_CLUSTER_ACTIVATION_ADMISSION_REFILL_RPS": cluster_refill_rps, @@ -329,6 +353,30 @@ def bring_redis_back(self) -> None: self._redis_back = None release.release() + def expect_enforcing_again( + self, sandbox_id: str, *, callers: int = 20, budget: float = 60 + ) -> None: + """Once the store is back, a connect burst draws a per-client 429 again. + + Connects rather than creates: the slot is reserved before the sandbox + is fetched, so the verdict costs no VM boots. The API's pooled + connections died with the old pod; the pool replaces them on use, so a + first burst may still fail open. Bursts repeat until one is limited or + the budget runs out, and the number it took is recorded. + """ + + deadline = self.now() + budget + bursts = 0 + while True: + bursts += 1 + outcomes = self.burst_connects(sandbox_id, callers) + limited = any(is_limiter_rejection(o, PER_CLIENT_LIMIT_MESSAGE) for o in outcomes) + if limited or self.now() >= deadline: + break + time.sleep(2) + self._fixture.record("recovery_bursts", bursts) + self.expect_per_client_limited(outcomes, label="per_client_after_recovery") + def expect_fail_open(self, outcomes: list[LimiterProbeOutcome]) -> None: """With the store down neither limiter may reject; other statuses are not judged.""" diff --git a/space_smoke/scenarios/local_rate_limiters_enforce_and_fail_open.py b/space_smoke/scenarios/local_rate_limiters_enforce_and_fail_open.py index 9c2866bdb1..44936a9412 100644 --- a/space_smoke/scenarios/local_rate_limiters_enforce_and_fail_open.py +++ b/space_smoke/scenarios/local_rate_limiters_enforce_and_fail_open.py @@ -1,4 +1,4 @@ -"""Both API rate limiters reject past their caps and admit when Redis is gone.""" +"""Both API rate limiters reject past their caps, admit when Redis is gone, and recover.""" import space_smoke as smoke @@ -6,12 +6,14 @@ FAIL_OPEN_CALLERS = 4 -# The shell suite this replaces reconfigured `deploy/space-api` with `kubectl -# set env` and reverted it in a trap. Here the override is a captured, -# Lease-guarded, kind-only operation whose restore is armed before the first -# write, the Redis fault has its scale-back armed before dispatch, and every -# sandbox the probes admit carries run metadata the teardown kills by. Caps -# are low because each admitted create boots a real Firecracker VM. +# The limiters already run against the chart-rendered Redis. The cap override +# is a captured, Lease-guarded, kind-only operation whose restore is armed +# before the first write and recorded on the Lease, where the next scenario +# finds it if this run never restores; the Redis fault is recorded the same +# way and has its scale-back armed before dispatch; every sandbox the probes +# admit carries run metadata the teardown kills by. +# Caps are low because each admitted create boots a real Firecracker VM: at +# the default burst the drain would need sixty. @smoke.scenario( "local.rate_limiters_enforce_and_fail_open", # Two API rollouts (enable, restore), a Redis scale-down/up, and about ten @@ -19,15 +21,17 @@ sandbox=smoke.SandboxConfig(timeout=1800), ) def rate_limiters_enforce_and_fail_open(test: smoke.KindEcw) -> None: - """With both limiters enabled in enforce mode against Redis, sequential - creates drain the cluster activation bucket to a 429 `Server rate limit:` - with Retry-After; twenty concurrent creates and twenty concurrent connects - for one client id each draw at least one 429 `Per-client rate limit:` with - Retry-After; and once Redis is unreachable neither limiter rejects.""" + """With both limiters' caps shrunk and per-client concurrency in enforce + mode, sequential creates drain the cluster activation bucket to a 429 + `Server rate limit:` with Retry-After; twenty concurrent creates and twenty + concurrent connects for one client id each draw at least one 429 + `Per-client rate limit:` with Retry-After; once Redis is unreachable + neither limiter rejects; and once it is back the per-client limiter + rejects again.""" limiters = test.rate_limiters - with test.phase("enable both limiters on the public API, low caps, enforce"): + with test.phase("shrink both limiters' caps on the public API, enforce per-client"): limiters.enable_limiters(template=test.sandbox_template_name()) with test.phase("preflight: one create is admitted or rate-limited"): @@ -57,5 +61,8 @@ def rate_limiters_enforce_and_fail_open(test: smoke.KindEcw) -> None: limiters.expect_fail_open(outcomes) limiters.bring_redis_back() + with test.phase("recovery: Redis back, the per-client limiter rejects again"): + limiters.expect_enforcing_again(target) + with test.phase("restore the public API limiter env"): limiters.restore_limiters() diff --git a/space_smoke/services.py b/space_smoke/services.py index 752c478c56..400fcc31e6 100644 --- a/space_smoke/services.py +++ b/space_smoke/services.py @@ -589,6 +589,27 @@ def from_environment( ) +def repair_orphaned_kind_faults(environment: Mapping[str, str]) -> bool: + """On the kind backend, undo a limiter fault an earlier run left, or refuse. + + Runs before every scenario, whatever its fixture: a scenario that takes no + Lease would otherwise run against shrunk caps or a store scaled to zero. + """ + + values = dict(environment) + if values.get("SPACE_SMOKE_BACKEND", KIND_BACKEND) != KIND_BACKEND: + return False + harness = KindHarness( + SubprocessExecutor(base_env=values), + KindConfig( + cluster=values.get("KIND_CLUSTER", "space-local"), + api_url=values.get("API_URL", "http://127.0.0.1:8080"), + ), + environment=values, + ) + return harness.repair_orphaned_limiter_faults() + + @dataclass(frozen=True) class _NativeScenarioSandboxes: factory: CommandSandboxFactory