[core][sandbox] Add a gRPC facade for external sandbox clients - #65839
Open
xyuzh wants to merge 15 commits into
Open
[core][sandbox] Add a gRPC facade for external sandbox clients#65839xyuzh wants to merge 15 commits into
xyuzh wants to merge 15 commits into
Conversation
Exposes ray.experimental.sandbox over a versioned REST API (/api/v1) served by Ray Serve, so sandboxes can be managed from outside the Ray cluster with nothing but an HTTP client and a bearer token — e.g. as an Anyscale service, or by agent-evaluation frameworks like Harbor. Design: - Each sandbox is a named, detached SandboxHost actor; the actors are the registry, so the Serve app is stateless and replicas can scale or restart without losing sandboxes. - Creation and execution are async submit + poll (with optional long-poll wait_seconds <= 30s) because image pulls and agent commands outlive HTTP requests and load-balancer limits. - The TTL reclaims both the sandbox and its hosting actor (the core runtime's TTL is deliberately disabled here so there is one owner). - Capabilities, network modes, DNS, shell, and workdir semantics are the core SandboxConfig's (ray-project#65570); the API validates network against VALID_NETWORK_MODES and defaults capabilities to DOCKER_DEFAULT_CAPABILITIES, patching nothing. - fastapi is only needed by this subpackage (ray[serve]); the base sandbox package never imports it. Testing: 54 unit tests run with no cluster and no runsc (fake runtime + fake actor resolver + FastAPI TestClient), including an OpenAPI contract snapshot; a runsc-gated integration test covers the real path. Validated end to end as a local 'serve run' and as an Anyscale service, driving real gVisor sandboxes. Signed-off-by: xyuzh <xinyzng@gmail.com>
POST /sandboxes now synthesizes its 202 response from the request instead of awaiting describe() on the new actor: on a saturated cluster the actor may be queued behind capacity for longer than a client (or load balancer) read timeout, and the endpoint has everything it needs to answer without touching the actor. Surfaced by a Harbor concurrency test that oversubscribed a single node. Signed-off-by: xyuzh <xinyzng@gmail.com>
A 16-way Terminal-Bench run against a cold cluster surfaced two capacity bugs: - The Serve deployment used the default max_ongoing_requests, but this API is long-poll based (requests deliberately hold a slot for up to ~30s), so a handful of concurrent clients saturated the replica and the platform load balancer answered 503 for everyone else. Raise it to 1000; the app is entirely async I/O. - Calls to a SandboxHost whose detached actor exists but has not been *scheduled* yet (cluster autoscaling) block indefinitely. Bound every actor call by the request's own long-poll budget plus a configurable scheduling grace: describe paths report a synthesized 'pending' instead of hanging, and exec/file paths return 409 with a retry hint. Signed-off-by: xyuzh <xinyzng@gmail.com>
An actor whose cpu/memory shape can never fit the cluster raises ActorUnschedulableError from any call; the app let it escape as an opaque 500. Terminal-Bench tasks declaring cpus=4/memory_mb=8192 on a cluster of 4CPU-16GB workers hit this for every large task. Map it to 409 'unschedulable' carrying Ray's own message, so clients see exactly which resource shape cannot be satisfied. Signed-off-by: xyuzh <xinyzng@gmail.com>
Proxies in front of a deployed service cap request bodies (an Anyscale ingress rejected a 4.8MB upload with 413 and killed an 11MB one mid-body), so single-request file uploads have a hidden size ceiling. PUT /files gains an append flag, plumbed through host, runtime, and backend (cat >> instead of cat >), so clients can chunk arbitrarily large uploads into proxy-sized pieces. Signed-off-by: xyuzh <xinyzng@gmail.com>
Match the style established in ray-project#65627 for the networking section: soft-wrapped prose, em-dash and semicolon clauses split into separate sentences, and bold list leads. Also document the new append flag on PUT /files and the 409 unschedulable error code. Signed-off-by: xyuzh <xinyzng@gmail.com>
… shared runsc cache, concurrent listing Three review findings: - A dead detached actor made its client_token permanently return 404; the idempotent-create path now clears the dead actor and creates a fresh sandbox under the same name. - The opt-in runsc download leaked a ~40MB temp dir per boot and raced concurrent boots; it now uses one shared cached path per node with an atomic rename. - GET /sandboxes described actors sequentially; it now gathers concurrently. Signed-off-by: xyuzh <xinyzng@gmail.com>
runsc exec takes numeric -user uid[:gid]; names are resolved against the image's own /etc/passwd host-side. Plumbed through runtime, the HTTP API (StartExecRequest.user), and the backend, with tests. Brings exec to parity with container engines' exec --user and the Harbor environment contract's user= parameter. Signed-off-by: xyuzh <xinyzng@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces an experimental HTTP REST API service and a Modal-compatible gRPC facade for Ray Sandbox, allowing users to manage sandboxes, execute commands, and perform file operations from outside the Ray cluster. Key feedback includes addressing a bug in the gVisor backend's user resolution logic when a group or GID is specified, preventing potential indefinite hangs during runsc downloads by replacing urlretrieve with a timed-out urlopen call, and fixing a timeout calculation bug in the Modal compatibility layer's SandboxWait endpoint that causes it to exit prematurely when no timeout is specified.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
xyuzh
force-pushed
the
sandbox-modal-facade
branch
from
September 1, 2026 18:04
e72ac4b to
9354d62
Compare
xyuzh
changed the title
…serve.run test_http_integration.py boots the API with `serve.run(build_app(...))`, but the http package did not declare a Bazel dependency on //python/ray/serve:serve_lib. Under bazel's sandboxed runfiles only a partial ray.serve namespace was present, so the test errored at setup with 'module ray.serve has no attribute run'. Declare the serve_lib dependency and grant serve_lib visibility to the sandbox http package so analysis succeeds. Signed-off-by: xyuzh <xinyzng@gmail.com>
The core sandbox test job builds with --install-mask all-ray-libraries, which removes python/ray/serve from the tree, so the //python/ray/serve:serve_lib bazel dep resolves to "no such package" and the runsc-gated integration test has no serve to import. app.py already imports serve lazily inside build_app, so the py_library needs no serve dep: drop it (and the now-moot visibility grant on serve/BUILD.bazel) and importorskip serve in the end-to-end test, which only runs in a full dev container where serve is installed. Signed-off-by: xyuzh <xinyzng@gmail.com>
Two failures surfaced once the core sandbox job started running these
tests (it installs with --install-mask all-ray-libraries):
- test_http_app: three tests drove TestClient with a bare client instead
of `with _client(...) as client:`, so each request ran on a fresh event
loop while the SandboxHost's asyncio.Events stayed bound to the first
one ("bound to a different event loop"). Wrap them like the others.
- test_http_integration: the mask leaves `ray.serve` importable but
without `serve.run`, so importorskip did not skip. Add a hasattr guard.
Signed-off-by: xyuzh <xinyzng@gmail.com>
Addresses review: a client_token create retry during a brief actor blip was misread as a permanent death and killed the live sandbox. _is_actor_gone matched ActorUnavailableError (Ray makes it a RayActorError subclass), so _actor_call mapped a transient blip to 404 sandbox_not_found — the create path then killed and recreated the actor, destroying a running sandbox. Split transient unreachability into _is_actor_unavailable, checked first (since it subclasses RayActorError), and map it to a retryable 503 sandbox_unavailable. Only genuine death (ActorDiedError / RayActorError) still maps to 404. Regression tests: test_transient_actor_unavailable_maps_to_503, test_dead_actor_maps_to_404. Signed-off-by: xyuzh <xinyzng@gmail.com>
dstrodtman
reviewed
Sep 4, 2026
dstrodtman
left a comment
dstrodtman
left a comment
Contributor
There was a problem hiding this comment.
Docs-team style pass from Douglas Strodtman (Anyscale docs). Claude Code assisted; I read every comment below and stand behind each one.
Scope: prose style, grammar, and docs conventions only. Technical accuracy — the endpoint contracts, status codes, the resource and capability semantics, the image tags in the examples — is the ray-core and ray-serve maintainers' call, not mine, and I'm not asserting anything about it.
This reads cleanly: sentence-case headings throughout, consistent serial commas, such as over like, and the #networking-and-dns anchors all resolve against the heading in this file. Two small prose nits inline, both optional. No approval implied — core and serve owners are the gate here.
…download and delete
- Resolve `name:group` exec users correctly: compare the passwd entry
against the user name, keep an explicit numeric gid, and resolve group
names via the image's /etc/group.
- Download runsc with a socket timeout instead of `urlretrieve`, which
can hang forever, and drop the partial file on failure.
- Bound `DELETE /sandboxes/{id}` like every other actor call so an
unscheduled actor is still killed instead of holding the request.
- Docs wording per the docs-team review.
Check in a minimal protobuf/grpclib stub set for the subset of a third-party sandbox SDK's control-plane and command-router services that the Ray Sandbox gRPC facade implements, generated from hand-authored `.proto` sources under `http/_proto/`. gRPC routes on the fully qualified method path and protobuf frames fields by number, so the identifiers reproduce the external service's on-the-wire names verbatim. The generated stubs are excluded from lint and formatting like the other checked-in `*_pb2.py` files.
Serve the detached `SandboxHost` actors behind the REST API over gRPC so an unmodified third-party sandbox client can create sandboxes, run commands, and use its filesystem API against a Ray cluster. - Stateless where possible: image and secret ids carry their payload and the sandbox id doubles as the client task id. Named creates are scoped to the client app and idempotent; a dead actor under a live name is killed and recreated. - Every actor call is bounded so an unscheduled actor surfaces as UNAVAILABLE (retried by the SDK) and a dead one as NOT_FOUND. - The filesystem API is emulated from the SDK's helper-binary argv with `SandboxHost` file operations; large writes flush in bounded slices. - The in-process exec table evicts finished records past a cap. Run with `python -m ray.experimental.sandbox.http.grpc_facade` (requires `grpclib`). Tests drive the facade over the wire with the vendored stubs and skip when `grpclib` is absent.
xyuzh
force-pushed
the
sandbox-modal-facade
branch
from
September 4, 2026 18:24
e41087a to
531b5f7
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit 531b5f7. Configure here.
| ) | ||
| logger.debug("exec %s on %s: %s", request.exec_id, request.task_id, command[:2]) | ||
| state.add_exec(request.exec_id, record) | ||
| await stream.send_message(sr_pb2.TaskExecStartResponse()) |
There was a problem hiding this comment.
Exec start is not idempotent
Medium Severity
TaskExecStart always starts a new SandboxHost job and overwrites state.execs[exec_id]. _bounded can return UNAVAILABLE after wait_for while the original Ray actor call is still queued, and the SDK retries that status. A retry then runs the command again and orphans the first job.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 531b5f7. Configure here.
| break | ||
| if loop.time() >= deadline: | ||
| break | ||
| await asyncio.sleep(1.0) |
There was a problem hiding this comment.
Zero-timeout wait still blocks
Low Severity
SandboxWait treats timeout=0 as the SDK’s non-blocking poll, but every loop iteration still calls describe.remote(wait_seconds=1.0) inside _bounded. A booting sandbox blocks for about a second, and an unscheduled actor can block for the full scheduling grace.
Reviewed by Cursor Bugbot for commit 531b5f7. Configure here.
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
2 participants
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.
Why are these changes needed?
Stacked on #65633 (the Ray Sandbox HTTP API); review the last two commits.
#65633 exposes the sandbox control plane as a REST service. This PR adds a gRPC facade in front of the same detached
SandboxHostactors, so an unmodified third-party sandbox client SDK pointed at this server can create sandboxes, exec commands, and use its filesystem API against a Ray cluster.The facade implements the subset of the SDK's control-plane (
ModalClient) and command-router (TaskCommandRouter) services that its Sandbox API uses. The wire contract is vendored underhttp/_proto/as a minimal protobuf/grpclib stub set generated from checked-in.protosources, so nothing third-party is needed to build or test. gRPC routes on the fully qualified method path and protobuf frames fields by number, so the vendored identifiers reproduce the external service's on-the-wire names verbatim; only the surface the facade uses is declared.Design:
im-<b64 image ref>,st-<b64 env json>) and the sandbox id doubles as the client task id. Named creates are scoped to the client app and idempotent; a dead actor under a live name is killed and recreated. The one in-process table is the exec table, which evicts finished records past a cap, so run a single facade process per cluster.SandboxHostfile operations; large writes flush in bounded slices.UNAVAILABLE(retried by the SDK) instead of hanging, and a dead actor maps toNOT_FOUND(the SDK's "task shut down" signal).Run with
python -m ray.experimental.sandbox.http.grpc_facade(requiresgrpclib). Documented in the sandboxes guide under the HTTP API service section.Commits:
[core][sandbox] Vendor the gRPC wire contract for the sandbox facade:.protosources, generated stubs, and the lint excludes for them.[core][sandbox] Add a gRPC facade for external sandbox clients: the facade, its tests, and docs.Review fixes that touch code shared with #65633 (
name:groupexec-user resolution, a bounded runsc download, a boundedDELETE, docs wording) landed on that branch and are included here through the stack.Related issue number
Stacked on #65633.
Checks
test_grpc_facade.pydrives the facade end to end over gRPC with the vendored stubs, backed by the same fake resolver and runtime seams the REST app tests use. It skips whengrpclibis absent, so the sandbox core CI job (which does not installgrpclib) adds no new dependency. Ran locally: 9 passed; the HTTP API suite (test_http_app.py,test_http_schemas.py,test_sandbox_host.py) still passes (63 passed).sandbox/__init__.pyor the RESTapp.pyimport graph, so it stays inert unless explicitly run.ruff check,ruff check --select I,black, andpydoclint(the pre-commit versions and flags) pass on the changed files.