-
Notifications
You must be signed in to change notification settings - Fork 688
[kubectl-plugin][Test] Use client-go reactors for FieldSelector filtering in fake client tests #4361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[kubectl-plugin][Test] Use client-go reactors for FieldSelector filtering in fake client tests #4361
Conversation
…t tests Add AddRayClusterFieldSelectorReactor helper that simulates server-side FieldSelector filtering in fake client tests. This addresses issue ray-project#4337 by allowing tests to verify filtering behavior without manual name checks. Refs: ray-project#4337
Use the new AddRayClusterFieldSelectorReactor in workergroup completion tests to properly simulate server-side filtering behavior. Refs: ray-project#4337
Remove the workaround that manually filtered clusters by name since the fake client now properly supports FieldSelector filtering via reactors. Refs: ray-project#4337
NewSimpleClientset is deprecated in favor of NewClientset for better server-side apply testing support. Note: scale_cluster_test.go is not migrated because it uses Update operations that require schema definitions missing from the generated applyconfiguration internal schema. Refs: ray-project#4337
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ikchifo, would you mind helping update
kubectl-plugin/pkg/cmd/scale/scale_cluster_test.go as well?
@justinyeh1995 Investigated this - and couldn't migrate |
…t setup Add a convenience wrapper that creates a fake Ray clientset with FieldSelector reactor pre-configured. This simplifies test setup and ensures consistent behavior across tests. Also applies reactor to get_cluster_test.go and fixes test data to match actual cluster names now that FieldSelector properly filters. Refs: ray-project#4337
Co-authored-by: JustinYeh <justinyeh1995@gmail.com> Signed-off-by: Ikenna <ikennachifo@gmail.com>
df9ad48 to
f3df8bc
Compare
Thanks for the investigation. I am fine with leaving it as-is for now. |
|
cc @machichima for another pass of review if you still have bandwidth. Thanks. |
Update clientset.go and reactor_test.go to use the renamed function AddRayClusterListFieldSelectorReactor.
f3df8bc to
e5fdf04
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if cluster != "" && rayCluster.Name != cluster { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why delete this line??
cc @cheyu @lorriexingfang @Jiekai for review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah since we use field selector now



Why are these changes needed?
NewSimpleClientsetand its successorNewClientsetdo not support FieldSelector filtering for List operations in unit tests. Tests currently rely on manual name checking as a workaround, which diverges from real Kubernetes API server behavior.This PR adds client-go reactors to simulate server-side FieldSelector filtering, making fake client behavior match the real Kubernetes API more closely.
Before
Tests manually filter results after List calls:
After
Reactor handles filtering automatically, matching real API behavior. Manual workarounds removed.
What did I do?
AddRayClusterFieldSelectorReactorhelper inpkg/util/client/testing/reactor.gocompletion_test.goandget_cluster_test.gocompletion.goNewSimpleClientsettoNewClientsetRelated issue number
Closes #4337
Checks
cc @justinyeh1995 @CheyuWu @rueian