[E2E] [RayCronJob] add e2e test for suspend behavior #4349

AndySung320 · 2026-01-06T08:00:42Z

Why are these changes needed?

This PR adds an e2e test covering RayCronJob suspend/resume behavior:

When spec.suspend=true, the RayCronJob controller should not create any new RayJobs even after the scheduled time passes.
After setting spec.suspend=false, the controller should resume scheduling and start creating RayJobs again.

To run this e2e test in CI, we also enable the RayCronJob feature gate in the test/CI-only operator overrides (Helm values override and kustomize test override).

Note: RayCronJob previously did not have // +genclient, so the typed client/applyconfiguration for RayCronJob was not generated. The PR adds +genclient and commits the resulting regenerated clientset/applyconfiguration/informer/lister code so the test can compile and run consistently.

Related issue number

Closes #4323

Checks

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

…guration Signed-off-by: AndySung320 <[email protected]>

Signed-off-by: AndySung320 <[email protected]>

seanlaii

Thank you!

seanlaii · 2026-01-06T21:28:45Z

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

 //+kubebuilder:printcolumn:name="age",type="date",JSONPath=".metadata.creationTimestamp",priority=0
 //+kubebuilder:printcolumn:name="suspend",type=boolean,JSONPath=".spec.suspend",priority=0

+// +genclient


Good catch!
Maybe we can also add the following annotations as well:

// +kubebuilder:storageversion // +kubebuilder:resource:categories=all

// +kubebuilder:storageversion: marker to indicate the GVK that should be used to store data by the API server.

// +kubebuilder:resource:categories=all: Puts the CRD into the all category, so it shows up in kubectl get all.

Ref:
https://book.kubebuilder.io/reference/markers/crd
https://book.kubebuilder.io/reference/generating-crd#multiple-versions

Cool!

seanlaii · 2026-01-06T21:37:25Z

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

+		g.Expect(err).NotTo(HaveOccurred())
+		ownerUID := rcj.UID
+
+		// No RayJob should be created


nit:

Suggested change

// No RayJob should be created

// No RayJob should be created

LogWithTimestamp(test.T(), "Waiting to ensure no RayJobs are created while suspended")

seanlaii · 2026-01-06T21:38:10Z

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

+			return n
+		}, 130*time.Second, 5*time.Second).Should(Equal(0))
+
+		// Resume


nit:

Suggested change

// Resume

LogWithTimestamp(test.T(), "Resuming RayCronJob %s/%s", rayCronJob.Namespace, rayCronJob.Name)

seanlaii · 2026-01-06T21:40:26Z

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

+			if err != nil {
+				return -1
+			}
+			return n


To preserve error:

Suggested change

if err != nil {

return -1

}

return n

g.Expect(err).NotTo(HaveOccurred())

return n

seanlaii · 2026-01-06T21:45:50Z

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

+
+		// Spec.suspend should be false
+		g.Eventually(func() bool {
+			rcj, err := test.Client().Ray().RayV1().RayCronJobs(namespace.Name).Get(test.Ctx(), rayCronJob.Name, metav1.GetOptions{})


Maybe we can replace this with a util function similar to GetRayJob.

Future-Outlier · 2026-01-07T04:27:21Z

cursor review

cursor

✅ Bugbot reviewed your changes and found no bugs!

Future-Outlier

cc @machichima @CheyuWu @justinyeh1995 @fscnick to take a look, thank you!

justinyeh1995 · 2026-01-08T01:08:21Z

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

+func rayCronJobACTemplate(name, namespace, schedule string) *rayv1ac.RayCronJobApplyConfiguration {
+	return rayv1ac.RayCronJob(name, namespace).
+		WithSpec(
+			rayv1ac.RayCronJobSpec().
+				WithSchedule(schedule).
+				WithJobTemplate(
+					rayv1ac.RayJobSpec().
+						WithEntrypoint("sleep 1").
+						WithRayClusterSpec(
+							rayv1ac.RayClusterSpec().
+								WithHeadGroupSpec(
+									rayv1ac.HeadGroupSpec().
+										WithTemplate(
+											corev1ac.PodTemplateSpec().
+												WithSpec(
+													corev1ac.PodSpec().
+														WithContainers(
+															corev1ac.Container().
+																WithName("ray-head").
+																WithImage(GetRayImage()).
+																WithResources(
+																	corev1ac.ResourceRequirements().
+																		WithRequests(corev1.ResourceList{
+																			corev1.ResourceCPU:    resource.MustParse("500m"),
+																			corev1.ResourceMemory: resource.MustParse("500Mi"),
+																		}),
+																),
+														),
+												),
+										),
+								),
+						),
+				),
+		)
+}


nit: for readability, would it make sense to construct it bottom-up using intermediate variables?

For example

func rayCronJobACTemplate(name, namespace, schedule string) *rayv1ac.RayCronJobApplyConfiguration { headContainer := corev1ac.Container(). WithName("ray-head"). WithImage(GetRayImage()). WithResources( corev1ac.ResourceRequirements(). WithRequests(corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("500m"), corev1.ResourceMemory: resource.MustParse("500Mi"), }), ) podTmpl := corev1ac.PodTemplateSpec(). WithSpec(corev1ac.PodSpec().WithContainers(headContainer)) cluster := ... job := ... spec := ... return rayv1ac.RayCronJob(name, namespace).WithSpec(spec) }

I think we can simply use:

func rayCronJobACTemplate(name, namespace, schedule string) *rayv1ac.RayCronJobApplyConfiguration { return rayv1ac.RayCronJob(name, namespace). WithSpec( rayv1ac.RayCronJobSpec(). WithSchedule(schedule). WithJobTemplate( rayv1ac.RayJobSpec(). WithEntrypoint("sleep 1"). WithRayClusterSpec(NewRayClusterSpec()), ), ) }

That's a great suggestion, thanks!

fscnick · 2026-01-09T15:14:59Z

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

+				return -1
+			}
+			return n
+		}, 130*time.Second, 5*time.Second).Should(Equal(0))


