Skip to content

[Core] Upgrade the bundled Ray to 2.56.1 - #10585

Open
DanielZhangQD wants to merge 5 commits into
masterfrom
sky-6826-ray-upgrade
Open

[Core] Upgrade the bundled Ray to 2.56.1#10585
DanielZhangQD wants to merge 5 commits into
masterfrom
sky-6826-ray-upgrade

Conversation

@DanielZhangQD

@DanielZhangQD DanielZhangQD commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #10582 — please review that one first; this PR's base is its branch, and the diff here is only the three commits below.

Summary

Ray 2.9.3 is from January 2024 and carries CVE-2025-62593, a Ray dashboard RCE (CVSS 9.4, listed in CISA KEV) fixed in Ray 2.52.0. This upgrades the bundled Ray to 2.56.1, the newest release that still supports Python 3.8–3.11.

Three commits:

1. [CI] Stop the whitespace hooks from rewriting the Ray patch diffs

trailing-whitespace and end-of-file-fixer have no excludes, and the Ray patches are diff files: a blank context line is a line holding a single space, so stripping it leaves a hunk that disagrees with its own @@ header. Staging a regenerated patch is enough to trigger it. This has been dormant only because pre-commit sees staged files and these are rarely touched — the copy on master carries such a line today.

Also adds ignore-paths for sky/backends/monkey_patches, matching the exclude mypy already has: those files are templates with {} placeholders, which pylint reads as undefined variables. (ignore does not cover it — that matches base names while recursing, not a path passed explicitly, which is how pre-commit invokes pylint.)

2. [Core] Pin the Ray version to the cluster, not to the constant

Whether to move to a new Ray is a property of the cluster, not of each node. Deciding it per node lets a worker upgrade on its own while the head keeps the old version: the head's ray status guard succeeds and skips the install, a worker's cannot (it dials 127.0.0.1, where only the head runs a GCS), and the raylet then crash-loops against a mismatched default_worker.py.

So the version a cluster was set up with is recorded under provider — Ray's cluster schema is additionalProperties: False at the top level, and provider is already restored wholesale for existing clusters — and rendered on later launches. A cluster only moves when nothing is running to disagree: sky stop followed by sky start.

Two things this commit gets right that are easy to get wrong, both found by running it rather than by reading it:

  • Recording the version is not enough on its own. RAY_INSTALLATION_COMMANDS baked the version in at import time, so the YAML said one version and installed another. It takes a {ray_version} placeholder now, substituted where the wheel hash already is.
  • The patch step keeps guarding on SKY_REMOTE_RAY_VERSION, not the cluster's pin. The install guard asks "is the node on the version this cluster wants?"; the patch guard asks "is the node's Ray what the shipped patch files target?". On a cluster that predates a bump those differ, and answering both with the cluster's pin applies the new patches to the old Ray — which the node cannot recover from, because the install guard then skips.

Kubernetes installs Ray from the pod args rather than from setup_commands, so it needs the same two strings; they move out of Kubernetes.make_deploy_resources_variables, which cannot know the cluster's pin, into write_cluster_config, which can. #10582's assertion that the deploy variable is actually produced moves with them.

While SKY_REMOTE_RAY_VERSION is unchanged this commit is a no-op — every path resolves to the same version — so it can be reviewed and reverted on its own.

3. [Core] Upgrade the bundled Ray to 2.56.1

The seven patches were re-triaged against the new source rather than regenerated blindly:

patch verdict
cli.py deleted — it rewrote ray job submit output, and nothing has called ray job submit since #4318
resource_demand_scheduler.py one of its two hunks dropped: the assert not unfulfilled it removed is gone upstream
log_monitor.py, worker.py still needed, regenerated against ray-2.56.1
autoscaler.py, command_runner.py, updater.py still needed, regenerated — but only reachable under ProvisionerVersion.RAY_AUTOSCALER

