Skip to content

Updating kube-proxy to support new EndpointSlice address types#85246

Merged
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
robscott:endpointslice-dualstack-proxy
Nov 14, 2019
Merged

Updating kube-proxy to support new EndpointSlice address types#85246
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
robscott:endpointslice-dualstack-proxy

Conversation

@robscott
Copy link
Copy Markdown
Member

@robscott robscott commented Nov 13, 2019

What type of PR is this?
/kind feature

What this PR does / why we need it:
This includes IPv4 and IPv6 address types and IPVS dual stack support. Importantly this ensures that EndpointSlices with a FQDN address type are not processed by kube-proxy.

Special notes for your reviewer:
This is intended to replace #84089

Does this PR introduce a user-facing change?:

kube-proxy now supports DualStack feature with EndpointSlices and IPVS.

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

/sig network
/priority important-soon
/cc @khenidak @andrewsykim @freehan @thockin

@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Nov 13, 2019
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. sig/network Categorizes an issue or PR as relevant to SIG Network. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/ipvs labels Nov 13, 2019
@robscott robscott mentioned this pull request Nov 13, 2019
8 tasks
Copy link
Copy Markdown
Contributor

@freehan freehan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just nits.

@robscott robscott force-pushed the endpointslice-dualstack-proxy branch from cb3c65d to 390c867 Compare November 13, 2019 23:23
@robscott
Copy link
Copy Markdown
Member Author

/retest

@andrewsykim
Copy link
Copy Markdown
Member

/milestone v1.17

@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Nov 14, 2019
This includes IPv4 and IPv6 address types and IPVS dual stack support.
Importantly this ensures that EndpointSlices with a FQDN address type
are not processed by kube-proxy.
@robscott robscott force-pushed the endpointslice-dualstack-proxy branch from 390c867 to 2a021d0 Compare November 14, 2019 03:51
proxier.ipv4Proxier.OnEndpointSliceAdd(endpointSlice)
case discovery.AddressTypeIPv6:
proxier.ipv6Proxier.OnEndpointSliceAdd(endpointSlice)
default:
Copy link
Copy Markdown
Member

@andrewsykim andrewsykim Nov 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for checking discovery.AddressTypeIP in the endpoints change tracker but not here? If we're deprecating but still supporting it I think we should we still check for it here as well. I guess we'd have to call both the ipv4 and ipv6 proxiers for that case.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not supporting the creation of new EndpointSlices with the type IP in 1.17, but are allowing existing ones to continue to exist in a deprecated state. The EndpointSlice controller should convert any EndpointSlice it manages with the type IP to IPv4, but there is of course a chance that EndpointSlices are managed by other entities or that kube-proxy is upgraded to 1.17 before the controller.

For a new feature like meta proxier with no previous support for the IP type, it didn't seem to make sense to support the IP address type. Of course it would also be a lot more complicated to support a type that could include both v4 and v6. Given that it was a deprecated alpha feature with no history of support for dual stack, it seemed fine to leave out here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying, makes sense!

@andrewsykim
Copy link
Copy Markdown
Member

/approve
/lgtm

For IPVS part

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 14, 2019
@khenidak
Copy link
Copy Markdown
Contributor

/lgtm

@freehan
Copy link
Copy Markdown
Contributor

freehan commented Nov 14, 2019

/lgtm
/approve

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewsykim, freehan, robscott

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 14, 2019
@k8s-ci-robot k8s-ci-robot merged commit 2a021d0 into kubernetes:master Nov 14, 2019
@robscott robscott deleted the endpointslice-dualstack-proxy branch March 11, 2021 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/ipvs cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/network Categorizes an issue or PR as relevant to SIG Network. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants