[RayService] Rollback Support for Incremental Upgrades by ryanaoleary · Pull Request #4109 · ray-project/kuberay

ryanaoleary · 2025-10-03T12:46:33Z

Why are these changes needed?

Implement rollback support for the IncrementalUpgrade RayService feature.

Related issue number

#3209

Checks

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

ryanaoleary · 2025-10-05T00:37:51Z

cc: @Future-Outlier the changes for full rollback support are just in this commit: f5d0243

Signed-off-by: Ryan O'Leary <[email protected]>

ryanaoleary · 2026-03-05T12:08:39Z

cc: @Future-Outlier this is rebased on the latest changes now and should be good to review

ray-operator/controllers/ray/rayservice_controller.go

Signed-off-by: Ryan O'Leary <[email protected]>

ray-operator/controllers/ray/rayservice_controller.go

Signed-off-by: Ryan O'Leary <[email protected]>

ray-operator/controllers/ray/rayservice_controller.go

Signed-off-by: Ryan O'Leary <[email protected]>

ray-operator/test/e2eincrementalupgrade/rayservice_incremental_upgrade_test.go

Signed-off-by: Ryan O'Leary <[email protected]>

ray-operator/controllers/ray/rayservice_controller.go

Signed-off-by: Ryan O'Leary <[email protected]>

ray-operator/controllers/ray/rayservice_controller.go

Future-Outlier · 2026-03-09T11:47:17Z

cc @JiangJiaWei1103 to take a look too

Signed-off-by: Ryan O'Leary <[email protected]>

ray-operator/controllers/ray/rayservice_controller.go

…ays evaluate false Signed-off-by: Ryan O'Leary <[email protected]>

ray-operator/controllers/ray/rayservice_controller.go

Signed-off-by: Ryan O'Leary <[email protected]>

andrewsykim

I had some nits about naming and terminology in the logs, but otherwise I think the logic seems fine. @Future-Outlier @rueian any concerns?

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

ray-operator/controllers/ray/rayservice_controller.go

andrewsykim · 2026-03-12T02:19:39Z

ray-operator/controllers/ray/rayservice_controller.go

