[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 :(

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!

thanks for the suggestion~

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!

Great point!

thanks for the suggestion~

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?

In this test Delete() is only cleanup; the assertions focus on suspend/resume affecting RayJob creation. The RayJob suspend e2e asserts cascade deletion because that’s part of its feature contract. Happy to add an Eventually(Get).Should(BeNotFound()) after Delete for explicitness if you prefer.
what do you think ?

Moreover, it could make sure this test exits safe and sound without leaving something to interfere others.

However, some tests do it and some don't. I guess it might not be mandatory. Feel free to do it or not.

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

machichima · 2026-01-12T03:04:20Z

We also need to add it to https://github.com/ray-project/kuberay/blob/master/.buildkite/test-e2e.yml so that the CI can run it. Could you please add Test RayCronJob E2E (nightly operator) to test-e2e.yml?

Future-Outlier

cc @JiangJiaWei1103 @win5923 @fsNick @machichima to do final pass

fscnick

LGTM

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

machichima

LGTM! Just a small nit

…guration Signed-off-by: AndySung320 <andysung0320@gmail.com>

Signed-off-by: AndySung320 <andysung0320@gmail.com>

Co-authored-by: Nary Yeh <60069744+machichima@users.noreply.github.com> Signed-off-by: AndySung320 <71032763+AndySung320@users.noreply.github.com>

Signed-off-by: AndySung320 <andysung0320@gmail.com>

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

.buildkite/test-e2e.yml

Signed-off-by: AndySung320 <andysung0320@gmail.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

Signed-off-by: AndySung320 <andysung0320@gmail.com>

Future-Outlier

I'm ok with current's implementation, but will it be better if we use label selector to filter RayJob?
https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#list-and-watch-filtering

AndySung320 · 2026-01-20T08:40:07Z

label selector could work, but I chose ownerRef UID filtering for correctness.
Using OwnerReferences.UID makes sure that we only count RayJobs created by this specific RayCronJob instance, while labels don’t provide uniqueness and could accidentally match other RayJobs if labels are reused/changed in the future. So UID-based filtering would be less error-prone for this e2e assertion.

Future-Outlier · 2026-01-20T09:49:03Z

label selector could work, but I chose ownerRef UID filtering for correctness.

Using OwnerReferences.UID makes sure that we only count RayJobs created by this specific RayCronJob instance, while labels don’t provide uniqueness and could accidentally match other RayJobs if labels are reused/changed in the future. So UID-based filtering would be less error-prone for this e2e assertion.

But we will have unique namespace, right? So this will still work

AndySung320 · 2026-01-20T19:12:53Z

I'm wondering that should we optimize for the current test scenario (where label selector is simpler), or consider potential future extensions?
For example, if we in the future add tests that create multiple RayCronJobs in the same namespace, OwnerReference.UID would be more robust. But if we don't foresee that scenario, I'm happy to switch to label selector for simplicity.
what do you think?

Future-Outlier · 2026-01-21T07:25:36Z

I'm wondering that should we optimize for the current test scenario (where label selector is simpler), or consider potential future extensions? For example, if we in the future add tests that create multiple RayCronJobs in the same namespace, OwnerReference.UID would be more robust. But if we don't foresee that scenario, I'm happy to switch to label selector for simplicity. what do you think?

make sense

Signed-off-by: Future-Outlier <eric901201@gmail.com>

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

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go Outdated Show resolved Hide resolved

win5923 reviewed Jan 9, 2026

View reviewed changes

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go Outdated Show resolved Hide resolved

Future-Outlier reviewed Jan 14, 2026

View reviewed changes

fscnick approved these changes Jan 14, 2026

View reviewed changes

win5923 approved these changes Jan 14, 2026

View reviewed changes

Future-Outlier moved this from to review to In Progress in @Future-Outlier's kuberay project Jan 15, 2026

machichima reviewed Jan 18, 2026

View reviewed changes

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go Outdated Show resolved Hide resolved

machichima approved these changes Jan 18, 2026

View reviewed changes

AndySung320 and others added 7 commits January 18, 2026 01:37

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

307dd63

…guration Signed-off-by: AndySung320 <andysung0320@gmail.com>

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

5e8dc71

Signed-off-by: AndySung320 <andysung0320@gmail.com>

add kubebuilder markers and refine RayCronJob test

05defb2

Signed-off-by: AndySung320 <andysung0320@gmail.com>

ci: add RayCronJob e2e test to Buildkite pipeline

e40bf8c

Signed-off-by: AndySung320 <andysung0320@gmail.com>

sync RayCronJob CRDs after kubebuilder marker update

8b8667d

Signed-off-by: AndySung320 <andysung0320@gmail.com>

Update ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go

54cca7c

Co-authored-by: Nary Yeh <60069744+machichima@users.noreply.github.com> Signed-off-by: AndySung320 <71032763+AndySung320@users.noreply.github.com>

Update generated code

9cc2e90

Signed-off-by: AndySung320 <andysung0320@gmail.com>

AndySung320 force-pushed the raycronjob/add-suspend-e2e-test branch from 91df5ac to 9cc2e90 Compare January 18, 2026 10:14

cursor bot reviewed Jan 18, 2026

View reviewed changes

.buildkite/test-e2e.yml Outdated Show resolved Hide resolved

.buildkite/test-e2e.yml Outdated Show resolved Hide resolved

Update test-e2e.yml to match the pattern

fb0ffdd

Signed-off-by: AndySung320 <andysung0320@gmail.com>

cursor bot reviewed Jan 18, 2026

View reviewed changes

ray-operator/test/e2eraycronjob/raycronjob_suspend_test.go Show resolved Hide resolved

test(e2e): avoid using Expect inside Eventually/Consistently callbacks

ddc2977

Signed-off-by: AndySung320 <andysung0320@gmail.com>

Future-Outlier moved this from In Progress to can be merged in @Future-Outlier's kuberay project Jan 19, 2026

Future-Outlier reviewed Jan 20, 2026

View reviewed changes

Future-Outlier approved these changes Jan 21, 2026

View reviewed changes

solve merge conflict

85d3b26

Signed-off-by: Future-Outlier <eric901201@gmail.com>

	// 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

Uh oh!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

machichima commented Jan 12, 2026

Uh oh!

Future-Outlier left a comment

Choose a reason for hiding this comment

Uh oh!

fscnick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

machichima left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

win5923 Jan 9, 2026 •

edited

Loading

justinyeh1995 Jan 8, 2026 •

edited

Loading