nit:

Suggested change

}, 130*time.Second, 5*time.Second).Should(Equal(0))

}, 130*time.Second, 5*time.Second).Should(BeZero())

fscnick · 2026-01-09T15:32:46Z

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

+		err = test.Client().Ray().RayV1().RayCronJobs(namespace.Name).Delete(test.Ctx(), rayCronJob.Name, metav1.DeleteOptions{})
+		g.Expect(err).NotTo(HaveOccurred())
+		LogWithTimestamp(test.T(), "Deleted RayCronJob %s/%s successfully", rayCronJob.Namespace, rayCronJob.Name)
+	})


Should we assert that the related resource has been deleted correctly?

win5923 · 2026-01-09T16:27:44Z

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

+		// Spec.suspend should be true
+		g.Eventually(func() bool {
+			job, err := test.Client().
+				Ray().
+				RayV1().
+				RayCronJobs(namespace.Name).
+				Get(test.Ctx(), rayCronJob.Name, metav1.GetOptions{})
+			if err != nil {
+				return false
+			}
+			return job.Spec.Suspend
+		}, TestTimeoutShort).Should(BeTrue())


Suggested change

// Spec.suspend should be true

g.Eventually(func() bool {

job, err := test.Client().

Ray().

RayV1().

RayCronJobs(namespace.Name).

Get(test.Ctx(), rayCronJob.Name, metav1.GetOptions{})

if err != nil {

return false

}

return job.Spec.Suspend

}, TestTimeoutShort).Should(BeTrue())

I think this check is redundant. Since the Server-Side Apply call already succeeded, the field is guaranteed to be set in the API server. The subsequent Consistently block provides sufficient validation by ensuring no jobs are actually created.

win5923 · 2026-01-09T16:46:13Z

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

+			return job.Spec.Suspend
+		}, TestTimeoutShort).Should(BeTrue())
+
+		rcj, err := test.Client().Ray().RayV1().RayCronJobs(namespace.Name).Get(test.Ctx(), rayCronJob.Name, metav1.GetOptions{})


Could we rename rcj to rayCronJob for clarity?

AndySung320 added 2 commits January 5, 2026 23:37

chore(raycronjob): generate client, informer, lister, and apply confi…

5d5eee7

…guration Signed-off-by: AndySung320 <[email protected]>

test(e2e): add RayCronJob suspend test and enable feature gate in CI

fd62246

Signed-off-by: AndySung320 <[email protected]>

AndySung320 requested review from andrewsykim, kevin85421 and rueian as code owners January 6, 2026 08:00

seanlaii reviewed Jan 6, 2026

View reviewed changes

cursor bot reviewed Jan 7, 2026

View reviewed changes

Future-Outlier moved this to can be merged in @Future-Outlier's kuberay project Jan 7, 2026

Future-Outlier added this to @Future-Outlier's kuberay project Jan 7, 2026

Future-Outlier reviewed Jan 7, 2026

View reviewed changes

Future-Outlier moved this from can be merged to Todo in @Future-Outlier's kuberay project Jan 7, 2026

CheyuWu self-requested a review January 7, 2026 16:33

justinyeh1995 reviewed Jan 8, 2026

View reviewed changes

fscnick reviewed Jan 9, 2026

View reviewed changes

win5923 reviewed Jan 9, 2026

View reviewed changes

	// No RayJob should be created
	// No RayJob should be created
	LogWithTimestamp(test.T(), "Waiting to ensure no RayJobs are created while suspended")

	// Resume
	LogWithTimestamp(test.T(), "Resuming RayCronJob %s/%s", rayCronJob.Namespace, rayCronJob.Name)

	}, 130time.Second, 5time.Second).Should(Equal(0))
	}, 130time.Second, 5time.Second).Should(BeZero())

[E2E] [RayCronJob] add e2e test for suspend behavior #4349

Are you sure you want to change the base?

[E2E] [RayCronJob] add e2e test for suspend behavior #4349

Conversation

AndySung320 commented Jan 6, 2026

Why are these changes needed?

Related issue number

Checks

Uh oh!

seanlaii left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

win5923 Jan 9, 2026 • 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!

Future-Outlier commented Jan 7, 2026

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Future-Outlier left a comment

Choose a reason for hiding this comment

Uh oh!

justinyeh1995 Jan 8, 2026 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

win5923 Jan 9, 2026 •

edited

Loading

justinyeh1995 Jan 8, 2026 •

edited

Loading