Skip to content

feat(server): place supervisor sessions by consistent hash - #3661

Open
FrostGod wants to merge 3 commits into
NVIDIA:mainfrom
FrostGod:gateway-session-placement
Open

FrostGod wants to merge 3 commits into
NVIDIA:mainfrom
FrostGod:gateway-session-placement

Conversation

@FrostGod

@FrostGod FrostGod commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Spreads supervisor sessions across gateway replicas and sends a sandbox's long-lived connections (SSH, port forwards, exec) straight to the replica holding its session, instead of relaying through a peer.

Related Issue

Part of #3528: covers graceful redistribution of supervisor sessions (placement plus handoff on shutdown) and adds owner routing for long-lived connections. Capacity metrics, autoscaling, advisory-lock scoping, and watch batching are out of scope. Builds on #1868.

Changes

  • Placement: replicas register in a shared membership table (expired rows are swept); a consistent hash picks each sandbox's replica. A supervisor on the wrong replica is redirected once and falls back to its original endpoint if that fails. Only supervisors that advertise supports_session_redirect are redirected, so older supervisors keep working across a gateway upgrade. Redirect dials verify TLS against the gateway Service name.
  • Graceful handoff: a replica shutting down leaves the ring first, then redirects its sessions to their new owners instead of dropping them.
  • Owner hint: the CLI asks GetSandbox for the owner (x-openshell-want-owner) and gets it back as x-openshell-owner; other callers pay no extra lookup. The CLI sends it as x-openshell-replica on SSH, forward, and exec, and retries once unrouted only when the request never reached a gateway.
  • Helm: opt-in grpcRoute.replicaRouting.enabled adds one Service per replica and header-matched GRPCRoute rules. Requires workload.kind=statefulset, workload.allowMultiReplicaStatefulSet=true, and at most 15 replicas. Off by default; rendered output is unchanged when off.

Testing

  • Unit tests and clippy pass for server, CLI, core, supervisor, and the Kubernetes driver; mise run helm:test, go:proto:check, and mise run pre-commit pass.
  • kubernetes_ha_rebalancing e2e passes on a 2-node cluster (scale 2→3→2, pod rolls, 32 MiB sync during rolls).
  • Routing, 42 checks across 6 sandboxes: peer relays dropped from 37 to 0, all checks pass. Still 42/42 with one replica's route broken (unrouted fallback).
  • Gateway SIGKILL: new and old CLI recover the same way; no regression.

Checklist

  • Conventional commits, signed off
  • Tests added
  • Docs updated (docs/kubernetes/ingress.mdx, Helm README, debug-openshell-cluster skill)
  • Go / TypeScript SDK clients do not request the owner hint yet (follow-up)

@copy-pr-bot

copy-pr-bot Bot commented Sep 24, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@FrostGod

FrostGod commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

/ok

@EmilienM EmilienM left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for taking this on. Placing sessions where the ring wants them should cut out a lot of peer relay hops, and handing sessions off on shutdown is a nice touch. I read through it with a multi-replica Kubernetes deployment in mind and left comments inline.

The three I'd want sorted before merging: redirects failing TLS verification against pod addresses, supervisors built before this PR that can't decode SessionRedirect after a gateway upgrade, and the Go SDK bindings. The rest are smaller, and a few are nits.

Comment thread crates/openshell-supervisor-process/src/supervisor_session.rs
Comment thread crates/openshell-server/src/supervisor_session.rs Outdated
Comment thread proto/openshell.proto
Comment thread crates/openshell-cli/src/tls.rs
Comment thread crates/openshell-server/src/gateway_members.rs
Comment thread crates/openshell-cli/src/ssh.rs
Comment thread crates/openshell-cli/src/run.rs Outdated
Comment thread crates/openshell-cli/src/run.rs
Comment thread crates/openshell-server/src/supervisor_session.rs Outdated
Comment thread crates/openshell-server/src/grpc/sandbox.rs Outdated
@FrostGod
FrostGod marked this pull request as ready for review October 2, 2026 17:26
@pimlock

pimlock commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 2bf8a00

@FrostGod
FrostGod force-pushed the gateway-session-placement branch from 2bf8a00 to 6227ab1 Compare October 5, 2026 07:34
Comment thread deploy/helm/openshell/templates/_helpers.tpl Outdated
Comment thread crates/openshell-supervisor-process/src/supervisor_session.rs
Comment thread crates/openshell-server/src/supervisor_session.rs
Comment thread proto/openshell.proto
Comment thread crates/openshell-server/src/grpc/sandbox.rs
@FrostGod
FrostGod force-pushed the gateway-session-placement branch from 6227ab1 to 00b9ded Compare October 5, 2026 20:27

@EmilienM EmilienM left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd be comfortable approving but there are some follow ups to do (see comments).

@EmilienM

EmilienM commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

LGTM

@drew

drew commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 25fd5ae

@FrostGod
FrostGod force-pushed the gateway-session-placement branch 2 times, most recently from 3787c69 to baa3fba Compare October 6, 2026 20:41
@pimlock

pimlock commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

/ok to test baa3fba

…hem off on shutdown

Signed-off-by: divesh <dgude@nvidia.com>
Signed-off-by: divesh <dgude@nvidia.com>
@FrostGod
FrostGod force-pushed the gateway-session-placement branch from baa3fba to aedb640 Compare October 6, 2026 22:46
@FrostGod

FrostGod commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Flaky Tests, @drew , can you run /ok again

This branch has not been deployed

No deployments
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.

4 participants