fix(ssh): add EMFILE backoff and exit notification to SSH accept loop - #2705
Open
politerealism wants to merge 8 commits into
Open
fix(ssh): add EMFILE backoff and exit notification to SSH accept loop#2705politerealism wants to merge 8 commits into
politerealism wants to merge 8 commits into
Conversation
…edly Add a oneshot notification channel to ProxyHandle that fires when the proxy task exits for any reason (panic, abort, or loop break). The sandbox main loop now races this signal alongside the entrypoint process and shutdown signals, terminating with a clear OCSF error if the proxy dies. This prevents the sandbox from continuing to report Ready with a dead proxy. Follows the existing sidecar control channel pattern. All five wait paths (process+sidecar, process-only, network+sidecar, network-only on Linux, and non-Linux) now monitor proxy liveness. Refs NVIDIA#2337 Signed-off-by: Sean Burdine <sburdine@nvidia.com> Signed-off-by: politerealism <burdcat17@gmail.com>
…it_receiver Verify the oneshot drop-guard pattern fires on normal task exit and abort, and document the take_exit_receiver contract including the double-call hazard. Signed-off-by: politerealism <burdcat17@gmail.com>
…unrecoverable failures Extract accept-loop error handling into classify_accept_error() with AcceptAction enum so the decision logic is independently testable. Terminal errors (EBADF/EINVAL/ENOTSOCK) now exit the loop immediately, allowing the supervisor to detect proxy death. Unknown errors exit after 10 consecutive failures. FD exhaustion retries with exponential backoff capped at 5s (fix unreachable cap from min(6) to min(7)). Adds 9 unit tests covering classification, counter interactions, backoff progression, and bounded exit behavior. Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Replace raw errno numbers with libc:: constants behind #[cfg(unix)] gates in classify_accept_error and its tests. Refactor ProxyHandle::exited_rx to Option<Receiver<()>> so take_exit_receiver uses .take() instead of allocating a dummy sender. Add a ProxyHandle-level test that verifies the full drop → abort → receiver fires path. Signed-off-by: Quinn Burdine <sburdine@redhat.com> Signed-off-by: politerealism <burdcat17@gmail.com>
The refactored classifier only recognized EMFILE/ENFILE as retryable, routing all other documented transient accept errors (ENOBUFS, ENOMEM, ECONNABORTED, ECONNRESET, EINTR, ENETDOWN, etc.) through the unknown- error budget which terminates the proxy after 10 consecutive failures. Restore the complete transient errno table from main so these errors retry indefinitely with appropriate backoff. Signed-off-by: Quinn Burdine <sburdine@redhat.com> Signed-off-by: politerealism <burdcat17@gmail.com>
Replace redundant closure |p| p.take_exit_receiver() with the method reference ProxyHandle::take_exit_receiver, and change all five select! arms from _ = &mut proxy_exited to () = &mut proxy_exited to satisfy clippy::ignored_unit_patterns. Also run cargo fmt on the new proxy test in proxy.rs. Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: Quinn Burdine <sburdine@redhat.com>
politerealism
requested review from
a team,
derekwaynecarr,
maxamillion and
mrunalp
as code owners
August 11, 2026 17:19
|
Thank you for your submission! We ask that you all sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text: I have read the DCO document and I hereby sign the DCO. 1 out of 2 committers have signed the DCO. |
Contributor
Author
|
Once #2370 merges into main, this branch will need to be rebased onto the updated main and force-pushed. The rebase should be clean — the proxy changes and SSH changes don't overlap. |
Apply the same two-layer defense from the proxy accept loop (NVIDIA#2369/NVIDIA#2370) to the SSH accept loop: classify transient vs terminal accept errors with exponential backoff on EMFILE/resource-exhaustion, and notify the sandbox when the accept loop exits so the container terminates instead of running without SSH access. - Add SshAcceptAction enum and classify_ssh_accept_error in ssh.rs, mirroring the proxy pattern (EMFILE/ENFILE/ENOBUFS → Retry with backoff, unknown errors → Terminal after 10 consecutive failures) - Replace the bare accept().await in run_ssh_server with a classify-and- retry loop; resets consecutive-error counter on each successful accept - Thread ssh_exit_tx: Option<oneshot::Sender<()>> through run_process; hold it as a drop-guard inside the SSH spawn so the receiver fires when the task ends for any reason - Wire ssh_exited future in lib.rs (created only when ssh_socket_path is Some) and select! on it in both process_enabled paths, returning an error so the sandbox container restarts Closes NVIDIA#2372 Signed-off-by: Quinn Burdine <119441270+politerealism@users.noreply.github.com>
politerealism
force-pushed
the
fix/ssh-accept-backoff-and-exit-notification
branch
from
August 11, 2026 18:11
0210986 to
3e0c305
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Applies the same two-layer defense from the proxy accept loop (#2369 / #2370) to the SSH accept loop:
Retry(EMFILE/ENFILE/ENOBUFS — exponential backoff) vsTerminal(≥10 consecutive unknown errors → break). Mirrorsclassify_accept_errorinopenshell-supervisor-network.oneshot::Sender<()>drop-guard throughrun_processso the sandbox is notified when the SSH task ends for any reason. The sandbox selects onssh_exitedin bothprocess_enabledpaths and returns an error, causing Kubernetes to restart the container rather than leaving it running without SSH access.Related Issue
Closes #2372
Changes
crates/openshell-supervisor-process/src/ssh.rsaccept().await?with classify-and-retry loop; addSshAcceptActionenum andclassify_ssh_accept_errorcrates/openshell-supervisor-process/src/run.rsssh_exit_tx: Option<oneshot::Sender<()>>parameter; hold as drop-guard in SSH spawn blockcrates/openshell-sandbox/src/lib.rs(ssh_exit_tx, ssh_exit_rx)channel whenssh_socket_pathisSome, pinssh_exitedfuture, select on it in bothprocess_enabledpathsTesting
cargo clippy -p openshell-sandbox -p openshell-supervisor-process— cleancargo fmt --check— cleanChecklist
Signed-off-bypresent