monkey_patch_hash_launch_conf now delegates to Ray's own hash_launch_conf instead of copying it. ray up runs against whatever Ray the client has installed, and Ray has already changed this digest once (sha1/hexdigest → sha256/base32hex); a copy silently disagrees with the hash Ray's own node_launcher writes for worker nodes. Only the ssh_proxy_command exclusion stays ours.

Images and docs are bumped alongside, with a test asserting every such pin still agrees with SKY_REMOTE_RAY_VERSION — a stale one fails silently today, since the patch guards simply skip.

Incidental: the hook in commit 1 strips one pre-existing trailing space from monkey_patch_ray_up.py's docstring. Unrelated to the change, but not something the hook will let us keep.

Test plan

Unit

pytest tests/unit_tests/test_sky/test_ray_version_per_cluster.py \
       tests/unit_tests/test_sky/test_ray_version_consistency.py \
       tests/unit_tests/test_sky/provision/test_ray_patches_payload.py \
       tests/unit_tests/test_sky/clouds/test_kubernetes_ray_template_snapshot.py

74 passed. Both new behaviours were revert-checked: pointing the patch guard at the cluster's pin, and dropping the per-cluster version, each turn test_an_existing_cluster_installs_its_own_ray_but_patches_for_ours red.

Commit 2 was also run in isolation (detached worktree at that commit): 20 passed.

tests/unit_tests/test_sky/clouds/test_kubernetes.py::TestKubernetesMakeDeployResourcesVariables::test_remote_identity_with_cluster_overrides fails, but it fails identically on master.

End-to-end, on real clusters

Two source trees swapped over one runtime dir by an isolated API server, so an in-place SkyPilot upgrade is reproducible. Every arm switch is asserted from inside the system (the served sky.__file__ and SKY_REMOTE_RAY_VERSION), never inferred from which command was typed.

AWS, 2× t3.small, before this change — cluster built by the old tree, then relaunched by the new one:

node Ray result
head 2.9.3 skipped the upgrade (ray status succeeded)
worker 2.56.1 upgraded on its own
(raylet, ip=…) default_worker.py: error: the following arguments are required: --node-id, --worker-id

AWS, same shape, with this change — both directions:

# relaunch of the existing cluster
RANK=0 (head)    ray=2.9.3   log_monitor=PATCHED   orig=…-v2.9.3.orig
RANK=1 (worker1) ray=2.9.3   log_monitor=PATCHED   orig=…-v2.9.3.orig
Job finished (status: SUCCEEDED)      # rendered YAML: provider.sky_ray_version: 2.9.3

# after sky stop + sky start
RANK=0 (head)    ray=2.56.1  log_monitor=PATCHED
RANK=1 (worker1) ray=2.56.1  log_monitor=PATCHED
Job finished (status: SUCCEEDED)

Kubernetes, 1-node and 2-node: cluster up, job runs, both nodes on Ray 2.56.1 with all six patches applied and .orig backups named -v2.56.1. Relaunching a cluster built by the old tree keeps both pods on 2.9.3, and a deleted worker pod comes back on 2.9.3 and rejoins — node_config is in the restore set, so Kubernetes already carried the old version forward implicitly. The exposure this PR fixes is VM-only; the fix simply makes the same behaviour explicit on both.

Not covered

  • IBM. ProvisionerVersion.RAY_AUTOSCALER is IBM alone, and it is the only consumer of the autoscaler / command_runner / resource_demand_scheduler / updater patches and of monkey_patch_ray_up.py. Nothing here exercises them — no IBM account. Worth a reviewer with access, or a smoke run.
  • Kubernetes after the per-cluster pin. The Kubernetes runs above predate commit 2. It should be a no-op there — the version is already held in place by the node_config restore — but that has not been measured.
  • docker-on-VM (DockerCommandRunner) and the image bake (Dockerfile_k8s, the skypilot.sh AMI). The image builds belong to the release sequence rather than to this PR; until they ship, a cluster on a pre-bump image installs 2.56.1 over the baked Ray at launch time, which is the existing behaviour for any version skew.
  • The worker.py patch's behaviour (\r-terminated output staying on one line) — the tests assert the patch is applied, not that it still does what it exists to do.

