Skip to content

Update --labels and add --labels-file options for Label Selector API #51706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 20 commits into from
Apr 4, 2025
Merged

Update --labels and add --labels-file options for Label Selector API #51706

merged 20 commits into from
Apr 4, 2025

Conversation

ryanaoleary
Copy link
Contributor

@ryanaoleary ryanaoleary commented Mar 26, 2025 •
edited
Loading

Why are these changes needed?

This PR updates the --labels option to ray start or ray init to accept a string list of key-value pairs mapping label names to label values. Labels follow Kubernetes label syntax. This PR also adds a --labels-file argument to support sourcing labels from a file. Files are expected to contain a valid JSON string containing a serialized key-value pair map. We use parse_node_labels_json to parse the argument passed to --labels-file.

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
Update --labels argument and add --labels-from-file
1c2668a 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary ryanaoleary mentioned this pull request Mar 26, 2025
Open
23 tasks
ryanaoleary and others added 2 commits March 26, 2025 07:00
@ryanaoleary ryanaoleary marked this pull request as ready for review March 26, 2025 07:00
@ryanaoleary
Handle empty string case
8734a47 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary ryanaoleary mentioned this pull request Mar 26, 2025
Merged
8 tasks
@ryanaoleary
Add implicit None check
3564945 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Fix test
5567fb7 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@jcotant1 jcotant1 added the core Issues that should be addressed in Ray Core label Mar 26, 2025
@MengjinYan MengjinYan self-assigned this Mar 26, 2025
@MengjinYan MengjinYan self-requested a review March 26, 2025 20:45
MengjinYan
MengjinYan reviewed Mar 27, 2025
View reviewed changes
Copy link
Collaborator

@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.

Thanks!

@ryanaoleary ryanaoleary mentioned this pull request Mar 28, 2025
Merged
@ryanaoleary
Add read from file and fix tests
b6e99ea 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary ryanaoleary requested a review from MengjinYan March 29, 2025 03:25
MengjinYan
MengjinYan approved these changes Mar 31, 2025
View reviewed changes
Copy link
Collaborator

@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.

Thanks!

@ryanaoleary
Handle empty path
04d8eb6 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@MengjinYan MengjinYan added the go add ONLY when ready to merge, run all tests label Mar 31, 2025
@MengjinYan MengjinYan requested a review from edoakes March 31, 2025 21:43
edoakes
edoakes reviewed Apr 2, 2025
View reviewed changes
Copy link
Collaborator

@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

Looks good, minor comments

One larger comment: let's rename the argument to --labels-file (it more closely matches some other CLI options we have elsewhere, such as --config-file for serve)

@ryanaoleary ryanaoleary changed the title Update --labels and add --labels-from-file options for Label Selector API Update --labels and add --labels-file options for Label Selector API Apr 2, 2025
@ryanaoleary
Move label utils to separate files
5a50d28 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
edoakes
edoakes reviewed Apr 2, 2025
View reviewed changes
@ryanaoleary @edoakes
Update python/ray/scripts/scripts.py
67dbc02 
Co-authored-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: ryanaoleary <113500783+ryanaoleary@users.noreply.github.com>
edoakes
edoakes approved these changes Apr 2, 2025
View reviewed changes
Copy link
Collaborator

@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

🚀

edoakes
edoakes reviewed Apr 2, 2025
View reviewed changes
Copy link
Collaborator

@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

I believe you will need to add entries to the tests BUILD file for these tests to run (unless something changed recently to automatically pick them up)

@ryanaoleary
Move cli_logger logic to CLI
e04ab0f 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Update --labels help comment
32f0d9c 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Add new test to BUILD file
0c6d430 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@edoakes
Copy link
Collaborator

edoakes commented Apr 2, 2025

ping when CI passes (want to spot check and make sure tests ran as expected)

ryanaoleary and others added 5 commits April 3, 2025 00:19
@ryanaoleary
Merge branch 'master' into label-based-scheduling
9740cc5
@ryanaoleary
Fix --labels commands in tests
05fb2a2 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Fix label utils test
7cd9849 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Fix java test
0e19c3d 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Fix --labels commands in tests
abe5c55 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@hainesmichaelc hainesmichaelc added the community-contribution Contributed by the community label Apr 4, 2025
@ryanaoleary
Parse out leading/trailing quotes
549f78d 
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
@ryanaoleary
Copy link
Contributor Author

ping when CI passes (want to spot check and make sure tests ran as expected)

@edoakes the PR is now passing the CI, I checked and it looks like the new test file is being ran:
image

@edoakes edoakes merged commit 0a5f970 into ray-project:master Apr 4, 2025
5 checks passed
jjyao added a commit to jjyao/ray that referenced this pull request Apr 7, 2025
@jjyao
Revert "Update --labels and add --labels-file options for Label S…
803dfc6 
…elector API (ray-project#51706)"

This reverts commit 0a5f970.
jjyao added a commit that referenced this pull request Apr 7, 2025
@jjyao
[Core] Revert "Update --labels and add --labels-file options for …
78959be 
…Label Selector API (#51706) (#52063)
@ryanaoleary ryanaoleary mentioned this pull request Apr 8, 2025
Merged
8 tasks
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

@MengjinYan MengjinYan MengjinYan approved these changes

Assignees

@MengjinYan MengjinYan

Labels
community-backlog 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
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ryanaoleary @edoakes @MengjinYan @hainesmichaelc @jcotant1