Dualstack support for kube-proxy iptables mode #82462

vllry · 2019-09-08T05:21:05Z

What type of PR is this?
/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

kube-proxy: Added dual-stack IPv4/IPv6 support to the iptables proxier.

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

vllry · 2019-09-08T05:37:00Z

/uncc @nicksardo
/uncc @caseydavenport

pkg/proxy/metaproxier/BUILD

cmd/kube-proxy/app/server_others.go

aojea · 2019-09-12T09:12:26Z

/test pull-kubernetes-integration

vllry · 2019-09-15T00:32:24Z

/test pull-kubernetes-kubemark-e2e-gce-big

…roxier.

thockin

relatively minor comments from me, I think, and even these could be done in a followup unless someone disagrees strongly?

thockin · 2019-12-17T19:20:53Z

cmd/kube-proxy/app/server_others.go

+			klog.V(0).Info("creating dualStackProxier for iptables.")
+
+			// Create iptables handlers for both families, one is already created
+			// Always ordered as IPv4, IPv6


To be fair, this is copying the ipvs logic. If we want to raise the bar, we should do so for both proxiers.

Perhaps, instead of [2]utiliptables.Interface it should be a map[core.IPFamily]utiliptables.Interface. Remove order from the equasion, since that seems to be meaningless anyway (and assumed ordering is bound to explode on us some day).

thockin · 2019-12-17T19:23:21Z

cmd/kube-proxy/app/server_others.go

@@ -164,6 +164,56 @@ func newProxyServer(
 			healthzServer,
 			config.NodePortAddresses,
 		)
