Skip to content
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

Merged
merged 1 commit into from Mar 10, 2021

Conversation

robscott
Copy link
Member

@robscott robscott commented Mar 9, 2021

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?

The Endpoints controller will now set the `endpoints.kubernetes.io/over-capacity` annotation to "warning" when an Endpoints resource contains more than 1000 addresses. In a future release, the controller will truncate Endpoints that exceed this limit. The EndpointSlice API can be used to support significantly larger number of addresses.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

/sig network
/priority important-soon
/triage accepted
/cc @aojea @swetharepakula
/assign @thockin

@k8s-ci-robot k8s-ci-robot added the release-note label Mar 9, 2021
@k8s-ci-robot k8s-ci-robot added kind/cleanup size/L kind/feature labels Mar 9, 2021
@k8s-ci-robot
Copy link
Contributor

@k8s-ci-robot k8s-ci-robot commented Mar 9, 2021

@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:

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?

The Endpoints controller will now set the `endpoints.kubernetes.io/over-capacity` annotation to "warning" when an Endpoints resource contains more than 1000 addresses. In a future release, the controller will truncate Endpoints that exceed this limit. The EndpointSlice API can be used to support significantly larger number of addresses.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

/sig network
/priority important-soon
/triage accepted
/cc @aojea @swetharepakula
/assign @thockin

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.

@k8s-ci-robot k8s-ci-robot added the sig/network label Mar 9, 2021
@k8s-ci-robot k8s-ci-robot requested a review from aojea Mar 9, 2021
@k8s-ci-robot k8s-ci-robot added priority/important-soon cncf-cla: yes do-not-merge/needs-kind do-not-merge/needs-sig triage/accepted needs-triage kind/api-change sig/apps and removed do-not-merge/needs-kind do-not-merge/needs-sig needs-triage labels Mar 9, 2021
@robscott robscott force-pushed the endpoints-over-capacity branch from 56cfbcc to a48a2fb Compare Mar 9, 2021
@fejta-bot
Copy link

@fejta-bot fejta-bot commented Mar 9, 2021

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.

Copy link
Member

@swetharepakula swetharepakula left a comment

LGTM

Copy link
Member

@thockin thockin left a comment

/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
Copy link
Member

@thockin thockin Mar 9, 2021

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

Copy link
Member Author

@robscott robscott Mar 9, 2021

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.

pkg/controller/endpoint/endpoints_controller.go Outdated Show resolved Hide resolved
staging/src/k8s.io/api/core/v1/annotation_key_constants.go Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the approved label Mar 9, 2021
@robscott robscott force-pushed the endpoints-over-capacity branch from a48a2fb to a376195 Compare Mar 9, 2021
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.
@robscott robscott force-pushed the endpoints-over-capacity branch from a376195 to 8a3f720 Compare Mar 9, 2021
@robscott
Copy link
Member Author

@robscott robscott commented Mar 9, 2021

@thockin Thanks for the review! Replaced "will" with "may", PTAL.

Copy link
Member

@thockin thockin left a comment

Thanks!

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Mar 9, 2021
@k8s-ci-robot
Copy link
Contributor

@k8s-ci-robot k8s-ci-robot commented Mar 9, 2021

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 05c4feb into kubernetes:master Mar 10, 2021
14 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.21 milestone Mar 10, 2021
valyala added a commit to VictoriaMetrics/VictoriaMetrics that referenced this issue Oct 19, 2021
…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)
valyala added a commit to VictoriaMetrics/VictoriaMetrics that referenced this issue Oct 19, 2021
…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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved cncf-cla: yes kind/api-change kind/cleanup kind/feature lgtm priority/important-soon release-note sig/apps sig/network size/L triage/accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants