-
Notifications
You must be signed in to change notification settings - Fork 402
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] Add RayService alb ingress CR #1169
[RayService] Add RayService alb ingress CR #1169
Conversation
Signed-off-by: Sihan Wang <[email protected]>
Signed-off-by: Sihan Wang <[email protected]>
Signed-off-by: Sihan Wang <[email protected]>
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.
LGTM
# Health Check Settings | ||
alb.ingress.kubernetes.io/healthcheck-protocol: HTTP | ||
alb.ingress.kubernetes.io/healthcheck-port: traffic-port | ||
alb.ingress.kubernetes.io/healthcheck-path: /-/routes |
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.
What's the behavior if this health check fails? Does traffic simply stop flowing to the pod, or does the pod get restarted?
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.
yes, traffic will stop sending traffic to the pod. (if there is only one pod which is unhealthy, the traffic will still be sent to that pod.)
No operation on the pod. it still relies on the kuberay operation to take care of it.
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.
Sounds good, that makes sense.
ray-operator/config/samples/ray_v1alpha1_rayservice-alb-ingress.yaml
Outdated
Show resolved
Hide resolved
Signed-off-by: Sihan Wang <[email protected]>
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.
LGTM
The change has no relationship with CI. Merge it. |
[RayService] Add RayService alb ingress CR
Why are these changes needed?
three updates comparing with this
rayservice-sample-serve-svc
With these two changes, customer will have workable CR out of the box for Ray Service.
Tested with eks.
I will put more details into the ray doc.
Related issue number
Checks