-
Notifications
You must be signed in to change notification settings - Fork 670
[Refactor][RayService] Unify ClusterAction decision to single function #2716
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
[Refactor][RayService] Unify ClusterAction decision to single function #2716
Conversation
f109493 to
7bb1438
Compare
Closes: ray-project#2706 Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
7bb1438 to
579310c
Compare
|
cc @rueian would you mind reviewing this PR? |
kevin85421
left a comment
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.
| return rayServiceInstance, nil | ||
| } | ||
|
|
||
| func isZeroDowntimeUpgradeEnabled(ctx context.Context, rayService *rayv1.RayService) bool { |
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.
add unit tests for this function.
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.
Already covered in this test
| func TestReconcileRayCluster(t *testing.T) { |
ray-operator/controllers/ray/rayservice_controller_unit_test.go
Outdated
Show resolved
Hide resolved
|
LGTM. |
Closes: ray-project#2706 Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
Why are these changes needed?
See the description in the corresponding issue for details.
Related issue number
Closes: #2706
Checks