From bfccfa7016be2b1609c39d29803a0bc5b581fbd4 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 1 Jul 2023 09:30:38 -0400 Subject: [PATCH 1/4] Add names to the getLocalDetector unit tests, use t.Run --- cmd/kube-proxy/app/server_others_test.go | 89 ++++++++++++++++-------- 1 file changed, 61 insertions(+), 28 deletions(-) diff --git a/cmd/kube-proxy/app/server_others_test.go b/cmd/kube-proxy/app/server_others_test.go index 9323f8327a1..a90d2c36d15 100644 --- a/cmd/kube-proxy/app/server_others_test.go +++ b/cmd/kube-proxy/app/server_others_test.go @@ -109,6 +109,7 @@ func Test_platformApplyDefaults(t *testing.T) { func Test_getLocalDetector(t *testing.T) { cases := []struct { + name string mode proxyconfigapi.LocalMode config *proxyconfigapi.KubeProxyConfiguration ipt utiliptables.Interface @@ -118,6 +119,7 @@ func Test_getLocalDetector(t *testing.T) { }{ // LocalModeClusterCIDR { + name: "LocalModeClusterCIDR, IPv4 cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, ipt: utiliptablestest.NewFake(), @@ -125,6 +127,7 @@ func Test_getLocalDetector(t *testing.T) { errExpected: false, }, { + name: "LocalModeClusterCIDR, IPv6 cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, ipt: utiliptablestest.NewIPv6Fake(), @@ -132,6 +135,7 @@ func Test_getLocalDetector(t *testing.T) { errExpected: false, }, { + name: "LocalModeClusterCIDR, IPv6 cluster with IPv6 config", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, ipt: utiliptablestest.NewIPv6Fake(), @@ -139,6 +143,7 @@ func Test_getLocalDetector(t *testing.T) { errExpected: true, }, { + name: "LocalModeClusterCIDR, IPv4 cluster with IPv6 config", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, ipt: utiliptablestest.NewFake(), @@ -146,6 +151,7 @@ func Test_getLocalDetector(t *testing.T) { errExpected: true, }, { + name: "LocalModeClusterCIDR, no ClusterCIDR", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, ipt: utiliptablestest.NewFake(), @@ -154,6 +160,7 @@ func Test_getLocalDetector(t *testing.T) { }, // LocalModeNodeCIDR { + name: "LocalModeNodeCIDR, IPv4 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, ipt: utiliptablestest.NewFake(), @@ -162,6 +169,7 @@ func Test_getLocalDetector(t *testing.T) { errExpected: false, }, { + name: "LocalModeNodeCIDR, IPv6 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, ipt: utiliptablestest.NewIPv6Fake(), @@ -170,6 +178,7 @@ func Test_getLocalDetector(t *testing.T) { errExpected: false, }, { + name: "LocalModeNodeCIDR, IPv6 cluster with IPv4 config", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, ipt: utiliptablestest.NewIPv6Fake(), @@ -178,6 +187,7 @@ func Test_getLocalDetector(t *testing.T) { errExpected: true, }, { + name: "LocalModeNodeCIDR, IPv4 cluster with IPv6 config", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, ipt: utiliptablestest.NewFake(), @@ -186,6 +196,7 @@ func Test_getLocalDetector(t *testing.T) { errExpected: true, }, { + name: "LocalModeNodeCIDR, no PodCIDRs", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, ipt: utiliptablestest.NewFake(), @@ -195,6 +206,7 @@ func Test_getLocalDetector(t *testing.T) { }, // unknown mode { + name: "unknown LocalMode", mode: proxyconfigapi.LocalMode("abcd"), config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, ipt: utiliptablestest.NewFake(), @@ -203,6 +215,7 @@ func Test_getLocalDetector(t *testing.T) { }, // LocalModeBridgeInterface { + name: "LocalModeBrideInterface", mode: proxyconfigapi.LocalModeBridgeInterface, config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{BridgeInterface: "eth"}, @@ -211,6 +224,7 @@ func Test_getLocalDetector(t *testing.T) { errExpected: false, }, { + name: "LocalModeBridgeInterface, strange bridge name", mode: proxyconfigapi.LocalModeBridgeInterface, config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{BridgeInterface: "1234567890123456789"}, @@ -220,6 +234,7 @@ func Test_getLocalDetector(t *testing.T) { }, // LocalModeInterfaceNamePrefix { + name: "LocalModeInterfaceNamePrefix", mode: proxyconfigapi.LocalModeInterfaceNamePrefix, config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{InterfaceNamePrefix: "eth"}, @@ -228,6 +243,7 @@ func Test_getLocalDetector(t *testing.T) { errExpected: false, }, { + name: "LocalModeInterfaceNamePrefix, strange interface name", mode: proxyconfigapi.LocalModeInterfaceNamePrefix, config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{InterfaceNamePrefix: "1234567890123456789"}, @@ -236,26 +252,29 @@ func Test_getLocalDetector(t *testing.T) { errExpected: false, }, } - for i, c := range cases { - r, err := getLocalDetector(c.mode, c.config, c.ipt, c.nodePodCIDRs) - if c.errExpected { - if err == nil { - t.Errorf("Case[%d] Expected error, but succeeded with %v", i, r) + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + r, err := getLocalDetector(c.mode, c.config, c.ipt, c.nodePodCIDRs) + if c.errExpected { + if err == nil { + t.Errorf("Expected error, but succeeded with %v", r) + } + return } - continue - } - if err != nil { - t.Errorf("Case[%d] Error resolving detect-local: %v", i, err) - continue - } - if !reflect.DeepEqual(r, c.expected) { - t.Errorf("Case[%d] Unexpected detect-local implementation, expected: %q, got: %q", i, c.expected, r) - } + if err != nil { + t.Errorf("Error resolving detect-local: %v", err) + return + } + if !reflect.DeepEqual(r, c.expected) { + t.Errorf("Unexpected detect-local implementation, expected: %q, got: %q", c.expected, r) + } + }) } } func Test_getDualStackLocalDetectorTuple(t *testing.T) { cases := []struct { + name string mode proxyconfigapi.LocalMode config *proxyconfigapi.KubeProxyConfiguration ipt [2]utiliptables.Interface @@ -265,6 +284,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { }{ // LocalModeClusterCIDR { + name: "LocalModeClusterCIDR, dual-stack IPv4-primary cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14,2002::1234:abcd:ffff:c0a8:101/64"}, ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, @@ -274,6 +294,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { errExpected: false, }, { + name: "LocalModeClusterCIDR, dual-stack IPv6-primary cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64,10.0.0.0/14"}, ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, @@ -283,6 +304,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { errExpected: false, }, { + name: "LocalModeClusterCIDR, single-stack IPv4 cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, @@ -292,6 +314,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { errExpected: false, }, { + name: "LocalModeClusterCIDR, single-stack IPv6 cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, @@ -301,6 +324,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { errExpected: false, }, { + name: "LocalModeClusterCIDR, no ClusterCIDR", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, @@ -309,6 +333,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { }, // LocalModeNodeCIDR { + name: "LocalModeNodeCIDR, dual-stack IPv4-primary cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14,2002::1234:abcd:ffff:c0a8:101/64"}, ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, @@ -319,6 +344,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { errExpected: false, }, { + name: "LocalModeNodeCIDR, dual-stack IPv6-primary cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64,10.0.0.0/14"}, ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, @@ -329,6 +355,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { errExpected: false, }, { + name: "LocalModeNodeCIDR, single-stack IPv4 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, @@ -339,6 +366,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { errExpected: false, }, { + name: "LocalModeNodeCIDR, single-stack IPv6 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, @@ -349,6 +377,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { errExpected: false, }, { + name: "LocalModeNodeCIDR, no PodCIDRs", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, @@ -358,6 +387,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { }, // LocalModeBridgeInterface { + name: "LocalModeBridgeInterface", mode: proxyconfigapi.LocalModeBridgeInterface, config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{BridgeInterface: "eth"}, @@ -369,6 +399,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { }, // LocalModeInterfaceNamePrefix { + name: "LocalModeInterfaceNamePrefix", mode: proxyconfigapi.LocalModeInterfaceNamePrefix, config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{InterfaceNamePrefix: "veth"}, @@ -379,21 +410,23 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { errExpected: false, }, } - for i, c := range cases { - r, err := getDualStackLocalDetectorTuple(c.mode, c.config, c.ipt, c.nodePodCIDRs) - if c.errExpected { - if err == nil { - t.Errorf("Case[%d] expected error, but succeeded with %q", i, r) + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + r, err := getDualStackLocalDetectorTuple(c.mode, c.config, c.ipt, c.nodePodCIDRs) + if c.errExpected { + if err == nil { + t.Errorf("Expected error, but succeeded with %q", r) + } + return } - continue - } - if err != nil { - t.Errorf("Case[%d] Error resolving detect-local: %v", i, err) - continue - } - if !reflect.DeepEqual(r, c.expected) { - t.Errorf("Case[%d] Unexpected detect-local implementation, expected: %q, got: %q", i, c.expected, r) - } + if err != nil { + t.Errorf("Error resolving detect-local: %v", err) + return + } + if !reflect.DeepEqual(r, c.expected) { + t.Errorf("Unexpected detect-local implementation, expected: %q, got: %q", c.expected, r) + } + }) } } From cefd50a753e16ee2753155b65c5d6558020d1280 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 1 Jul 2023 09:11:28 -0400 Subject: [PATCH 2/4] Improve the single-stack LocalDetector behavior 1. When bringing up a single-stack kube-proxy in a dual-stack cluster, allow using either the primary or secondary IP family. 2. Since the earlier config-checking code will already have bailed out if the single-stack configuration is unusably broken, we don't need to do that here. Instead, just return a no-op local detector if there are no usable CIDRs of the expected IP family. --- cmd/kube-proxy/app/server_others.go | 37 ++++++++++++------ cmd/kube-proxy/app/server_others_test.go | 49 +++++++++++++++++++----- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index 061152a8882..349a8e31132 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -50,6 +50,7 @@ import ( utilipset "k8s.io/kubernetes/pkg/proxy/ipvs/ipset" utilipvs "k8s.io/kubernetes/pkg/proxy/ipvs/util" proxymetrics "k8s.io/kubernetes/pkg/proxy/metrics" + proxyutil "k8s.io/kubernetes/pkg/proxy/util" proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" utiliptables "k8s.io/kubernetes/pkg/util/iptables" "k8s.io/utils/exec" @@ -179,7 +180,7 @@ func (s *ProxyServer) createProxier(config *proxyconfigapi.KubeProxyConfiguratio } else { // Create a single-stack proxier if and only if the node does not support dual-stack (i.e, no iptables support). var localDetector proxyutiliptables.LocalTrafficDetector - localDetector, err = getLocalDetector(config.DetectLocalMode, config, iptInterface, s.podCIDRs) + localDetector, err = getLocalDetector(s.PrimaryIPFamily, config.DetectLocalMode, config, iptInterface, s.podCIDRs) if err != nil { return nil, fmt.Errorf("unable to create proxier: %v", err) } @@ -250,7 +251,7 @@ func (s *ProxyServer) createProxier(config *proxyconfigapi.KubeProxyConfiguratio ) } else { var localDetector proxyutiliptables.LocalTrafficDetector - localDetector, err = getLocalDetector(config.DetectLocalMode, config, iptInterface, s.podCIDRs) + localDetector, err = getLocalDetector(s.PrimaryIPFamily, config.DetectLocalMode, config, iptInterface, s.podCIDRs) if err != nil { return nil, fmt.Errorf("unable to create proxier: %v", err) } @@ -402,28 +403,40 @@ func detectNumCPU() int { return numCPU } -func getLocalDetector(mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, ipt utiliptables.Interface, nodePodCIDRs []string) (proxyutiliptables.LocalTrafficDetector, error) { +func getLocalDetector(ipFamily v1.IPFamily, mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, ipt utiliptables.Interface, nodePodCIDRs []string) (proxyutiliptables.LocalTrafficDetector, error) { switch mode { case proxyconfigapi.LocalModeClusterCIDR: // LocalModeClusterCIDR is the default if --detect-local-mode wasn't passed, // but --cluster-cidr is optional. - if len(strings.TrimSpace(config.ClusterCIDR)) == 0 { + clusterCIDRs := strings.TrimSpace(config.ClusterCIDR) + if len(clusterCIDRs) == 0 { klog.InfoS("Detect-local-mode set to ClusterCIDR, but no cluster CIDR defined") break } - return proxyutiliptables.NewDetectLocalByCIDR(config.ClusterCIDR, ipt) - case proxyconfigapi.LocalModeNodeCIDR: - if len(nodePodCIDRs) == 0 { - klog.InfoS("Detect-local-mode set to NodeCIDR, but no PodCIDR defined at node") - break + + cidrsByFamily := proxyutil.MapCIDRsByIPFamily(strings.Split(clusterCIDRs, ",")) + if len(cidrsByFamily[ipFamily]) != 0 { + return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0], ipt) } - return proxyutiliptables.NewDetectLocalByCIDR(nodePodCIDRs[0], ipt) + + klog.InfoS("Detect-local-mode set to ClusterCIDR, but no cluster CIDR for family", "ipFamily", ipFamily) + + case proxyconfigapi.LocalModeNodeCIDR: + cidrsByFamily := proxyutil.MapCIDRsByIPFamily(nodePodCIDRs) + if len(cidrsByFamily[ipFamily]) != 0 { + return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0], ipt) + } + + klog.InfoS("Detect-local-mode set to NodeCIDR, but no PodCIDR defined at node for family", "ipFamily", ipFamily) + case proxyconfigapi.LocalModeBridgeInterface: return proxyutiliptables.NewDetectLocalByBridgeInterface(config.DetectLocal.BridgeInterface) + case proxyconfigapi.LocalModeInterfaceNamePrefix: return proxyutiliptables.NewDetectLocalByInterfaceNamePrefix(config.DetectLocal.InterfaceNamePrefix) } - klog.InfoS("Defaulting to no-op detect-local", "detectLocalMode", string(mode)) + + klog.InfoS("Defaulting to no-op detect-local") return proxyutiliptables.NewNoOpLocalDetector(), nil } @@ -482,7 +495,7 @@ func getDualStackLocalDetectorTuple(mode proxyconfigapi.LocalMode, config *proxy } return localDetectors, err case proxyconfigapi.LocalModeBridgeInterface, proxyconfigapi.LocalModeInterfaceNamePrefix: - localDetector, err := getLocalDetector(mode, config, ipt[0], nodePodCIDRs) + localDetector, err := getLocalDetector("", mode, config, nil, nodePodCIDRs) if err == nil { localDetectors[0] = localDetector localDetectors[1] = localDetector diff --git a/cmd/kube-proxy/app/server_others_test.go b/cmd/kube-proxy/app/server_others_test.go index a90d2c36d15..797fcfe0301 100644 --- a/cmd/kube-proxy/app/server_others_test.go +++ b/cmd/kube-proxy/app/server_others_test.go @@ -112,6 +112,7 @@ func Test_getLocalDetector(t *testing.T) { name string mode proxyconfigapi.LocalMode config *proxyconfigapi.KubeProxyConfiguration + family v1.IPFamily ipt utiliptables.Interface expected proxyutiliptables.LocalTrafficDetector nodePodCIDRs []string @@ -122,6 +123,7 @@ func Test_getLocalDetector(t *testing.T) { name: "LocalModeClusterCIDR, IPv4 cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, + family: v1.IPv4Protocol, ipt: utiliptablestest.NewFake(), expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14", utiliptablestest.NewFake())), errExpected: false, @@ -130,6 +132,7 @@ func Test_getLocalDetector(t *testing.T) { name: "LocalModeClusterCIDR, IPv6 cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, + family: v1.IPv6Protocol, ipt: utiliptablestest.NewIPv6Fake(), expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/64", utiliptablestest.NewIPv6Fake())), errExpected: false, @@ -138,22 +141,34 @@ func Test_getLocalDetector(t *testing.T) { name: "LocalModeClusterCIDR, IPv6 cluster with IPv6 config", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, + family: v1.IPv6Protocol, ipt: utiliptablestest.NewIPv6Fake(), - expected: nil, - errExpected: true, + expected: proxyutiliptables.NewNoOpLocalDetector(), + errExpected: false, }, { name: "LocalModeClusterCIDR, IPv4 cluster with IPv6 config", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, + family: v1.IPv4Protocol, ipt: utiliptablestest.NewFake(), - expected: nil, - errExpected: true, + expected: proxyutiliptables.NewNoOpLocalDetector(), + errExpected: false, + }, + { + name: "LocalModeClusterCIDR, IPv4 kube-proxy in dual-stack IPv6-primary cluster", + mode: proxyconfigapi.LocalModeClusterCIDR, + config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64,10.0.0.0/14"}, + family: v1.IPv4Protocol, + ipt: utiliptablestest.NewFake(), + expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14", utiliptablestest.NewFake())), + errExpected: false, }, { name: "LocalModeClusterCIDR, no ClusterCIDR", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, + family: v1.IPv4Protocol, ipt: utiliptablestest.NewFake(), expected: proxyutiliptables.NewNoOpLocalDetector(), errExpected: false, @@ -163,6 +178,7 @@ func Test_getLocalDetector(t *testing.T) { name: "LocalModeNodeCIDR, IPv4 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, + family: v1.IPv4Protocol, ipt: utiliptablestest.NewFake(), expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24", utiliptablestest.NewFake())), nodePodCIDRs: []string{"10.0.0.0/24"}, @@ -172,6 +188,7 @@ func Test_getLocalDetector(t *testing.T) { name: "LocalModeNodeCIDR, IPv6 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, + family: v1.IPv6Protocol, ipt: utiliptablestest.NewIPv6Fake(), expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96", utiliptablestest.NewIPv6Fake())), nodePodCIDRs: []string{"2002::1234:abcd:ffff:c0a8:101/96"}, @@ -181,24 +198,37 @@ func Test_getLocalDetector(t *testing.T) { name: "LocalModeNodeCIDR, IPv6 cluster with IPv4 config", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, + family: v1.IPv6Protocol, ipt: utiliptablestest.NewIPv6Fake(), - expected: nil, + expected: proxyutiliptables.NewNoOpLocalDetector(), nodePodCIDRs: []string{"10.0.0.0/24"}, - errExpected: true, + errExpected: false, }, { name: "LocalModeNodeCIDR, IPv4 cluster with IPv6 config", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, + family: v1.IPv4Protocol, ipt: utiliptablestest.NewFake(), - expected: nil, + expected: proxyutiliptables.NewNoOpLocalDetector(), nodePodCIDRs: []string{"2002::1234:abcd:ffff:c0a8:101/96"}, - errExpected: true, + errExpected: false, + }, + { + name: "LocalModeNodeCIDR, IPv6 kube-proxy in dual-stack IPv4-primary cluster", + mode: proxyconfigapi.LocalModeNodeCIDR, + config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14,2002::1234:abcd:ffff:c0a8:101/64"}, + family: v1.IPv6Protocol, + ipt: utiliptablestest.NewIPv6Fake(), + expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96", utiliptablestest.NewIPv6Fake())), + nodePodCIDRs: []string{"10.0.0.0/24", "2002::1234:abcd:ffff:c0a8:101/96"}, + errExpected: false, }, { name: "LocalModeNodeCIDR, no PodCIDRs", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, + family: v1.IPv4Protocol, ipt: utiliptablestest.NewFake(), expected: proxyutiliptables.NewNoOpLocalDetector(), nodePodCIDRs: []string{}, @@ -209,6 +239,7 @@ func Test_getLocalDetector(t *testing.T) { name: "unknown LocalMode", mode: proxyconfigapi.LocalMode("abcd"), config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, + family: v1.IPv4Protocol, ipt: utiliptablestest.NewFake(), expected: proxyutiliptables.NewNoOpLocalDetector(), errExpected: false, @@ -254,7 +285,7 @@ func Test_getLocalDetector(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - r, err := getLocalDetector(c.mode, c.config, c.ipt, c.nodePodCIDRs) + r, err := getLocalDetector(c.family, c.mode, c.config, c.ipt, c.nodePodCIDRs) if c.errExpected { if err == nil { t.Errorf("Expected error, but succeeded with %v", r) From 7690c6e81210b03ab7fb93c3a5c4392ba9e5befd Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 13 Apr 2023 13:19:26 -0400 Subject: [PATCH 3/4] Simplify getDualStackLocalDetectorTuple Since the single-stack and dual-stack local-detector-getters now have the same behavior in terms of error-checking and dual-stack config, we can just replace the contents of getDualStackLocalDetectorTuple() with a pair of calls to getLocalDetector(). --- cmd/kube-proxy/app/server_others.go | 92 +++-------------------------- 1 file changed, 7 insertions(+), 85 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index 349a8e31132..2bdedc413e2 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -54,7 +54,6 @@ import ( proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" utiliptables "k8s.io/kubernetes/pkg/util/iptables" "k8s.io/utils/exec" - netutils "k8s.io/utils/net" "k8s.io/klog/v2" ) @@ -441,97 +440,20 @@ func getLocalDetector(ipFamily v1.IPFamily, mode proxyconfigapi.LocalMode, confi } func getDualStackLocalDetectorTuple(mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, ipt [2]utiliptables.Interface, nodePodCIDRs []string) ([2]proxyutiliptables.LocalTrafficDetector, error) { + var localDetectors [2]proxyutiliptables.LocalTrafficDetector var err error - localDetectors := [2]proxyutiliptables.LocalTrafficDetector{proxyutiliptables.NewNoOpLocalDetector(), proxyutiliptables.NewNoOpLocalDetector()} - switch mode { - case proxyconfigapi.LocalModeClusterCIDR: - // LocalModeClusterCIDR is the default if --detect-local-mode wasn't passed, - // but --cluster-cidr is optional. - if len(strings.TrimSpace(config.ClusterCIDR)) == 0 { - klog.InfoS("Detect-local-mode set to ClusterCIDR, but no cluster CIDR defined") - break - } - clusterCIDRs := cidrTuple(config.ClusterCIDR) - - if len(strings.TrimSpace(clusterCIDRs[0])) == 0 { - klog.InfoS("Detect-local-mode set to ClusterCIDR, but no IPv4 cluster CIDR defined, defaulting to no-op detect-local for IPv4") - } else { - localDetectors[0], err = proxyutiliptables.NewDetectLocalByCIDR(clusterCIDRs[0], ipt[0]) - if err != nil { // don't loose the original error - return localDetectors, err - } - } - - if len(strings.TrimSpace(clusterCIDRs[1])) == 0 { - klog.InfoS("Detect-local-mode set to ClusterCIDR, but no IPv6 cluster CIDR defined, defaulting to no-op detect-local for IPv6") - } else { - localDetectors[1], err = proxyutiliptables.NewDetectLocalByCIDR(clusterCIDRs[1], ipt[1]) - } + localDetectors[0], err = getLocalDetector(v1.IPv4Protocol, mode, config, ipt[0], nodePodCIDRs) + if err != nil { + return localDetectors, err + } + localDetectors[1], err = getLocalDetector(v1.IPv6Protocol, mode, config, ipt[1], nodePodCIDRs) + if err != nil { return localDetectors, err - case proxyconfigapi.LocalModeNodeCIDR: - if len(nodePodCIDRs) == 0 { - klog.InfoS("No node info available to configure detect-local-mode NodeCIDR") - break - } - // localDetectors, like ipt, need to be of the order [IPv4, IPv6], but PodCIDRs is setup so that PodCIDRs[0] == PodCIDR. - // so have to handle the case where PodCIDR can be IPv6 and set that to localDetectors[1] - if netutils.IsIPv6CIDRString(nodePodCIDRs[0]) { - localDetectors[1], err = proxyutiliptables.NewDetectLocalByCIDR(nodePodCIDRs[0], ipt[1]) - if err != nil { - return localDetectors, err - } - if len(nodePodCIDRs) > 1 { - localDetectors[0], err = proxyutiliptables.NewDetectLocalByCIDR(nodePodCIDRs[1], ipt[0]) - } - } else { - localDetectors[0], err = proxyutiliptables.NewDetectLocalByCIDR(nodePodCIDRs[0], ipt[0]) - if err != nil { - return localDetectors, err - } - if len(nodePodCIDRs) > 1 { - localDetectors[1], err = proxyutiliptables.NewDetectLocalByCIDR(nodePodCIDRs[1], ipt[1]) - } - } - return localDetectors, err - case proxyconfigapi.LocalModeBridgeInterface, proxyconfigapi.LocalModeInterfaceNamePrefix: - localDetector, err := getLocalDetector("", mode, config, nil, nodePodCIDRs) - if err == nil { - localDetectors[0] = localDetector - localDetectors[1] = localDetector - } - return localDetectors, err - default: - klog.InfoS("Unknown detect-local-mode", "detectLocalMode", mode) } - klog.InfoS("Defaulting to no-op detect-local", "detectLocalMode", string(mode)) return localDetectors, nil } -// cidrTuple takes a comma separated list of CIDRs and return a tuple (ipv4cidr,ipv6cidr) -// The returned tuple is guaranteed to have the order (ipv4,ipv6) and if no cidr from a family is found an -// empty string "" is inserted. -func cidrTuple(cidrList string) [2]string { - cidrs := [2]string{"", ""} - foundIPv4 := false - foundIPv6 := false - - for _, cidr := range strings.Split(cidrList, ",") { - if netutils.IsIPv6CIDRString(cidr) && !foundIPv6 { - cidrs[1] = cidr - foundIPv6 = true - } else if !foundIPv4 { - cidrs[0] = cidr - foundIPv4 = true - } - if foundIPv6 && foundIPv4 { - break - } - } - - return cidrs -} - // cleanupAndExit remove iptables rules and ipset/ipvs rules func cleanupAndExit() error { execer := exec.New() From e2900da46aac8202d4387aba4ef035400bc6920e Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 18 May 2023 21:35:52 -0400 Subject: [PATCH 4/4] Remove unnecessary utiliptables.Interface arg from local detectors getLocalDetector() used to pass a utiliptables.Interface to NewDetectLocalByCIDR() so that NewDetectLocalByCIDR() could verify that the passed-in CIDR was of the same family as the iptables interface. It would make more sense for getLocalDetector() to verify this itself and just *not call NewDetectLocalByCIDR* if the families don't match, and that's what the code does now. So there's no longer any need to pass the utiliptables.Interface to the local detector. --- cmd/kube-proxy/app/server_others.go | 20 +++---- cmd/kube-proxy/app/server_others_test.go | 71 ++++++++---------------- pkg/proxy/iptables/proxier_test.go | 2 +- pkg/proxy/util/iptables/traffic.go | 6 +- pkg/proxy/util/iptables/traffic_test.go | 27 +-------- 5 files changed, 38 insertions(+), 88 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index 2bdedc413e2..517f44e20cf 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -154,7 +154,7 @@ func (s *ProxyServer) createProxier(config *proxyconfigapi.KubeProxyConfiguratio if dualStack { // Always ordered to match []ipt var localDetectors [2]proxyutiliptables.LocalTrafficDetector - localDetectors, err = getDualStackLocalDetectorTuple(config.DetectLocalMode, config, ipt, s.podCIDRs) + localDetectors, err = getDualStackLocalDetectorTuple(config.DetectLocalMode, config, s.podCIDRs) if err != nil { return nil, fmt.Errorf("unable to create proxier: %v", err) } @@ -179,7 +179,7 @@ func (s *ProxyServer) createProxier(config *proxyconfigapi.KubeProxyConfiguratio } else { // Create a single-stack proxier if and only if the node does not support dual-stack (i.e, no iptables support). var localDetector proxyutiliptables.LocalTrafficDetector - localDetector, err = getLocalDetector(s.PrimaryIPFamily, config.DetectLocalMode, config, iptInterface, s.podCIDRs) + localDetector, err = getLocalDetector(s.PrimaryIPFamily, config.DetectLocalMode, config, s.podCIDRs) if err != nil { return nil, fmt.Errorf("unable to create proxier: %v", err) } @@ -219,7 +219,7 @@ func (s *ProxyServer) createProxier(config *proxyconfigapi.KubeProxyConfiguratio if dualStack { // Always ordered to match []ipt var localDetectors [2]proxyutiliptables.LocalTrafficDetector - localDetectors, err = getDualStackLocalDetectorTuple(config.DetectLocalMode, config, ipt, s.podCIDRs) + localDetectors, err = getDualStackLocalDetectorTuple(config.DetectLocalMode, config, s.podCIDRs) if err != nil { return nil, fmt.Errorf("unable to create proxier: %v", err) } @@ -250,7 +250,7 @@ func (s *ProxyServer) createProxier(config *proxyconfigapi.KubeProxyConfiguratio ) } else { var localDetector proxyutiliptables.LocalTrafficDetector - localDetector, err = getLocalDetector(s.PrimaryIPFamily, config.DetectLocalMode, config, iptInterface, s.podCIDRs) + localDetector, err = getLocalDetector(s.PrimaryIPFamily, config.DetectLocalMode, config, s.podCIDRs) if err != nil { return nil, fmt.Errorf("unable to create proxier: %v", err) } @@ -402,7 +402,7 @@ func detectNumCPU() int { return numCPU } -func getLocalDetector(ipFamily v1.IPFamily, mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, ipt utiliptables.Interface, nodePodCIDRs []string) (proxyutiliptables.LocalTrafficDetector, error) { +func getLocalDetector(ipFamily v1.IPFamily, mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, nodePodCIDRs []string) (proxyutiliptables.LocalTrafficDetector, error) { switch mode { case proxyconfigapi.LocalModeClusterCIDR: // LocalModeClusterCIDR is the default if --detect-local-mode wasn't passed, @@ -415,7 +415,7 @@ func getLocalDetector(ipFamily v1.IPFamily, mode proxyconfigapi.LocalMode, confi cidrsByFamily := proxyutil.MapCIDRsByIPFamily(strings.Split(clusterCIDRs, ",")) if len(cidrsByFamily[ipFamily]) != 0 { - return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0], ipt) + return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0]) } klog.InfoS("Detect-local-mode set to ClusterCIDR, but no cluster CIDR for family", "ipFamily", ipFamily) @@ -423,7 +423,7 @@ func getLocalDetector(ipFamily v1.IPFamily, mode proxyconfigapi.LocalMode, confi case proxyconfigapi.LocalModeNodeCIDR: cidrsByFamily := proxyutil.MapCIDRsByIPFamily(nodePodCIDRs) if len(cidrsByFamily[ipFamily]) != 0 { - return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0], ipt) + return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0]) } klog.InfoS("Detect-local-mode set to NodeCIDR, but no PodCIDR defined at node for family", "ipFamily", ipFamily) @@ -439,15 +439,15 @@ func getLocalDetector(ipFamily v1.IPFamily, mode proxyconfigapi.LocalMode, confi return proxyutiliptables.NewNoOpLocalDetector(), nil } -func getDualStackLocalDetectorTuple(mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, ipt [2]utiliptables.Interface, nodePodCIDRs []string) ([2]proxyutiliptables.LocalTrafficDetector, error) { +func getDualStackLocalDetectorTuple(mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, nodePodCIDRs []string) ([2]proxyutiliptables.LocalTrafficDetector, error) { var localDetectors [2]proxyutiliptables.LocalTrafficDetector var err error - localDetectors[0], err = getLocalDetector(v1.IPv4Protocol, mode, config, ipt[0], nodePodCIDRs) + localDetectors[0], err = getLocalDetector(v1.IPv4Protocol, mode, config, nodePodCIDRs) if err != nil { return localDetectors, err } - localDetectors[1], err = getLocalDetector(v1.IPv6Protocol, mode, config, ipt[1], nodePodCIDRs) + localDetectors[1], err = getLocalDetector(v1.IPv6Protocol, mode, config, nodePodCIDRs) if err != nil { return localDetectors, err } diff --git a/cmd/kube-proxy/app/server_others_test.go b/cmd/kube-proxy/app/server_others_test.go index 797fcfe0301..32311315fc8 100644 --- a/cmd/kube-proxy/app/server_others_test.go +++ b/cmd/kube-proxy/app/server_others_test.go @@ -38,8 +38,6 @@ import ( clientgotesting "k8s.io/client-go/testing" proxyconfigapi "k8s.io/kubernetes/pkg/proxy/apis/config" proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" - utiliptables "k8s.io/kubernetes/pkg/util/iptables" - utiliptablestest "k8s.io/kubernetes/pkg/util/iptables/testing" netutils "k8s.io/utils/net" "k8s.io/utils/pointer" ) @@ -113,7 +111,6 @@ func Test_getLocalDetector(t *testing.T) { mode proxyconfigapi.LocalMode config *proxyconfigapi.KubeProxyConfiguration family v1.IPFamily - ipt utiliptables.Interface expected proxyutiliptables.LocalTrafficDetector nodePodCIDRs []string errExpected bool @@ -124,8 +121,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, family: v1.IPv4Protocol, - ipt: utiliptablestest.NewFake(), - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14", utiliptablestest.NewFake())), + expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14")), errExpected: false, }, { @@ -133,8 +129,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, family: v1.IPv6Protocol, - ipt: utiliptablestest.NewIPv6Fake(), - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/64", utiliptablestest.NewIPv6Fake())), + expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/64")), errExpected: false, }, { @@ -142,7 +137,6 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, family: v1.IPv6Protocol, - ipt: utiliptablestest.NewIPv6Fake(), expected: proxyutiliptables.NewNoOpLocalDetector(), errExpected: false, }, @@ -151,7 +145,6 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, family: v1.IPv4Protocol, - ipt: utiliptablestest.NewFake(), expected: proxyutiliptables.NewNoOpLocalDetector(), errExpected: false, }, @@ -160,8 +153,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64,10.0.0.0/14"}, family: v1.IPv4Protocol, - ipt: utiliptablestest.NewFake(), - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14", utiliptablestest.NewFake())), + expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14")), errExpected: false, }, { @@ -169,7 +161,6 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, family: v1.IPv4Protocol, - ipt: utiliptablestest.NewFake(), expected: proxyutiliptables.NewNoOpLocalDetector(), errExpected: false, }, @@ -179,8 +170,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, family: v1.IPv4Protocol, - ipt: utiliptablestest.NewFake(), - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24", utiliptablestest.NewFake())), + expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24")), nodePodCIDRs: []string{"10.0.0.0/24"}, errExpected: false, }, @@ -189,8 +179,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, family: v1.IPv6Protocol, - ipt: utiliptablestest.NewIPv6Fake(), - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96", utiliptablestest.NewIPv6Fake())), + expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96")), nodePodCIDRs: []string{"2002::1234:abcd:ffff:c0a8:101/96"}, errExpected: false, }, @@ -199,7 +188,6 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, family: v1.IPv6Protocol, - ipt: utiliptablestest.NewIPv6Fake(), expected: proxyutiliptables.NewNoOpLocalDetector(), nodePodCIDRs: []string{"10.0.0.0/24"}, errExpected: false, @@ -209,7 +197,6 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, family: v1.IPv4Protocol, - ipt: utiliptablestest.NewFake(), expected: proxyutiliptables.NewNoOpLocalDetector(), nodePodCIDRs: []string{"2002::1234:abcd:ffff:c0a8:101/96"}, errExpected: false, @@ -219,8 +206,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14,2002::1234:abcd:ffff:c0a8:101/64"}, family: v1.IPv6Protocol, - ipt: utiliptablestest.NewIPv6Fake(), - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96", utiliptablestest.NewIPv6Fake())), + expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96")), nodePodCIDRs: []string{"10.0.0.0/24", "2002::1234:abcd:ffff:c0a8:101/96"}, errExpected: false, }, @@ -229,7 +215,6 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, family: v1.IPv4Protocol, - ipt: utiliptablestest.NewFake(), expected: proxyutiliptables.NewNoOpLocalDetector(), nodePodCIDRs: []string{}, errExpected: false, @@ -240,7 +225,6 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalMode("abcd"), config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, family: v1.IPv4Protocol, - ipt: utiliptablestest.NewFake(), expected: proxyutiliptables.NewNoOpLocalDetector(), errExpected: false, }, @@ -251,6 +235,7 @@ func Test_getLocalDetector(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{BridgeInterface: "eth"}, }, + family: v1.IPv4Protocol, expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByBridgeInterface("eth")), errExpected: false, }, @@ -260,6 +245,7 @@ func Test_getLocalDetector(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{BridgeInterface: "1234567890123456789"}, }, + family: v1.IPv4Protocol, expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByBridgeInterface("1234567890123456789")), errExpected: false, }, @@ -270,6 +256,7 @@ func Test_getLocalDetector(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{InterfaceNamePrefix: "eth"}, }, + family: v1.IPv4Protocol, expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByInterfaceNamePrefix("eth")), errExpected: false, }, @@ -279,13 +266,14 @@ func Test_getLocalDetector(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{InterfaceNamePrefix: "1234567890123456789"}, }, + family: v1.IPv4Protocol, expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByInterfaceNamePrefix("1234567890123456789")), errExpected: false, }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - r, err := getLocalDetector(c.family, c.mode, c.config, c.ipt, c.nodePodCIDRs) + r, err := getLocalDetector(c.family, c.mode, c.config, c.nodePodCIDRs) if c.errExpected { if err == nil { t.Errorf("Expected error, but succeeded with %v", r) @@ -308,7 +296,6 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name string mode proxyconfigapi.LocalMode config *proxyconfigapi.KubeProxyConfiguration - ipt [2]utiliptables.Interface expected [2]proxyutiliptables.LocalTrafficDetector nodePodCIDRs []string errExpected bool @@ -318,29 +305,26 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name: "LocalModeClusterCIDR, dual-stack IPv4-primary cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14,2002::1234:abcd:ffff:c0a8:101/64"}, - ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, expected: resolveDualStackLocalDetectors(t)( - proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14", utiliptablestest.NewFake()))( - proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/64", utiliptablestest.NewIPv6Fake())), + proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14"))( + proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/64")), errExpected: false, }, { name: "LocalModeClusterCIDR, dual-stack IPv6-primary cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64,10.0.0.0/14"}, - ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, expected: resolveDualStackLocalDetectors(t)( - proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14", utiliptablestest.NewFake()))( - proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/64", utiliptablestest.NewIPv6Fake())), + proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14"))( + proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/64")), errExpected: false, }, { name: "LocalModeClusterCIDR, single-stack IPv4 cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, - ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, expected: [2]proxyutiliptables.LocalTrafficDetector{ - resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14", utiliptablestest.NewFake())), + resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14")), proxyutiliptables.NewNoOpLocalDetector()}, errExpected: false, }, @@ -348,17 +332,15 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name: "LocalModeClusterCIDR, single-stack IPv6 cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, - ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, expected: [2]proxyutiliptables.LocalTrafficDetector{ proxyutiliptables.NewNoOpLocalDetector(), - resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/64", utiliptablestest.NewIPv6Fake()))}, + resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/64"))}, errExpected: false, }, { name: "LocalModeClusterCIDR, no ClusterCIDR", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, - ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, expected: [2]proxyutiliptables.LocalTrafficDetector{proxyutiliptables.NewNoOpLocalDetector(), proxyutiliptables.NewNoOpLocalDetector()}, errExpected: false, }, @@ -367,10 +349,9 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name: "LocalModeNodeCIDR, dual-stack IPv4-primary cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14,2002::1234:abcd:ffff:c0a8:101/64"}, - ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, expected: resolveDualStackLocalDetectors(t)( - proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24", utiliptablestest.NewFake()))( - proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96", utiliptablestest.NewIPv6Fake())), + proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24"))( + proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96")), nodePodCIDRs: []string{"10.0.0.0/24", "2002::1234:abcd:ffff:c0a8:101/96"}, errExpected: false, }, @@ -378,10 +359,9 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name: "LocalModeNodeCIDR, dual-stack IPv6-primary cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64,10.0.0.0/14"}, - ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, expected: resolveDualStackLocalDetectors(t)( - proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24", utiliptablestest.NewFake()))( - proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96", utiliptablestest.NewIPv6Fake())), + proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24"))( + proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96")), nodePodCIDRs: []string{"2002::1234:abcd:ffff:c0a8:101/96", "10.0.0.0/24"}, errExpected: false, }, @@ -389,9 +369,8 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name: "LocalModeNodeCIDR, single-stack IPv4 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, - ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, expected: [2]proxyutiliptables.LocalTrafficDetector{ - resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24", utiliptablestest.NewFake())), + resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24")), proxyutiliptables.NewNoOpLocalDetector()}, nodePodCIDRs: []string{"10.0.0.0/24"}, errExpected: false, @@ -400,10 +379,9 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name: "LocalModeNodeCIDR, single-stack IPv6 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002::1234:abcd:ffff:c0a8:101/64"}, - ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, expected: [2]proxyutiliptables.LocalTrafficDetector{ proxyutiliptables.NewNoOpLocalDetector(), - resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96", utiliptablestest.NewIPv6Fake()))}, + resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:c0a8:101/96"))}, nodePodCIDRs: []string{"2002::1234:abcd:ffff:c0a8:101/96"}, errExpected: false, }, @@ -411,7 +389,6 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name: "LocalModeNodeCIDR, no PodCIDRs", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, - ipt: [2]utiliptables.Interface{utiliptablestest.NewFake(), utiliptablestest.NewIPv6Fake()}, expected: [2]proxyutiliptables.LocalTrafficDetector{proxyutiliptables.NewNoOpLocalDetector(), proxyutiliptables.NewNoOpLocalDetector()}, nodePodCIDRs: []string{}, errExpected: false, @@ -443,7 +420,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - r, err := getDualStackLocalDetectorTuple(c.mode, c.config, c.ipt, c.nodePodCIDRs) + r, err := getDualStackLocalDetectorTuple(c.mode, c.config, c.nodePodCIDRs) if c.errExpected { if err == nil { t.Errorf("Expected error, but succeeded with %q", r) diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 166f7e1ffbc..7d104da9987 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -291,7 +291,7 @@ func NewFakeProxier(ipt utiliptables.Interface) *Proxier { ipfamily = v1.IPv6Protocol podCIDR = "fd00::/64" } - detectLocal, _ := proxyutiliptables.NewDetectLocalByCIDR(podCIDR, ipt) + detectLocal, _ := proxyutiliptables.NewDetectLocalByCIDR(podCIDR) networkInterfacer := proxyutiltest.NewFakeNetwork() itf := net.Interface{Index: 0, MTU: 0, Name: "lo", HardwareAddr: nil, Flags: 0} diff --git a/pkg/proxy/util/iptables/traffic.go b/pkg/proxy/util/iptables/traffic.go index 4666c6c3de6..f27d89e9a57 100644 --- a/pkg/proxy/util/iptables/traffic.go +++ b/pkg/proxy/util/iptables/traffic.go @@ -19,7 +19,6 @@ package iptables import ( "fmt" - utiliptables "k8s.io/kubernetes/pkg/util/iptables" netutils "k8s.io/utils/net" ) @@ -62,10 +61,7 @@ type detectLocalByCIDR struct { // NewDetectLocalByCIDR implements the LocalTrafficDetector interface using a CIDR. This can be used when a single CIDR // range can be used to capture the notion of local traffic. -func NewDetectLocalByCIDR(cidr string, ipt utiliptables.Interface) (LocalTrafficDetector, error) { - if netutils.IsIPv6CIDRString(cidr) != ipt.IsIPv6() { - return nil, fmt.Errorf("CIDR %s has incorrect IP version: expect isIPv6=%t", cidr, ipt.IsIPv6()) - } +func NewDetectLocalByCIDR(cidr string) (LocalTrafficDetector, error) { _, _, err := netutils.ParseCIDRSloppy(cidr) if err != nil { return nil, err diff --git a/pkg/proxy/util/iptables/traffic_test.go b/pkg/proxy/util/iptables/traffic_test.go index f74b850b2fa..f60169de9da 100644 --- a/pkg/proxy/util/iptables/traffic_test.go +++ b/pkg/proxy/util/iptables/traffic_test.go @@ -19,9 +19,6 @@ package iptables import ( "reflect" "testing" - - utiliptables "k8s.io/kubernetes/pkg/util/iptables" - iptablestest "k8s.io/kubernetes/pkg/util/iptables/testing" ) func TestNoOpLocalDetector(t *testing.T) { @@ -44,52 +41,35 @@ func TestNoOpLocalDetector(t *testing.T) { func TestNewDetectLocalByCIDR(t *testing.T) { cases := []struct { cidr string - ipt utiliptables.Interface errExpected bool }{ { cidr: "10.0.0.0/14", - ipt: iptablestest.NewFake(), errExpected: false, }, { cidr: "2002::1234:abcd:ffff:c0a8:101/64", - ipt: iptablestest.NewIPv6Fake(), errExpected: false, }, - { - cidr: "10.0.0.0/14", - ipt: iptablestest.NewIPv6Fake(), - errExpected: true, - }, - { - cidr: "2002::1234:abcd:ffff:c0a8:101/64", - ipt: iptablestest.NewFake(), - errExpected: true, - }, { cidr: "10.0.0.0", - ipt: iptablestest.NewFake(), errExpected: true, }, { cidr: "2002::1234:abcd:ffff:c0a8:101", - ipt: iptablestest.NewIPv6Fake(), errExpected: true, }, { cidr: "", - ipt: iptablestest.NewFake(), errExpected: true, }, { cidr: "", - ipt: iptablestest.NewIPv6Fake(), errExpected: true, }, } for i, c := range cases { - r, err := NewDetectLocalByCIDR(c.cidr, c.ipt) + r, err := NewDetectLocalByCIDR(c.cidr) if c.errExpected { if err == nil { t.Errorf("Case[%d] expected error, but succeeded with: %q", i, r) @@ -105,25 +85,22 @@ func TestNewDetectLocalByCIDR(t *testing.T) { func TestDetectLocalByCIDR(t *testing.T) { cases := []struct { cidr string - ipt utiliptables.Interface expectedIfLocalOutput []string expectedIfNotLocalOutput []string }{ { cidr: "10.0.0.0/14", - ipt: iptablestest.NewFake(), expectedIfLocalOutput: []string{"-s", "10.0.0.0/14"}, expectedIfNotLocalOutput: []string{"!", "-s", "10.0.0.0/14"}, }, { cidr: "2002::1234:abcd:ffff:c0a8:101/64", - ipt: iptablestest.NewIPv6Fake(), expectedIfLocalOutput: []string{"-s", "2002::1234:abcd:ffff:c0a8:101/64"}, expectedIfNotLocalOutput: []string{"!", "-s", "2002::1234:abcd:ffff:c0a8:101/64"}, }, } for _, c := range cases { - localDetector, err := NewDetectLocalByCIDR(c.cidr, c.ipt) + localDetector, err := NewDetectLocalByCIDR(c.cidr) if err != nil { t.Errorf("Error initializing localDetector: %v", err) continue