+
+		if utilfeature.DefaultFeatureGate.Enabled(features.IPv6DualStack) {


ipvs follows this same logic. If dual-stack is enabled, make a dual-stack proxier. We want to diminish the meaning of cluster-cidr here, so I do not think testing that should be used. TL;DR is that we have no real way to know whether dual-stack is enabled or not except to try it, and even that could change at run time. Very little cost (I think) to setting up both modes, but we do need to handle failure gracefully if dual-stack is enabled on a single-stack node.

thockin · 2019-12-17T19:25:41Z

pkg/proxy/iptables/proxier.go

+	nodePortAddresses []string,
+) (proxy.Provider, error) {
+	// Create an ipv4 instance of the single-stack proxier
+	ipv4Proxier, err := NewProxier(ipt[0], sysctl,


ipt[] is now guaranteed to be v4, v6. Are clusterCidr[] and nodeIP? It seems they are, though same comments as above - perhaps turning it into maps would be better

thockin · 2019-12-17T19:29:00Z

In other words, I will approve and hold, @vllry can decide to clear the hold or fix now.

/lgtm
/approve

thockin · 2019-12-17T19:29:08Z

/retest

k8s-ci-robot · 2019-12-17T19:29:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: thockin, vllry

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:

~~cmd/kube-proxy/OWNERS~~ [thockin]
~~pkg/proxy/OWNERS~~ [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

vllry · 2019-12-17T19:34:47Z

Perhaps, instead of [2]utiliptables.Interface it should be a map[core.IPFamily]utiliptables.Interface. Remove order from the equasion, since that seems to be meaningless anyway (and assumed ordering is bound to explode on us some day).

I do prefer this idea, I think it was actually in an earlier draft of this PR. Would probably suggest this be a followup, and I'll try to handle the other suggestions before merging.

danwinship · 2019-12-17T19:51:34Z

In other words, I will approve and hold

except you forgot to actually
/hold
🙂

(Assuming the hold is wanted since @vllry said she was going to address your other suggestions.)

thockin · 2019-12-17T20:12:19Z

@vllry my comments seem to be exclusively about maps. If you want to do a follow-up, feel free to clear the hold.

Thanks @danwinship for catching my flub

vllry · 2020-01-07T04:46:41Z

Thanks everything, this was a very long time coming. Will look at the map/ordering comments as followups, as they address issues that existed prior to this PR.

/hold cancel

vllry · 2020-01-07T05:41:01Z

/retest

fejta-bot · 2020-01-07T08:58:46Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

aojea · 2020-01-07T09:57:46Z

/retest
known flake #75355

lachie83 · 2020-01-07T16:52:13Z

What what!!!

lachie83 · 2020-01-07T16:52:45Z

Awesome work getting this merged. Thank you very much @vllry and sig-network!

k8s-ci-robot requested review from caseydavenport and nicksardo September 8, 2019 05:22

vllry force-pushed the dualstack-iptables branch from 896405c to c8f301b Compare September 8, 2019 05:32

k8s-ci-robot removed request for nicksardo and caseydavenport September 8, 2019 05:37

vllry force-pushed the dualstack-iptables branch 4 times, most recently from ac34b97 to 17dcc69 Compare September 12, 2019 05:27

k8s-ci-robot added the area/ipvs label Sep 12, 2019

vllry force-pushed the dualstack-iptables branch 2 times, most recently from c3db649 to f64d914 Compare September 12, 2019 05:56

vllry mentioned this pull request Sep 12, 2019

WIP: IPv4/IPv6 dual stack support in kube-proxy (part of "dual stack phase 2") #79576

Closed

vllry force-pushed the dualstack-iptables branch from f64d914 to 735881c Compare September 12, 2019 07:17

aojea reviewed Sep 12, 2019

View reviewed changes

pkg/proxy/metaproxier/BUILD Show resolved Hide resolved

aojea reviewed Sep 12, 2019

View reviewed changes

cmd/kube-proxy/app/server_others.go Show resolved Hide resolved

aojea mentioned this pull request Sep 12, 2019

Add dual stack support kubernetes-sigs/kind#692

Merged

8 tasks

vllry force-pushed the dualstack-iptables branch from 735881c to cab0a8f Compare September 14, 2019 21:38

k8s-ci-robot assigned dcbw, robscott and thockin Dec 12, 2019

vllry force-pushed the dualstack-iptables branch from 23cb1fe to 7bd40f2 Compare December 17, 2019 06:48

Allow kube-proxy iptables mode to support dual-stack, with the meta-p…

23957a6

…roxier.

vllry force-pushed the dualstack-iptables branch from 7bd40f2 to 23957a6 Compare December 17, 2019 06:50

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 17, 2019

thockin reviewed Dec 17, 2019

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 17, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 17, 2019

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 17, 2019

aojea mentioned this pull request Jan 2, 2020

DualStack: Fail to deploy dualstack cluster, kube-proxy panics #86773

Closed

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 7, 2020

k8s-ci-robot merged commit 5373fa3 into kubernetes:master Jan 7, 2020

k8s-ci-robot added this to the v1.18 milestone Jan 7, 2020

vllry mentioned this pull request Jan 13, 2020

Remove {ipv4, ipv6} tuple conventions in kube-proxy #87123

Closed

neolit123 mentioned this pull request Feb 28, 2020

Failed to get IPv4 and IPv6 interface kubernetes/kubeadm#2045

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dualstack support for kube-proxy iptables mode #82462

Dualstack support for kube-proxy iptables mode #82462

vllry commented Sep 8, 2019 •

edited

Loading

vllry commented Sep 8, 2019

aojea commented Sep 12, 2019

vllry commented Sep 15, 2019

thockin left a comment

thockin Dec 17, 2019

thockin Dec 17, 2019

thockin Dec 17, 2019

thockin commented Dec 17, 2019

thockin commented Dec 17, 2019

k8s-ci-robot commented Dec 17, 2019

vllry commented Dec 17, 2019

danwinship commented Dec 17, 2019

thockin commented Dec 17, 2019

vllry commented Jan 7, 2020

vllry commented Jan 7, 2020

fejta-bot commented Jan 7, 2020

aojea commented Jan 7, 2020

lachie83 commented Jan 7, 2020

lachie83 commented Jan 7, 2020

Dualstack support for kube-proxy iptables mode #82462

Dualstack support for kube-proxy iptables mode #82462

Conversation

vllry commented Sep 8, 2019 • edited Loading

vllry commented Sep 8, 2019

aojea commented Sep 12, 2019

vllry commented Sep 15, 2019

thockin left a comment

Choose a reason for hiding this comment

thockin Dec 17, 2019

Choose a reason for hiding this comment

thockin Dec 17, 2019

Choose a reason for hiding this comment

thockin Dec 17, 2019

Choose a reason for hiding this comment

thockin commented Dec 17, 2019

thockin commented Dec 17, 2019

k8s-ci-robot commented Dec 17, 2019

vllry commented Dec 17, 2019

danwinship commented Dec 17, 2019

thockin commented Dec 17, 2019

vllry commented Jan 7, 2020

vllry commented Jan 7, 2020

fejta-bot commented Jan 7, 2020

aojea commented Jan 7, 2020

lachie83 commented Jan 7, 2020

lachie83 commented Jan 7, 2020

vllry commented Sep 8, 2019 •

edited

Loading