feat: add API and webhook validation for GCS active-passive-head#4910
feat: add API and webhook validation for GCS active-passive-head#4910gangli113 wants to merge 1 commit into
Conversation
gangli113
requested review from
andrewsykim,
kevin85421 and
rueian
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a638b94649
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
gangli113
force-pushed
the
feat/active-passive-head
branch
from
a638b94 to
2f43881
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f43881caf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // EnableActivePassiveHead enables active-passive high availability for the GCS. | ||
| // If enabled, KubeRay will provision a standby head node to ensure quick recovery. | ||
| // +kubebuilder:default:=false | ||
| EnableActivePassiveHead *bool `json:"enableActivePassiveHead,omitempty"` |
There was a problem hiding this comment.
Adding these API fields only to config/crd/bases leaves Helm installs with the old schemas: I checked helm-chart/kuberay-operator/crds/ray.io_ray{clusters,jobs,services,cronjobs}.yaml in this commit and none contains enableActivePassiveHead or the leader-election fields. Users who install/upgrade KubeRay via the published Helm chart will have these fields rejected under strict validation or pruned before the webhook/controller sees them, so the new active-passive configuration cannot be used in that deployment path.
Useful? React with 👍 / 👎.
gangli113
force-pushed
the
feat/active-passive-head
branch
from
2f43881 to
32767e2
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 32767e2. Configure here.
|
|
||
| if err := w.validateGcsFTOptions(rayCluster); err != nil { | ||
| allErrs = append(allErrs, err) | ||
| } |
There was a problem hiding this comment.
Active-passive rules webhook-only
Medium Severity
Active-passive GCS checks live only on the RayCluster admission webhook, not in ValidateRayClusterSpec, which RayJob and RayService reconcilers use via ValidateRayJobSpec / ValidateRayServiceSpec. Invalid embedded gcsFaultToleranceOptions can pass controller validation and leave RayJobs requeueing on failed RayCluster creates instead of ValidationFailed.
Reviewed by Cursor Bugbot for commit 32767e2. Configure here.
… Head HA Signed-off-by: Gang Li <ganglica@google.com>
gangli113
force-pushed
the
feat/active-passive-head
branch
from
32767e2 to
4788d90
Compare
1 participant
Why are these changes needed?
This PR implements "Customer-Facing API Changes" part of GCS active-passive-head feature
Related issue number
ray-project/ray#63643
Checks