[RayJob] Directly fail CR if is invalid #3981

machichima · 2025-08-23T10:50:05Z

Set RayJob status to Failed if validation error
Not requeue if validation error

Why are these changes needed?

When trying to create rayjob with name that's too long, we get the validation error. However, the RayJob just keep retrying and does not fail, which cause RayJob to stuck without any status update

Result after my modification, RayJob will directly fail when validation error:

Related issue number

Closes #3980

Checks

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

Signed-off-by: machichima <nary12321@gmail.com>

Future-Outlier · 2025-08-25T08:31:17Z

ray-operator/controllers/ray/rayjob_controller.go

+	// Please do NOT modify `originalRayJobInstance` in the following code.
+	originalRayJobInstance := rayJobInstance.DeepCopy()

-	if err := utils.ValidateRayJobSpec(rayJobInstance); err != nil {
-		logger.Error(err, "The RayJob spec is invalid")
-		r.Recorder.Eventf(rayJobInstance, corev1.EventTypeWarning, string(utils.InvalidRayJobSpec),
-			"The RayJob spec is invalid %s/%s: %v", rayJobInstance.Namespace, rayJobInstance.Name, err)
-		return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
-	}
+	// Perform all validations and directly fail the RayJob if any of the validation fails
+	validationErrors := []struct {
+		err     error
+		errType utils.K8sEventType
+		message string
+	}{
+		{utils.ValidateRayJobMetadata(rayJobInstance.ObjectMeta), utils.InvalidRayJobMetadata, "The RayJob metadata is invalid"},
+		{utils.ValidateRayJobSpec(rayJobInstance), utils.InvalidRayJobSpec, "The RayJob spec is invalid"},
+		{utils.ValidateRayJobStatus(rayJobInstance), utils.InvalidRayJobStatus, "The RayJob status is invalid"},
+	}
+
+	for _, validation := range validationErrors {
+		if validation.err != nil {
+			logger.Error(validation.err, validation.message)
+			r.Recorder.Eventf(rayJobInstance, corev1.EventTypeWarning, string(validation.errType),
+				"%s %s/%s: %v", validation.message, rayJobInstance.Namespace, rayJobInstance.Name, validation.err)
+
+			rayJobInstance.Status.JobStatus = rayv1.JobStatusFailed
+			rayJobInstance.Status.JobDeploymentStatus = rayv1.JobDeploymentStatusFailed
+			rayJobInstance.Status.Reason = rayv1.ValidationFailed
+			rayJobInstance.Status.Message = fmt.Sprintf("%s: %v", validation.message, validation.err)
+
+			if err = r.updateRayJobStatus(ctx, originalRayJobInstance, rayJobInstance); err != nil {
+				logger.Info("Failed to update RayJob status", "error", err)
+				return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
+			}

-	if err := utils.ValidateRayJobStatus(rayJobInstance); err != nil {
-		logger.Error(err, "The RayJob status is invalid")
-		r.Recorder.Eventf(rayJobInstance, corev1.EventTypeWarning, string(utils.InvalidRayJobStatus),
-			"The RayJob status is invalid %s/%s: %v", rayJobInstance.Namespace, rayJobInstance.Name, err)
-		return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
+			return ctrl.Result{}, nil
+		}


I am thinking that should we wrap this whole part to a function?
maybe the name can be checkRayJobSpecAndUpdateStatusIfNeeded?

SG! Updated in 3685d4d

Future-Outlier · 2025-08-25T08:31:42Z

ray-operator/controllers/ray/rayjob_controller.go

+	validationErrors := []struct {
+		err     error


Maybe validationRule is more accurate than validationErrors?

Modified in 771f198, thanks!

fscnick · 2025-08-25T14:31:15Z

ray-operator/controllers/ray/rayjob_controller.go

+	}{
+		{utils.ValidateRayJobMetadata(rayJobInstance.ObjectMeta), utils.InvalidRayJobMetadata, "The RayJob metadata is invalid"},
+		{utils.ValidateRayJobSpec(rayJobInstance), utils.InvalidRayJobSpec, "The RayJob spec is invalid"},
+		{utils.ValidateRayJobStatus(rayJobInstance), utils.InvalidRayJobStatus, "The RayJob status is invalid"},
+	}


This slice might cause some unnecessary execution. If the former validation return an error, the latter still gets executed but it only update the status according the first error.

Fixed in 5066d66 to prevent executing the next validation if the previous one failed, thank you!

fscnick · 2025-08-25T14:39:57Z

ray-operator/controllers/ray/rayjob_controller_unit_test.go

+					},
+				},
+			},
+			shouldContain: "The RayJob metadata is invalid",


