Skip to content

[kubectl-plugin] Add kubectl ray create cluster#2607

Merged
andrewsykim merged 1 commit intoray-project:masterfrom
chiayi:kubectl-plugin-create
Dec 11, 2024
Merged

[kubectl-plugin] Add kubectl ray create cluster#2607
andrewsykim merged 1 commit intoray-project:masterfrom
chiayi:kubectl-plugin-create

Conversation

@chiayi
Copy link
Contributor

@chiayi chiayi commented Dec 4, 2024

Why are these changes needed?

Adds kubectl ray create cluster command.

Related issue number

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

@chiayi chiayi mentioned this pull request Dec 4, 2024
14 tasks
@chiayi
Copy link
Contributor Author

chiayi commented Dec 4, 2024

Going to post Screenshot of manual test soon.

kubectl ray create cluster -f raycluster-stample.yaml

# Creates Ray Cluster from flags input
kubectl ray create cluster sample-cluster --ray-version 2.9.0 --image rayproject/ray:2.9.0 --head-cpu 1 head-memory 5Gi --worker-grp-name worker-group1 --worker-replicas 3 --worker-cpu 1 --worker-memory 5Gi
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a --dry-run or -o yaml option to print the generated YAML from this command?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see from the implementation that we're already printing the generated YAML

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would having --dry-run be a better option than just printing out the yaml and waiting for user input?

Copy link
Member

Choose a reason for hiding this comment

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

would having --dry-run be a better option than just printing out the yaml and waiting for user input?

Yeah maybe -- by default the output should just be "Created Ray cluster ...." and --dry-run can output the YAML? Not sure we need to print the YAML every time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a --dry-run flag that will print out the yaml if its set and will just apply the yaml if unset.

@chiayi chiayi force-pushed the kubectl-plugin-create branch 3 times, most recently from 3a13d4b to 7cd01b2 Compare December 6, 2024 00:11
@chiayi
Copy link
Contributor Author

chiayi commented Dec 6, 2024

Screenshot of run:
Screenshot 2024-12-05 at 4 31 02 PM
Screenshot 2024-12-05 at 4 31 17 PM

@chiayi chiayi force-pushed the kubectl-plugin-create branch from 7cd01b2 to 8b9e893 Compare December 6, 2024 00:33
MortalHappiness

This comment was marked as duplicate.

Copy link
Member

@MortalHappiness MortalHappiness left a comment

Choose a reason for hiding this comment

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

BTW, we can try using RayClusterApplyConfiguration if importing that package doesn't result in a dependency conflict. It might be simpler than the current implementation of generating YAML files.

@chiayi chiayi force-pushed the kubectl-plugin-create branch 3 times, most recently from 006150a to 0a4815a Compare December 10, 2024 04:12
@chiayi chiayi force-pushed the kubectl-plugin-create branch 2 times, most recently from 2d73a55 to 274c03d Compare December 10, 2024 04:20
@chiayi
Copy link
Contributor Author

chiayi commented Dec 10, 2024

BTW, we can try using RayClusterApplyConfiguration if importing that package doesn't result in a dependency conflict. It might be simpler than the current implementation of generating YAML files.

@MortalHappiness @andrewsykim I was able to use RayClusterApplyConfiguration instead of manually creating the RayCluster object, converting to a map, and then removing unnecessary fields. But I had do a workaround for rayStartParams since the applyconfigurations wouldn't accept an empty string map by using default values for dashboard-host and metrics-export-port for headGroupSpec and workerGroupSpecs respectively. PTAL! Thank you!

@chiayi chiayi force-pushed the kubectl-plugin-create branch from 7bec5a9 to 557252d Compare December 10, 2024 17:16
@chiayi chiayi force-pushed the kubectl-plugin-create branch from 557252d to 54a77e2 Compare December 10, 2024 18:56
@andrewsykim andrewsykim enabled auto-merge (squash) December 10, 2024 23:48
auto-merge was automatically disabled December 10, 2024 23:55

Head branch was pushed to by a user without write access

@chiayi chiayi force-pushed the kubectl-plugin-create branch from 7e1032e to 7f8e8a0 Compare December 10, 2024 23:55
@andrewsykim andrewsykim enabled auto-merge (squash) December 11, 2024 15:00
@andrewsykim
Copy link
Member

@MortalHappiness can you do another review?

@andrewsykim andrewsykim merged commit 12babc8 into ray-project:master Dec 11, 2024
19 checks passed
Ygnas pushed a commit to Ygnas/kuberay that referenced this pull request Mar 20, 2025
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.

3 participants