Skip to content

feat(supervisor): recover shared instances with hash-based HA - #4864

Merged
drew merged 16 commits into
mainfrom
feat/4860-supervisor-ha/drew
Oct 27, 2026
Merged

drew merged 16 commits into
mainfrom
feat/4860-supervisor-ha/drew

Conversation

@drew

@drew drew commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Add experimental high availability for the shared supervisor introduced by #4702. A hash ring places new sandbox generations, while durable ownership and gateway-signed takeover grants let a replacement supervisor adopt existing runtimes, processes, and memory without restarting workloads. Shared supervisors always use pool coordination, including at one replica.

Compute drivers are unchanged from base 9a85667d0. Coordination remains in the gateway and supervisor control layer.

Related Issue

Closes #4860

Changes

  • Persist membership, desired assignments, ownership terms, and term-scoped reports across gateway replicas. Store launch material in the configured credential store.
  • Simplify pool registration to an opaque member ID and reachable runtime address. Remove the unused Pod UID field, CLI flag, and environment variable; preserve process-instance and reconnect fencing without compute-driver changes.
  • Keep launches in an admitting state until one transaction publishes the assignment, creates its owner, and charges member capacity. Refused launches remove only their own publication and material; failed cleanup is fenced and collected. Concurrent successful retries are preserved, and rejected launches cannot be revived by later capacity.
  • Advance credential epochs, revoke the previous owner's credentials, and require signed takeover grants. Preserve process identity, binary identity pins, policy, sandbox CA, and SSH host keys during adoption; route runtimes to their current owner even while a stale owner remains reachable.
  • Confirm apparent owner failure with a point read of its current membership before recording takeover intent; a membership read failure leaves ownership unchanged. Fence local supervision observations by the current acknowledged ownership term, including ownership returning to the same process.
  • Retain successful snapshot acknowledgements per stream across retries and coalesced refusals. Suppress reports from unconfirmed/dead owners while preserving their assignments for recovery.
  • Fence streamed stdin and EOF at delivery under the current supervisor owner. Reserve input capacity before short ownership guards, and retire old attachments without waiting for queue room.
  • Use atomic per-member capacity accounting, keyset pagination with absence confirmation, independently refreshed membership, acknowledged ownership terms, stale-stream rejection, and bounded reconnects. Process restarts can reclaim existing routes even in a full pool.
  • Remove the non-pool shared-supervisor path and its enable switch. [openshell.gateway.supervisor_pool] optionally tunes the recovery deadline (60 seconds by default); member identity is required, and configuration errors name the setting to fix. Per-sandbox supervision remains available.
  • Render a rolling supervisor Deployment and a disruption budget enabled by default (supervisor.podDisruptionBudget.enabled can disable it for maintenance). Keep each supervisor resource in its own Helm template. Supervisor affinity defaults to {}; operators can set supervisor.affinity independently of gateway affinity, with a node-spreading example in the HA guide. Changing only supervisor.replicas scales 1 → 2 → 1 without changing gateway configuration or replacing surviving members.
  • Consolidate setup, recovery tuning, failover, manual scaling, and memory autoscaling under the experimental multi-tenant supervisor section in the Kubernetes HA guide. Restore the gateway configuration page to its PR-base version.
  • Optionally autoscale the supervisor Deployment from average per-Pod memory using supervisor.autoscaling. The HPA owns replicas when enabled, with validated bounds and conservative downscale behavior; fixed-count deployments remain the default. Existing workloads stay with their live owners.
  • Add deterministic acknowledgement, input-backpressure/takeover, stale-readiness, admission/cleanup/retry, pagination, report-ordering, restart, and reconnect regressions, plus serialized live scaling and abrupt-loss E2Es that require distinct owners and preserved workload identity and memory.

Release Notes

Experimental shared supervisors always run as a pool; set supervisor.replicas to change capacity. One replica has no spare for immediate failover. During takeover, workloads can briefly freeze and existing proxied connections or interactive sessions must reconnect. Workloads terminate if supervision is not restored within the recovery deadline.