Devin Review

Review pass (self-review before this was opened for review)

Three things came out of it, all fixed in the commits above.

The version marker moved config_hash. It is written to the cluster YAML before _deterministic_cluster_yaml_hash runs, so every pre-existing cluster — which has no marker in its stored YAML — would have produced a different hash from the one the previous launch stored, and lost its sky launch --fast skip exactly once. Measured, not reasoned:

without marker: 9f66b46e92f69273425bdb2908e31e8b3a0f7ee44a301eaed24bb1df5e8ddef8
with marker   : 848003a5be2aa826de4a53935d04e41d71f226c42b2260b81e08ec80193b6a24

('provider', _RAY_VERSION_KEY) now joins _RAY_YAML_KEYS_TO_REMOVE_FOR_HASH, whose stated purpose is exactly this: keys that change on an existing cluster without meaning a re-provision. A real version change still moves the hash, through the install commands themselves. This also makes the dryrun hash agree with the real one — dryrun returns before the marker is written.

A pinned cluster said nothing about why. A cluster that stays on the old Ray had no way to tell the user, which is the same complaint #10582 got about its silent patch skip. write_cluster_config now says which version the cluster is pinned to, and how to move it.

How to move it is cloud-dependent, and the first version of this got it wrong by naming sky stop unconditionally. sky stop + sky start is the only in-place path, and a good many clouds cannot stop at all — Kubernetes, Slurm, RunPod, Lambda, Cudo, Fluidstack, and AWS/GCP for spot, among others — so there the version is fixed for the life of the cluster and that advice points at a command that errors out. The notice now asks the cloud, through check_features_are_supported(..., {STOP}), which is resources-aware as well as cloud-aware (AWS on-demand can stop, AWS spot cannot):

  • can stop → info, naming sky stop / sky start. Actionable, and transient: one stop/start clears it.
  • cannot stop → debug, naming sky down instead. Not actionable in place and permanent for that cluster, so it does not belong in every launch's output.

Note this is not a new restriction on Kubernetes. A Kubernetes cluster already carried its old Ray forward through the node_config restore, before this PR — the marker only makes the same behaviour explicit and gives the other clouds the same property.

Why a restart moves the version at all, given every cloud gets the same YAML restore. It comes down to where each template puts the install, checked across all 23 *-ray.yml.j2:

template Ray install lives under in the restore set? restart moves the version?
kubernetes-ray.yml.j2 available_node_typesnode_config yes no — and Kubernetes cannot sky stop either, so nothing does
the other 22 top-level setup_commands no yes

_RAY_YAML_KEYS_TO_RESTORE_FOR_BACK_COMPATIBILITY is {cluster_name, provider, auth, node_config, docker}setup_commands is not in it, so on every VM cloud the newly rendered install command survives the restore and a stopped cluster comes back on the new Ray. Measured on AWS: relaunch keeps 2.9.3, sky stop + sky start puts both nodes on 2.56.1.

That asymmetry is the load-bearing assumption of this whole design and it was previously only prose plus one AWS run, so it is a test now: node_config is restored, setup_commands is not, and every template installs Ray under setup_commands except the Kubernetes one. A new cloud template that inlined the install into node_config would silently acquire Kubernetes' behaviour; that now fails.

test_ray_version_consistency missed one of the four files this PR had to update. Its docstring claims every place pinning a Ray version, but docs/source/reference/architecture/internals.rst states the version in prose, which neither the pip nor the shell pattern matches — it would have gone stale on the next bump without anything failing. Covered now by a third pattern.

Also verified against Ray 2.56.1's own source rather than taken on trust: hash_launch_conf is sha256 + base32hex there, where the copy in monkey_patch_ray_up.py was sha1 + hexdigest — so on the IBM path the copy would have disagreed with the hash Ray's own node_launcher writes for worker nodes. And resource_demand_scheduler.py no longer contains the assert not unfulfilled that patch used to comment out, which is why that hunk is dropped rather than regenerated.

