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
Graduate EndpointSlice Controllers to GA #99870
Graduate EndpointSlice Controllers to GA #99870
Conversation
Hi @swetharepakula. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Thanks @swetharepakula! Should this PR also include graduating the EndpointSlice feature gate to GA? That's what guards the enablement of these controllers. Related to that, I think this file also needs to be updated: https://github.com/kubernetes/kubernetes/blob/master/cmd/kube-controller-manager/app/discovery.go#L40. /triage accepted |
@@ -186,6 +187,17 @@ func objectRefPtrEqual(ref1, ref2 *corev1.ObjectReference) bool { | |||
return true | |||
} | |||
|
|||
// stringPtrEqual returns true if a set of string pointers have equivalent values. | |||
func stringPtrEqual(ptr1, ptr2 *string) bool { |
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.
we have stringPtrChanged
in endpointslices/utils.go, @robscott if the goal is to consolidate this code does make sense to have only one and negate the logic in one of the places?
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.
Good catch, it would be good to use that util function instead.
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.
That is a good point. I wasn't sure where the function should go. Is endpointslices/utils.go
the best place and it can be called in the other controller as well?
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.
I added a function to utils/pointer just yesterday, LOL.
Ideally we can all just use pointer.StringEqual()
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.
needs to be re-vendored, so "later"
it seems this integration test has to be changed to
|
ce4b0a4
to
aa594e3
Compare
I just pushed up an update that consolidates the util functions that were being used by both EPS controllers and removes an extraneous check when starting the controllers since the feature gate is being removed. Also added the beta comment to the EndpointSlice feature gate. Thanks for all the quick reviews on this! |
aa594e3
to
5f0f5d6
Compare
/retest |
/retest |
@swetharepakula Now that #99662 is in it looks like this needs to be rebased. I can LGTM whenever you're able to rebase. |
- EndpointSlice controller will stop writing to Topology field - EndpointSlice controller will only provide NodeName and Zone on EndpointSlices
5f0f5d6
to
108fd44
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: swetharepakula, thockin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Thanks! /lgtm |
The hold was just waiting for #99662 to merge in, removing that now that it has. /hold cancel |
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Graduates EPS controllers to use V1 Endpoint Slice Resources and API
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
Please use the following format for linking documentation:
KEP: EndpointSlice KEP
Enhancement Issue: kubernetes/enhancements#752
/cc @aojea @andrewsykim @liggitt @robscott @wojtek-t
/assign @thockin
/sig network
/priority important-soon