Testing

  • Rebased onto main (f07127e5d), preserving suspend/resume and required-middleware support. Updated two suspend test fixtures for the HA interfaces (853684bf6). All 3,103 affected Rust tests passed (33 skipped), full workspace all-target compile checks passed, Helm tests passed (405 gateway + 6 workspace), and normal commit checks passed. Rebuilt all three images from the rebased runtime and reran live 1 → 2 → 1: all eight tests passed (74.47 s), preserving workload identity and in-memory state. Readiness follow-up is tracked in bug(supervisor): gate rollout readiness on gateway attachment #4960.
  • Optional supervisor PDB and template split (f59e8dc94): 401 gateway chart tests and 6 workspace chart tests passed, plus full Helm, docs, Markdown and normal commit checks. Semantic renders match the previous chart across four deployment configurations; disabling the PDB changes only that resource in fixed and HPA modes. Live disable/re-enable preserved Pod templates, UIDs and restart counts. Live 1 → 2 → 1 passed all eight tests (81.32 s), preserving workload identity and memory.
  • Configurable supervisor affinity (6bee825e3): 394 gateway chart tests and 6 workspace chart tests passed, including custom-rule rendering and gateway/supervisor separation. Full Helm checks, docs validation, Markdown lint, and normal commit hooks passed. Live 1 → 2 → 1 with the empty affinity default passed all eight tests (82.75 s), preserving workload identity and memory.
  • Pool identity simplification (7cd665138): 2,686 Rust tests passed (33 skipped), strict affected-crate Clippy and normal commit hooks passed, and full helm:test passed. Rebuilt all three images and reran live 1 → 2 → 1 against the current chart: all eight tests passed (77.25 s), preserving workload identity and memory. The final supervisor Deployment has one replica and no member-UID setting.
  • Documentation consolidation: Markdown/Mermaid, navigation, Fern validation, and normal commit hooks passed. Fern reports three existing warnings, including an MDX parse warning in the gateway configuration page restored byte-for-byte from the PR base.
  • Affected Rust suites: 3,063 passed, 33 skipped across server (test-support), supervisor-control, supervisor, sandbox, and sandbox-backend. Both acknowledgement regressions passed again after the mechanical Clippy fix.
  • Disposable PostgreSQL suite: 32/32 passed, including the admission/cleanup races; the container was removed afterward.
  • Mutations removing refused-launch cleanup, atomic admission, or stale-admission fencing were caught by the regressions.
  • Gateway chart 391/391 and workspace chart 6/6; full helm:test, rendered-TOML loader validation, count-independent 1/2/3 renders, Helm lint/docs, affected Clippy with warnings denied, formatting, and Markdown lint passed.
  • Live scaling on all three images built from 4a7f97f8f: 1 → 2 → 1 passed (81.75 s), plus all seven helpers. Workload Pod UID/generation/PID/start ticks/in-memory counter and gateway/surviving-supervisor identity stayed unchanged; a new sandbox worked after downscale. All exec and network probes succeeded, including the handed-off workload.
  • Live abrupt-loss recovery on the same rebuilt images passed (47.79 s), plus all seven helpers: direct supervisor SIGKILL, no graceful Leave, expiry-based takeover, preserved workload identity/memory, restored exec/network, and an unaffected workload.
  • Normal full pre-commit hook passed (53.69 s), including workspace/extra-configuration Rust lint, format/license/lockfile, Helm, protobuf, Markdown/Mermaid, Python, and TypeScript checks. Strict Clippy with warnings denied passed for all six affected runtime/control crates (all targets).

