[RayCluster][Kueue] Delete Services during suspension by apullo777 · Pull Request #4935 · ray-project/kuberay

apullo777 · 2026-06-22T18:12:48Z

Why are these changes needed?

Suspending a RayCluster deleted its Pods but left the head, serve, and headless Services behind. This made standalone RayCluster inconsistent with the other suspend paths.

This PR deletes those Services while suspended and recreates them on resume. It preserves the existing suspend commit point, but tears Pods down before Services so a failed Service deletion cannot block quota release.

The status path now tolerates the head Service being absent during suspension, allowing the suspend/resume transition to complete normally.

Scope is limited to the Pods and Services called out in #4876. Other owned resources have different lifecycle semantics and are left unchanged.

One behavior worth calling out: RayClusterSuspended still becomes true once the Pods are gone, rather than waiting for every owned resource to be deleted.

Related issue number

Closes #4876

Checks

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

AndySung320

Agree with the potential follow-up, I have also proposed that in #4896 (comment)

Just to make sure I understand the intended scope: is this issue about RayCluster-owned resources only, or about the cross-controller case where the suspended RayCluster is owned by a RayJob and we also need to clean up RayJob-owned resources such as the submitter Job/Pod? If it is the latter, I think the test should probably involve a RayJob-owned RayCluster rather than only a standalone RayCluster.

apullo777 · 2026-06-23T01:28:47Z

Just to make sure I understand the intended scope: is this issue about RayCluster-owned resources only, or about the cross-controller case where the suspended RayCluster is owned by a RayJob and we also need to clean up RayJob-owned resources such as the submitter Job/Pod? If it is the latter, I think the test should probably involve a RayJob-owned RayCluster rather than only a standalone RayCluster.

Thanks for pointing this out. I agree the scope is a bit easy to mix up.

My understanding is that this PR is scoped to the standalone RayCluster suspend path, not the RayJob-owned / cross-controller cleanup path. That’s why this PR uses a standalone RayCluster and only checks that RayCluster-owned Pods are deleted after suspend.

Happy to adjust if maintainers intended this to cover the broader RayJob-owned cleanup path instead.

Future-Outlier · 2026-06-25T03:29:37Z

I am actually thinking that we might not need this PR, since this PR already does the same thing.
#4896

apullo777 · 2026-06-25T03:53:30Z

I am actually thinking that we might not need this PR, since this PR already does the same thing. #4896

Thanks for checking. I agree there is overlap in the observable outcome that RayCluster-related Pods are gone.

The distinction I had in mind is that #4896 covers the RayJob suspend path, where the RayCluster CR is deleted and child resources are cleaned up by Kubernetes GC. This PR covers standalone RayCluster spec.suspend, where TestRayClusterSuspend previously only checked status conditions, not Pod deletion.

If you think the #4896 coverage is sufficient for #4876, I’m happy to close this PR as duplicate!

Future-Outlier

plz update the pr description
plz don't link the original rayjob suspend issue

apullo777 · 2026-06-25T04:41:55Z

plz update the pr description

plz don't link the original rayjob suspend issue

