Skip to content

[Core] Add default Ray Node labels at Node init#53360

Merged
jjyao merged 39 commits intoray-project:masterfrom
ryanaoleary:add-default-labels
Jul 29, 2025
Merged

[Core] Add default Ray Node labels at Node init#53360
jjyao merged 39 commits intoray-project:masterfrom
ryanaoleary:add-default-labels

Conversation

@ryanaoleary
Copy link
Copy Markdown
Contributor

@ryanaoleary ryanaoleary commented May 28, 2025

Why are these changes needed?

This PR adds support for populating several default Ray node labels (described here) in the Ray runtime environment when a node is initialized. This change will help support autoscaling with the Label Selector API. This PR is related to ray-project/kuberay#3699 which passes several environment variables from the K8s stack which are used to set ray.io/ labels. I'll leave a comment on this PR with manual tests showing the ray.io/accelerator-type and ray.io/availability-zone labels getting set.

Related issue number

#51564

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@ryanaoleary
Copy link
Copy Markdown
Contributor Author

ryanaoleary commented May 28, 2025

cc: @MengjinYan

MengjinYan
MengjinYan reviewed May 28, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@MengjinYan MengjinYan left a comment

Choose a reason for hiding this comment

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

The logic change looks good to me! Just one comment about testing.

@ryanaoleary ryanaoleary mentioned this pull request May 28, 2025
Closed
4 tasks
@MengjinYan MengjinYan added the go add ONLY when ready to merge, run all tests label May 29, 2025
@MengjinYan MengjinYan self-assigned this May 29, 2025
@MengjinYan
Copy link
Copy Markdown
Contributor

MengjinYan commented Jun 3, 2025

@edoakes @jjyao Wondering if you can help take a look and merge the change to get default set the default labels for ray nodes? Thanks!

@mascharkh mascharkh added the k8s-proj K8s and Ray OSS label Jun 5, 2025
@mascharkh mascharkh moved this to In Progress in K8s and Ray (go/k8s-ray-oss) Jun 5, 2025
jjyao
jjyao reviewed Jun 5, 2025
View reviewed changes
@jjyao jjyao changed the title Add default Ray Node labels at Node init [Core] Add default Ray Node labels at Node init Jun 5, 2025
@ryanaoleary ryanaoleary mentioned this pull request Jun 5, 2025
Open
36 tasks
edoakes
edoakes reviewed Jun 5, 2025
View reviewed changes
edoakes
edoakes reviewed Jun 6, 2025
View reviewed changes
@ryanaoleary ryanaoleary requested a review from edoakes June 12, 2025 01:30
@ryanaoleary ryanaoleary requested a review from a team as a code owner June 12, 2025 08:44
@edoakes
Copy link
Copy Markdown
Collaborator

edoakes commented Jun 12, 2025

CI tests failures @ryanaoleary

@ryanaoleary
Copy link
Copy Markdown
Contributor Author

ryanaoleary commented Jun 13, 2025

CI tests failures @ryanaoleary

@edoakes Think I fixed the CI failure with bd51036. test_exit_logging was failing because the calls to AcceleratorManager in Node init would add the Traceback to the driver log, when none was expected. I updated the test to patch calls to _get_default_ray_node_labels so that these logs could be ignored for the test. An alternative would be to update calls like this in each AcceleratorManager definition to log but not add to the traceback. I also added a try: except block when we make calls to AcceleratorManager in the new code to ensure Node init doesn't fail for whatever reason when adding the default labels.

@edoakes
Copy link
Copy Markdown
Collaborator

edoakes commented Jun 13, 2025

The solution looks worrisome/hacky. What causes the accelerator manager calls to fail on startup? We might be breaking some kind of assumption there.

@jjyao PTAL at the usage of accelerator manager here

@ryanaoleary
Copy link
Copy Markdown
Contributor Author

ryanaoleary commented Jun 13, 2025

The solution looks worrisome/hacky. What causes the accelerator manager calls to fail on startup? We might be breaking some kind of assumption there.

@jjyao PTAL at the usage of accelerator manager here

It'd raise an exception here due to missing dependencies such as ModuleNotFoundError: No module named 'acl' for NPU. Detecting the accelerator type would fail, but node startup would still succeed as expected. I added the try: except just to be safe since the code is on a critical path, but maybe it's not needed, since it's not used in other places AcceleratorManager is used. Maybe a better solution is just to logger.debug when we see a ModuleNotFoundError rather than raise an exception.