Live Kubernetes validation uses one local k3d node and fixed-count scaling. The latest pool-identity scaling run used the current chart; the earlier scaling and abrupt-loss runs used the pre-autoscaling chart. The added HPA is covered by unit/render/config checks; metric-driven HPA scaling was not exercised live. Physical node loss and multi-gateway partitions are not exercised. Cross-replica wall-clock comparisons remain deferred. Aligning the ownership design record with this implementation is deferred to RFC 0018 (#4377).

Pool members share the supervisor credential, so declared member identity is within that trust boundary. Identity-bound provider placeholders without stable handles can fail closed after takeover if their provider revision changed. Service discovery adds bounded periodic probes per pool sandbox. A crash between credential creation and candidate staging can leave orphaned material, as in the existing SSH host-key path.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Configuration and operational documentation updated

@copy-pr-bot

copy-pr-bot Bot commented Oct 23, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown

@drew
drew requested a review from FrostGod October 23, 2026 06:23

@matthewgrossman matthewgrossman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review of head 06cd2e2 from a multi-agent review panel (security, correctness, API/UX, and scope reviewers, plus a verification and triage step). I checked the one blocking finding against the code by hand. The reviewers read the code but didn't build it or run tests.

Summary: a large but coherent HA feature for #4860. The security reviewer found no new problems with the takeover grants, credential-epoch fencing, or the state preserved during adoption. One correctness issue should be fixed before merge. Everything else is polish or follow-up.

Fix before merge

  • A refused pool launch leaves its assignment record and launch material behind, and another replica can later give it a takeover owner (supervisor_pool.rs:1731).

Small fixes

  • A config error message contains runs of spaces (lib.rs:1876).
  • The pool/non-pool mismatch errors don't name the setting to change (controller.rs:1037).
  • The HA docs don't say what an upgrade does to running sandboxes (high-availability.mdx).

Follow-ups (fine as separate issues)

  • Member freshness and report ordering compare wall-clock times written by different replicas.
  • A design record (RFC) for pool ownership, takeover grants, and epoch fencing.
  • Decide whether the non-pool shared-supervisor mode stays, since Helm always enables the pool.

Considered and not raised: splitting this ~24k-line PR into smaller PRs. It's one coherent, tested feature, so that's your call.

Details are in the inline comments.

Comment thread crates/openshell-server/src/supervisor_pool.rs Outdated
Comment thread crates/openshell-server/src/lib.rs Outdated
Comment thread crates/openshell-supervisor-control/src/controller.rs Outdated
Comment thread docs/kubernetes/high-availability.mdx Outdated
Comment thread crates/openshell-server/src/supervisor_pool.rs
Comment thread rfc/0012-isolation-backend/README.md
Comment thread crates/openshell-server/src/config_file.rs Outdated
@drew
drew marked this pull request as ready for review October 23, 2026 16:06
@github-actions

Copy link
Copy Markdown

Compatibility review (advisory)

New or updated breaking changes; see the full report for all findings.

1. cli — No explicit risk acceptance found

The existing openshell-supervisor --mode=shared invocation now additionally requires --pool-member-id, --pool-member-uid, and --pool-advertise-ip, or their environment equivalents.

  • Impact: Existing custom launchers exit with a missing-pool-identity error before opening their listeners, although the updated Helm chart supplies the required environment variables.
  • Action: Preserve startup compatibility for the old shared-mode invocation in v0.1.6, or defer mandatory pool identity to a permitted minor release with launcher migration guidance.
  • PR: feat(supervisor): recover shared instances with hash-based HA #4864

2. rust-sdk — No explicit risk acceptance found

raw::proto::supervisor_host::v1::{HostHello, AssignmentSnapshot, SandboxInstanceAssignment, HostStatus} gain fields that invalidate existing exhaustive Rust literals and patterns.

  • Impact: Raw-message consumers, including mock/server fixtures, fail to compile unless updated, while high-level clients and construction using ..Default::default() remain source-compatible.
  • Action: Preserve the existing exported message layouts in v0.1.6, or defer their source-breaking expansion to a permitted minor release with explicit field and default-construction migration guidance.
  • PR: feat(supervisor): recover shared instances with hash-based HA #4864
Review details
  • Execution: complete
  • Candidate: PR #4864 (8ab1d50d340b6f47fff0d01ebb8d1cc720ebed79)
  • Baseline: 9a85667d0d87e8131b64351ec1fc4eb26e7bb865 (9a85667d0d87e8131b64351ec1fc4eb26e7bb865)
  • Reviewed: rust-sdk, python-sdk, go-sdk, typescript-sdk, api-behavior, cli, configuration, policy, helm, state

Advisory review; publication gates are unchanged.

Branch check run

@TaylorMutch TaylorMutch 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 head abffde246 locally. I found two correctness issues, each reproduced with a focused Rust test against this commit; details and reproduction steps are inline.

Validation: 30 supervisor Helm tests, the render-only 1/2/3-replica scaling check, 13 sandbox authentication tests, and the original 12 supervisor-control tests passed. The two additional regression assertions fail at the expected behavior checks. Temporary source changes were restored. These reproductions exercise the Rust controller and SQLite-backed pool; I did not run live Kubernetes recovery E2Es or PostgreSQL validation.

Comment thread crates/openshell-server/src/supervisor_pool.rs Outdated
Comment thread crates/openshell-supervisor-control/src/controller.rs

@FrostGod FrostGod 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.

One inline comment on preserving snapshot acknowledgements across retries.

Comment thread crates/openshell-supervisor/src/shared_host.rs Outdated
Comment thread crates/openshell-server/src/supervisor_pool.rs
Comment thread crates/openshell-sandbox/src/boundary_server.rs Outdated
Comment thread crates/openshell-supervisor/src/shared_host.rs
@drew drew added test:e2e Requires end-to-end coverage test:e2e-kubernetes Requires Kubernetes end-to-end coverage labels Oct 27, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for 4a7f97f. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@github-actions

Copy link
Copy Markdown

Label test:e2e-kubernetes applied for 4a7f97f. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute Kubernetes HA and credential-driver E2E after building the required gateway, sandbox, and supervisor images once. This is an optional proof-of-life suite; failures are visible in the workflow run but do not publish a required CI gate status.

Comment thread deploy/helm/openshell/templates/supervisor.yaml Outdated
Comment thread deploy/helm/openshell/templates/supervisor.yaml Outdated
Comment thread deploy/helm/openshell/templates/supervisor.yaml
TaylorMutch
TaylorMutch previously approved these changes Oct 27, 2026
drew added 7 commits October 26, 2026 20:38
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
drew added 9 commits October 26, 2026 20:38
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
@drew
drew added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit e5adf6f Oct 27, 2026
123 of 124 checks passed
@drew
drew deleted the feat/4860-supervisor-ha/drew branch October 27, 2026 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage test:e2e-kubernetes Requires Kubernetes end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add hash-based HA recovery to shared Kubernetes supervisors

4 participants