Skip to content

perf(kubernetes): use a TCP readiness probe for the supervisor - #3700

Open
FrostGod wants to merge 1 commit into
NVIDIA:mainfrom
FrostGod:perf/k8s-supervisor-tcp-readiness
Open

perf(kubernetes): use a TCP readiness probe for the supervisor#3700
FrostGod wants to merge 1 commit into
NVIDIA:mainfrom
FrostGod:perf/k8s-supervisor-tcp-readiness

Conversation

@FrostGod

@FrostGod FrostGod commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Kubernetes now checks supervisor readiness by connecting to TCP port 5501
  • Stop starting a supervisor process in every sandbox each second
  • The supervisor opens the port only while its gateway session is up
  • Keep the health socket for Docker, Podman, and debugging
  • Add tests and update the docs

Related Issue

localized performance fix. The Kubernetes supervisor readiness probe switches from exec to a TCP check.

Changes

  • Kubernetes now checks supervisor readiness with a tcpSocket probe on port 5501, instead of starting openshell-supervisor health in every sandbox each second (about 30m CPU per idle sandbox).
  • The supervisor accepts connections on that port only while its gateway session is up. The Unix health socket stays for Docker, Podman and debugging.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated: new TCP readiness test (open → session lost → session back → shutdown); pod test now requires the TCP check and forbids an exec check

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable): architecture/build.md, plus docs/kubernetes/sandbox-runtime.mdx for the port 5501 requirement

@copy-pr-bot

copy-pr-bot Bot commented Sep 25, 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 force-pushed the perf/k8s-supervisor-tcp-readiness branch from d9fe4d7 to 41dda1f Compare September 25, 2026 02:46
@drew drew added the test:e2e Requires end-to-end coverage label Sep 25, 2026
@github-actions

github-actions Bot commented Sep 25, 2026

Copy link
Copy Markdown

Label test:e2e applied, but pull-request/3700 does not exist yet. A maintainer needs to comment /ok to test 41dda1fde2920b6638d877868539843ad81d07e4 to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

@drew

drew commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 41dda1f

@drew drew added test:e2e-kubernetes Requires Kubernetes end-to-end coverage and removed test:e2e Requires end-to-end coverage labels Sep 25, 2026
@github-actions

github-actions Bot commented Sep 25, 2026

Copy link
Copy Markdown

Label test:e2e-kubernetes applied for 41dda1f. 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.

drew
drew reviewed Sep 25, 2026
View reviewed changes

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

gator-agent

PR Review Status

This is a project-valid, focused Kubernetes performance fix with the relevant architecture and Fern documentation updated. The initial code review found one blocking readiness regression for IPv4 probes in network namespaces configured with IPv6-only wildcard sockets.

Action required: @FrostGod, please make the TCP readiness listener cover the Pod's active IP family and add the bindv6only=1 regression case.

Blocking findings:

  • GATOR-41dda1fd-01: See the inline review comment.

Carried findings:

  • None

Non-blocking suggestions:

  • None
Gator metadata
  • Validation: Localized Kubernetes and supervisor performance fix with a clear runtime cost and test path
  • Docs: Fern and architecture docs updated
  • Checks: Current-head required checks are still running
  • E2E: test:e2e-kubernetes is applied; its helper requests a current-head workflow rerun after review feedback is resolved
  • Head SHA: 41dda1fde2920b6638d877868539843ad81d07e4
  • Base SHA: 1374672967c002400f43c0e9c891f7429ccfd236
  • Merge base SHA: 1374672967c002400f43c0e9c891f7429ccfd236
  • Patch ID: 57cf6256cba431ef5924e01651b6efbba5739720
  • Gator payload: 10
  • Review mode: initial
  • Previous reviewed SHA: none
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:in-review

Comment thread crates/openshell-supervisor/src/lib.rs Outdated
@drew drew added the gator:in-review Gator is reviewing or awaiting PR review feedback label Sep 25, 2026
@FrostGod
perf(kubernetes): use a TCP readiness probe for the supervisor
980a55d 
- Kubernetes now checks supervisor readiness by connecting to TCP port 5501
- Stop starting a supervisor process in every sandbox each second
- The supervisor opens the port only while its gateway session is up
- Accept IPv4 and IPv6 probes, even when net.ipv6.bindv6only is set
- Keep the health socket for Docker, Podman, and debugging
- Add tests and update the docs

Signed-off-by: divesh <dgude@nvidia.com>
@FrostGod
FrostGod force-pushed the perf/k8s-supervisor-tcp-readiness branch from 41dda1f to 980a55d Compare September 25, 2026 06:13
@drew

drew commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 980a55d

drew
drew reviewed Sep 25, 2026
View reviewed changes

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

gator-agent

PR Review Status

Thanks @FrostGod. I checked your readiness-listener update: the IPv6 socket now explicitly clears IPV6_V6ONLY, retains the IPv4 fallback when IPv6 is unavailable, and adds an IPv4-connect regression check. That resolves GATOR-41dda1fd-01, and the follow-up review found no new blocking issues.

Blocking findings:

  • No blocking findings remain

Carried findings:

  • None
Gator metadata
  • Validation: Localized Kubernetes and supervisor performance fix with a clear runtime cost and test path
  • Docs: Fern and architecture docs updated
  • Checks: The current-head mirror was refreshed; required workflow runs are not yet confirmed queued
  • E2E: test:e2e-kubernetes is applied and awaits a current-head workflow run
  • Head SHA: 980a55d2363fe90ff24ea274cdf231b53734ef1a
  • Base SHA: 7a50c0899f12a97d6f9bc7de415903c451865e5a
  • Merge base SHA: 1374672967c002400f43c0e9c891f7429ccfd236
  • Patch ID: e820c33277ce963103dc5ec7bbcddf27d5c9b1b0
  • Gator payload: 10
  • Review mode: follow_up
  • Previous reviewed SHA: 41dda1fde2920b6638d877868539843ad81d07e4
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:in-review

@drew drew added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status labels Sep 25, 2026
@FrostGod

FrostGod commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

seems like flaky tests

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

Reviewers

@drew drew drew left review comments
@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner
@sjenning sjenning Awaiting requested review from sjenning sjenning is a code owner
@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr derekwaynecarr is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

gator:blocked Gator is blocked by process or repository gates test:e2e-kubernetes Requires Kubernetes end-to-end coverage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@FrostGod @drew