-
Notifications
You must be signed in to change notification settings - Fork 321
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
[RayService] Avoid Duplicate Serve Service #1867
[RayService] Avoid Duplicate Serve Service #1867
Conversation
453c4d7
to
c433ce4
Compare
Signed-off-by: Yicheng-Lu-llll <luyc58576@gmail.com>
c433ce4
to
519aec8
Compare
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! Just leave some nits.
@@ -43,6 +43,8 @@ const ( | |||
// Finalizers for GCS fault tolerance | |||
GCSFaultToleranceRedisCleanupFinalizer = "ray.io/gcs-ft-redis-cleanup-finalizer" | |||
|
|||
// EnableServeServiceKey is exclusively utilized to indicate if a Raycluster is directly used for serving. |
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.
nit
// EnableServeServiceKey is exclusively utilized to indicate if a Raycluster is directly used for serving. | |
// EnableServeServiceKey is exclusively utilized to indicate if a RayCluster is directly used for serving. |
@@ -1220,7 +1224,7 @@ func TestInitLivenessAndReadinessProbe(t *testing.T) { | |||
// Test 2: User does not define a custom probe. KubeRay will inject Exec probe. |
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.
Update the 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.
Can I get more hint on which part do I need to update? Thank you!
val, ok := pod.Labels[utils.RayClusterServingServiceLabelKey] | ||
assert.True(t, ok, "Expected serve label is not present") | ||
assert.Equal(t, utils.EnableRayClusterServingServiceFalse, val, "Wrong serve label value") | ||
CheckHasCorrectDeathEnv(t, &pod.Spec.Containers[utils.RayContainerIndex]) |
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.
Use utils.EnvVarExists
instead.
Signed-off-by: Yicheng-Lu-llll <luyc58576@gmail.com>
Why are these changes needed?
Currently, when creating a RayService, two identical serve services are created as shown below:
rayservice-sample-serve-svc
is the original serve service, whereasrayservice-sample-raycluster-bw4xz-serve-svc
is the serve service exclusively used to support Raycluster with serve support. Clearly, the latter one should not be created when creating RayService.Changes in this PR
Currently, we need to add specific labels and readiness probe for Raycluster created by RayService. However, the
ray.io/enable-serve-service
annotation has been used to determine whether a Raycluster is created by RayService. This method is not always accurate, especially following Raycluster with serve support, which also uses the same annotation to indicate whether a Raycluster is directly used for serving. This overlap is the primary reason why two serve services are being created. To resolve this, this PR uses theray.io/originated-from-crd
, introduced in Raycluster with serve support, as the sole indicator to identify if a Raycluster is created by RayService. Meanwhile, keepray.io/enable-serve-service
annotation continue to be used to indicate whether a Raycluster is directly used for serving.As describe in 1,
ray.io/enable-serve-service
annotation is not utilized for Raycluster created by Rayservice. Hence, This PR removes it in rayservice controller.Summary of the current serving behavior
When using Rayservice:
ray.io/serve
annotation is added to both head and worker Pods.When using Raycluster with serve support:
Related issue number
Checks