[RayService] Add skipGateway option for NewClusterWithIncrementalUpgrade#13
Draft
savitagm wants to merge 7 commits into
Draft
[RayService] Add skipGateway option for NewClusterWithIncrementalUpgrade#13savitagm wants to merge 7 commits into
savitagm wants to merge 7 commits into
Conversation
…port Introduces `skipGateway` field in `ClusterUpgradeOptions` for the `NewClusterWithIncrementalUpgrade` strategy. When enabled: - Gateway and HTTPRoute objects are not created or reconciled - Gateway/HTTPRoute readiness checks are skipped in the target_capacity advancement gate (checkIfNeedTargetCapacityUpdate) - TrafficRoutedPercent advances via the existing time-step logic (IntervalSeconds + StepSizePercent) instead of being read from HTTPRoute backend ref weights This allows deployments using client-side load balancers to use incremental upgrade without requiring a Gateway API implementation. The client-side LB is responsible for reading targetCapacity from RayService status and splitting traffic accordingly. GatewayClassName is now optional when skipGateway is true. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
In skipGateway mode, TrafficRoutedPercent previously advanced on a fixed timer (intervalSeconds + stepSizePercent). This caused capacity gaps when pod cold-start exceeded intervalSeconds, routing traffic to pending before replicas were ready. Replace with a serve-deployment health gate: TrafficRoutedPercent is set to match TargetCapacity only when the pending cluster's Serve deployments report HEALTHY, meaning autoscaling has fully settled at the current target_capacity. The existing reconcileServeTargetCapacity gate (pendingTargetCapacity != pendingTrafficRoutedPercent) then naturally advances to the next step — no timer needed. IntervalSeconds and StepSizePercent are made optional (omitempty) since they are not used in skipGateway mode. GatewayClassName was already optional. MaxSurgePercent remains the only functional field for skipGateway: it controls how large each target_capacity step is. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GatewayClassName, StepSizePercent, and IntervalSeconds are not needed when skipGateway is enabled — guard the checks accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ixes - Replace arePendingServeDeploymentsHealthy with isPendingClusterCapacityReady at the TrafficRoutedPercent gate so that the pending cluster must have enough available worker replicas (ceil(pendingTargetCap/100 * active.availableWorkers)) before traffic routing advances. Includes deadlock-safe bypass when pendingMaxReplicas < activeAvailableWorkers (intentional capacity reduction). - Add UpgradeTimeoutSeconds field to ClusterUpgradeOptions. When set, the operator aborts the upgrade by deleting the pending RayCluster and emitting a Warning event if the pending cluster's age exceeds the timeout. - Add UpgradeTimeout event type constant. - Add validation: upgradeTimeoutSeconds must be >= 0 when set. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
autoscaler is required for gateway-based incremental traffic routing but not for skipGateway mode, where traffic routing is handled by a client-side load balancer reading targetCapacity from RayService status. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… waivers autoscaler is required regardless of skipGateway: it drives targetCapacity scaling so the pending cluster ramps up workers proportionally instead of starting at full replicas. Without autoscaler the gradual capacity transfer semantics are broken. skipGateway only waives stepSizePercent, intervalSeconds, and gatewayClassName which are gateway-routing-specific fields not used in the client-side LB path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rade in progress When upgradeStrategy.type=NewClusterWithIncrementalUpgrade with skipGateway=true, the TC/TRP gate code ran every reconcile regardless of whether a pending cluster existed. With no pending cluster, arePendingServeDeploymentsHealthy(nil) returns false, so the log fired on every reconcile post-upgrade. Fix: guard the SkipGateway TC/TRP block with pendingCluster != nil. Initialize activeWeight/pendingWeight to -1 (the sentinel for "don't update") so the no- pending-cluster case correctly skips the update rather than falling through to the HTTPRoute weight reader. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
skipGateway: truesupport toNewClusterWithIncrementalUpgrade, enabling zero-downtime rolling upgrades for deployments that use a client-side load balancer instead of a Gateway/HTTPRoute for traffic splitting.When
skipGateway: trueis set:targetCapacityinactiveServiceStatus/pendingServiceStatus— the client-side LB reads these values to route trafficTrafficRoutedPercentreflects the actual split only after the pending cluster's Serve deployments are HEALTHY and worker replica parity is met (capacity-ready gate)UpgradeInProgress=true; the updated config goes to the pending clusterChanges
6cbc10a0skipGatewayfield toClusterUpgradeOptionsCRD; skip Gateway/HTTPRoute reconciliation when setd85d6397isPendingClusterCapacityReady)3ee51aa4skipGateway=truedc97d621ceil(TC% × activeWorkers)available workers; addupgradeTimeoutSecondssupport88b0001c/951e4fbdskipGatewayservices from in-tree autoscaler requirement (gateway path requires it; skipGateway does not)d68528baKey behaviors
Capacity-ready gate — TC advances only when:
HEALTHYpendingAvailableWorkers >= ceil(pendingTC% × activeAvailableWorkers)— or pendingmaxReplicas < activeAvailable(capacity reduction path, always passes)TC stepping — With
maxSurgePercent=20and 2 active workers:ceil(60%×2)=2workers needed)Serve config consistency — Both gateway and skipGateway paths use the same blocking logic: while
UpgradeInProgress=true, serve config changes to the active cluster are held; they go to the pending cluster. Escape hatches:upgradeTimeoutSeconds— Acts as a per-attempt circuit breaker: after timeout, the pending cluster is deleted and a Warning event is emitted. The operator will retry (create a new pending cluster) on the next reconcile since the spec still differs. To permanently stop a timed-out upgrade, either revert the spec or delete the pending cluster manually.Future work
Permanent-abort on timeout (
upgradeTimeoutSecondsas a hard stop): currently the timeout is a per-attempt circuit breaker — after deleting the pending cluster, the operator immediately retries becauseshouldPrepareNewClusterdetects the spec hash still differs. A future enhancement would annotate the RayService with the timed-out spec hash (ray.io/upgrade-timeout-hash) and check it inshouldPrepareNewClusterto skip retrying for the same hash. The annotation would be cleared when a new (different) spec change arrives. This was deferred to keep this PR focused.Testing
Manually tested all 7 scenarios via the
/skipgw-rollout-testskill against a live RayService (llm-experiments/aipts-1466-skip-gw-test) with A100 GPUs:maxSurgePercent=100(TC jumps 0→100)upgradeTimeoutSecondsminReplicas=0Unit tests added for
isPendingClusterCapacityReadycovering: nil apps, nil clusters, capacity reduction bypass, insufficient/sufficient workers, and TC=0.