[RayService] Submit requests to the Dashboard after the head Pod is running and ready

[kevin85421]

Why are these changes needed?

In #1065, the RayCluster initialization resulted in 47 status updates due to the controller sending HTTP requests to the Dashboard to create Serve Deployments before it was ready to handle requests. See this gist for more details.

• updateState {"error": ...: connect: connection refused"} => Send requests to the Dashboard before it was ready to handle requests.
• r.Status().Update() isHealthy && !isReady => The Serve Deployments are created but not ready.

In this PR, the RayService controller will submit the HTTP request to the Dashboard after the head Pod is running and ready. Note that the Dashboard and GCS may take a few seconds to start up after the head pod is running and ready. Hence, some requests to the Dashboard (e.g. UpdateDeployments) may fail. This is not a big issue since UpdateDeployments is an idempotent operation.

Compare with RayJob

Unlike RayJob, RayJob will submit a job to the RayCluster when the cluster is ready (by definition, "ready" means that all Pods are running, but the implementation may be a bit different.).

kuberay/ray-operator/controllers/ray/rayjob_controller.go

Lines 173 to 174 in 795db0d

	// Check the current status of ray cluster before submitting.
	if rayClusterInstance.Status.State != rayv1alpha1.Ready {

The reasons are:

• Typically, RayService should be available 24/7. In addition, Ray Serve deployments may scale up/down based on the QPS, and cause the RayCluster up/down with Ray Autoscaler. Only updating Ray Serve deployments when all Pods are ready and running may cause big overhead.
• In addition, there may be some edge cases that cause some Pods to crash unexpectedly.

Hence, if RayService only updates Ray Serve Deployments when all Pods are running and ready, it may be risky.

Why do I update rayservice_controller_test.go?

Prior to this PR, the controller would submit an HTTP request to the Dashboard regardless of the status of the head Pod. As a result, the fake Dashboard client would create Ray Serve deployments with a HEALTHY status. However, in a real Kubernetes cluster, it is impossible for Ray Serve Deployments to be HEALTHY before the Dashboard is ready.

• r.getAndCheckServeStatus -> r.updateRayClusterInfo -> Turn PendingRayCluster into ActiveRayCluster.

kuberay/ray-operator/controllers/ray/rayservice_controller_test.go

Lines 553 to 559 in 795db0d

	func prepareFakeRayDashboardClient() utils.FakeRayDashboardClient {
	client := utils.FakeRayDashboardClient{}
	client.SetServeStatus(generateServeStatus(metav1.Now(), "HEALTHY"))
	return client
	}

With this PR, the function r.getAndCheckServeStatus will not be executed if the head Pod is not running and ready. However, envtest doesn't create a full K8s cluster. It only has the control plane. There's no container runtime or any other Kubernetes controllers. Pod's status will not be updated by Kubernetes built-in Pod controller, and thus the status will always be "pending". Hence, we need to manually update the status from "pending" to "running". The function r.getAndCheckServeStatus will be executed after the head Pod becomes running and ready, and then turn PendingRayCluster into ActiveRayCluster. See https://book.kubebuilder.io/reference/envtest.html for more details.

Other updates

• Typo: CheckAllPodsRunnning -> CheckAllPodsRunning

• rayservice_controller_test.go => rayiov1alpha1 -> v1alpha1

Related issue number

#1061
#1062
#1065
#983

Checks

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

I performed an experiment similar to #1062.

• status updates in the RayCluster initialization. => 47 -> 13

2023-05-10T06:44:04.862Z	INFO	controllers.RayService	r.Status().Update() Update RayService Status since reconcileRayCluster may mark RayCluster restart.
2023-05-10T06:44:06.908Z	INFO	controllers.RayService	r.Status().Update() updateState	{"error": "Service \"rayservice-sample-raycluster-c6d8t-dashboard-svc\" not found"}
2023-05-10T06:44:07.046Z	INFO	controllers.RayService	r.Status().Update() updateState	{"error": "Service \"rayservice-sample-raycluster-c6d8t-dashboard-svc\" not found"}
2023-05-10T06:45:27.955Z	INFO	controllers.RayService	r.Status().Update() updateState	{"error": "Put \"http://rayservice-sample-raycluster-c6d8t-dashboard-svc.default.svc.cluster.local:52365/api/serve/deployments/\": dial tcp 10.96.243.100:52365: connect: connection refused"}
2023-05-10T06:45:28.978Z	INFO	controllers.RayService	r.Status().Update() updateState	{"error": "Put \"http://rayservice-sample-raycluster-c6d8t-dashboard-svc.default.svc.cluster.local:52365/api/serve/deployments/\": dial tcp 10.96.243.100:52365: connect: connection refused"}
2023-05-10T06:45:31.002Z	INFO	controllers.RayService	r.Status().Update() updateState	{"error": "Put \"http://rayservice-sample-raycluster-c6d8t-dashboard-svc.default.svc.cluster.local:52365/api/serve/deployments/\": dial tcp 10.96.243.100:52365: connect: connection refused"}
2023-05-10T06:45:33.060Z	INFO	controllers.RayService	r.Status().Update() updateState	{"error": "Put \"http://rayservice-sample-raycluster-c6d8t-dashboard-svc.default.svc.cluster.local:52365/api/serve/deployments/\": dial tcp 10.96.243.100:52365: connect: connection refused"}
2023-05-10T06:45:42.919Z	INFO	controllers.RayService	r.Status().Update() isHealthy && !isReady
2023-05-10T06:45:42.963Z	INFO	controllers.RayService	r.Status().Update() isHealthy && !isReady
2023-05-10T06:45:44.958Z	INFO	controllers.RayService	r.Status().Update() isHealthy && !isReady
2023-05-10T06:45:46.995Z	INFO	controllers.RayService	r.Status().Update() isHealthy && !isReady
2023-05-10T06:45:49.060Z	INFO	controllers.RayService	r.Status().Update() isHealthy && !isReady
2023-05-10T06:45:51.102Z	INFO	controllers.RayService	r.Status().Update() isHealthy && !isReady
2023-05-10T06:45:53.261Z	INFO	controllers.RayService	r.Status().Update() Final status update for any CR modification.

[kevin85421]

cc @msumitjain would you mind giving some feedbacks? Thanks!

[gvspraveen]

nice catch

[gvspraveen]

what is TODO here? Do you expect to make a fix in future to wait for Dashboard agent before UpdateDeployments? if not remove this TODO prefix

[kevin85421]

Do you expect to make a fix in the future to wait for the Dashboard agent before UpdateDeployments

Yes. The behavior is correct without the TODO. However, it may cause unnecessary network traffic.

[gvspraveen]

when do we expect this case? having multiple head nodes?

[kevin85421]

Yes, in KubeRay, it is impossible to have more than one head Pod for a RayCluster. If there is more than one Pod that fulfills the matching labels, it may be caused by either a KubeRay bug or a user misconfiguration.

client.MatchingLabels{common.RayClusterLabelKey: instance.Name, common.RayNodeTypeLabelKey: string(rayv1alpha1.HeadNode)}

[shrekris-anyscale]

Nice work! This change looks good to me pending @gvspraveen's comments.

[msumitjain]

cc @msumitjain would you mind giving some feedbacks? Thanks!

Thanks @kevin85421 for picking this up. I will test them tomorrow and let you know.

[architkulkarni]

Looks great!

[architkulkarni]

rayiov1alpha1 -> v1alpha1 what's the reason for this out of curiosity?

[kevin85421]

rayiov1alpha1 -> v1alpha1 what's the reason for this out of curiosity?

There are rayiov1alpha1, rayv1alpha1, and v1alpha1 in the codebase, and I would like to ensure consistency across them. This can be a good first issue for new contributors.