KEP-4794: initial proposal for deprecating v1.Endpoints #4975
+569
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
sig/network
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship 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 |
k8s-ci-robot
added
cncf-cla: yes
danwinship
force-pushed
the
deprecate-endpoints
branch
from
4f695f2
to
85ba04c
Compare
sftim
reviewed
Nov 23, 2024
Comment on lines
+104
to
+106
| - Ensure that all documentation reflects the fact that the Endpoints | ||
| API is deprecated and should not be used by new code, and that | ||
| Endpoints objects might not be created or mirrored in some clusters. |
There was a problem hiding this comment.
Suggested change
| - Ensure that all documentation reflects the fact that the Endpoints | |
| API is deprecated and should not be used by new code, and that | |
| Endpoints objects might not be created or mirrored in some clusters. | |
| - Ensure that end users favor EndpointSlice over Endpoints, and understand that the | |
| Endpoints API is deprecated. |
Comment on lines
+108
to
+109
| - Add apiserver warnings when users (other than the Endpoints | ||
| controller) create Endpoints objects. |
There was a problem hiding this comment.
Suggested change
| - Add apiserver warnings when users (other than the Endpoints | |
| controller) create Endpoints objects. | |
| - Serve warnings (`Warning:` headers) when users, other than the Endpoints | |
| controller, create Endpoints objects. |
Not sure this is a goal - that's more of a “how will we achieve“. But I'm not outright opposed to leaving it in.
Comment on lines
+111
to
+144
| - Update the e2e test suite to ensure that all tests that care about | ||
| endpoints only as a generic concept are using EndpointSlices rather | ||
| than Endpoints. (For example, the utility function | ||
| `framework.WaitForServiceEndpointsNum` should check that | ||
| EndpointSlices exist, not that Endpoints exist). | ||
|
|
||
| - Adjust the conformance test suite to not require the Endpoints and | ||
| EndpointSlice Mirroring controllers to be running: | ||
|
|
||
| - Fix tests such as `Service endpoints latency should not be very | ||
| high` to check the latency of EndpointSlices rather than | ||
| Endpoints, since that is what is actually important in | ||
| real-world usage. | ||
|
|
||
| - Demote tests such as `EndpointSliceMirroring should mirror a | ||
| custom Endpoints resource through create update and delete` to | ||
| non-conformance. | ||
|
|
||
| - Split up tests such as `EndpointSlice should create Endpoints | ||
| and EndpointSlices for Pods matching a Service` into a | ||
| conformance test for EndpointSlice and a non-conformance test | ||
| for Endpoints. | ||
|
|
||
| - Update the e2e test suite so that all remaining tests that depend on | ||
| the outputs of the Endpoints or EndpointSlice Mirroring controllers | ||
| are feature-tagged so they can be skipped in clusters that do not | ||
| run those controllers, and set up a periodic e2e job running in such | ||
| a configuration. | ||
|
|
||
| - Explicitly document that disabling `endpoints-controller` and/or | ||
| `endpointslice-mirroring-controller` via kube-controller-manager's | ||
| `--controllers` flag is a supported and conforming configuration. | ||
|
|
||
| - Figure out what to do with stale Endpoints in such a cluster. |
There was a problem hiding this comment.
These seems like a really long Goals section. How about
- ensure that Conformance doesn't require a running controller that writes Endpoints objects
- reduce the end to end tests for Endpoints, to the minimum tests required to support Endpoints as a
stable (but deprecated) API
- design, implement and document cleanup for stale Endpoints in clusters that still have these
objects present| controllers via kube-controller-manager's `--controllers` option, and | ||
| we believe that this will have no ill effects in a vanilla Kubernetes | ||
| cluster (though, currently, it will cause the e2e tests to fail). | ||
|
|
There was a problem hiding this comment.
If a controller is (still) making Endpoints objects, do we want to surface that somehow once the API is deprecated?
eg: .status on the associated Service?
Comment on lines
+196
to
+219
| We do not want to leave stale `Endpoints` objects around forever if | ||
| the associated controllers are not running. (This is both a waste of | ||
| disk space and a potential source of confusion since the Endpoints | ||
| objects would quickly become out-of-date and incorrect.) | ||
|
|
||
| One possibility would be to just document that administrators should | ||
| delete all existing Endpoints themselves if they are going to disable | ||
| the controllers. | ||
|
|
||
| Another possibility would be to have kube-controller-manager do this | ||
| automatically. (Though the idea of having it automatically take action | ||
| because a controller _isn't_ enabled seems slightly magical? But | ||
| perhaps there is precedent somewhere else?) | ||
|
|
||
| Another possibility would be to create an `endpoints-cleanup` | ||
| controller, that could be enabled explicitly, and document that admins | ||
| should (probably) enable that controller if they disable the others. | ||
|
|
||
| Or perhaps the EndpointSlice controller could delete Endpoints objects | ||
| that were more than 24 hours out of date with respect to their | ||
| EndpointSlices? | ||
|
|
||
| In all cases, we should probably not automatically delete Endpoints | ||
| that were not originally created by the Endpoints controller. |
There was a problem hiding this comment.
We could mark this as an unresolved point.
| ### Graduation Criteria | ||
|
|
||
| #### Alpha | ||
|
|
There was a problem hiding this comment.
Suggested change
| - Deprecate the Endpoints API | |
Comment on lines
+371
to
+373
| - The documentation explains how to disable the Endpoints and | ||
| EndpointSlice Mirroring controllers, but warns that third-party | ||
| components may still depend on them. |
There was a problem hiding this comment.
Bear in mind that in managed Kubernetes, people who use the cluster may have no way to disable these controllers.
|
|
||
| - <test>: <link to test coverage> | ||
|
|
||
| ### Graduation Criteria |
There was a problem hiding this comment.
We're not looking to graduate to GA here. This is a deprecation plan.
Comment on lines
+26
to
+30
| # The milestone at which this feature was, or is targeted to be, at each stage. | ||
| milestone: | ||
| alpha: "v1.33" | ||
| beta: "v1.35" | ||
| stable: "v1.37" |
There was a problem hiding this comment.
We're not planning to graduate to stable. This is a deprecation plan.
| - "/keps/sig-network/0752-endpointslices" | ||
|
|
||
| # The target maturity stage in the current dev cycle for this KEP. | ||
| stage: alpha |
There was a problem hiding this comment.
We're not planning to graduate to stable. This is a deprecation plan.
sftim
reviewed
Nov 23, 2024
|
|
||
| - We have a passing "no Endpoints" e2e job. | ||
|
|
||
| - Warnings are emitted when users create Endpoints. |
There was a problem hiding this comment.
Maybe kubectl also warns on get / list, provided the API server doesn't serve an actual warning instead?
Just a thought.
sftim
reviewed
Nov 23, 2024
Comment on lines
+149
to
+154
| - MAYBE change kube-apiserver to optionally not generate Endpoints for | ||
| kubernetes.default, though this would require adding a | ||
| kube-apiserver configuration option, and the benefit from removing | ||
| just that 1 Endpoints object is small. Perhaps instead it could just | ||
| add an annotation to the object pointing out the fact that Endpoints | ||
| is deprecated. |
There was a problem hiding this comment.
From experience, I can say that it would make teaching easier if there were an EndpointSlice for the kubernetes Service in the default namespace.
If we deprecate, we should mean it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#4974
/sig network
/cc @aojea @robscott