Done. Updated the PR description and kept it focused on RayCluster suspend (#4876). I also removed the RayJob suspend cross-reference from my previous comment where possible. Thanks!

Future-Outlier · 2026-06-27T01:06:07Z

cc @ruiean to merge, tks!

win5923

IIUC, I think #4876 is about deleting all child resources managed by the RayCluster. We should also delete the Service so that the suspend behavior is consistent across all KubeRay CRs.

kevin85421 · 2026-09-02T20:18:36Z

cc @win5923 @machichima

apullo777 · 2026-09-02T20:18:51Z

We should also delete the Service so that the suspend behavior is consistent across all KubeRay CRs.

@win5923 Thanks, I see! I went back through #4876 and expanded this beyond the original test-only scope.

It now deletes the RayCluster-owned Services on suspend and brings them back on resume. I also kept the existing suspend commit-point semantics, but moved Pod teardown ahead of Service teardown so a failed Service deletion can't hold up quota release.

The status path now handles the missing head Service as well, and the suspend coverage exercises the full suspend/resume path.

For now I've kept this to the Pods + Services called out in #4876. The other owned resources have different lifecycle semantics, especially the GCS PVC, so I'd rather treat those separately if we want to extend suspend further.

cursor

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

^{Reviewed by Cursor Bugbot for commit dbbbf78. Configure here.}

win5923 · 2026-09-03T18:49:40Z

+// rayClusterSuspendDeletesServices stays armed for as long as a cluster is suspended,
+// unlike the Pod teardown, which ends the moment suspension completes. Pods cannot
+// reappear on their own; Services left behind by an older operator still have to
+// converge, without asking the user to resume first.
+func rayClusterSuspendDeletesServices(instance *rayv1.RayCluster) bool {
+	return rayClusterSuspendCommitted(instance) ||
+		utils.FindRayClusterSuspendStatus(instance) == rayv1.RayClusterSuspended
+}


Just wondering, should we follow the same suspend state machine as RayJob and RayService?

(no suspend) --Spec.Suspend=true--> Suspending --owned resources deleted--> Suspended --Spec.Suspend=false--> (no suspend)

The current implementation transitions to RayClusterSuspended based only on the Pod count:

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

Lines 2065 to 2072 in 215b98f

case rayv1.RayClusterSuspending:

if len(runtimePods.Items) == 0 {

meta.SetStatusCondition(&newInstance.Status.Conditions, metav1.Condition{

Type: string(rayv1.RayClusterProvisioned),

Status: metav1.ConditionFalse,

Reason: rayv1.RayClusterPodsProvisioning,

Message: "RayCluster has been suspended",

})

This means a K8s Service deletion can still be in progress or failing in the Suspended state.

Would it make more sense to wait until all owned resources, are deleted before transitioning to RayClusterSuspended, consistent with RayJob and RayService?

Thanks! I think that makes sense and ended up going with that model. I changed the condition to follow that model: RayClusterSuspended now waits until the Pods and owned Services are actually gone, including Services left behind by an older operator. A terminating Service keeps it in Suspending and is reported in the condition message.

I kept .Status.State Pod-gated, though. Kueue still uses State == Ready as the RayCluster's active signal, so waiting on Service teardown there could delay quota release behind a stuck finalizer.

I think this gives us the stricter condition semantics without changing the existing Kueue contract, but curious what you think?

I also moved the suspend Pod/Service reconciliation ahead of unrelated resources so failures there can't block teardown. The wait stays scoped to the Pods + Services from #4876; the other resource lifecycles are unchanged.

Suspending a RayCluster deleted its Pods but left the head, serve and headless Services behind, which made standalone RayCluster inconsistent with the other suspend paths. Delete those Services while the cluster is suspended and recreate them on resume. The existing suspend commit point is unchanged. The RayClusterSuspending -> RayClusterSuspended transition now waits for those Services to be deleted, so the condition means the teardown this path performs is done. While it waits, the Suspending condition names the Services that are still going, so a deletion that never finishes is visible rather than an unexplained stall. A cluster suspended by an operator that predates this teardown gives up its Suspended condition until its leftover Services are collected, rather than reporting a teardown that has not happened. .Status.State keeps its older, Pod-gated meaning. It is deprecated in favour of the conditions, but Kueue reads it as the RayCluster's active signal and releases an evicted Workload's quota only once it stops reading Ready, so gating it on the Services as well would hold that quota behind a Service that is slow, or unable, to go away. During a committed suspend the Pods and Services are reconciled ahead of the other resources. The pass stops at the first error, so an unrelated permanent failure would otherwise leave the Services undeleted and the cluster stuck in RayClusterSuspending, unable to resume. The status path now tolerates the head Service being absent while the suspension explains it, or while a failed reconcile does. Without that, calculateStatus would discard the status it had just computed and the cluster could never finish suspending or resuming. Scope is the Pods and Services called out in ray-project#4876. Other owned resources have different lifecycle semantics and are left unchanged.

AndySung320 reviewed Jun 22, 2026

View reviewed changes

Future-Outlier self-assigned this Jun 23, 2026

Future-Outlier moved this to this week in @Future-Outlier's kuberay project Jun 23, 2026

Future-Outlier added this to @Future-Outlier's kuberay project Jun 23, 2026

Future-Outlier moved this from this week to close to merge in @Future-Outlier's kuberay project Jun 24, 2026

Future-Outlier reviewed Jun 24, 2026

View reviewed changes

Comment thread ray-operator/test/e2e/raycluster_test.go Outdated

Future-Outlier reviewed Jun 25, 2026

View reviewed changes

Comment thread ray-operator/test/e2e/raycluster_test.go Outdated

apullo777 force-pushed the test/raycluster-suspend-delete-owned-pods branch from a995063 to 3418e9a Compare June 25, 2026 03:07

Future-Outlier reviewed Jun 25, 2026

View reviewed changes

Comment thread ray-operator/test/e2e/raycluster_test.go Outdated

Future-Outlier moved this from close to merge to to review in @Future-Outlier's kuberay project Jun 25, 2026

apullo777 force-pushed the test/raycluster-suspend-delete-owned-pods branch from 3418e9a to 49ec22d Compare June 25, 2026 03:47

Future-Outlier approved these changes Jun 25, 2026

View reviewed changes

Future-Outlier moved this from to review to can be merged in @Future-Outlier's kuberay project Jun 25, 2026

AndySung320 approved these changes Jun 25, 2026

View reviewed changes

win5923 reviewed Aug 5, 2026

View reviewed changes

apullo777 force-pushed the test/raycluster-suspend-delete-owned-pods branch from 49ec22d to 1eced7d Compare September 2, 2026 20:12

apullo777 requested review from MortalHappiness, andrewsykim, kevin85421 and rueian as code owners September 2, 2026 20:12

apullo777 force-pushed the test/raycluster-suspend-delete-owned-pods branch from 1eced7d to 9c41925 Compare September 2, 2026 20:16

apullo777 changed the title ~~[Test][RayCluster] Assert owned Pods deleted on suspend~~ [RayCluster][Kueue] Delete Services during suspension Sep 2, 2026

apullo777 force-pushed the test/raycluster-suspend-delete-owned-pods branch from 9c41925 to dbbbf78 Compare September 2, 2026 20:36

cursor Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread ray-operator/controllers/ray/raycluster_controller.go

win5923 reviewed Sep 3, 2026

View reviewed changes

Comment thread ray-operator/controllers/ray/raycluster_controller.go Outdated

Comment thread ray-operator/controllers/ray/raycluster_controller.go Outdated

apullo777 force-pushed the test/raycluster-suspend-delete-owned-pods branch from dbbbf78 to 17ccc02 Compare September 5, 2026 15:38

	case rayv1.RayClusterSuspending:
	if len(runtimePods.Items) == 0 {
	meta.SetStatusCondition(&newInstance.Status.Conditions, metav1.Condition{
	Type: string(rayv1.RayClusterProvisioned),
	Status: metav1.ConditionFalse,
	Reason: rayv1.RayClusterPodsProvisioning,
	Message: "RayCluster has been suspended",
	})

Uh oh!

Conversation

apullo777 commented Jun 22, 2026 • 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!

AndySung320 left a comment

Choose a reason for hiding this comment

Uh oh!

apullo777 commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Future-Outlier commented Jun 25, 2026

Uh oh!

apullo777 commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Future-Outlier left a comment

Choose a reason for hiding this comment

Uh oh!

apullo777 commented Jun 25, 2026

Uh oh!

Future-Outlier commented Jun 27, 2026

Uh oh!

win5923 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!

kevin85421 commented Sep 2, 2026

Uh oh!

apullo777 commented Sep 2, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

win5923 Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

apullo777 Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

apullo777 commented Jun 22, 2026 •

edited

Loading

apullo777 commented Jun 23, 2026 •

edited

Loading

apullo777 commented Jun 25, 2026 •

edited

Loading

win5923 left a comment •

edited

Loading