+		if isRollbackInProgress {
+			// Gradually shift traffic from the pending to the active cluster.
+			// Rollback traffic migration occurs regardless of pending cluster readiness.
+			logger.Info("Rollback in progress. Shifting traffic back to active cluster.", "stepSize", *options.StepSizePercent)


I think the active / pending terminology will get confusing going forward, especially as we add rollback support. Do we use these terms in the APIs or only in the logs? If only in the logs we should consider changing the terminology in a future PR. Maybe next cluster and previous cluster might be easier to understand.

active and pending are used in the API in ActiveServiceStatus and PendingServiceStatus, so I used similar terminology when referring to the cluster that's being scaled for each service. Currently we only refer to the active/pending clusters in the comments/logs though so that should be fine to change to next cluster and previous cluster. Can follow-up in the next PR if needed

andrewsykim · 2026-03-12T02:24:22Z

cc @spencer-p for review as well since you've been thinking about Gateway integrations in KubeRay as well

Signed-off-by: Ryan O'Leary <[email protected]>

Future-Outlier

I will come back to review this after kuberay 1.6 release
due to my personal bandwidth, I might don't have bandwidth review this now
if @andrewsykim and @rueian approve, I am ok to merge this
we will need to merge @JiangJiaWei1103 's e2e test for incremental upgrades before this get merged

Future-Outlier · 2026-03-14T06:31:15Z

We committed in the KubeRay community sync that the E2E tests must be merged before we can merge the incremental upgrade PR. I may not have communicated that clearly enough earlier. If Ryan and Jia-Wei’s E2E tests are not merged, I will revert this PR before the 1.6 release.

Future-Outlier · 2026-03-14T06:50:55Z

@codex review

Future-Outlier · 2026-03-14T06:51:14Z

@cursor review

cursor

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

cursor · 2026-03-14T07:00:09Z

ray-operator/controllers/ray/rayservice_controller.go

+	if meta.IsStatusConditionTrue(rayServiceInstance.Status.Conditions, string(rayv1.RollbackInProgress)) {
+		// Rollback the upgrade. The active RayCluster should be scaled back to 100% target_capacity,
+		// while the pending RayCluster is scaled to 0%. This is the inverse of the regular upgrade path.
+		activeTrafficRoutedPercent := ptr.Deref(activeRayServiceStatus.TrafficRoutedPercent, 0)


Inconsistent default for active TrafficRoutedPercent causes potential deadlock

Medium Severity

During rollback, reconcileServeTargetCapacity defaults active TrafficRoutedPercent to 0 via ptr.Deref(..., 0), while calculateTrafficRoutedPercent defaults the same field to 100 via ptr.Deref(..., 100). If TrafficRoutedPercent is ever nil during rollback, the capacity reconciler sees activeTargetCapacity(100) != activeTrafficRoutedPercent(0) and defers, while the traffic calculator sees activeClusterWeight(100) == activeClusterTargetCapacity(100) and stops migration. Neither function makes progress, creating a deadlock where the rollback can never complete.

Additional Locations (1)

ray-operator/controllers/ray/rayservice_controller.go#L734-L735

cc @ryanaoleary to take a look

I think the correct solution is to default it to TargetCapacity if it's ever nil, rather than 100 or 0 since either can result in deadlock (100 (capacity) != 0 (traffic) -> defer or 100 (traffic) == 100 (capacity) -> defer if we default when it becomes nil.

fixed in d02b7be and added unit tests for this case

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8b66d42a5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-14T07:02:21Z

ray-operator/controllers/ray/rayservice_controller.go

+		if isRollbackInProgress {
+			// Gradually shift traffic from the pending to the active cluster.
+			// Rollback traffic migration occurs regardless of pending cluster readiness.
+			logger.Info("Rollback in progress. Shifting traffic back to active cluster.", "stepSize", *options.StepSizePercent)


Gate rollback traffic migration on active readiness

The rollback branch starts shifting HTTPRoute weight back to the active cluster whenever isRollbackInProgress is true, but it never checks whether the active cluster is actually ready to serve. In the case where the active cluster is unhealthy during an in-progress upgrade and the user changes spec (triggering rollback), this logic can still move traffic onto a non-ready backend and cause request failures; rollback traffic shifts should be conditioned on active-cluster readiness the same way forward migration is conditioned on pending readiness.

Useful? React with 👍 / 👎.

cc @ryanaoleary to take a look

done in 81ad0cf

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

This reverts commit b8b66d4.

JiangJiaWei1103 · 2026-03-18T12:01:00Z

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

 	NoPendingCluster               RayServiceConditionReason = "NoPendingCluster"
 	NoActiveCluster                RayServiceConditionReason = "NoActiveCluster"
 	RayServiceValidationFailed     RayServiceConditionReason = "ValidationFailed"
+	TargetClusterChanged           RayServiceConditionReason = "TargetClusterChanged"


As a follow-up, we should also consider renaming this reason as discussed here.

I'd suggest renaming it to GoalClusterSpecChanged (or a clearer name). Here's the rationale:

Goal prefix: Consistent with the naming convention used across all log and error messages in the reconcileRollbackState method

SpecChanged suffix: More accurate, since this reflects a spec change (RayService.Spec.RayClusterSpec) rather than a target cluster switch

Avoids ambiguity: The term "target cluster" is already used in a different context, where the head and serve services point to the ready cluster (please see here), so reusing it here could be misleading.

Happy to take this on in a follow-up PR!

"TargetCluster" or "DesireCluster" is more commonly used term in Kubernetes, so I suggest changing the logs or other naming conventions if possible while we're still in alpha. For 3) the "target cluster" is only used as a reference for a Go variable and not actually exposed to users, so not sure it matters in this case?

Thanks so much for the context and guidance. Really helpful to understand the broader Kubernetes conventions here!

Aligning with idiomatic "Target" / "Desired" terminology makes a lot of sense. And fair enough on point 3 since the variable isn't user-facing, the ambiguity concern is less of an issue.

With that in mind, here's what I'm thinking for the follow-up:

Rename rollback-related logs and local variables in reconcileRollbackState to use "Target" (or "Desired") consistently

Update the condition reason to align with that same terminology (e.g. TargetClusterSpecChanged)

Do you have a preference between Target and Desired? Happy to consolidate everything into a single follow-up change once we land on the right term. Thanks!

+1 on changing the naming, DesiredClusterSpecChanged sounds good to me since we already use "desired" terminology in the API (

kuberay/ray-operator/apis/ray/v1/raycluster_types.go

Line 287 in 911c383

DesiredCPU resource.Quantity `json:"desiredCPU,omitempty"`

). We also already have TargetCapacity which refers to the Serve capacity to scale within a cluster. I think we should use "target" to refer to fields that the controller is changing, while "desired" more clearly refers to user-declared intent - like the spec that they submit.

Thanks Ryan, the naming makes a lot of sense!

* Disable RayMultiHostIndexing feature for TestReconcile_Multihost_Replicas (#4583) Signed-off-by: Future-Outlier <[email protected]> * Disable the field alignment lint check (#4560) * chore: disable govet fieldalignment lint Signed-off-by: jinbum9958 <[email protected]> * chore: remove redundant //nolint:govet comments Signed-off-by: jinbum9958 <[email protected]> * chore: restore RayCronJobSpec/RayCronJob comments Signed-off-by: jinbum9958 <[email protected]> * chore: restore RayCronJob comment Signed-off-by: jinbum9958 <[email protected]> * chore: regenerate artifacts for RayCronJob comment changes Signed-off-by: jinbum9958 <[email protected]> --------- Signed-off-by: jinbum9958 <[email protected]> * Mark RC releases as pre-release in GoReleaser (#4590) Signed-off-by: Future-Outlier <[email protected]> * Add example YAML for manually enabling Ray k8s auth (#4582) Signed-off-by: Andrew Sy Kim <[email protected]> * [RayService] Rollback Support for Incremental Upgrades (#4109) * Implement rollback support Signed-off-by: Ryan O'Leary <[email protected]> * Fix unit test file Signed-off-by: Ryan O'Leary <[email protected]> * Ensure upgrade in progress status is cleared Signed-off-by: Ryan O'Leary <[email protected]> * Clarify rollback scenarios and clear pending apps during rollback Signed-off-by: Ryan O'Leary <[email protected]> * Fix typo Signed-off-by: Ryan O'Leary <[email protected]> * Add guard for rollback to prepare new cluster Signed-off-by: Ryan O'Leary <[email protected]> * fix e2e test checking pending cluster name Signed-off-by: Ryan O'Leary <[email protected]> * Fix rollback calculation Signed-off-by: Ryan O'Leary <[email protected]> * Fix httproute name in test Signed-off-by: Ryan O'Leary <[email protected]> * Increase timeout for checking RayCluster deletion Signed-off-by: Ryan O'Leary <[email protected]> * Fix rollback check and change httproute equal check so it doesn't always evaluate false Signed-off-by: Ryan O'Leary <[email protected]> * Adjust logic to use timestamp of growing cluster Signed-off-by: Ryan O'Leary <[email protected]> * Fix naming of vars Signed-off-by: Ryan O'Leary <[email protected]> --------- Signed-off-by: Ryan O'Leary <[email protected]> Signed-off-by: Ryan O'Leary <[email protected]> * [RayService] Promote Incremental Upgrade Feature to Beta (#4599) Signed-off-by: Ryan O'Leary <[email protected]> * Revert "[RayService] Promote Incremental Upgrade Feature to Beta (#4599)" (#4602) This reverts commit 0c8aab9. * [Github Action] Skip krew-index update for pre-release tags (#4587) * [CI] Skip krew-index update for pre-release tags Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> --------- Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> * [Helm] update ray-cluster chart to apiVersion: v2 (#4593) * Update kind configs and docs to v1.29.0 (#4595) * Remove ingress template support for k8s < 1.19 (#4591) * [Helm] Update ray-cluster default resource values (#4588) * update kuberay 1.6.0 helm chart and readme Signed-off-by: Future-Outlier <[email protected]> * [Helm] Fix apiserver README chart version and align ray-cluster resource defaults Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * fix ctl test Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> --------- Signed-off-by: Future-Outlier <[email protected]> * fix lint Signed-off-by: Future-Outlier <[email protected]> * chore: Use RayCluster name as SA name for RBAC auth (#4611) Signed-off-by: JiangJiaWei1103 <[email protected]> * Add Google Artifact Registry image build/push guide (#4618) * [History Server] Fix API response format to match Ray Dashboard frontend schema (#4615) * [History Server] Fix API response format to match Ray Dashboard frontend schema Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * fix bug 1 Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * add comments to explain frontend fields naming issue Signed-off-by: Future-Outlier <[email protected]> * fix tests Signed-off-by: Future-Outlier <[email protected]> * Add comments Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * Add frontend reference for this PR Signed-off-by: Future-Outlier <[email protected]> * fix codex and cursor bug Signed-off-by: Future-Outlier <[email protected]> * upadte Signed-off-by: Future-Outlier <[email protected]> * add actor.LabelSelector Signed-off-by: Future-Outlier <[email protected]> * better query for additional endpoints, request URI (path + query) Signed-off-by: Future-Outlier <[email protected]> * Add comments Signed-off-by: Future-Outlier <[email protected]> --------- Signed-off-by: Future-Outlier <[email protected]> --------- Signed-off-by: Future-Outlier <[email protected]> Signed-off-by: jinbum9958 <[email protected]> Signed-off-by: Andrew Sy Kim <[email protected]> Signed-off-by: Ryan O'Leary <[email protected]> Signed-off-by: Ryan O'Leary <[email protected]> Signed-off-by: JiangJiaWei1103 <[email protected]> Co-authored-by: Han-Ju Chen (Future-Outlier) <[email protected]> Co-authored-by: Jinbum Kim <[email protected]> Co-authored-by: Andrew Sy Kim <[email protected]> Co-authored-by: Ryan O'Leary <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Matt Boersma <[email protected]> Co-authored-by: Jia-Wei Jiang <[email protected]> Co-authored-by: Chia-Yi Liang <[email protected]>

Future-Outlier mentioned this pull request Oct 30, 2025

[Docs] Add guide for RayService Incremental Upgrade KubeRay feature ray-project/ray#58293

Merged

Copilot AI mentioned this pull request Feb 5, 2026

Review all open pull requests #4482

Closed

4 tasks

ryanaoleary mentioned this pull request Feb 20, 2026

[Feature] RayService Incremental Upgrade Project Tracker #3209

Open

5 tasks

JiangJiaWei1103 moved this to To Review in My Kuberay & Ray Feb 28, 2026

JiangJiaWei1103 added this to My Kuberay & Ray Feb 28, 2026

Implement rollback support

5938b5b

Signed-off-by: Ryan O'Leary <[email protected]>

ryanaoleary force-pushed the rollback-support branch from f5d0243 to 5938b5b Compare March 5, 2026 12:03

ryanaoleary and others added 2 commits March 5, 2026 04:03

Merge branch 'master' into rollback-support

31733e4

Signed-off-by: Ryan O'Leary <[email protected]>

Fix unit test file

bdc3007

Signed-off-by: Ryan O'Leary <[email protected]>

ryanaoleary marked this pull request as ready for review March 5, 2026 12:08

ryanaoleary requested review from MortalHappiness, andrewsykim, kevin85421 and rueian as code owners March 5, 2026 12:08

cursor bot reviewed Mar 5, 2026

View reviewed changes

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

Ensure upgrade in progress status is cleared

7613de3

Signed-off-by: Ryan O'Leary <[email protected]>

cursor bot reviewed Mar 5, 2026

View reviewed changes

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

ray-operator/controllers/ray/rayservice_controller.go Show resolved Hide resolved

ryanaoleary added 2 commits March 5, 2026 23:43

Clarify rollback scenarios and clear pending apps during rollback

3615825

Signed-off-by: Ryan O'Leary <[email protected]>

Fix typo

b93aeb4

Signed-off-by: Ryan O'Leary <[email protected]>

cursor bot reviewed Mar 5, 2026

View reviewed changes

ray-operator/controllers/ray/rayservice_controller.go Show resolved Hide resolved

Add guard for rollback to prepare new cluster

11299d6

Signed-off-by: Ryan O'Leary <[email protected]>

cursor bot reviewed Mar 6, 2026

View reviewed changes

ray-operator/test/e2eincrementalupgrade/rayservice_incremental_upgrade_test.go Outdated Show resolved Hide resolved

fix e2e test checking pending cluster name

e263d1f

Signed-off-by: Ryan O'Leary <[email protected]>

cursor bot reviewed Mar 6, 2026

View reviewed changes

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

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

ryanaoleary added 2 commits March 7, 2026 11:10

Fix rollback calculation

9d659d0

Signed-off-by: Ryan O'Leary <[email protected]>

Fix httproute name in test

eb52d96

Signed-off-by: Ryan O'Leary <[email protected]>

cursor bot reviewed Mar 9, 2026

View reviewed changes

ray-operator/controllers/ray/rayservice_controller.go Show resolved Hide resolved

Increase timeout for checking RayCluster deletion

a7867fa

Signed-off-by: Ryan O'Leary <[email protected]>

cursor bot reviewed Mar 9, 2026

View reviewed changes

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

Fix rollback check and change httproute equal check so it doesn't alw…

9978cc8

…ays evaluate false Signed-off-by: Ryan O'Leary <[email protected]>

cursor bot reviewed Mar 9, 2026

View reviewed changes

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

Adjust logic to use timestamp of growing cluster

196ea1f

Signed-off-by: Ryan O'Leary <[email protected]>

andrewsykim reviewed Mar 12, 2026

View reviewed changes

Fix naming of vars

cc0a5a3

Signed-off-by: Ryan O'Leary <[email protected]>

Future-Outlier reviewed Mar 13, 2026

View reviewed changes

rueian approved these changes Mar 13, 2026

View reviewed changes

andrewsykim merged commit b8b66d4 into ray-project:master Mar 13, 2026
31 checks passed

github-project-automation bot moved this from To Review to Done in My Kuberay & Ray Mar 13, 2026

cursor bot reviewed Mar 14, 2026

View reviewed changes

chatgpt-codex-connector bot reviewed Mar 14, 2026

View reviewed changes

ryanaoleary mentioned this pull request Mar 14, 2026

[RayService] Promote Incremental Upgrade Feature to Beta #4599

Merged

4 tasks

Future-Outlier added a commit that referenced this pull request Mar 16, 2026

Revert "[RayService] Rollback Support for Incremental Upgrades (#4109)"

5eef493

This reverts commit b8b66d4.

Future-Outlier mentioned this pull request Mar 16, 2026

Revert "[RayService] Rollback Support for Incremental Upgrades" #4604

Closed

JiangJiaWei1103 reviewed Mar 18, 2026

View reviewed changes

Conversation

ryanaoleary commented Oct 3, 2025

Why are these changes needed?

Related issue number

Checks

Uh oh!

ryanaoleary commented Oct 5, 2025

Uh oh!

ryanaoleary commented Mar 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Future-Outlier commented Mar 9, 2026

Uh oh!

Uh oh!

Uh oh!

andrewsykim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewsykim commented Mar 12, 2026

Uh oh!

Future-Outlier left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Future-Outlier commented Mar 14, 2026

Uh oh!

Future-Outlier commented Mar 14, 2026

Uh oh!

Future-Outlier commented Mar 14, 2026

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Mar 14, 2026

Choose a reason for hiding this comment

Inconsistent default for active TrafficRoutedPercent causes potential deadlock

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!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 14, 2026

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!

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