@jjyao jjyao self-assigned this Jun 18, 2025
jjyao
jjyao reviewed Jun 21, 2025
View reviewed changes
@ryanaoleary ryanaoleary requested a review from jjyao June 25, 2025 00:24
jjyao
jjyao reviewed Jun 25, 2025
View reviewed changes
jjyao
jjyao reviewed Jun 25, 2025
View reviewed changes
ryanaoleary and others added 21 commits July 29, 2025 17:44
@ryanaoleary
remove change to test_basic_5
147dcad 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary @jjyao
Update python/ray/_private/resource_and_label_spec.py
38cc250 
Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
@ryanaoleary @jjyao
Update python/ray/_private/node.py
33018f7 
Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
@ryanaoleary @jjyao
Update python/ray/_private/resource_and_label_spec.py
d518549 
Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
@ryanaoleary @jjyao
Update python/ray/_private/resource_and_label_spec.py
8ba4a05 
Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
@ryanaoleary @jjyao
Update python/ray/_private/resource_and_label_spec.py
1ba0d6f 
Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
@ryanaoleary
Update num-gpus being set and fix nits
3116caf 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Remove test_sql.py change
a469b3f 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Fix GPU tests, use monkeypatch, and fix comment
ad6895c 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Fix num_gpus edge case and add additional unit tests
775b631 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Move unit tests file, change to call public APIs
2ac985b 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary @MengjinYan
Update python/ray/_private/resource_and_label_spec.py
683f5cb 
Co-authored-by: Mengjin Yan <mengjinyan3@gmail.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
@ryanaoleary @MengjinYan
Update python/ray/_private/resource_and_label_spec.py
cd39761 
Co-authored-by: Mengjin Yan <mengjinyan3@gmail.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
@ryanaoleary
Add additional unit tests and move resource merge/load from env to Re…
3363509 
…sourceAndLabelSpec

Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Make functions that don't use self static
a5a68cc 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Remove change to unit/BUILD
9df1245 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Format
fd43e71 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary @jjyao
Update python/ray/_private/resource_and_label_spec.py
e0f6f9d 
Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
@ryanaoleary @jjyao
Update python/ray/_private/resource_and_label_spec.py
ee4b20d 
Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
@ryanaoleary
Change tests to use FakeAcceleratorManager, add _resolve_resources, a…
5805a54 
…nd move record hardware usage to node.py

Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Fix import of usage_lib
4148cc0 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Copy link
Copy Markdown
Contributor Author

ryanaoleary commented Jul 29, 2025

@MengjinYan rebased and re-pushed since there were some changes to resource_spec.py in master which gets deleted in this PR. I made sure the same change is included in this PR (changed import for from ray._common.usage import usage_lib in node.py to match).

@jjyao jjyao merged commit 8e0d494 into ray-project:master Jul 29, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in K8s and Ray (go/k8s-ray-oss) Jul 29, 2025
krishnakalyan3 pushed a commit to krishnakalyan3/ray that referenced this pull request Jul 30, 2025
@ryanaoleary @jjyao
@MengjinYan @krishnakalyan3
[Core] Add default Ray Node labels at Node init (ray-project#53360)
a8655a4 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
Co-authored-by: Mengjin Yan <mengjinyan3@gmail.com>
Signed-off-by: Krishna Kalyan <krishnakalyan3@gmail.com>
kamil-kaczmarek pushed a commit that referenced this pull request Aug 4, 2025
@ryanaoleary @jjyao
@MengjinYan @kamil-kaczmarek
[Core] Add default Ray Node labels at Node init (#53360)
2ddb548 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
Co-authored-by: Mengjin Yan <mengjinyan3@gmail.com>
Signed-off-by: Kamil Kaczmarek <kamil@anyscale.com>
jugalshah291 pushed a commit to jugalshah291/ray_fork that referenced this pull request Sep 11, 2025
@ryanaoleary @jjyao
@MengjinYan @jugalshah291
[Core] Add default Ray Node labels at Node init (ray-project#53360)
c0f8262 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
Co-authored-by: Mengjin Yan <mengjinyan3@gmail.com>
Signed-off-by: jugalshah291 <shah.jugal291@gmail.com>
dstrodtman pushed a commit to dstrodtman/ray that referenced this pull request Oct 6, 2025
@ryanaoleary @jjyao
@MengjinYan @dstrodtman
[Core] Add default Ray Node labels at Node init (ray-project#53360)
8740c1f 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
Co-authored-by: Mengjin Yan <mengjinyan3@gmail.com>
Signed-off-by: Douglas Strodtman <douglas@anyscale.com>
@noemotiovon noemotiovon mentioned this pull request Oct 29, 2025
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@edoakes edoakes edoakes approved these changes

@jjyao jjyao jjyao approved these changes

@MengjinYan MengjinYan MengjinYan approved these changes

@kfstorm kfstorm Awaiting requested review from kfstorm kfstorm is a code owner

@raulchen raulchen Awaiting requested review from raulchen raulchen is a code owner

@WangTaoTheTonic WangTaoTheTonic Awaiting requested review from WangTaoTheTonic WangTaoTheTonic is a code owner

@SongGuyang SongGuyang Awaiting requested review from SongGuyang SongGuyang is a code owner

Assignees

@jjyao jjyao

@MengjinYan MengjinYan

Labels

community-contribution Contributed by the community core Issues that should be addressed in Ray Core go add ONLY when ready to merge, run all tests k8s-proj K8s and Ray OSS

Projects

K8s and Ray (go/k8s-ray-oss)
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ryanaoleary @MengjinYan @edoakes @jjyao @mascharkh