-
Notifications
You must be signed in to change notification settings - Fork 364
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.
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
[Bug] label rayNodeType is useless #698
[Bug] label rayNodeType is useless #698
Conversation
@shrekris-anyscale can you help me review the changes in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work so far! I left a comment for the ray_v1alpha1_rayservice.yaml
file.
It looks good to me. Have you check all |
Thank @Jeffwan for your review! I have checked all occurrences of |
The label rayNodeType is not used in the KubeRay operator code. This PR removes the label from all sample configs.
Why are these changes needed?
Label rayNodeType is useless in the whole KubeRay operator. I guess people are trying to add the label
ray.io/node-type
which will be set by the Kuberay operator.kuberay/ray-operator/config/samples/ray-cluster.mini.yaml
Lines 36 to 42 in f3415f4
kuberay/ray-operator/controllers/ray/common/constant.go
Line 7 in fd06b5b
Related issue number
Closes #679
Checks
For RayCluster YAMLs in
ray-operator/config/samples
, I used the configuration test framework to test them.ray_v1alpha1_rayjob.yaml
: I tested it by following the rayjob.md.ray_v1alpha1_rayservice.yaml
: I tested it by following the rayservice.md. I faced a bug mentioned by a user in KubeRay slack channel (thread) and solved it by settingSERVE_DEPLOYMENT_HANDLE_IS_SYNC
(solution from @shrekris-anyscale).rayjob_controller_test.go
,rayservice_controller_test.go
,rayjob_types_test.go
,rayservice_types_test.go
: Usemake build
to test them.I tested the helm chart.