[Feature][RayJob] Support light-weight job submission #1893

kevin85421 · 2024-01-31T08:21:43Z

Why are these changes needed?

Some users are not willing to allocate additional compute resources for the submitter Kubernetes Job. Hence, this PR implements the light-weight job submission which uses a HTTP request to submit the Ray job.

Related issue number

Closes #1558

Checks

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

kevin85421 · 2024-01-31T23:41:12Z

ray-operator/test/e2e/rayjob_lightweight_test.go

+				WithEntrypoint("python /home/ray/jobs/counter.py").
+				WithRuntimeEnvYAML(`
+env_vars:
+  counter_name: test_counter


TODO: If the indentation is set to 4 spaces, the YAML string will fail to unmarshal correctly. Maybe we should verify whether runtimeEnvYAML is a valid YAML or not.

kevin85421 · 2024-01-31T23:42:40Z

ray-operator/controllers/ray/utils/dashboard_httpclient.go

@@ -197,10 +198,10 @@ type RayJobInfo struct {
 }

 // RayJobRequest is the request body to submit.
-// Reference to https://docs.ray.io/en/latest/cluster/jobs-package-ref.html#jobsubmissionclient.
+// Reference to https://docs.ray.io/en/latest/cluster/running-applications/job-submission/rest.html#ray-job-rest-api-spec


Check the OpenAPI https://docs.ray.io/en/latest/cluster/running-applications/job-submission/api.html#/paths/~1api~1jobs/post for more details. Some types seem to be wrong.

The OpenAPI doc is currently manually written and might itself have bugs; it might be better to use the Python as the source of truth.

(And if we notice something is off, we can update the OpenAPI doc)

kevin85421 · 2024-02-01T00:06:41Z

ray-operator/controllers/ray/utils/dashboard_httpclient.go

@@ -222,6 +223,7 @@ type RayJobLogsResponse struct {
 }

 // Note that RayJobInfo and error can't be nil at the same time.
+// Please make sure if the Ray job with JobId can't be found. Return a BadRequest error.


This is somewhat unusual, but the function NewNotFound requires schema.GroupResource as a parameter.

func NewNotFound(qualifiedResource schema.GroupResource, name string) *StatusError

kevin85421 · 2024-02-01T00:08:22Z

ray-operator/controllers/ray/utils/dashboard_httpclient.go

@@ -222,6 +223,7 @@ type RayJobLogsResponse struct {
 }

 // Note that RayJobInfo and error can't be nil at the same time.


TODO: KubeRay uses many different methods to create errors. We should try to unify them.

"k8s.io/apimachinery/pkg/api/errors"

"github.com/pkg/errors"

fmt.Errorf

kevin85421 · 2024-02-01T00:10:31Z

cc @andrewsykim @astefanutti

architkulkarni

The API looks reasonable and the code looks good to me! In the accompanying Ray doc PR, we can add a brief explanation of the pros and cons of setting this parameter.

architkulkarni · 2024-02-01T00:51:52Z

ray-operator/controllers/ray/rayjob_controller.go

@@ -197,6 +202,15 @@ func (r *RayJobReconciler) Reconcile(ctx context.Context, request ctrl.Request)
 		rayDashboardClient.InitClient(rayJobInstance.Status.DashboardURL)
 		jobInfo, err := rayDashboardClient.GetJobInfo(ctx, rayJobInstance.Status.JobId)
 		if err != nil {
+			// If the Ray job was not found, GetJobInfo returns a BadRequest error.


Interesting, I would expect it to return a 404 according to the doc (https://docs.ray.io/en/latest/cluster/running-applications/job-submission/api.html#/paths/~1api~1jobs~1%7Bsubmission_id%7D/get), but the doc is not automatically generated and it might be inaccurate. Do you think it's worth changing the behavior in Ray to make it return 404? I don't think any other users have reported it, so maybe it's not important.

The Ray RESTful API will return 404, but the function GetJobInfo returns BadRequest here. The reason that I decide not to use NewNotFound at this moment is that it requires to pass schema.GroupResource into the function (example). We should figure out a better way for "errors". See #1893 (comment) for more details.

architkulkarni · 2024-02-01T00:58:46Z

ray-operator/controllers/ray/utils/dashboard_httpclient.go

@@ -197,10 +198,10 @@ type RayJobInfo struct {
 }

 // RayJobRequest is the request body to submit.
-// Reference to https://docs.ray.io/en/latest/cluster/jobs-package-ref.html#jobsubmissionclient.
+// Reference to https://docs.ray.io/en/latest/cluster/running-applications/job-submission/rest.html#ray-job-rest-api-spec


The OpenAPI doc is currently manually written and might itself have bugs; it might be better to use the Python as the source of truth.

andrewsykim · 2024-02-01T01:32:40Z

ray-operator/apis/ray/v1/rayjob_types.go

+	// If LightWeightSubmissionMode is true, KubeRay operator sends a request to the RayCluster to create a
+	// Ray job. Otherwise, the operator creates a submitter Kubernetes Job to submit the Ray job.
+	// +kubebuilder:default:=false
+	LightWeightSubmissionMode bool `json:"lightWeightSubmissionMode,omitempty"`


I wonder if it's cleaner to enumerate the submission type instead of a bool.

type RayJobSpec struct { ... ... SubmissionMode JobSubmissionMode } type JobSubmissionMode string const ( // Submit job via Kubernetes Job BatchJob JobSubmissionMode = "batchv1.Job" // Submit job via HTTP request HTTP JobSubmissionMode = "HTTP" ... )

I guess it depends on whether we anticipate more modes of job submission going forward.

Good idea. Updated.

andrewsykim · 2024-02-01T14:56:48Z

ray-operator/apis/ray/v1/rayjob_types.go

+type JobSubmissionMode string
+
+const (
+	K8sJobMode JobSubmissionMode = "K8sJobMode" // Submit job via Kubernetes Job


nit: Is the "Mode" part neeeed? I think it's implied from the field name. Maybe just K8sJob and HTTP is enough?

I prefer to add "Mode". Without it, users might find the documentation confusing, as they would have to guess whether "HTTP" refers to the submission mode or the protocol. In addition, "K8sJob" and "K8s Job" can also be confusing.

"HTTP" refers to the submission mode or the protocol

Users will rarely see "HTTP" by itself though. They will usually see submissionMode: HTTP and submissionMode: batchv1.Job.

They will usually see submissionMode: HTTP and submissionMode: batchv1.Job.

This is reasonable in the YAML files and in the KubeRay codebase, but it may not be accurate for the Ray documentation.

ray-operator/apis/ray/v1/rayjob_types.go

docs/reference/api.md

kevin85421 added 5 commits January 31, 2024 08:21

update

dd24623

update

e441029

update

d3db275

update

b1d422d

update

1e26476

kevin85421 commented Jan 31, 2024

View reviewed changes

update

7d8897f

kevin85421 commented Feb 1, 2024

View reviewed changes

kevin85421 changed the title ~~WIP~~ [Feature][RayJob] Support light-weight job submission Feb 1, 2024

kevin85421 marked this pull request as ready for review February 1, 2024 00:09

kevin85421 requested a review from architkulkarni February 1, 2024 00:09

kevin85421 assigned architkulkarni Feb 1, 2024

architkulkarni approved these changes Feb 1, 2024

View reviewed changes

andrewsykim reviewed Feb 1, 2024

View reviewed changes

kevin85421 added 2 commits February 1, 2024 05:52

update

568dcfa

update

fd065aa

andrewsykim reviewed Feb 1, 2024

View reviewed changes

update

ea752a8

kevin85421 merged commit fab00b5 into ray-project:master Feb 1, 2024
23 checks passed

kevin85421 mentioned this pull request Feb 2, 2024

[Bug] Fix rebase error #1897

Merged

4 tasks

rueian mentioned this pull request Feb 5, 2024

[Feature][RayJob] Support light-weight job submission with entrypoint_num_cpus, entrypoint_num_gpus and entrypoint_resources #1904

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature][RayJob] Support light-weight job submission #1893

[Feature][RayJob] Support light-weight job submission #1893

kevin85421 commented Jan 31, 2024 •

edited

Loading

kevin85421 Jan 31, 2024

kevin85421 Jan 31, 2024

architkulkarni Feb 1, 2024

architkulkarni Feb 1, 2024

kevin85421 Feb 1, 2024

kevin85421 Feb 1, 2024

kevin85421 commented Feb 1, 2024

architkulkarni left a comment

architkulkarni Feb 1, 2024

kevin85421 Feb 1, 2024

architkulkarni Feb 1, 2024

andrewsykim Feb 1, 2024 •

edited

Loading

andrewsykim Feb 1, 2024

kevin85421 Feb 1, 2024

andrewsykim Feb 1, 2024

kevin85421 Feb 1, 2024

andrewsykim Feb 1, 2024

kevin85421 Feb 1, 2024

		@@ -222,6 +223,7 @@ type RayJobLogsResponse struct {
		}

		// Note that RayJobInfo and error can't be nil at the same time.

[Feature][RayJob] Support light-weight job submission #1893

[Feature][RayJob] Support light-weight job submission #1893

Conversation

kevin85421 commented Jan 31, 2024 • edited Loading

Why are these changes needed?

Related issue number

Checks

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

kevin85421 commented Feb 1, 2024

architkulkarni 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

andrewsykim Feb 1, 2024 • edited Loading

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

kevin85421 commented Jan 31, 2024 •

edited

Loading

andrewsykim Feb 1, 2024 •

edited

Loading