Would it be difficult to include the message of error in shouldContain? That could be more specific that it did cause a certain error.

No problem! Added in bd6c07d

Signed-off-by: machichima <nary12321@gmail.com>

owenowenisme · 2025-08-26T15:22:38Z

ray-operator/controllers/ray/rayjob_controller.go

+				return false, ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
+			}
+
+			return false, ctrl.Result{}, nil


Is there any reason not to return the error?

Sorry that's a mistake, updated here: 4de789e

owenowenisme · 2025-08-26T15:40:33Z

ray-operator/controllers/ray/rayjob_controller.go

+			rayJobInstance.Status.Reason = rayv1.ValidationFailed
+			rayJobInstance.Status.Message = fmt.Sprintf("%s: %v", validation.message, err)
+
+			if err = r.updateRayJobStatus(ctx, originalRayJobInstance, rayJobInstance); err != nil {


Should we update rayJob with Kubernetes here?
We should only update rayJob here.

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

Lines 458 to 463 in edf7de8

// This is the only place where we update the RayJob status. Please do NOT add any code

// between `checkBackoffLimitAndUpdateStatusIfNeeded` and the following code.

if err = r.updateRayJobStatus(ctx, originalRayJobInstance, rayJobInstance); err != nil {

logger.Info("Failed to update RayJob status", "error", err)

return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err

}

I tried not to update the ray job status here. However, it will then directly goes into case rayv1.JobDeploymentStatusComplete, rayv1.JobDeploymentStatusFailed and return here:

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

Line 435 in bd6c07d

return ctrl.Result{}, nil

, which will not update the status

I think it's safe to update the status here, as I believe the code you are pointing to is trying to prevent us updating the rayjob status within the switch rayJobInstance.Status.JobDeploymentStatus block. If the rayjob status is updated, here, it will directly return and won't affect anything

owenowenisme · 2025-08-26T15:42:27Z

ray-operator/controllers/ray/rayjob_controller.go

 	// Please do NOT modify `originalRayJobInstance` in the following code.
 	originalRayJobInstance := rayJobInstance.DeepCopy()

+	// Perform all validations and directly fail the RayJob if any of the validation fails
+	if passed, result, err := r.validateRayJobAndUpdateStatusIfNeeded(ctx, rayJobInstance, originalRayJobInstance); !passed || err != nil {


Why use "or " here?
In what scenario would ‎passed be ‎true and ‎err be ‎nil?

That's a mistake I made on not returning the error here return false, ctrl.Result{}, nil. Fixed in 4de789e

Signed-off-by: machichima <nary12321@gmail.com>

rueian · 2025-08-27T05:25:41Z

ray-operator/controllers/ray/rayjob_controller.go

+			if updateErr := r.updateRayJobStatus(ctx, originalRayJobInstance, rayJobInstance); updateErr != nil {
+				logger.Info("Failed to update RayJob status", "error", updateErr)
+				return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, updateErr
+			}


Can we move this out? Ideally, we should have only one updateRayJobStatus usage, the one at the bottom of the reconciliation loop, and we may need to distinguish whether the result is a validation error or a status update error. For validation errors, I think we'd better even stop requeueing reconciliations.

I agree this one, this will keep the RayJob reconciler's logic clean.

Thank you! I added a new status JobDeploymentStatusValidationFailed and ensure there's only one updateRayJobStatus in the very end. Also, if we got validation error, it will not be requeue.

commit: 3002c05

400Ping · 2025-08-27T11:14:40Z

ray-operator/controllers/ray/rayjob_controller.go

+
+			rayJobInstance.Status.JobStatus = rayv1.JobStatusFailed
+			rayJobInstance.Status.JobDeploymentStatus = rayv1.JobDeploymentStatusFailed
+			rayJobInstance.Status.Reason = rayv1.ValidationFailed


Consider emitting a Condition (type=Ready/Valid) alongside Status.Reason=ValidationFailed so kubectl describe surfaces a machine-readable state.

I think this is a great idea and maybe more closer to Kubernetes's best practice, however, I think it will be too much change, we should wait for more users give us feedback if this really bother them.

Thank you! Although not exactly your suggestion, I added a new status ValidationFailed in 3002c05

We can see the ValidationFailed as the DEPLOYMENT STATUS when we do ``kubectl get`

Signed-off-by: machichima <nary12321@gmail.com>

Future-Outlier · 2025-08-28T23:05:13Z

ray-operator/controllers/ray/rayjob_controller.go

+	// Do not requeue if validation error occurs
+	if rayJobInstance.Status.JobDeploymentStatus == rayv1.JobDeploymentStatusValidationFailed {
+		return ctrl.Result{}, validateErr
+	}


I think it would be really weird to add this statement here, we should consider other implementation in my opinion, this makes the code harder to read.

To be more specific, I want to keep the logic of reconciliation clean

rueian · 2025-08-28T23:43:56Z

ray-operator/controllers/ray/rayjob_controller.go

@@ -462,9 +446,42 @@ func (r *RayJobReconciler) Reconcile(ctx context.Context, request ctrl.Request)
 		return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
 	}
 	emitRayJobMetrics(r.options.RayJobMetricsManager, rayJobInstance.Name, rayJobInstance.Namespace, originalRayJobInstance.Status, rayJobInstance.Status)
+	// Do not requeue if validation error occurs
+	if rayJobInstance.Status.JobDeploymentStatus == rayv1.JobDeploymentStatusValidationFailed {
+		return ctrl.Result{}, validateErr


Suggested change

return ctrl.Result{}, validateErr

return ctrl.Result{}, nil

If I remember correctly, returning an error will requeue reconciliation.

Thank you for pointing this out! Fixed in e98978b

Future-Outlier · 2025-08-29T06:36:56Z

ray-operator/controllers/ray/rayjob_controller.go

+	case rayv1.JobDeploymentStatusValidationFailed:
+		// Fall through to status update


I prefer change the deployment status to failed here, so r.updateRayJobStatus(ctx, originalRayJobInstance, rayJobInstance); can help use update the rayJobInstace (CR).
and the next reconciliation, we can remove the CR in the queue.

Suggested change

case rayv1.JobDeploymentStatusValidationFailed:

// Fall through to status update

case rayv1.JobDeploymentStatusValidationFailed:

rayJobInstance.Status.JobDeploymentStatus = rayv1.JobDeploymentStatusFailed

kevin85421

The validation error will appear in the CRD events, right? If so, I don’t think we need to update the status.

[update] On second thought, we still need to update the CR status so that platform teams can program against it more easily.

kevin85421 · 2025-08-30T19:02:21Z

ray-operator/controllers/ray/rayjob_controller.go

 	// Please do NOT modify `originalRayJobInstance` in the following code.
 	originalRayJobInstance := rayJobInstance.DeepCopy()

+	// Perform all validations and directly fail the RayJob if any of the validation fails
+	validateErr := r.validateRayJobAndUpdateStatusIfNeeded(ctx, rayJobInstance)
+


How about calling updateRayJobStatus here directly so that we don't need to go into the switch statement? This makes the implementation simpler.

The comment "This is the only place where we update the RayJob status." is a good suggestion, as it helps avoid calling Status().Update() inside the switch statement, where complexity typically increases significantly. However, since validation is not in the switch statement, updating the status immediately after validation is acceptable and actually reduces complexity.

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

Lines 459 to 461 in f18b1a4

// This is the only place where we update the RayJob status. Please do NOT add any code

// between `checkBackoffLimitAndUpdateStatusIfNeeded` and the following code.

if err = r.updateRayJobStatus(ctx, originalRayJobInstance, rayJobInstance); err != nil {

Fixed in e98978b, thank you!

kevin85421 · 2025-09-01T06:47:07Z

ray-operator/controllers/ray/rayjob_controller.go

@@ -466,6 +456,34 @@ func (r *RayJobReconciler) Reconcile(ctx context.Context, request ctrl.Request)
 	return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, nil
 }

+func (r *RayJobReconciler) validateRayJobAndUpdateStatusIfNeeded(ctx context.Context, rayJobInstance *rayv1.RayJob) error {


Can you move the logic for updating rayJobInstance.Status outside of this function? This function is only responsible to return an error if any.

Rename the function from validateRayJobAndUpdateStatusIfNeeded to validateRayJob.

This no longer needs to be a member function of RayJobReconciler.

Fixed in 70c3e12, thank you!

kevin85421 · 2025-09-01T06:51:03Z

ray-operator/controllers/ray/rayjob_controller.go

+			r.Recorder.Eventf(rayJobInstance, corev1.EventTypeWarning, string(validation.errType),
+				"%s/%s: %v", rayJobInstance.Namespace, rayJobInstance.Name, err)
+
+			rayJobInstance.Status.JobStatus = rayv1.JobStatusFailed


Setting JobStatus to JobStatusFailed is misleading. JobStatus should always reflect the Ray job status retrieved from the Ray dashboard API.

Suggested change

rayJobInstance.Status.JobStatus = rayv1.JobStatusFailed

I agree this one, since we never run ray job submit.

Done! Thanks

Future-Outlier · 2025-09-02T02:01:06Z

I am thinking that if we do this change for RayJob, should we also do it for RayService?

machichima · 2025-09-02T09:30:25Z

Hi @Future-Outlier ,
The test passed, PTAL.
Thank you!

Signed-off-by: Nary <nary@Narys-MacBook-Pro.local>

Future-Outlier · 2025-09-04T03:27:16Z

ray-operator/controllers/ray/rayjob_controller.go

+		if err = r.updateRayJobStatus(ctx, originalRayJobInstance, rayJobInstance); err != nil {
+			logger.Info("Failed to update RayJob status", "error", err)
+			return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
+		}


Suggested change

if err = r.updateRayJobStatus(ctx, originalRayJobInstance, rayJobInstance); err != nil {

logger.Info("Failed to update RayJob status", "error", err)

return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err

}

// This is the only 2 places where we update the RayJob status.

if err = r.updateRayJobStatus(ctx, originalRayJobInstance, rayJobInstance); err != nil {

logger.Info("Failed to update RayJob status", "error", err)

return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err

}

Can you also help me add comments to the code below?

// This is the only place where we update the RayJob status. Please do NOT add any code // between `checkBackoffLimitAndUpdateStatusIfNeeded` and the following code. if err = r.updateRayJobStatus(ctx, originalRayJobInstance, rayJobInstance); err != nil { logger.Info("Failed to update RayJob status", "error", err) return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err }

Added in 0663f3e

Future-Outlier · 2025-09-04T03:29:39Z

ray-operator/controllers/ray/rayjob_controller_unit_test.go

+							Template: corev1.PodTemplateSpec{
+								Spec: corev1.PodSpec{
+									Containers: []corev1.Container{{Image: "rayproject/ray"}},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedMessage: "The RayJob metadata is invalid: RayJob name should be no more than 47 characters",
+		},
+		{
+			name: "spec validation failure - empty containers",
+			rayJob: &rayv1.RayJob{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "valid-name",
+					Namespace: "default",
+				},
+				Spec: rayv1.RayJobSpec{
+					RayClusterSpec: &rayv1.RayClusterSpec{
+						HeadGroupSpec: rayv1.HeadGroupSpec{
+							Template: corev1.PodTemplateSpec{
+								Spec: corev1.PodSpec{
+									Containers: []corev1.Container{},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedMessage: "The RayJob spec is invalid: headGroupSpec should have at least one container",
+		},
+		{
+			name: "status validation failure - invalid status combination",
+			rayJob: &rayv1.RayJob{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "valid-name",
+					Namespace: "default",
+				},
+				Spec: rayv1.RayJobSpec{
+					SubmissionMode: rayv1.K8sJobMode, // not interactive
+					RayClusterSpec: &rayv1.RayClusterSpec{
+						HeadGroupSpec: rayv1.HeadGroupSpec{
+							Template: corev1.PodTemplateSpec{
+								Spec: corev1.PodSpec{
+									Containers: []corev1.Container{{Image: "rayproject/ray"}},
+								},
+							},
+						},
+					},
+				},
+				Status: rayv1.RayJobStatus{
+					JobDeploymentStatus: rayv1.JobDeploymentStatusWaiting,
+				},
+			},
+			expectedMessage: "The RayJob status is invalid: JobDeploymentStatus cannot be `Waiting` when SubmissionMode is not InteractiveMode",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create a fake client with the RayJob
+			fakeClient := clientFake.NewClientBuilder().
+				WithScheme(scheme).
+				WithRuntimeObjects(tt.rayJob).
+				WithStatusSubresource(&rayv1.RayJob{}).
+				Build()
+
+			// Create recorder for events
+			recorder := record.NewFakeRecorder(10)
+			reconciler := &RayJobReconciler{
+				Client:   fakeClient,
+				Recorder: recorder,
+				Scheme:   scheme,
+			}
+
+			// Call Reconcile
+			ctx := context.Background()
+			req := ctrl.Request{
+				NamespacedName: types.NamespacedName{
+					Name:      tt.rayJob.Name,
+					Namespace: tt.rayJob.Namespace,
+				},
+			}
+			result, err := reconciler.Reconcile(ctx, req)
+			require.NoError(t, err)
+
+			// Make sure no requeue happening
+			assert.Equal(t, time.Duration(0), result.RequeueAfter)
+
+			// Verify the status was properly set
+			updatedRayJob := &rayv1.RayJob{}
+			err = fakeClient.Get(ctx, req.NamespacedName, updatedRayJob)
+			require.NoError(t, err)
+			assert.Equal(t, rayv1.JobDeploymentStatusValidationFailed, updatedRayJob.Status.JobDeploymentStatus)
+			assert.Equal(t, rayv1.ValidationFailed, updatedRayJob.Status.Reason)
+			assert.Equal(t, tt.expectedMessage, updatedRayJob.Status.Message)
+		})
+	}
+}


Can we add at least an integration test for this PR?

Sure! Added in 987b4f0

Future-Outlier

Can you help me merge master and solve the conflict? I can find time review this next week, thank you!

machichima · 2025-09-12T13:51:00Z

Can you help me merge master and solve the conflict? I can find time review this next week, thank you!

Sure! I am adding the e2e test you mentioned in the review. Will address all your review this week. Thank you!

…job-metadata-invalid

Signed-off-by: machichima <nary12321@gmail.com>

ray-operator/controllers/ray/rayjob_controller_unit_test.go

machichima · 2025-09-15T07:16:52Z

After discussed with @Future-Outlier , we decided to remove the unit test as it is somehow covered already by test cases in validation_test.go and rayjob_test.go. Will add tests in the future PR if needed

Removed test in: 6a4c5dd

Future-Outlier

LGTM, thank you!

Future-Outlier · 2025-09-15T08:05:34Z

cc @rueian for review

machichima added 3 commits August 23, 2025 18:47

fix: directly raise error if validation fail

a0b0f24

Signed-off-by: machichima <nary12321@gmail.com>

feat: directly return if any validation failed

bc2d327

Signed-off-by: machichima <nary12321@gmail.com>

test: validation fail and rayjob fail

83d959e

Signed-off-by: machichima <nary12321@gmail.com>

machichima marked this pull request as ready for review August 24, 2025 12:58

machichima requested review from kevin85421, andrewsykim, rueian and MortalHappiness as code owners August 24, 2025 12:58

test: invalid yaml fail rayjob

95312cd

Signed-off-by: machichima <nary12321@gmail.com>

Future-Outlier reviewed Aug 25, 2025

View reviewed changes

fscnick reviewed Aug 25, 2025

View reviewed changes

machichima added 3 commits August 26, 2025 20:39

fix: wrap validate func into func in struct

5066d66

Signed-off-by: machichima <nary12321@gmail.com>

refactor: validationErrors to validationRules

771f198

Signed-off-by: machichima <nary12321@gmail.com>

refactor: move validation to function

53cb27d

Signed-off-by: machichima <nary12321@gmail.com>

machichima force-pushed the 3980-rayjob-metadata-invalid branch from 3685d4d to 53cb27d Compare August 26, 2025 13:29

test: use exact error message match

bd6c07d

Signed-off-by: machichima <nary12321@gmail.com>

owenowenisme reviewed Aug 26, 2025

View reviewed changes

fix: return error if return bool is false

4de789e

Signed-off-by: machichima <nary12321@gmail.com>

machichima force-pushed the 3980-rayjob-metadata-invalid branch from f5b5576 to 4de789e Compare August 27, 2025 01:07

rueian reviewed Aug 27, 2025

View reviewed changes

400Ping reviewed Aug 27, 2025

View reviewed changes

machichima added 2 commits August 28, 2025 20:46

feat: add validation fail status&keep update status in one place

3002c05

Signed-off-by: machichima <nary12321@gmail.com>

test: use new status

83d4e2c

Signed-off-by: machichima <nary12321@gmail.com>

machichima force-pushed the 3980-rayjob-metadata-invalid branch from a220ac1 to 83d4e2c Compare August 28, 2025 12:57

Future-Outlier reviewed Aug 28, 2025

View reviewed changes

rueian reviewed Aug 28, 2025

View reviewed changes

Future-Outlier reviewed Aug 29, 2025

View reviewed changes

kevin85421 reviewed Aug 30, 2025

View reviewed changes

kevin85421 reviewed Sep 1, 2025

View reviewed changes

machichima force-pushed the 3980-rayjob-metadata-invalid branch from a1059ba to 70c3e12 Compare September 2, 2025 05:55

machichima added 3 commits September 2, 2025 06:13

refactor: update status outside validation func

b575a13

test: fix test fail

36bfe3f

Trigger CI

920ae1d

Signed-off-by: Nary <nary@Narys-MacBook-Pro.local>

machichima force-pushed the 3980-rayjob-metadata-invalid branch from 5bf5706 to 920ae1d Compare September 2, 2025 13:14

Future-Outlier reviewed Sep 4, 2025

View reviewed changes

Future-Outlier reviewed Sep 12, 2025

View reviewed changes

machichima added 4 commits September 12, 2025 07:05

Merge branch 'master' of github.com:ray-project/kuberay into 3980-ray…

7591f40

…job-metadata-invalid

refactor: update comment for updateJobStatus

0663f3e

test: add e2e test for rayjob validate

987b4f0

Trigger CI

7ba0b48

Signed-off-by: machichima <nary12321@gmail.com>

Future-Outlier mentioned this pull request Sep 15, 2025

[RayJob] ClusterSelector shouldn't support SidecarMode #4074

Merged

4 tasks

Future-Outlier reviewed Sep 15, 2025

View reviewed changes

ray-operator/controllers/ray/rayjob_controller_unit_test.go Outdated Show resolved Hide resolved

machichima added 2 commits September 15, 2025 15:04

refactor: use string.Repeat in test for better readability

2d2134c

test: remove unit test with too much details

6a4c5dd

Future-Outlier approved these changes Sep 15, 2025

View reviewed changes

400Ping approved these changes Sep 15, 2025

View reviewed changes

rueian approved these changes Sep 15, 2025

View reviewed changes

rueian merged commit 6e70fd2 into ray-project:master Sep 16, 2025
27 checks passed

Future-Outlier changed the title ~~[Fix] Directly fail if RayJob metadata is invalid~~ [RayJob] Directly fail if metadata is invalid Sep 16, 2025

Future-Outlier changed the title ~~[RayJob] Directly fail if metadata is invalid~~ [RayJob] Directly fail if spec is invalid Sep 16, 2025

Future-Outlier changed the title ~~[RayJob] Directly fail if spec is invalid~~ [RayJob] Directly fail CR is invalid Sep 16, 2025

Future-Outlier changed the title ~~[RayJob] Directly fail CR is invalid~~ [RayJob] Directly fail CR if is invalid Sep 16, 2025

	// This is the only place where we update the RayJob status. Please do NOT add any code
	// between `checkBackoffLimitAndUpdateStatusIfNeeded` and the following code.
	if err = r.updateRayJobStatus(ctx, originalRayJobInstance, rayJobInstance); err != nil {
	logger.Info("Failed to update RayJob status", "error", err)
	return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
	}

		case rayv1.JobDeploymentStatusValidationFailed:
		// Fall through to status update

[RayJob] Directly fail CR if is invalid #3981

[RayJob] Directly fail CR if is invalid #3981

Conversation

machichima commented Aug 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Related issue number

Checks

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machichima Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rueian Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevin85421 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

machichima commented Aug 23, 2025 •

edited

Loading

machichima Aug 27, 2025 •

edited

Loading

rueian Aug 27, 2025 •

edited

Loading

kevin85421 left a comment •

edited

Loading

Future-Outlier commented Sep 2, 2025 •

edited

Loading

Future-Outlier Sep 4, 2025 •

edited

Loading