Skip to content

Commit 52014d2

Browse files
committed
feat(server): add HA capacity metrics, scoped locks, and graceful drain
Multi-replica gateways route correctly, but they exposed no capacity signals, dropped every supervisor session at once on shutdown, serialized all cross-object mutations fleet-wide on one PostgreSQL advisory lock, and polled one full sandbox record per watched sandbox per second on every replica. Graceful drain: on SIGTERM the gateway closes supervisor admission and reports "draining" on /readyz and /health (/healthz stays 200). Gateways with PostgreSQL and a peer endpoint (every Helm install on an external database) keep the listener open for a 3-second propagation delay, then close their sessions at most 100 ms apart within 12 seconds; each session keeps serving relays and heartbeats until its slot. The existing shutdown then runs with up to 10 seconds of cleanup, 25 seconds in total. A draining owner fails peer relays for sessions it no longer holds immediately, and disconnect cleanup skips the endpoint-status lock when a replacement session exists. Supervisors reset their reconnect backoff after an accepted session and jitter retries, and a new session receives RelayOpen only after SessionAccepted is queued. Capacity metrics: a gateway_metrics module exports per-replica held supervisor sessions, draining state, pending relays against the 256 and 32 caps, relay rejections, expiries and claim latency, peer request rate, outcome (with the owner's code before the remap to UNAVAILABLE) and latency, mutation-lock waits and timeouts, and watch-poller cost. Labels are bounded, series are zero-initialized, new latency metrics are histograms, and existing duration summaries are unchanged. Scoped mutation locks: hierarchical intention locks on a global key, a key per workspace, and a key per sandbox replace the single fleet-wide key. Global settings and platform profiles hold the global key exclusively; provider and workspace-profile writes hold their workspace key exclusively; every sandbox mutation, admin or supervisor, holds only its sandbox key exclusively. Keys are taken in a process-local RwLock table, then as PostgreSQL advisory locks in ascending order on one connection from a dedicated 4-connection lock pool, with one 10-second deadline. Timeouts return UNAVAILABLE with reason MUTATION_LOCK_TIMEOUT instead of INTERNAL. The global key keeps its legacy value, so mixed-version rollouts stay mutually exclusive. DeleteProvider now takes its workspace guard, the settings mutex is removed, and startup endpoint-status reconciliation takes one sandbox guard at a time instead of holding the global key for the whole scan. Watch polling: Store::get_resource_versions reads only id and resource_version (PostgreSQL id = ANY($2), SQLite IN, 1000 ids per statement), and the cross-replica poller makes one batched call per tick with unchanged notification semantics. Helm: the default termination grace period rises from 5 to 30 seconds, and an optional autoscaling/v2 HorizontalPodAutoscaler (disabled by default) omits spec.replicas from the workload when enabled. Chart validation covers the effective maximum replicas and resource requests or limits for utilization targets, and the certgen hook pods no longer match the gateway selector. Tests and docs: mise run test:rust:postgres runs the ignored PostgreSQL-backed tests (batched lookups, cross-store lock exclusion, cancellation, pool bounds, a drain-rate envelope); a Kubernetes HA e2e test verifies that supervisor sessions move off gateway pods during a rollout; unit and integration tests cover relay saturation, 5000 watched sandboxes, drain pacing, and lock scopes. The HA guide, a new gateway metrics reference, the architecture notes, the API errors reference, and the cluster debugging skills describe the drain lifecycle, capacity signals, autoscaling, and PostgreSQL connection sizing. Part of #3528 Signed-off-by: Emilien Macchi <emacchi@redhat.com>
1 parent 9244868 commit 52014d2

51 files changed

Lines changed: 9851 additions & 663 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.agents/skills/helm-dev-environment/SKILL.md‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,10 @@ namespace with a `uri` key. For local manual testing, either create your own
134134
PostgreSQL Secret or use the e2e PostgreSQL fixture manifest in
135135
`e2e/kubernetes/postgres-fixture.yaml`.
136136

137+
Gateway pods in the `high-availability` profile use external PostgreSQL, so
138+
they drain their supervisor sessions when deleted or rolled and can take up to
139+
30 seconds to terminate.
140+
137141
For the `high-availability` profile, return to the repository root and apply the
138142
GatewayClass and BackendTrafficPolicy manifest after Skaffold has installed
139143
Envoy Gateway:
@@ -271,16 +275,24 @@ The kube e2e wrapper creates only one port-forward, to `svc/openshell`; it no
271275
longer forwards the unauthenticated health listener or runs a `/readyz` e2e
272276
target. `/readyz` remains covered by server unit/integration tests.
273277

274-
Use `mise run e2e:kubernetes:ha-rebalancing` for full-suite HA coverage. The
275-
task creates an external PostgreSQL fixture, installs Envoy Gateway, applies
278+
Use `mise run e2e:kubernetes:ha-rebalancing` for HA coverage. The task creates
279+
an external PostgreSQL fixture, installs Envoy Gateway, applies
276280
`deploy/kube/manifests/envoy-gateway-openshell.yaml`, enables the chart
277-
`GRPCRoute`, and runs the full Kubernetes e2e suite, including
278-
`kubernetes_ha_rebalancing`. That coverage validates sandbox create/watch and
281+
`GRPCRoute`, and runs the CLI conformance profile and the
282+
`kubernetes_ha_rebalancing` tests. That coverage validates sandbox create/watch and
279283
exec through the Envoy proxy while gateway replicas scale up, scale down, and
280284
rotate. It also keeps a long-running sandbox alive and runs upload/download
281285
operations while gateway pods roll, so file sync exercises the same relay retry
282286
path as interactive sessions.
283287

288+
`supervisor_sessions_redistribute_across_gateway_pod_rolls` restarts the
289+
gateway Deployment, reads `openshell_server_draining` from the terminating
290+
pods, and checks that the new pods' `openshell_server_supervisor_sessions` add
291+
up to the Ready sandbox count. It scrapes each pod through
292+
`kubectl get --raw /api/v1/namespaces/<ns>/pods/<pod>:9090/proxy/metrics`.
293+
Gateway pods take up to 30 seconds to terminate because each drains its
294+
sessions, and the HA tests are allowed ten minutes each.
295+
284296
If you reuse an existing Skaffold cluster for the full kube suite, make sure the
285297
chart has `server.hostGatewayIP` set so sandbox pods can resolve
286298
`host.openshell.internal` back to the test host. The e2e wrapper detects this on
@@ -446,6 +458,7 @@ for dependencies still declared in `Chart.yaml`.
446458
| `deploy/helm/openshell/ci/values-cert-manager.yaml` | cert-manager PKI overlay (opt-in; disables pkiInitJob) |
447459
| `deploy/helm/openshell/ci/values-gateway.yaml` | Envoy Gateway GRPCRoute + Gateway overlay |
448460
| `deploy/helm/openshell/ci/values-high-availability.yaml` | HA test overlay (`replicaCount: 2` with external PostgreSQL Secret) |
461+
| `deploy/helm/openshell/ci/values-autoscaling.yaml` | Render-only overlay for the optional gateway HorizontalPodAutoscaler (helm lint and helm-unittest) |
449462
| `deploy/helm/openshell/ci/values-keycloak.yaml` | Keycloak OIDC overlay |
450463
| `deploy/helm/openshell/ci/values-spire.yaml` | SPIFFE/SPIRE provider token grant overlay |
451464
| `deploy/helm/openshell/ci/values-spire-stack.yaml` | SPIRE hardened chart values for local dev |

‎.config/nextest.toml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ kubernetes-ha = { max-threads = 1 }
2626
[[profile.e2e-kubernetes.overrides]]
2727
filter = "test(/gateway_(scale_and_rollout|pod_rolls)$/)"
2828
test-group = "kubernetes-ha"
29+
# Each rolled gateway pod drains its supervisor sessions for up to the 30s
30+
# termination grace period, so these tests get ten minutes, matching
31+
# HA_SYNC_TIMEOUT in kubernetes_ha_rebalancing.rs.
32+
slow-timeout = { period = "60s", terminate-after = 10 }
2933

3034
# Relative to the profile store dir (`e2e/rust/target/nextest/e2e-kubernetes/`).
3135
[profile.e2e-kubernetes.junit]

‎TESTING.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,23 @@ mise run test:rust # cargo test --workspace
4848

4949
Rust validation checks tracked Cargo lockfiles; run `mise run rust:lockfiles:check` to check them directly. If one is stale, refresh it with Cargo using its adjacent manifest, review the diff, and commit the update.
5050

51+
### PostgreSQL-backed tests
52+
53+
Tests that need a real PostgreSQL server, such as advisory-lock concurrency
54+
across two stores, are `#[ignore]`d and named `postgres_*`. Run them with:
55+
56+
```shell
57+
mise run test:rust:postgres
58+
```
59+
60+
The task starts a disposable PostgreSQL container with Docker or Podman
61+
(set `CONTAINER_ENGINE` to choose), runs the tests one at a time, and removes
62+
the container. Each test works in its own temporary schema. To use your own
63+
disposable database, set `OPENSHELL_TEST_POSTGRES_URL`. Never point it at a
64+
database that a running gateway uses: the tests take fleet-wide advisory locks.
65+
CI does not run these tests; the Kubernetes HA e2e suite covers PostgreSQL end
66+
to end.
67+
5168
### Native Windows validation
5269

5370
Use `mise run --skip-tools pre-commit` with the existing Rust/MSVC toolchain.
@@ -403,6 +420,7 @@ Available task variants:
403420
|---|---|
404421
| `e2e:kubernetes` | Default Rust e2e against Helm-deployed gateway |
405422
| `e2e:kubernetes:db` | All database backend scenarios (SQLite + external PostgreSQL) |
423+
| `e2e:kubernetes:ha-rebalancing` | Two gateway replicas behind Envoy with external PostgreSQL: scale, pod deletion, rollout drain and session redistribution, and file sync during pod rolls |
406424
| `e2e:kubernetes:sidecar` | Supervisor sidecar topology overlay |
407425
| `e2e:kubernetes:credential-drivers` | Kubernetes Secrets and Vault credential storage |
408426
| `e2e:kubernetes:workspace-managed` | Managed workspace mode (auto-created namespaces) |

‎architecture/gateway.md‎

Lines changed: 120 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -415,10 +415,44 @@ validates the current supervisor session and keeps the in-memory evidence; a
415415
non-owner never accepts evidence from a stale local session or projects a
416416
remote session as disconnected.
417417

418-
Nothing redistributes established sessions, so after a rolling restart the last
419-
surviving replica holds most sessions and a new replica serves none until
420-
sandboxes reconnect. That skew decays only as sandboxes churn. Client traffic
421-
stays correct throughout because a non-owner relays to the owner.
418+
Planned shutdown moves ownership instead of waiting for churn. On SIGTERM a
419+
gateway closes supervisor admission and reports `draining` on `/readyz`. A
420+
gateway that uses an external PostgreSQL database and advertises a peer
421+
endpoint (every Helm chart install on an external database: Deployment pods
422+
advertise their pod IP and StatefulSet pods their stable DNS name) then keeps
423+
its listener open for a 3-second propagation delay, so endpoint removal
424+
reaches kube-proxy and ingress, and closes its supervisor sessions at most 100 ms apart within 12
425+
seconds. Peers keep following the durable owner record, which changes only
426+
when a session is closed and reconnects elsewhere. Until its turn, a session keeps serving relays and
427+
heartbeats. Each supervisor redials the gateway Service, lands on a ready
428+
replica, and publishes ownership there. Supervisors reset their reconnect
429+
backoff after an accepted session and jitter failed retries, so a drained
430+
session normally reconnects within about a second. The old replica demotes the
431+
sandbox to `Provisioning` when it closes the session, so for that second new
432+
exec, SSH, and forward requests fail the readiness check with
433+
`FAILED_PRECONDITION`; requests already routed wait up to 15 seconds for the
434+
supervisor. A new session receives relay requests only after the gateway has
435+
sent `SessionAccepted`, and a draining owner answers peer relays for sessions
436+
it no longer holds with `UNAVAILABLE` at once so the requester re-reads
437+
ownership. Up to 120 sessions close one every 100 ms; above that the close rate
438+
is the session count divided by 12 seconds. Each reconnect takes one or two
439+
short mutation locks on the receiving replicas, so the mutation-lock pool
440+
bounds how many sessions a replica can drain without queuing. Other gateways
441+
skip the drain. A lone replica, such as a single-replica StatefulSet, has no
442+
peer to receive its sessions, so its drain adds up to 15 seconds to shutdown;
443+
a StatefulSet creates the replacement only after the old pod exits, so the
444+
restart outage grows by the same amount. Supervisors keep backing off while no
445+
replica is ready, so some reconnect up to one maximum backoff interval (30
446+
seconds) after the replacement is ready. Skipping the drain needs a view of
447+
live replicas, which the gateway does not have yet.
448+
449+
The drain decides when sessions leave, not where they land. Reconnects go to
450+
whichever replicas are ready at that moment, so a two-replica surge rollout
451+
leaves the first replacement pod with most sessions (about 62/38), and pod
452+
deletion, StatefulSet updates, and scale-out leave new replicas nearly empty
453+
until sandboxes churn. Connect-time placement and rebalancing are future work.
454+
Client traffic stays correct throughout because a non-owner relays to the
455+
owner.
422456

423457
File upload and download use tar-over-SSH through the same relay path. A gateway
424458
pod termination drops the active SSH proxy byte stream, so the CLI retries the
@@ -434,18 +468,54 @@ also trust the chart CA, present the chart-generated client certificate for
434468
mTLS, and verify the stable gateway Service DNS name even when connecting to a
435469
Deployment pod IP.
436470

437-
`WatchSandbox` uses the local update bus for same-replica writes. On
438-
multi-replica backends one shared poller per gateway observes resource-version
439-
changes made by other replicas and feeds that bus for all local watchers,
440-
avoiding a database poll per client stream. SQLite deployments do not run the
441-
poller because they are single-replica and the local bus already sees every
442-
write.
443-
444-
Mutations whose invariants span sandbox, provider-profile, policy, or provider
445-
records take a process-local mutex and a shared PostgreSQL advisory lock. The
446-
database session remains dedicated to the request and closes when the guard is
447-
dropped, which releases the lock on normal completion, cancellation, or error.
448-
SQLite deployments use only the local mutex because they are single-replica.
471+
Each replica exports per-replica capacity signals on its metrics listener:
472+
held supervisor sessions, draining state, pending relays against the fixed
473+
limits of 256 per replica and 32 per sandbox, relay rejections and expiries,
474+
outbound peer RPC outcomes and latency, mutation-lock waits and timeouts, and
475+
watch-poller cost. Session and pending-relay gauges are held by the registry
476+
entries themselves, so every removal path keeps them exact, and peer request
477+
metrics record the owner's status before relay failures are reported to
478+
clients as `UNAVAILABLE`. Labels are bounded; no metric carries a sandbox,
479+
channel, endpoint, or replica identifier, because the scrape target already
480+
identifies the replica.
481+
482+
`WatchSandbox` uses the local update bus for same-replica writes. Every
483+
PostgreSQL-backed gateway, even with a single replica, runs one shared poller
484+
that observes writes made by other replicas and feeds that bus for all local
485+
watchers. Every second it reads the id and `resource_version` of each sandbox
486+
that has a local watcher in one store call, with one statement per 1000 ids
487+
and no payload reads, so database load follows the number of distinct watched
488+
sandboxes on the replica rather than the number of client streams. It
489+
notifies watchers when it first observes a sandbox, when the version changes,
490+
and when the row disappears. A failed read keeps the last known versions and
491+
retries on the next tick. SQLite deployments do not run the poller because
492+
they are single-replica and the local bus already sees every write.
493+
494+
Mutations whose invariants span sandbox, provider, provider-profile, policy, or
495+
settings records take a hierarchical mutation guard. It names a global key, one
496+
key per workspace, and one key per sandbox, each held shared or exclusive.
497+
Global policy and settings updates and platform-scope profile changes hold the
498+
global key exclusively. Provider and workspace-scoped profile mutations hold
499+
the global key shared and their workspace key exclusively. Sandbox-scoped
500+
mutations, including every supervisor report, hold the global and workspace
501+
keys shared and their sandbox key exclusively. Unrelated sandboxes proceed
502+
concurrently, and a provider change still excludes every sandbox mutation in
503+
its workspace. Each replica takes the keys in a process-local lock table first,
504+
then, on PostgreSQL, as session-level advisory locks in ascending key order on
505+
one connection from a dedicated four-connection lock pool. Returning that
506+
connection runs `pg_advisory_unlock_all()`, and a cancelled acquisition closes
507+
its session. Acquisition is bounded at 10 seconds, PostgreSQL enforces the
508+
remaining deadline on every lock wait, and a timeout fails with `UNAVAILABLE`.
509+
The global key is the legacy cross-object key, so a replica from an earlier
510+
release, which holds it exclusively for every mutation, still excludes new
511+
replicas during a rolling upgrade. Lifecycle, driver-watch, and reconcile paths
512+
take only process-local keys, the global key shared and their sandbox key
513+
exclusively, and rely on compare-and-swap across replicas. Provisioning-deadline
514+
reconciliation also holds its workspace key shared, because it re-derives
515+
configuration from provider and profile records. SQLite deployments use only
516+
the local table. Startup endpoint-status reconciliation takes each candidate's
517+
full sandbox-scoped guard, up to four at a time, and never holds a guard across
518+
the whole scan.
449519

450520
## API Surface
451521

@@ -936,13 +1006,13 @@ coverage:
9361006
| Provider | `MustCreate` | `update_message_cas` | `list_messages` |
9371007
| ProviderProfile | `MustCreate` | `MatchResourceVersion` | `list_messages` |
9381008
| SandboxPolicy | scoped versioning | scoped versioning | scoped query |
939-
| Settings | `Mutex`-guarded | `Mutex`-guarded | single-row |
1009+
| Settings | mutation-guarded | mutation-guarded | single-row |
9401010

941-
Global settings updates use a Tokio `Mutex` to serialize multi-step
942-
validation within a single gateway process, with CAS on the underlying
943-
persistence write as defense in depth. In an HA deployment with multiple
944-
gateways, the Mutex alone would be insufficient. Sandbox-scoped settings
945-
rely entirely on CAS without a Mutex.
1011+
Global settings and policy updates hold the global mutation key exclusively,
1012+
and sandbox-scoped settings updates hold their sandbox key with the global key
1013+
shared. The precedence check between a sandbox setting and a globally managed
1014+
key therefore cannot interleave with a global change on any replica. Settings
1015+
writes also use CAS as defense in depth.
9461016

9471017
The `resource_version` is surfaced to clients through `ObjectMeta` in proto
9481018
responses. Provider profiles are the exception: custom profile get/list/export
@@ -952,13 +1022,14 @@ requests also carry an explicit target profile ID; the payload ID must match the
9521022
target so an edited export cannot overwrite a different profile. Database
9531023
migrations backfill existing rows with version 1.
9541024

955-
Provider profile imports, updates, and deletes hold the sandbox synchronization
956-
guard while checking attached-sandbox dynamic token grant ambiguity or in-use
957-
state and writing the profile record. Sandbox creation with initial providers and
958-
sandbox provider attach/detach use the same guard, so gateway replicas cannot
959-
interleave a profile mutation with a sandbox provider-set mutation that would
960-
leave an ambiguous final dynamic-token state or a deleted custom profile that is
961-
still referenced by a sandbox.
1025+
Provider profile imports, updates, and deletes hold their workspace mutation
1026+
key exclusively (the global key for platform-scope profiles) while checking
1027+
attached-sandbox dynamic token grant ambiguity or in-use state and writing the
1028+
profile record. Sandbox creation with initial providers and sandbox provider
1029+
attach/detach hold the same workspace key shared plus their sandbox key, so
1030+
gateway replicas cannot interleave a profile mutation with a sandbox
1031+
provider-set mutation that would leave an ambiguous final dynamic-token state or
1032+
a deleted custom profile that is still referenced by a sandbox.
9621033

9631034
Policy and runtime settings are delivered together through the effective sandbox
9641035
config path. A gateway-global policy can override sandbox-scoped policy. The
@@ -1081,14 +1152,21 @@ The same relay pattern backs interactive SSH, command execution, file sync, and
10811152
local service forwarding. The gateway tracks live sessions in memory and
10821153
persists session records so tokens can expire or be revoked.
10831154

1084-
Graceful gateway shutdown closes supervisor-session admission before stopping
1085-
local compute. It then signals the remaining control sessions to exit and waits
1086-
up to ten seconds for their cleanup, including conditional deletion of persisted
1087-
ownership. Pending connection setup and sessions already removed from the live
1088-
registry remain tracked until cleanup finishes. This lets a replacement
1089-
supervisor claim ownership immediately after restart without deleting a newer
1090-
replica's claim. An incomplete drain is reported as a shutdown error. Closing
1091-
these control sessions does not stop Kubernetes-owned workloads.
1155+
Graceful gateway shutdown first closes supervisor-session admission and reports
1156+
`draining` from readiness on every gateway. A gateway that uses an external
1157+
PostgreSQL database and advertises a peer endpoint then drains its sessions
1158+
with the listener still open (see HA Supervisor Ownership): 3 seconds of
1159+
propagation delay, then paced closes within 12 seconds. Other gateways skip
1160+
the drain. The gateway then stops its listener and local compute, signals the
1161+
remaining control sessions to exit, and waits up to ten seconds for their
1162+
cleanup, including conditional deletion of persisted ownership. Pending
1163+
connection setup and sessions already removed from the live registry remain
1164+
tracked until cleanup finishes. This lets a replacement supervisor claim
1165+
ownership immediately after restart without deleting a newer replica's claim.
1166+
An incomplete cleanup is reported as a shutdown error. The worst case is 25
1167+
seconds, inside the chart's 30-second termination grace period; the final OTLP
1168+
trace flush runs after that and is not counted. Closing these control sessions
1169+
does not stop Kubernetes-owned workloads.
10921170

10931171
Relay liveness has two backstops so a reset supervisor session cannot leave a
10941172
request parked forever. The gateway runs server-side HTTP/2 keepalive on
@@ -1268,11 +1346,11 @@ and that span continues incoming W3C trace context when present or starts a new
12681346
trace otherwise. It is named for the RPC and carries the request ID that also
12691347
appears in the gateway's logs — the identifier that lets an operator pivot
12701348
between a trace and its log lines. Store and compute-driver spans become
1271-
children of the request span. Reconciliation, provider refresh, and
1272-
driver-watch loops create their own operation spans because they have no
1273-
inbound request to provide a parent. gRPC status is recorded when response
1274-
trailers arrive. Gateway spans carry resource attributes for the gateway
1275-
identity and configured compute driver.
1349+
children of the request span. Reconciliation, provider refresh, the
1350+
cross-replica watch poller, and driver-watch loops create their own operation
1351+
spans because they have no inbound request to provide a parent. gRPC status is
1352+
recorded when response trailers arrive. Gateway spans carry resource attributes
1353+
for the gateway identity and configured compute driver.
12761354

12771355
The gateway forwards OTLP configuration, its configured gateway name, and W3C
12781356
trace context to managed external drivers. Built-in drivers use dedicated

0 commit comments

Comments
 (0)