Adding new EndpointSlice Mirroring Controller #91637
Conversation
|
Skipping CI for Draft Pull Request. |
k8s-ci-robot
added
do-not-merge/work-in-progress
|
Unknown CLA label state. Rechecking for CLA labels. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
k8s-ci-robot
added
cncf-cla: yes
|
/cc @bowei |
|
/cc |
robscott
force-pushed
the
endpointslice-mirroring
branch
from
0d60745
to
f4e8d04
Compare
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
This is not quite ready for review, but it's close. I'm transitioning this from draft status to get tests running. |
k8s-ci-robot
added
sig/cloud-provider
| return endpointHash(endpointutil.DeepHashObjectToString(hashObj)) | ||
| } | ||
|
|
||
| // endpointSet provides simple methods for comparing sets of Endpoints. |
There was a problem hiding this comment.
You are implementing this somewhat as in a "functional programming" map style wrt to the func sigs, but it seems like it actually has side effects on the mutation?
Might want to consider matching OO-style vs doing this, as it can become confusing when the original object is modified vs not.
There was a problem hiding this comment.
Commented below about wanting to maintain consistency with the EndpointSlice controller for now so we can potentially share this code in the future.
| } | ||
|
|
||
| // Returns an endpoint matching the hash if contained in the set. | ||
| func (s endpointSet) Get(item *discovery.Endpoint) *discovery.Endpoint { |
There was a problem hiding this comment.
This is actually a direct copy of the set defined by the EndpointSlice controller. I think that may have been based on another common k8s Set datastructure, but can't quite remember. I'm open to changing the signature of this function at some point, but would prefer to do it once we're able to combine/share some of the code for these controllers. For the sake of simpler backporting I'm trying to avoid any changes like that here.
pkg/controller/endpointslicemirroring/endpointslicemirroring_controller.go
Outdated
Show resolved
Hide resolved
|
/retest Review the full test history for this PR. Silence the bot with an |
|
/retest |
|
/retest Review the full test history for this PR. Silence the bot with an |
2 similar comments
|
/retest Review the full test history for this PR. Silence the bot with an |
|
/retest Review the full test history for this PR. Silence the bot with an |
|
/retest |
|
/retest Review the full test history for this PR. Silence the bot with an |
|
/retest |
|
/retest Review the full test history for this PR. Silence the bot with an |
1 similar comment
|
/retest Review the full test history for this PR. Silence the bot with an |
|
/lgtm |
|
/retest |
1 similar comment
|
/retest |
|
/retest Review the full test history for this PR. Silence the bot with an |
|
/retest |
1 similar comment
|
/retest |
|
Will this be backported to 1.18? Currently facing this issue with Cilium 1.8.2 and K8s 1.18.8 with services without selector. |
|
@Antiarchitect Is that with the EndpointSliceProxying feature gate enabled on kube-proxy? Although I'd love to cherry pick this I think it's likely too big to pull back to a previous Kubernetes version. |
|
@robscott There is no kube-proxy with Cilium - it provides a full replacement for it. |
|
@Antiarchitect woops, I missed that detail. The assumption in 1.18 and earlier was that anything publishing custom Endpoints would starting publishing custom EndpointSlices. That ended up being naive so we added the EndpointSliceMirroring controller in 1.19. Unfortunately any use of EndpointSlices in 1.18 by a proxy is going to require any custom Endpoints to be replicated to EndpointSlices through some other means. |
|
Actually there is a way in Cilium to cut the consequences: cilium/cilium#12513 (comment) I'll stick with that until I upgrade to K8s 1.19. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This adds an EndpointSlice mirroring controller to mirror custom Endpoints to EndpointSlices as described by the KEP.
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
Test Coverage
Progress on required test coverage from the KEP.
Unit Tests
Stale EndpointSlices managed by EndpointSliceMirroring controller are cleaned up by the controller when it starts up.
Covered by controller tests, primarily an issue for garbage collector since these are owned by Endpoints resources.
Only Endpoints with the skip label set to "true" will be mirrored by the EndpointSliceMirroring controller.
This has changed a bit, but is covered by controller tests.
Endpoints transitioning between values of the skip label should result in corresponding EndpointSlices being created or deleted.
Covered by controller tests, could potentially use additional coverage in e2e or integration tests
Mirrored EndpointSlices will:
Include the appropriate labels, endpoints, and ports of the corresponding Endpoints resource.
Covered by unit tests.
Refer to the original Endpoints resource with an OwnerReference and a kubernetes.io/service-name label.
Covered by unit tests.
Include a endpointslice.kubernetes.io/managed-by label set to endpointslicemirroring-controller.k8s.io.
Covered by unit tests.
Endpoints with dual stack Endpoints should be represented with multiple EndpointSlices, at least 1 per IP family.
Covered by unit tests, could potentially add some scale with unit or integration tests.
Endpoints with multiple subsets should be represented with multiple EndpointSlices, at least 1 per subset.
Covered by unit tests, could potentially add some scale with unit or integration tests.
Endpoints with multiple subsets and IP families should be represented with multiple EndpointSlices, at least 1 per subset and IP family.
Covered by unit tests, could potentially add some scale with unit or integration tests.
Endpoints with more than 1000 endpoints should result in exactly 1000 endpoints being mirrored to corresponding EndpointSlices.
Ended up with a variation that will limit replication to 1000 endpoints per subset, covered by controller tests.
EndpointSlices with a different endpointslice.kubernetes.io/managed-by will not be modified by the EndpointSlice mirroring controller.
Covered in controller tests.
When a selector is added to a Service with a preexisting Endpoints resource:
The Endpoints controller will modify the existing Endpoints resource to add the endpointslice.kubernetes.io/skip-mirror label.Covered by unit and integration tests.The EndpointSliceMirroring controller will delete any EndpointSlices it has mirrored for that Endpoints resource.
EndpointSliceMirroring controller doesn't watch Services, but other tests
verify that skip label is respected.
E2E Tests
Custom Endpoints are mirrored through create, update, and delete process.
Covered by e2e tests.
Endpoints created by the Endpoints controller include the skip label.Covered by e2e and integration tests.EndpointSlices are created with the appropriate labels, endpoints, ports, and Service references (label and owner ref).
Covered fully by unit tests, partially by e2e tests.
/sig network
/hold