Each new behaviour was revert-checked: putting the marker back into the hash, pointing the patch guard at the cluster's pin, dropping the per-cluster version, and letting internals.rst go stale each turn the corresponding test red.

@devin-ai-integration devin-ai-integration 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.

Devin Review found 1 potential issue.

Devin Review

Comment thread sky/backends/monkey_patches/monkey_patch_ray_up.py
@DanielZhangQD

Copy link
Copy Markdown
Collaborator Author

Fair, and it was the riskiest untested change here — so I dug into it rather than just re-declaring the gap.

Verified against Ray 2.56.1's own source (ray-2.56.1 tag, not the locally installed Ray):

  • _private/util.pyhash_launch_conf(node_conf, auth) is hashlib.sha256() + base32hex(hasher.digest()). The copy this replaces was sha1 + hexdigest, so on the IBM path it would have disagreed with the hash Ray's own node_launcher writes for worker nodes. That divergence is the reason for the change, not a side effect of it.
  • _private/commands.py:60 still does from ...util import (hash_launch_conf, ...), and line 791 calls the bare name — so the script's sdk.sdk.commands.hash_launch_conf = ... assignment still lands on the name Ray actually calls. Same for _should_create_new_head (defined at 999, called at 792). Had either become a qualified util.hash_launch_conf(...) call, the patch would have become a silent no-op and the ssh_proxy_command exclusion would have stopped working — worth checking, and it holds.
  • No recursion: the patch overwrites commands.hash_launch_conf while the new implementation reads util.hash_launch_conf.

And it is no longer untested. tests/unit_tests/test_sky/backends/test_monkey_patch_ray_up.py lifts the function out of the script's AST — it cannot be imported, since it is a template that runs ray up at import — and covers four things: that the names the script assigns onto still exist on commands; that the hash is produced by Ray's function rather than reimplemented; that ssh_proxy_command, and nothing else, is stripped from what reaches it; and that it does not recurse into the patched name.

The delegation assertion is deliberately "Ray's function produced this answer", not "equals Ray's output". My first version asserted the latter and the revert check exposed it as useless: with Ray 2.46 installed locally, a reintroduced sha1 copy still matched, because 2.46 predates the digest change. A value assertion only bites when the installed Ray is new enough — which is exactly when the test is needed least. All four now fail when the delegation is reverted to a copy, and the ssh_proxy_command one also fails if the exclusion is dropped.

What this still does not cover, and the PR description says so: the IBM provisioning path end to end. Nothing here calls ray up. If someone with IBM access can run a launch against this branch, that would close it properly.

@DanielZhangQD
DanielZhangQD force-pushed the sky-6826-ray-upgrade branch 4 times, most recently from 4316d57 to 3273b4a Compare August 28, 2026 14:45
@DanielZhangQD
DanielZhangQD force-pushed the sky-6826-k8s-patch-src branch from fb4fb86 to f5ed862 Compare August 31, 2026 01:20
@DanielZhangQD
DanielZhangQD force-pushed the sky-6826-ray-upgrade branch 2 times, most recently from 5061ac6 to e9daeb9 Compare August 31, 2026 01:48
Base automatically changed from sky-6826-k8s-patch-src to master August 31, 2026 07:57
The Ray patches are diff files: a blank context line is a line holding a
single space, so trailing-whitespace and end-of-file-fixer edit them into
hunks that disagree with their own @@ headers. They have never been caught
because pre-commit only sees staged files and these are rarely touched --
the copy on master carries such a line today.

Also skip sky/backends/monkey_patches in pylint, matching the exclude mypy
already has: those files are templates with {} placeholders, so pylint reads
them as undefined variables.
Whether to move to a new Ray is a property of the cluster, not of each node.
Deciding it per node lets a worker upgrade on its own while the head keeps
the old version: the head's `ray status` guard succeeds and skips the
install, a worker's cannot -- it dials 127.0.0.1, where only the head runs a
GCS -- and the raylet then crash-loops against a mismatched
default_worker.py. Reproduced on a two-node AWS cluster relaunched by a
server with a newer pin.

