Skip to content

fix(supervisor): guard shared supervisor pool configuration - #4988

Merged
drew merged 3 commits into
NVIDIA:mainfrom
FrostGod:fix/supervisor-pool-config-guardrails/FrostGod
Oct 28, 2026
Merged

drew merged 3 commits into
NVIDIA:mainfrom
FrostGod:fix/supervisor-pool-config-guardrails/FrostGod

Conversation

@FrostGod

Copy link
Copy Markdown
Collaborator

Summary

Three fixes found while testing #4864: reject a pool recovery timeout shorter than member failure detection, reject a supervisor_pool table no driver uses in preflight and before driver startup, and deflake a supervisor startup test.

Related Issue

No issue. Observed while testing #4864:

  • A recovery timeout of 10 to 23 s expires before the gateway can detect a failed member (worst case about 24 s), so the runtime ends workloads another member would have adopted.
  • config preflight accepted [openshell.gateway.supervisor_pool] without supervisor_topology = "gateway". Startup rejected it only after building the driver.
  • shared_startup_uses_selected_backend_through_readiness_and_shutdown hit its 10 s readiness deadline under full-workspace load.

Changes

  • Gateway and Helm accept recovery_timeout_secs / supervisor.recoveryTimeoutSeconds from 30 to 300 (was 10 to 300). The Sandbox Protocol range stays 10 to 300. A test pins the minimum above worst-case detection.
  • Add ComputeDriverFactory::uses_shared_supervision (default false; Kubernetes returns true for the gateway topology). Preflight for the selected driver and startup before build reject an unused pool table with the existing messages.
  • Widen that test's readiness deadline to 30 s and its child deadline to 1 min.
  • Update the Helm values, chart README, HA docs page, and debug-openshell-cluster skill.

Testing

  • cargo clippy -p openshell-supervisor-control -p openshell-server -p openshell-gateway -p openshell-supervisor --all-targets -- -D warnings
  • cargo nextest run on the same crates: 2816 passed
  • helm unittest deploy/helm/openshell: 424 passed
  • Manual openshell-gateway config preflight on Helm-rendered configs: shared topology passes; a pool table without it and a 20 s timeout fail; startup fails before building the driver
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

Raise the gateway's minimum supervisor pool recovery timeout from 10 to
30 seconds. A failed pool member takes up to about 24 seconds to detect,
so a shorter deadline ended workloads another member would have adopted.
The Sandbox Protocol range stays 10 to 300 seconds for compatibility.

Refuse an [openshell.gateway.supervisor_pool] table during config
preflight, and at startup before the compute driver is built, unless the
selected driver hosts sandboxes on the shared supervisor pool. A new
ComputeDriverFactory::uses_shared_supervision hook reports this; the
Kubernetes factory returns true with supervisor_topology = "gateway".

Widen the readiness deadline of a shared-startup supervisor test that
timed out under full-workspace load.

Signed-off-by: divesh <dgude@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 27, 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 requested a review from drew October 27, 2026 23:26
Comment thread crates/openshell-server/src/cli.rs
drew added 2 commits October 27, 2026 20:29
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
@drew
drew enabled auto-merge October 28, 2026 03:52
@FrostGod

Copy link
Copy Markdown
Collaborator Author

/ok to test 24648e9

@drew
drew added this pull request to the merge queue Oct 28, 2026
Merged via the queue into NVIDIA:main with commit 44ef984 Oct 28, 2026
83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants