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
Adding new EndpointsOverCapacity annotation for Endpoints controller #99975
Adding new EndpointsOverCapacity annotation for Endpoints controller #99975
Conversation
@robscott: GitHub didn't allow me to request PR reviews from the following users: swetharepakula. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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. |
56cfbcc
to
a48a2fb
Compare
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. |
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.
LGTM
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.
/approve
// EndpointsOverCapacity will be set on an Endpoints resource when it | ||
// exceeds the maximum capacity of 1000 addresses. Inititially the Endpoints | ||
// controller will set this annotation with a value of "warning". In a | ||
// future release, the controller will set this annotation with a value of |
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.
s/will/may ? I am not 100% sure that we WILL - I think it depends on data
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 call, "may" is better than "will" here, updated. I personally think we should enforce some kind of limit here, maybe 1k is too low, but if we don't enforce some kind of limit, the Endpoints controller is just going to keep retrying endlessly when it hits the etcd object size limit. Without some kind of limit here, we're limiting how far EndpointSlices can go with this controller enabled.
a48a2fb
to
a376195
Compare
Now that the EndpointSlice API and controllers are GA, the Endpoints controller will use this annotation to warn when Endpoints are over capacity. In a future release, this warning will be replaced with truncation.
a376195
to
8a3f720
Compare
@thockin Thanks for the review! Replaced "will" with "may", PTAL. |
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.
Thanks!
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: robscott, 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 |
…s` discovers more than 1000 targets per a single endpoint In this case `role: endpointslice` must be used instead. See the following references: * https://kubernetes.io/docs/reference/labels-annotations-taints/#endpoints-kubernetes-io-over-capacity * kubernetes/kubernetes#99975 * prometheus/prometheus#7572 (comment)
…s` discovers more than 1000 targets per a single endpoint In this case `role: endpointslice` must be used instead. See the following references: * https://kubernetes.io/docs/reference/labels-annotations-taints/#endpoints-kubernetes-io-over-capacity * kubernetes/kubernetes#99975 * prometheus/prometheus#7572 (comment)
What type of PR is this?
/kind cleanup
/kind feature
What this PR does / why we need it:
Now that the EndpointSlice API and controllers are GA, the Endpoints controller will use this annotation to warn when Endpoints are over capacity. In a future release, this warning will be replaced with truncation.
Special notes for your reviewer:
This represents the last item in the v1.21 roll out plan for the EndpointSlice KEP.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
/sig network
/priority important-soon
/triage accepted
/cc @aojea @swetharepakula
/assign @thockin