So record the version a cluster was set up with, under 'provider' (Ray's
cluster schema is additionalProperties=False at the top level, and 'provider'
is already restored wholesale for existing clusters), and render that version
on later launches. A cluster only moves when nothing is running to disagree:
`sky stop` followed by `sky start`.

Recording it is not enough on its own -- RAY_INSTALLATION_COMMANDS baked the
version in at import time, so the YAML said one version and installed
another. It takes a {ray_version} placeholder now, substituted where the
wheel hash already is.

The patch step keeps guarding on SKY_REMOTE_RAY_VERSION rather than the
cluster's pin: those ask different questions. The install guard asks whether
the node runs the version this cluster wants; the patch guard asks whether it
runs the version the shipped patch files were generated against. On a cluster
that predates a bump they differ, and answering both with the cluster's pin
applies the new patches to the old Ray -- which it cannot recover from, since
the install guard then skips.

Kubernetes installs Ray from the pod args rather than from setup_commands, so
it needs the same two strings; they move out of
Kubernetes.make_deploy_resources_variables, which cannot know the cluster's
pin, into write_cluster_config, which can.
Ray 2.9.3 is from January 2024 and carries CVE-2025-62593, a dashboard RCE
(CVSS 9.4, in CISA KEV) fixed in 2.52.0. 2.56.1 is the newest release that
still supports Python 3.8-3.11.

The seven patches were re-triaged against the new source rather than
regenerated blindly:

- cli.py is deleted. It rewrote `ray job submit` output, and nothing has
  called `ray job submit` since #4318.
- resource_demand_scheduler loses one of its two hunks: the `assert not
  unfulfilled` it removed is gone upstream.
- The remaining five still apply, and are regenerated against ray-2.56.1.
  Four of them (autoscaler, command_runner, resource_demand_scheduler,
  updater) only run under ProvisionerVersion.RAY_AUTOSCALER, which is IBM
  alone -- untested here, see the PR description.

monkey_patch_hash_launch_conf now delegates to Ray's own hash_launch_conf
instead of copying it. `ray up` runs against whatever Ray the client has, and
Ray has already changed this digest once (sha1/hexdigest -> sha256/base32hex);
a copy silently disagrees with the hash Ray's node_launcher writes for worker
nodes. Only the ssh_proxy_command exclusion stays ours.

Images and docs are bumped alongside, and a test asserts every such pin still
agrees with SKY_REMOTE_RAY_VERSION -- a stale one fails silently, since the
patch guards simply skip.
_log_pinned_ray_version asks the cloud whether it can stop a cluster, purely
to choose between "run sky stop/sky start" and "recreate it". On Kubernetes
that question is answered by loading the kubeconfig and probing the context's
API, and only KubeAPIUnreachableError is caught in there -- a context missing
from the kubeconfig raises a bare ValueError, which propagated out of
write_cluster_config and failed the launch. On `sky launch --dryrun` too: the
notice sits well above the dryrun return.

So catch everything and fall back to saying nothing, and pass the region
write_cluster_config already has, so Kubernetes probes the one context this
cluster targets instead of enumerating every allowed one. The answer never
depended on any of it -- kubernetes.py assigns unsupported_features[STOP]
unconditionally, before the context loop.

Also: a stored cluster YAML carrying a bare `provider:` parses to
{'provider': None}, so reading the version marker off it raised
AttributeError. Only a malformed YAML gets there, but it gets there on the
launch path.

The launch-cannot-fail test uses the real failure rather than a mocked one --
a context name that is not in the kubeconfig.
@DanielZhangQD

DanielZhangQD commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

/quicktest-core --kubernetes --base-branch master
https://buildkite.com/skypilot-1/quicktest-core/builds/3672#_
/smoke-test --kubernetes
https://buildkite.com/skypilot-1/smoke-tests/builds/12716#_

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.

1 participant