Support updating RayServices using the KubeRay API Server #633

scarlet25151 · 2022-10-13T22:21:32Z

Why are these changes needed?

support update ray service for current apiserver.

curl -XPUT localhost:8888/apis/v1alpha2/namespaces/ray-system/services/test-1
{
  "name": "test-1",
  "namespace": "ray-system",
  "serveConfigs": [
      {
        "deploymentName": "PearStand",
        "replicas": 2,
        "userConfig": "price: 1",
        "actorOptions": {
          "cpusPerActor": 1.0
        }
      }
  ],
  "workerGroupUpdateSpec": [
    {
      "groupName": "small-wg",
      "replicas": 2,
      "minReplicas": 1,
      "maxReplicas": 5
    }]
}
// Response:
{
    "name": "test-1",
    "namespace": "ray-system",
    "user": "scarlet25151",
    "serveDeploymentGraphSpec": {
        "importPath": "https://github.com/ray-project/test_dag/archive/c620251044717ace0a4c19d766d43c5099af8a77.zip\"\n",
        "serveConfigs": [
            {
                "deploymentName": "RayServeModel",
                "replicas": 1,
                "routePrefix": "/model",
                "actorOptions": {
                    "cpusPerActor": 0.4
                }
            },
            {
                "deploymentName": "OrangeStand",
                "replicas": 1,
                "userConfig": "price: 2",
                "actorOptions": {
                    "cpusPerActor": 0.1
                }
            },
            {
                **"deploymentName": "PearStand",
                "replicas": 2,
                "userConfig": "price: 1",
                "actorOptions": {
                    "cpusPerActor": 1**
                }
            },
            {
                "deploymentName": "FruitMarket",
                "replicas": 1,
                "actorOptions": {
                    "cpusPerActor": 0.1
                }
            },
            {
                "deploymentName": "DAGDriver",
                "replicas": 1,
                "routePrefix": "/",
                "actorOptions": {
                    "cpusPerActor": 0.1
                }
            }
        ]
    },
    "clusterSpec": {
        "headGroupSpec": {
            "computeTemplate": "default-template",
            "image": "hub.byted.org/kuberay/ray:2.0.0",
            "serviceType": "NodePort",
            "rayStartParams": {
                "dashboard-host": "0.0.0.0",
                "metrics-export-port": "8080",
                "node-ip-address": "$MY_POD_IP",
                "port": "6379"
            }
        },
        "workerGroupSpec": [
            {
                "groupName": "small-wg",
                "computeTemplate": "default-template",
                "image": "hub.byted.org/kuberay/ray:2.0.0",
                **"replicas": 2,**
                "minReplicas": 5,
                "maxReplicas": 1,
                "rayStartParams": {
                    "node-ip-address": "$MY_POD_IP"
                }
            }
        ]
    },
    "rayServiceStatus": {
//...
}

Related issue number

Checks

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

Jeffwan

/cc @brucez-anyscale @simon-mo

Let's ask anyscale team to help review it and see if the PR makes sense.

Jeffwan · 2022-10-18T16:35:44Z

proto/serve.proto

@@ -67,6 +73,19 @@ message CreateRayServiceRequest {
  string namespace = 2;
 }

+message UpdateRayServiceRequest {
+  // The ray service to be updated.
+  RayService service = 1;


why do we need entire object here? is name + namespace good enough?

Updated, remove the service from the body.

is service removed?

Jeffwan · 2022-10-18T16:36:14Z

proto/serve.proto

+  // The namespace of the ray service to be updated.
+  string namespace = 3;
+  // specify the worker group to be update
+  repeated WorkerGroupUpdateSpec worker_group_update_spec = 4;


Seems we want to use patch mode (partial update)?

How will you make sure it only find the right spec and update it? Do you plan to use name for consistency?

here I would like to use patch mode, the logic in util/service.go would check the field by workergroup name and serviceconfig name as the key and if need to modify then update it.

brucez-anyscale · 2022-10-20T19:05:51Z

apiserver/pkg/manager/resource_manager.go

+		service.Spec.ServeDeploymentGraphSpec.ServeConfigSpecs = newServeConfigSpecs
+	}
+
+	newService, err := client.Update(ctx, service, metav1.UpdateOptions{})


so if service resourceVersion changes, the update could fail ?

we just leverage the client go's update and it will directly put the update event to the k8s workerqueue and wait for previous resourceVersion changed?

yeah, but k8s do update in a reconcile loop. So if it fails, it will retry in next loop. But this api call is 1 time call. Do we want to retry here? Or the caller should retry their side?

apiserver/pkg/model/converter.go

brucez-anyscale · 2022-10-20T19:12:08Z

apiserver/pkg/util/service.go

+		}
+	}
+	for i, spec := range workerGroupSpecs {
+		if updateSpec, ok := specMap[spec.GroupName]; ok {


this is update logic. what if updateSpecs has new worker groups, do we support adding new worker groups?

Good question, for now if we add or delete the worker group by name, do we think it's better to add/delete or just keep the old one? I think we need to support adding/deleting worker groups. WDYT? @Jeffwan

RayService should be able to handle any spec update. So here we just need a design about http request, how to support add/delete/update

brucez-anyscale · 2022-10-20T19:12:57Z

apiserver/pkg/util/service.go

+	}
+	for i, spec := range serveConfigSpecs {
+		if updateSpec, ok := specMap[spec.Name]; ok {
+			newSpec := updateServeConfigSpec(updateSpec, spec)


the same, do we support adding service config? Also how to support deleting?

brucez-anyscale · 2022-10-20T19:13:22Z

thanks for the pr. I leave some comments

scarlet25151 · 2022-10-27T17:58:27Z

Hi @Jeffwan @brucez-anyscale I've made a new modification that support two types of APIs to update the ray service,

curl -XPUT {BaseURL}/apis/v1alpha2/namespaces/{namespace}/services/{name}
in this case, we can make any changes to the rayservice spec, including adding/deleting raycluster spec, The request body would be the complete definition of rayservice and we would update it.
curl -XPATCH {BaseURL}/apis/v1alpha2/namespaces/{namespace}/services/{name}
this case is used by our downstream user in the case that they would just modify the serveConfig sections and the raycluster workers replicas without restarting the new raycluster under the hook,

this might be tricky things since for now in rayservice, we just doing simple check if the raycluster spec should update, we just recreate a new one, I will create a issue to discuss this in detail.

DmitriGekhtman · 2022-11-04T01:38:21Z

@brucez-anyscale if you have a spare moment to re-review this, that would be great
(I imagine you might not have a spare moment these days...)

brucez-anyscale · 2022-11-04T23:41:07Z

it is fine. I think we can merge and improve based on it

Support updating Ray Services using the KubeRay ApiServer. Co-authored-by: chenyu.jiang <chenyu.jiang@bytedance.com>

scarlet25151 marked this pull request as draft October 13, 2022 22:22

Jeffwan reviewed Oct 18, 2022

View reviewed changes

scarlet25151 force-pushed the support-update-rayservice branch from 919d7ef to 3bc8e57 Compare October 20, 2022 00:03

scarlet25151 marked this pull request as ready for review October 20, 2022 00:05

scarlet25151 force-pushed the support-update-rayservice branch from 3bc8e57 to 488e9b4 Compare October 20, 2022 18:01

brucez-anyscale reviewed Oct 20, 2022

View reviewed changes

apiserver/pkg/model/converter.go Outdated Show resolved Hide resolved

brucez-anyscale reviewed Oct 20, 2022

View reviewed changes

scarlet25151 force-pushed the support-update-rayservice branch from 488e9b4 to c04a75b Compare October 21, 2022 01:51

chenyu.jiang added 2 commits October 26, 2022 15:46

support apiserver update rayservice

1fed8b7

add put and patch update in apiserver

ac40deb

scarlet25151 force-pushed the support-update-rayservice branch from c04a75b to ac40deb Compare October 26, 2022 22:46

simon-mo requested review from shrekris-anyscale and sihanwang41 October 27, 2022 18:04

DmitriGekhtman requested a review from Jeffwan November 4, 2022 01:37

DmitriGekhtman approved these changes Nov 5, 2022

View reviewed changes

DmitriGekhtman merged commit 599e74b into ray-project:master Nov 5, 2022

DmitriGekhtman changed the title ~~support update rayservice~~ Support updating RayServices using the KubeRay API Server Nov 5, 2022

lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023

support update rayservice (ray-project#633)

ff7b8bb

Support updating Ray Services using the KubeRay ApiServer. Co-authored-by: chenyu.jiang <chenyu.jiang@bytedance.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support updating RayServices using the KubeRay API Server #633

Support updating RayServices using the KubeRay API Server #633

scarlet25151 commented Oct 13, 2022 •

edited

Jeffwan left a comment

Jeffwan Oct 18, 2022

scarlet25151 Oct 20, 2022

brucez-anyscale Oct 28, 2022

Jeffwan Oct 18, 2022

Jeffwan Oct 18, 2022

scarlet25151 Oct 20, 2022

brucez-anyscale Oct 20, 2022

scarlet25151 Oct 21, 2022

brucez-anyscale Oct 21, 2022

brucez-anyscale Oct 20, 2022

scarlet25151 Oct 20, 2022

brucez-anyscale Oct 20, 2022

brucez-anyscale Oct 20, 2022

brucez-anyscale commented Oct 20, 2022

scarlet25151 commented Oct 27, 2022

DmitriGekhtman commented Nov 4, 2022

brucez-anyscale commented Nov 4, 2022

Support updating RayServices using the KubeRay API Server #633

Support updating RayServices using the KubeRay API Server #633

Conversation

scarlet25151 commented Oct 13, 2022 • edited

Why are these changes needed?

Related issue number

Checks

Jeffwan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brucez-anyscale commented Oct 20, 2022

scarlet25151 commented Oct 27, 2022

DmitriGekhtman commented Nov 4, 2022

brucez-anyscale commented Nov 4, 2022

scarlet25151 commented Oct 13, 2022 •

edited