Skip to content

[core][sandbox] Docker-parity /etc/hosts and /tmp handling - #65744

Merged
pcmoritz merged 6 commits into
ray-project:masterfrom
xyuzh:sandbox-docker-parity-fs
Aug 31, 2026
Merged

[core][sandbox] Docker-parity /etc/hosts and /tmp handling#65744
pcmoritz merged 6 commits into
ray-project:masterfrom
xyuzh:sandbox-docker-parity-fs

Conversation

@xyuzh

@xyuzh xyuzh commented Aug 26, 2026

Copy link
Copy Markdown
Member

Why are these changes needed?

Two filesystem-behavior divergences from Docker, both found running Terminal-Bench 2.1 oracle evaluations in Ray sandboxes (via the Harbor integration):

  1. localhost does not resolve. Container engines inject /etc/hosts at run time, so images do not ship a usable one — and the sandbox never provided it either. glibc consults hosts files before DNS, and public resolvers will not answer for localhost, so every task that starts a local server and connects to it fails with Name or service not known (6 of 89 TB tasks). Fix: generate a per-sandbox hosts file next to the generated resolv.conf (seeded from the node's file under host networking) and bind-mount it read-write, matching engine behavior — the source is always a per-sandbox copy, never the node's file.

  2. /tmp sits on a different device than the rootfs. runsc mounts a private tmpfs over an empty /tmp, so the common tempfile-then-rename(2) pattern fails with EXDEV where it works under Docker (plain rootfs /tmp). Fix: seed the extracted rootfs /tmp with a placeholder so runsc keeps it on the rootfs; readonly sandboxes get an explicit tmpfs mount instead, preserving the always-writable /tmp they have today.

Related issue number

Follow-up to #65570, sibling of #65737.

Checks

  • Signed off (DCO); pre-commit hooks pass on the changed files.
  • Unit tests included (test_oci_spec_docker_parity_hosts_and_tmp, test_prepare_oci_bundle_writes_hosts_file); both run without gVisor.

@xyuzh
xyuzh requested a review from a team as a code owner August 26, 2026 16:52

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces changes to improve Docker parity in the sandbox environment. Specifically, it adds support for bind-mounting a per-sandbox /etc/hosts file to ensure localhost resolves correctly, and ensures /tmp remains writable by either keeping it on the rootfs using a placeholder file or mounting an explicit tmpfs when the rootfs is read-only. The feedback suggests optimizing the placeholder file creation by checking for its existence first to avoid redundant disk writes, and handling potential UnicodeDecodeError exceptions when reading /etc/hosts by using errors="replace" to prevent sandbox creation crashes.

Comment thread python/ray/experimental/sandbox/image_manager.py
Comment thread python/ray/experimental/sandbox/image_manager.py
Comment thread python/ray/experimental/sandbox/image_manager.py
Comment thread python/ray/experimental/sandbox/image_manager.py
Comment thread python/ray/experimental/sandbox/_internal/image_utils.py Outdated
@ray-gardener ray-gardener Bot added the core Issues that should be addressed in Ray Core label Aug 26, 2026
Comment thread python/ray/experimental/sandbox/_internal/image_utils.py Outdated
xyuzh added 4 commits August 27, 2026 22:26
Two filesystem-behavior divergences from Docker, both found running
Terminal-Bench 2.1 oracle evaluations in sandboxes:

- No /etc/hosts: container engines inject one at run time, so images do
  not ship a usable file — and without it 'localhost' does not resolve
  at all (public DNS servers will not answer for it). Every task that
  starts a local server and connects to localhost fails. Generate a
  per-sandbox hosts file next to the generated resolv.conf (seeded from
  the node's file under host networking) and bind-mount it read-write,
  exactly like container engines do.

- /tmp on a different device: runsc mounts a private tmpfs over an
  empty /tmp, so the rename(2)-from-/tmp pattern that works under
  Docker fails with EXDEV. Seed the extracted rootfs /tmp with a
  placeholder so it stays on the rootfs; readonly sandboxes get an
  explicit tmpfs mount instead, keeping /tmp writable there.

Signed-off-by: xyuzh <xinyzng@gmail.com>
extract_tar_layer created directories with the umask default instead
of the archived mode, so every directory in the rootfs came out 0755
root-owned. Keeping /tmp on the rootfs (previous commit) exposed it:
a 0755 /tmp breaks every non-root writer, apt-key first among them
('Couldn't create temporary file /tmp/apt.conf.*'), failing apt-get
update across a full Terminal-Bench sweep. Apply the archived mode to
directories exactly as regular files already get it, and make the
/tmp seed enforce 1777 unconditionally as a backstop.

Signed-off-by: xyuzh <xinyzng@gmail.com>
…node

Review feedback: a node /etc/hosts with stray non-UTF-8 bytes must
not fail bundle preparation; replace undecodable bytes instead.

Signed-off-by: xyuzh <xinyzng@gmail.com>
Two review findings:

- Directory modes are now applied after the extraction loop, children
  before parents, exactly like the mtime pass (and merged with it):
  tar lists a directory before its contents, so applying a restrictive
  archived mode (0500) inline would break extracting the children.
  Preserved UsrMerge symlinks are skipped so chmod never follows a
  link. Regression test with a 0500 directory included.
- The /tmp placeholder that keeps runsc from mounting its private
  tmpfs no longer leaks into sandboxes: writable sandboxes remove it
  from their overlay right after boot (the shared cache stays seeded),
  and readonly sandboxes already hide it under the explicit tmpfs.

Signed-off-by: xyuzh <xinyzng@gmail.com>
@xyuzh
xyuzh force-pushed the sandbox-docker-parity-fs branch from 47ac0aa to a5a3800 Compare August 28, 2026 05:29

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit a5a3800. Configure here.

Comment thread python/ray/experimental/sandbox/_internal/image_utils.py
Comment thread python/ray/experimental/sandbox/backend/gvisor.py Outdated
@xyuzh xyuzh added the go add ONLY when ready to merge, run all tests label Aug 28, 2026
Signed-off-by: xyuzh <xinyzng@gmail.com>
@xyuzh

xyuzh commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

Filed the upstream ask for issue 2 so the .ray-sandbox-keep seeding can eventually go away: google/gvisor#14474 — a minimal Ray-free reproduction (same spec, one file in the rootfs /tmp toggles tmpfs-and-EXDEV vs rootfs-and-OK) plus a request for a --tmp-mount knob. Until gVisor grows that, the seed + post-boot cleanup in this PR is the workaround, now annotated by the issue.

One empty dotfile in scratch space is harmless, and dropping the
post-boot runsc exec saves a subprocess round-trip on every sandbox
start. Readonly sandboxes still hide it under their tmpfs.
@xyuzh
xyuzh requested a review from pcmoritz August 31, 2026 18:31
@pcmoritz
pcmoritz merged commit 5e834fb into ray-project:master Aug 31, 2026
6 checks passed
pcmoritz added a commit that referenced this pull request Sep 1, 2026
## Why are these changes needed?

Sandbox image pulls are anonymous Docker Hub pulls. A cluster of nodes
pulling distinct multi-GB benchmark images concurrently runs straight
into Docker Hub's anonymous rate limits and pays WAN latency on every
node — during a Terminal-Bench 2.1 evaluation under Harbor, big-image
tasks failed in the concurrent sweep but passed in isolation.

`RAY_SANDBOX_REGISTRY_MIRROR` names a registry that mirrors Docker Hub,
as `host[:port][/repo-prefix]`:

- an **ECR pull-through cache**
(`<acct>.dkr.ecr.<region>.amazonaws.com/dockerhub`),
- an **Artifact Registry remote repository**, or
- an in-cluster `registry:2` proxy.

Docker Hub pulls are rewritten to the mirror (the prefix prepended to
the repository, as ECR requires); other registries pass through
untouched. This mirrors Docker's own `registry-mirrors` semantics, minus
the fallback: when set, the mirror is authoritative, and it uses the
same anonymous token flow as any registry.

## Related issue number

Follow-up to #65570; sibling of #65737 and #65744.

## Checks

- [x] Signed off (DCO); pre-commit hooks pass on the changed files.
- [ ] Unit test included
(`test_registry_mirror_rewrites_docker_hub_only`); runs without gVisor
or network.

---------

Signed-off-by: xyuzh <xinyzng@gmail.com>
Signed-off-by: Philipp Moritz <pcmoritz@gmail.com>
Co-authored-by: Philipp Moritz <pcmoritz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Issues that should be addressed in Ray Core go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants