[Core] Upgrade the bundled Ray to 2.56.1 - #10585
Conversation
4532350 to
f27f946
Compare
f27f946 to
fe88173
Compare
|
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 (
And it is no longer untested. 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 What this still does not cover, and the PR description says so: the IBM provisioning path end to end. Nothing here calls |
4316d57 to
3273b4a
Compare
fb4fb86 to
f5ed862
Compare
5061ac6 to
e9daeb9
Compare
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.
e9daeb9 to
508e394
Compare
_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.
|
/quicktest-core --kubernetes --base-branch master |
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 diffstrailing-whitespaceandend-of-file-fixerhave 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-pathsforsky/backends/monkey_patches, matching the exclude mypy already has: those files are templates with{}placeholders, which pylint reads as undefined variables. (ignoredoes 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 constantWhether 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 statusguard 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 mismatcheddefault_worker.py.So the version a cluster was set up with is recorded under
provider— Ray's cluster schema isadditionalProperties: Falseat the top level, andprovideris already restored wholesale for existing clusters — and rendered on later launches. A cluster only moves when nothing is running to disagree:sky stopfollowed bysky start.Two things this commit gets right that are easy to get wrong, both found by running it rather than by reading it:
RAY_INSTALLATION_COMMANDSbaked 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.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 ofKubernetes.make_deploy_resources_variables, which cannot know the cluster's pin, intowrite_cluster_config, which can. #10582's assertion that the deploy variable is actually produced moves with them.While
SKY_REMOTE_RAY_VERSIONis 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.1The seven patches were re-triaged against the new source rather than regenerated blindly:
cli.pyray job submitoutput, and nothing has calledray job submitsince #4318resource_demand_scheduler.pyassert not unfulfilledit removed is gone upstreamlog_monitor.py,worker.pyray-2.56.1autoscaler.py,command_runner.py,updater.pyProvisionerVersion.RAY_AUTOSCALERmonkey_patch_hash_launch_confnow delegates to Ray's ownhash_launch_confinstead of copying it.ray upruns 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 ownnode_launcherwrites for worker nodes. Only thessh_proxy_commandexclusion 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
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_oursred.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_overridesfails, 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__andSKY_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:ray statussucceeded)AWS, same shape, with this change — both directions:
Kubernetes, 1-node and 2-node: cluster up, job runs, both nodes on Ray 2.56.1 with all six patches applied and
.origbackups 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_configis 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
ProvisionerVersion.RAY_AUTOSCALERis IBM alone, and it is the only consumer of theautoscaler/command_runner/resource_demand_scheduler/updaterpatches and ofmonkey_patch_ray_up.py. Nothing here exercises them — no IBM account. Worth a reviewer with access, or a smoke run.node_configrestore — but that has not been measured.DockerCommandRunner) and the image bake (Dockerfile_k8s, theskypilot.shAMI). 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.worker.pypatch'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.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_hashruns, 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 itssky launch --fastskip exactly once. Measured, not reasoned:('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_confignow 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 stopunconditionally.sky stop+sky startis 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, throughcheck_features_are_supported(..., {STOP}), which is resources-aware as well as cloud-aware (AWS on-demand can stop, AWS spot cannot):info, namingsky stop/sky start. Actionable, and transient: one stop/start clears it.debug, namingsky downinstead. 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_configrestore, 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:kubernetes-ray.yml.j2available_node_types→node_configsky stopeither, so nothing doessetup_commands_RAY_YAML_KEYS_TO_RESTORE_FOR_BACK_COMPATIBILITYis{cluster_name, provider, auth, node_config, docker}—setup_commandsis 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 startputs 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_configis restored,setup_commandsis not, and every template installs Ray undersetup_commandsexcept the Kubernetes one. A new cloud template that inlined the install intonode_configwould silently acquire Kubernetes' behaviour; that now fails.test_ray_version_consistencymissed one of the four files this PR had to update. Its docstring claims every place pinning a Ray version, butdocs/source/reference/architecture/internals.rststates 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_confissha256+base32hexthere, where the copy inmonkey_patch_ray_up.pywassha1+hexdigest— so on the IBM path the copy would have disagreed with the hash Ray's ownnode_launcherwrites for worker nodes. Andresource_demand_scheduler.pyno longer contains theassert not unfulfilledthat 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.rstgo stale each turn the corresponding test red.