From dc1155bd53320ebbe4badece021a900aab282b28 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 4 Nov 2023 19:46:47 -0400 Subject: [PATCH 1/3] Move LocalTrafficDetector from pkg/proxy/util/iptables to pkg/proxy/util Since it's used for nftables as well now. --- cmd/kube-proxy/app/server_linux.go | 21 ++-- cmd/kube-proxy/app/server_linux_test.go | 104 +++++++++--------- pkg/proxy/iptables/proxier.go | 7 +- pkg/proxy/iptables/proxier_test.go | 5 +- pkg/proxy/ipvs/proxier.go | 7 +- pkg/proxy/ipvs/proxier_test.go | 3 +- pkg/proxy/nftables/proxier.go | 7 +- pkg/proxy/nftables/proxier_test.go | 5 +- .../{iptables/traffic.go => localdetector.go} | 2 +- .../traffic_test.go => localdetector_test.go} | 2 +- 10 files changed, 78 insertions(+), 85 deletions(-) rename pkg/proxy/util/{iptables/traffic.go => localdetector.go} (99%) rename pkg/proxy/util/{iptables/traffic_test.go => localdetector_test.go} (99%) diff --git a/cmd/kube-proxy/app/server_linux.go b/cmd/kube-proxy/app/server_linux.go index adeb91c75a6..7e93bbb29a9 100644 --- a/cmd/kube-proxy/app/server_linux.go +++ b/cmd/kube-proxy/app/server_linux.go @@ -53,7 +53,6 @@ import ( proxymetrics "k8s.io/kubernetes/pkg/proxy/metrics" "k8s.io/kubernetes/pkg/proxy/nftables" 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" ) @@ -165,8 +164,8 @@ func (s *ProxyServer) platformCheckSupported(ctx context.Context) (ipv4Supported func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi.KubeProxyConfiguration, dualStack, initOnly bool) (proxy.Provider, error) { logger := klog.FromContext(ctx) var proxier proxy.Provider - var localDetectors [2]proxyutiliptables.LocalTrafficDetector - var localDetector proxyutiliptables.LocalTrafficDetector + var localDetectors [2]proxyutil.LocalTrafficDetector + var localDetector proxyutil.LocalTrafficDetector var err error if config.Mode == proxyconfigapi.ProxyModeIPTables { @@ -505,7 +504,7 @@ func detectNumCPU() int { return numCPU } -func getLocalDetector(logger klog.Logger, ipFamily v1.IPFamily, mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, nodePodCIDRs []string) (proxyutiliptables.LocalTrafficDetector, error) { +func getLocalDetector(logger klog.Logger, ipFamily v1.IPFamily, mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, nodePodCIDRs []string) (proxyutil.LocalTrafficDetector, error) { switch mode { case proxyconfigapi.LocalModeClusterCIDR: // LocalModeClusterCIDR is the default if --detect-local-mode wasn't passed, @@ -518,7 +517,7 @@ func getLocalDetector(logger klog.Logger, ipFamily v1.IPFamily, mode proxyconfig cidrsByFamily := proxyutil.MapCIDRsByIPFamily(strings.Split(clusterCIDRs, ",")) if len(cidrsByFamily[ipFamily]) != 0 { - return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0].String()) + return proxyutil.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0].String()) } logger.Info("Detect-local-mode set to ClusterCIDR, but no cluster CIDR for family", "ipFamily", ipFamily) @@ -526,24 +525,24 @@ func getLocalDetector(logger klog.Logger, ipFamily v1.IPFamily, mode proxyconfig case proxyconfigapi.LocalModeNodeCIDR: cidrsByFamily := proxyutil.MapCIDRsByIPFamily(nodePodCIDRs) if len(cidrsByFamily[ipFamily]) != 0 { - return proxyutiliptables.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0].String()) + return proxyutil.NewDetectLocalByCIDR(cidrsByFamily[ipFamily][0].String()) } logger.Info("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) + return proxyutil.NewDetectLocalByBridgeInterface(config.DetectLocal.BridgeInterface) case proxyconfigapi.LocalModeInterfaceNamePrefix: - return proxyutiliptables.NewDetectLocalByInterfaceNamePrefix(config.DetectLocal.InterfaceNamePrefix) + return proxyutil.NewDetectLocalByInterfaceNamePrefix(config.DetectLocal.InterfaceNamePrefix) } logger.Info("Defaulting to no-op detect-local") - return proxyutiliptables.NewNoOpLocalDetector(), nil + return proxyutil.NewNoOpLocalDetector(), nil } -func getDualStackLocalDetectorTuple(logger klog.Logger, mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, nodePodCIDRs []string) ([2]proxyutiliptables.LocalTrafficDetector, error) { - var localDetectors [2]proxyutiliptables.LocalTrafficDetector +func getDualStackLocalDetectorTuple(logger klog.Logger, mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, nodePodCIDRs []string) ([2]proxyutil.LocalTrafficDetector, error) { + var localDetectors [2]proxyutil.LocalTrafficDetector var err error localDetectors[0], err = getLocalDetector(logger, v1.IPv4Protocol, mode, config, nodePodCIDRs) diff --git a/cmd/kube-proxy/app/server_linux_test.go b/cmd/kube-proxy/app/server_linux_test.go index 0b9c0162aa9..11ebc4bc849 100644 --- a/cmd/kube-proxy/app/server_linux_test.go +++ b/cmd/kube-proxy/app/server_linux_test.go @@ -39,7 +39,7 @@ import ( clientsetfake "k8s.io/client-go/kubernetes/fake" clientgotesting "k8s.io/client-go/testing" proxyconfigapi "k8s.io/kubernetes/pkg/proxy/apis/config" - proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" + proxyutil "k8s.io/kubernetes/pkg/proxy/util" "k8s.io/kubernetes/test/utils/ktesting" netutils "k8s.io/utils/net" "k8s.io/utils/ptr" @@ -114,7 +114,7 @@ func Test_getLocalDetector(t *testing.T) { mode proxyconfigapi.LocalMode config *proxyconfigapi.KubeProxyConfiguration family v1.IPFamily - expected proxyutiliptables.LocalTrafficDetector + expected proxyutil.LocalTrafficDetector nodePodCIDRs []string errExpected bool }{ @@ -124,7 +124,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14")), + expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("10.0.0.0/14")), errExpected: false, }, { @@ -132,7 +132,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, family: v1.IPv6Protocol, - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002:0:0:1234::/64")), + expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64")), errExpected: false, }, { @@ -140,7 +140,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, family: v1.IPv6Protocol, - expected: proxyutiliptables.NewNoOpLocalDetector(), + expected: proxyutil.NewNoOpLocalDetector(), errExpected: false, }, { @@ -148,7 +148,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, family: v1.IPv4Protocol, - expected: proxyutiliptables.NewNoOpLocalDetector(), + expected: proxyutil.NewNoOpLocalDetector(), errExpected: false, }, { @@ -156,7 +156,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64,10.0.0.0/14"}, family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14")), + expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("10.0.0.0/14")), errExpected: false, }, { @@ -164,7 +164,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, family: v1.IPv4Protocol, - expected: proxyutiliptables.NewNoOpLocalDetector(), + expected: proxyutil.NewNoOpLocalDetector(), errExpected: false, }, // LocalModeNodeCIDR @@ -173,7 +173,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24")), + expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("10.0.0.0/24")), nodePodCIDRs: []string{"10.0.0.0/24"}, errExpected: false, }, @@ -182,7 +182,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, family: v1.IPv6Protocol, - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), + expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), nodePodCIDRs: []string{"2002::1234:abcd:ffff:0:0/96"}, errExpected: false, }, @@ -191,7 +191,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, family: v1.IPv6Protocol, - expected: proxyutiliptables.NewNoOpLocalDetector(), + expected: proxyutil.NewNoOpLocalDetector(), nodePodCIDRs: []string{"10.0.0.0/24"}, errExpected: false, }, @@ -200,7 +200,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, family: v1.IPv4Protocol, - expected: proxyutiliptables.NewNoOpLocalDetector(), + expected: proxyutil.NewNoOpLocalDetector(), nodePodCIDRs: []string{"2002::1234:abcd:ffff:0:0/96"}, errExpected: false, }, @@ -209,7 +209,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14,2002:0:0:1234::/64"}, family: v1.IPv6Protocol, - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), + expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), nodePodCIDRs: []string{"10.0.0.0/24", "2002::1234:abcd:ffff:0:0/96"}, errExpected: false, }, @@ -218,7 +218,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, family: v1.IPv4Protocol, - expected: proxyutiliptables.NewNoOpLocalDetector(), + expected: proxyutil.NewNoOpLocalDetector(), nodePodCIDRs: []string{}, errExpected: false, }, @@ -228,7 +228,7 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalMode("abcd"), config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, family: v1.IPv4Protocol, - expected: proxyutiliptables.NewNoOpLocalDetector(), + expected: proxyutil.NewNoOpLocalDetector(), errExpected: false, }, // LocalModeBridgeInterface @@ -239,7 +239,7 @@ func Test_getLocalDetector(t *testing.T) { DetectLocal: proxyconfigapi.DetectLocalConfiguration{BridgeInterface: "eth"}, }, family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByBridgeInterface("eth")), + expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByBridgeInterface("eth")), errExpected: false, }, { @@ -249,7 +249,7 @@ func Test_getLocalDetector(t *testing.T) { DetectLocal: proxyconfigapi.DetectLocalConfiguration{BridgeInterface: "1234567890123456789"}, }, family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByBridgeInterface("1234567890123456789")), + expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByBridgeInterface("1234567890123456789")), errExpected: false, }, // LocalModeInterfaceNamePrefix @@ -260,7 +260,7 @@ func Test_getLocalDetector(t *testing.T) { DetectLocal: proxyconfigapi.DetectLocalConfiguration{InterfaceNamePrefix: "eth"}, }, family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByInterfaceNamePrefix("eth")), + expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByInterfaceNamePrefix("eth")), errExpected: false, }, { @@ -270,7 +270,7 @@ func Test_getLocalDetector(t *testing.T) { DetectLocal: proxyconfigapi.DetectLocalConfiguration{InterfaceNamePrefix: "1234567890123456789"}, }, family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByInterfaceNamePrefix("1234567890123456789")), + expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByInterfaceNamePrefix("1234567890123456789")), errExpected: false, }, } @@ -300,7 +300,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name string mode proxyconfigapi.LocalMode config *proxyconfigapi.KubeProxyConfiguration - expected [2]proxyutiliptables.LocalTrafficDetector + expected [2]proxyutil.LocalTrafficDetector nodePodCIDRs []string errExpected bool }{ @@ -310,8 +310,8 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14,2002:0:0:1234::/64"}, expected: resolveDualStackLocalDetectors(t)( - proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14"))( - proxyutiliptables.NewDetectLocalByCIDR("2002:0:0:1234::/64")), + proxyutil.NewDetectLocalByCIDR("10.0.0.0/14"))( + proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64")), errExpected: false, }, { @@ -319,33 +319,33 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64,10.0.0.0/14"}, expected: resolveDualStackLocalDetectors(t)( - proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14"))( - proxyutiliptables.NewDetectLocalByCIDR("2002:0:0:1234::/64")), + proxyutil.NewDetectLocalByCIDR("10.0.0.0/14"))( + proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64")), errExpected: false, }, { name: "LocalModeClusterCIDR, single-stack IPv4 cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, - expected: [2]proxyutiliptables.LocalTrafficDetector{ - resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/14")), - proxyutiliptables.NewNoOpLocalDetector()}, + expected: [2]proxyutil.LocalTrafficDetector{ + resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("10.0.0.0/14")), + proxyutil.NewNoOpLocalDetector()}, errExpected: false, }, { name: "LocalModeClusterCIDR, single-stack IPv6 cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, - expected: [2]proxyutiliptables.LocalTrafficDetector{ - proxyutiliptables.NewNoOpLocalDetector(), - resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002:0:0:1234::/64"))}, + expected: [2]proxyutil.LocalTrafficDetector{ + proxyutil.NewNoOpLocalDetector(), + resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64"))}, errExpected: false, }, { name: "LocalModeClusterCIDR, no ClusterCIDR", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, - expected: [2]proxyutiliptables.LocalTrafficDetector{proxyutiliptables.NewNoOpLocalDetector(), proxyutiliptables.NewNoOpLocalDetector()}, + expected: [2]proxyutil.LocalTrafficDetector{proxyutil.NewNoOpLocalDetector(), proxyutil.NewNoOpLocalDetector()}, errExpected: false, }, // LocalModeNodeCIDR @@ -354,8 +354,8 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14,2002:0:0:1234::/64"}, expected: resolveDualStackLocalDetectors(t)( - proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24"))( - proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), + proxyutil.NewDetectLocalByCIDR("10.0.0.0/24"))( + proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), nodePodCIDRs: []string{"10.0.0.0/24", "2002::1234:abcd:ffff:0:0/96"}, errExpected: false, }, @@ -364,8 +364,8 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64,10.0.0.0/14"}, expected: resolveDualStackLocalDetectors(t)( - proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24"))( - proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), + proxyutil.NewDetectLocalByCIDR("10.0.0.0/24"))( + proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), nodePodCIDRs: []string{"2002::1234:abcd:ffff:0:0/96", "10.0.0.0/24"}, errExpected: false, }, @@ -373,9 +373,9 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name: "LocalModeNodeCIDR, single-stack IPv4 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, - expected: [2]proxyutiliptables.LocalTrafficDetector{ - resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("10.0.0.0/24")), - proxyutiliptables.NewNoOpLocalDetector()}, + expected: [2]proxyutil.LocalTrafficDetector{ + resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("10.0.0.0/24")), + proxyutil.NewNoOpLocalDetector()}, nodePodCIDRs: []string{"10.0.0.0/24"}, errExpected: false, }, @@ -383,9 +383,9 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name: "LocalModeNodeCIDR, single-stack IPv6 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, - expected: [2]proxyutiliptables.LocalTrafficDetector{ - proxyutiliptables.NewNoOpLocalDetector(), - resolveLocalDetector(t)(proxyutiliptables.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96"))}, + expected: [2]proxyutil.LocalTrafficDetector{ + proxyutil.NewNoOpLocalDetector(), + resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96"))}, nodePodCIDRs: []string{"2002::1234:abcd:ffff:0:0/96"}, errExpected: false, }, @@ -393,7 +393,7 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { name: "LocalModeNodeCIDR, no PodCIDRs", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, - expected: [2]proxyutiliptables.LocalTrafficDetector{proxyutiliptables.NewNoOpLocalDetector(), proxyutiliptables.NewNoOpLocalDetector()}, + expected: [2]proxyutil.LocalTrafficDetector{proxyutil.NewNoOpLocalDetector(), proxyutil.NewNoOpLocalDetector()}, nodePodCIDRs: []string{}, errExpected: false, }, @@ -405,8 +405,8 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { DetectLocal: proxyconfigapi.DetectLocalConfiguration{BridgeInterface: "eth"}, }, expected: resolveDualStackLocalDetectors(t)( - proxyutiliptables.NewDetectLocalByBridgeInterface("eth"))( - proxyutiliptables.NewDetectLocalByBridgeInterface("eth")), + proxyutil.NewDetectLocalByBridgeInterface("eth"))( + proxyutil.NewDetectLocalByBridgeInterface("eth")), errExpected: false, }, // LocalModeInterfaceNamePrefix @@ -417,8 +417,8 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { DetectLocal: proxyconfigapi.DetectLocalConfiguration{InterfaceNamePrefix: "veth"}, }, expected: resolveDualStackLocalDetectors(t)( - proxyutiliptables.NewDetectLocalByInterfaceNamePrefix("veth"))( - proxyutiliptables.NewDetectLocalByInterfaceNamePrefix("veth")), + proxyutil.NewDetectLocalByInterfaceNamePrefix("veth"))( + proxyutil.NewDetectLocalByInterfaceNamePrefix("veth")), errExpected: false, }, } @@ -455,8 +455,8 @@ func makeNodeWithPodCIDRs(cidrs ...string) *v1.Node { } } -func resolveLocalDetector(t *testing.T) func(proxyutiliptables.LocalTrafficDetector, error) proxyutiliptables.LocalTrafficDetector { - return func(localDetector proxyutiliptables.LocalTrafficDetector, err error) proxyutiliptables.LocalTrafficDetector { +func resolveLocalDetector(t *testing.T) func(proxyutil.LocalTrafficDetector, error) proxyutil.LocalTrafficDetector { + return func(localDetector proxyutil.LocalTrafficDetector, err error) proxyutil.LocalTrafficDetector { t.Helper() if err != nil { t.Fatalf("Error resolving detect-local: %v", err) @@ -465,18 +465,18 @@ func resolveLocalDetector(t *testing.T) func(proxyutiliptables.LocalTrafficDetec } } -func resolveDualStackLocalDetectors(t *testing.T) func(localDetector proxyutiliptables.LocalTrafficDetector, err1 error) func(proxyutiliptables.LocalTrafficDetector, error) [2]proxyutiliptables.LocalTrafficDetector { - return func(localDetector proxyutiliptables.LocalTrafficDetector, err error) func(proxyutiliptables.LocalTrafficDetector, error) [2]proxyutiliptables.LocalTrafficDetector { +func resolveDualStackLocalDetectors(t *testing.T) func(localDetector proxyutil.LocalTrafficDetector, err1 error) func(proxyutil.LocalTrafficDetector, error) [2]proxyutil.LocalTrafficDetector { + return func(localDetector proxyutil.LocalTrafficDetector, err error) func(proxyutil.LocalTrafficDetector, error) [2]proxyutil.LocalTrafficDetector { t.Helper() if err != nil { t.Fatalf("Error resolving dual stack detect-local: %v", err) } - return func(otherLocalDetector proxyutiliptables.LocalTrafficDetector, err1 error) [2]proxyutiliptables.LocalTrafficDetector { + return func(otherLocalDetector proxyutil.LocalTrafficDetector, err1 error) [2]proxyutil.LocalTrafficDetector { t.Helper() if err1 != nil { t.Fatalf("Error resolving dual stack detect-local: %v", err) } - return [2]proxyutiliptables.LocalTrafficDetector{localDetector, otherLocalDetector} + return [2]proxyutil.LocalTrafficDetector{localDetector, otherLocalDetector} } } } diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 4d1c52e8e87..ac07749463a 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -51,7 +51,6 @@ import ( "k8s.io/kubernetes/pkg/proxy/metaproxier" "k8s.io/kubernetes/pkg/proxy/metrics" proxyutil "k8s.io/kubernetes/pkg/proxy/util" - proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" "k8s.io/kubernetes/pkg/util/async" utiliptables "k8s.io/kubernetes/pkg/util/iptables" utilexec "k8s.io/utils/exec" @@ -107,7 +106,7 @@ func NewDualStackProxier( masqueradeAll bool, localhostNodePorts bool, masqueradeBit int, - localDetectors [2]proxyutiliptables.LocalTrafficDetector, + localDetectors [2]proxyutil.LocalTrafficDetector, hostname string, nodeIPs map[v1.IPFamily]net.IP, recorder events.EventRecorder, @@ -168,7 +167,7 @@ type Proxier struct { masqueradeAll bool masqueradeMark string conntrack conntrack.Interface - localDetector proxyutiliptables.LocalTrafficDetector + localDetector proxyutil.LocalTrafficDetector hostname string nodeIP net.IP recorder events.EventRecorder @@ -229,7 +228,7 @@ func NewProxier(ctx context.Context, masqueradeAll bool, localhostNodePorts bool, masqueradeBit int, - localDetector proxyutiliptables.LocalTrafficDetector, + localDetector proxyutil.LocalTrafficDetector, hostname string, nodeIP net.IP, recorder events.EventRecorder, diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index d3381c1ca35..5ae5788d8af 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -54,7 +54,6 @@ import ( "k8s.io/kubernetes/pkg/proxy/healthcheck" proxyutil "k8s.io/kubernetes/pkg/proxy/util" - proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" proxyutiltest "k8s.io/kubernetes/pkg/proxy/util/testing" "k8s.io/kubernetes/pkg/util/async" utiliptables "k8s.io/kubernetes/pkg/util/iptables" @@ -94,7 +93,7 @@ func NewFakeProxier(ipt utiliptables.Interface) *Proxier { ipfamily = v1.IPv6Protocol podCIDR = "fd00:10::/64" } - detectLocal, _ := proxyutiliptables.NewDetectLocalByCIDR(podCIDR) + detectLocal, _ := proxyutil.NewDetectLocalByCIDR(podCIDR) networkInterfacer := proxyutiltest.NewFakeNetwork() itf := net.Interface{Index: 0, MTU: 0, Name: "lo", HardwareAddr: nil, Flags: 0} @@ -5588,7 +5587,7 @@ func TestInternalExternalMasquerade(t *testing.T) { fp := NewFakeProxier(ipt) fp.masqueradeAll = tc.masqueradeAll if !tc.localDetector { - fp.localDetector = proxyutiliptables.NewNoOpLocalDetector() + fp.localDetector = proxyutil.NewNoOpLocalDetector() } setupTest(fp) diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index c11fbb6ab8d..193e1f832ca 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -53,7 +53,6 @@ import ( "k8s.io/kubernetes/pkg/proxy/metaproxier" "k8s.io/kubernetes/pkg/proxy/metrics" proxyutil "k8s.io/kubernetes/pkg/proxy/util" - proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" "k8s.io/kubernetes/pkg/util/async" utiliptables "k8s.io/kubernetes/pkg/util/iptables" utilkernel "k8s.io/kubernetes/pkg/util/kernel" @@ -127,7 +126,7 @@ func NewDualStackProxier( udpTimeout time.Duration, masqueradeAll bool, masqueradeBit int, - localDetectors [2]proxyutiliptables.LocalTrafficDetector, + localDetectors [2]proxyutil.LocalTrafficDetector, hostname string, nodeIPs map[v1.IPFamily]net.IP, recorder events.EventRecorder, @@ -207,7 +206,7 @@ type Proxier struct { conntrack conntrack.Interface masqueradeAll bool masqueradeMark string - localDetector proxyutiliptables.LocalTrafficDetector + localDetector proxyutil.LocalTrafficDetector hostname string nodeIP net.IP recorder events.EventRecorder @@ -282,7 +281,7 @@ func NewProxier( udpTimeout time.Duration, masqueradeAll bool, masqueradeBit int, - localDetector proxyutiliptables.LocalTrafficDetector, + localDetector proxyutil.LocalTrafficDetector, hostname string, nodeIP net.IP, recorder events.EventRecorder, diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 1ede4509fd6..fe9bb6a0e86 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -51,7 +51,6 @@ import ( ipvstest "k8s.io/kubernetes/pkg/proxy/ipvs/util/testing" "k8s.io/kubernetes/pkg/proxy/metrics" proxyutil "k8s.io/kubernetes/pkg/proxy/util" - proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" proxyutiltest "k8s.io/kubernetes/pkg/proxy/util/testing" "k8s.io/kubernetes/pkg/util/async" utiliptables "k8s.io/kubernetes/pkg/util/iptables" @@ -148,7 +147,7 @@ func NewFakeProxier(ctx context.Context, ipt utiliptables.Interface, ipvs utilip ipset: ipset, conntrack: conntrack.NewFake(), strictARP: false, - localDetector: proxyutiliptables.NewNoOpLocalDetector(), + localDetector: proxyutil.NewNoOpLocalDetector(), hostname: testHostname, serviceHealthServer: healthcheck.NewFakeServiceHealthServer(), ipvsScheduler: defaultScheduler, diff --git a/pkg/proxy/nftables/proxier.go b/pkg/proxy/nftables/proxier.go index 31b7c77e1be..f3e1f68256f 100644 --- a/pkg/proxy/nftables/proxier.go +++ b/pkg/proxy/nftables/proxier.go @@ -50,7 +50,6 @@ import ( "k8s.io/kubernetes/pkg/proxy/metaproxier" "k8s.io/kubernetes/pkg/proxy/metrics" proxyutil "k8s.io/kubernetes/pkg/proxy/util" - proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" "k8s.io/kubernetes/pkg/util/async" utilexec "k8s.io/utils/exec" netutils "k8s.io/utils/net" @@ -111,7 +110,7 @@ func NewDualStackProxier( minSyncPeriod time.Duration, masqueradeAll bool, masqueradeBit int, - localDetectors [2]proxyutiliptables.LocalTrafficDetector, + localDetectors [2]proxyutil.LocalTrafficDetector, hostname string, nodeIPs map[v1.IPFamily]net.IP, recorder events.EventRecorder, @@ -170,7 +169,7 @@ type Proxier struct { masqueradeAll bool masqueradeMark string conntrack conntrack.Interface - localDetector proxyutiliptables.LocalTrafficDetector + localDetector proxyutil.LocalTrafficDetector hostname string nodeIP net.IP recorder events.EventRecorder @@ -207,7 +206,7 @@ func NewProxier(ctx context.Context, minSyncPeriod time.Duration, masqueradeAll bool, masqueradeBit int, - localDetector proxyutiliptables.LocalTrafficDetector, + localDetector proxyutil.LocalTrafficDetector, hostname string, nodeIP net.IP, recorder events.EventRecorder, diff --git a/pkg/proxy/nftables/proxier_test.go b/pkg/proxy/nftables/proxier_test.go index 91422373cc0..833aa5798c1 100644 --- a/pkg/proxy/nftables/proxier_test.go +++ b/pkg/proxy/nftables/proxier_test.go @@ -44,7 +44,6 @@ import ( "k8s.io/kubernetes/pkg/proxy/healthcheck" "k8s.io/kubernetes/pkg/proxy/metrics" proxyutil "k8s.io/kubernetes/pkg/proxy/util" - proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" proxyutiltest "k8s.io/kubernetes/pkg/proxy/util/testing" "k8s.io/kubernetes/pkg/util/async" netutils "k8s.io/utils/net" @@ -85,7 +84,7 @@ func NewFakeProxier(ipFamily v1.IPFamily) (*knftables.Fake, *Proxier) { podCIDR = "fd00:10::/64" serviceCIDRs = "fd00:10:96::/112" } - detectLocal, _ := proxyutiliptables.NewDetectLocalByCIDR(podCIDR) + detectLocal, _ := proxyutil.NewDetectLocalByCIDR(podCIDR) nodePortAddresses := []string{fmt.Sprintf("%s/32", testNodeIP), fmt.Sprintf("%s/128", testNodeIPv6)} networkInterfacer := proxyutiltest.NewFakeNetwork() @@ -3905,7 +3904,7 @@ func TestInternalExternalMasquerade(t *testing.T) { nft, fp := NewFakeProxier(v1.IPv4Protocol) fp.masqueradeAll = tc.masqueradeAll if !tc.localDetector { - fp.localDetector = proxyutiliptables.NewNoOpLocalDetector() + fp.localDetector = proxyutil.NewNoOpLocalDetector() } setupTest(fp) diff --git a/pkg/proxy/util/iptables/traffic.go b/pkg/proxy/util/localdetector.go similarity index 99% rename from pkg/proxy/util/iptables/traffic.go rename to pkg/proxy/util/localdetector.go index 6c4f54b0c01..a4c5bc62987 100644 --- a/pkg/proxy/util/iptables/traffic.go +++ b/pkg/proxy/util/localdetector.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package iptables +package util import ( "fmt" diff --git a/pkg/proxy/util/iptables/traffic_test.go b/pkg/proxy/util/localdetector_test.go similarity index 99% rename from pkg/proxy/util/iptables/traffic_test.go rename to pkg/proxy/util/localdetector_test.go index 1741261a352..f21e2014f7e 100644 --- a/pkg/proxy/util/iptables/traffic_test.go +++ b/pkg/proxy/util/localdetector_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package iptables +package util import ( "reflect" From 59cecf8a366c3e0bf19f243d535314db831a9e8e Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 27 Jan 2024 11:49:11 -0500 Subject: [PATCH 2/3] Simplify redundant LocalTrafficDetector implementations All of the LocalTrafficDetector implementations were essentially identical after construction time, so just reduce them to a single implementation with multiple constructors. Also, improve the comments. --- pkg/proxy/util/localdetector.go | 166 +++++++++----------------------- 1 file changed, 47 insertions(+), 119 deletions(-) diff --git a/pkg/proxy/util/localdetector.go b/pkg/proxy/util/localdetector.go index a4c5bc62987..6f2d77a9b0d 100644 --- a/pkg/proxy/util/localdetector.go +++ b/pkg/proxy/util/localdetector.go @@ -22,61 +22,62 @@ import ( netutils "k8s.io/utils/net" ) -// LocalTrafficDetector in a interface to take action (jump) based on whether traffic originated locally -// at the node or not +// LocalTrafficDetector generates iptables or nftables rules to detect traffic from local pods. type LocalTrafficDetector interface { - // IsImplemented returns true if the implementation does something, false otherwise + // IsImplemented returns true if the implementation does something, false + // otherwise. You should not call the other methods if IsImplemented() returns + // false. IsImplemented() bool - // IfLocal returns iptables arguments that will match traffic from a pod + // IfLocal returns iptables arguments that will match traffic from a local pod. IfLocal() []string - // IfNotLocal returns iptables arguments that will match traffic that is not from a pod + // IfNotLocal returns iptables arguments that will match traffic that is not from + // a local pod. IfNotLocal() []string - // IfLocalNFT returns nftables arguments that will match traffic from a pod + // IfLocalNFT returns nftables arguments that will match traffic from a local pod. IfLocalNFT() []string - // IfNotLocalNFT returns nftables arguments that will match traffic that is not from a pod + // IfNotLocalNFT returns nftables arguments that will match traffic that is not + // from a local pod. IfNotLocalNFT() []string } -type noOpLocalDetector struct{} - -// NewNoOpLocalDetector is a no-op implementation of LocalTrafficDetector -func NewNoOpLocalDetector() LocalTrafficDetector { - return &noOpLocalDetector{} -} - -func (n *noOpLocalDetector) IsImplemented() bool { - return false -} - -func (n *noOpLocalDetector) IfLocal() []string { - return nil // no-op; matches all traffic -} - -func (n *noOpLocalDetector) IfNotLocal() []string { - return nil // no-op; matches all traffic -} - -func (n *noOpLocalDetector) IfLocalNFT() []string { - return nil // no-op; matches all traffic -} - -func (n *noOpLocalDetector) IfNotLocalNFT() []string { - return nil // no-op; matches all traffic -} - -type detectLocalByCIDR struct { +type detectLocal struct { ifLocal []string ifNotLocal []string ifLocalNFT []string ifNotLocalNFT []string } -// 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 (d *detectLocal) IsImplemented() bool { + return len(d.ifLocal) > 0 +} + +func (d *detectLocal) IfLocal() []string { + return d.ifLocal +} + +func (d *detectLocal) IfNotLocal() []string { + return d.ifNotLocal +} + +func (d *detectLocal) IfLocalNFT() []string { + return d.ifLocalNFT +} + +func (d *detectLocal) IfNotLocalNFT() []string { + return d.ifNotLocalNFT +} + +// NewNoOpLocalDetector returns a no-op implementation of LocalTrafficDetector. +func NewNoOpLocalDetector() LocalTrafficDetector { + return &detectLocal{} +} + +// NewDetectLocalByCIDR returns a LocalTrafficDetector that considers traffic from the +// provided cidr to be from a local pod, and other traffic to be non-local. func NewDetectLocalByCIDR(cidr string) (LocalTrafficDetector, error) { _, parsed, err := netutils.ParseCIDRSloppy(cidr) if err != nil { @@ -88,7 +89,7 @@ func NewDetectLocalByCIDR(cidr string) (LocalTrafficDetector, error) { nftFamily = "ip6" } - return &detectLocalByCIDR{ + return &detectLocal{ ifLocal: []string{"-s", cidr}, ifNotLocal: []string{"!", "-s", cidr}, ifLocalNFT: []string{nftFamily, "saddr", cidr}, @@ -96,40 +97,14 @@ func NewDetectLocalByCIDR(cidr string) (LocalTrafficDetector, error) { }, nil } -func (d *detectLocalByCIDR) IsImplemented() bool { - return true -} - -func (d *detectLocalByCIDR) IfLocal() []string { - return d.ifLocal -} - -func (d *detectLocalByCIDR) IfNotLocal() []string { - return d.ifNotLocal -} - -func (d *detectLocalByCIDR) IfLocalNFT() []string { - return d.ifLocalNFT -} - -func (d *detectLocalByCIDR) IfNotLocalNFT() []string { - return d.ifNotLocalNFT -} - -type detectLocalByBridgeInterface struct { - ifLocal []string - ifNotLocal []string - ifLocalNFT []string - ifNotLocalNFT []string -} - -// NewDetectLocalByBridgeInterface implements the LocalTrafficDetector interface using a bridge interface name. -// This can be used when a bridge can be used to capture the notion of local traffic from pods. +// NewDetectLocalByBridgeInterface returns a LocalTrafficDetector that considers traffic +// from interfaceName to be from a local pod, and traffic from other interfaces to be +// non-local. func NewDetectLocalByBridgeInterface(interfaceName string) (LocalTrafficDetector, error) { if len(interfaceName) == 0 { return nil, fmt.Errorf("no bridge interface name set") } - return &detectLocalByBridgeInterface{ + return &detectLocal{ ifLocal: []string{"-i", interfaceName}, ifNotLocal: []string{"!", "-i", interfaceName}, ifLocalNFT: []string{"iif", interfaceName}, @@ -137,64 +112,17 @@ func NewDetectLocalByBridgeInterface(interfaceName string) (LocalTrafficDetector }, nil } -func (d *detectLocalByBridgeInterface) IsImplemented() bool { - return true -} - -func (d *detectLocalByBridgeInterface) IfLocal() []string { - return d.ifLocal -} - -func (d *detectLocalByBridgeInterface) IfNotLocal() []string { - return d.ifNotLocal -} - -func (d *detectLocalByBridgeInterface) IfLocalNFT() []string { - return d.ifLocalNFT -} - -func (d *detectLocalByBridgeInterface) IfNotLocalNFT() []string { - return d.ifNotLocalNFT -} - -type detectLocalByInterfaceNamePrefix struct { - ifLocal []string - ifNotLocal []string - ifLocalNFT []string - ifNotLocalNFT []string -} - -// NewDetectLocalByInterfaceNamePrefix implements the LocalTrafficDetector interface using an interface name prefix. -// This can be used when a pod interface name prefix can be used to capture the notion of local traffic. Note -// that this will match on all interfaces that start with the given prefix. +// NewDetectLocalByInterfaceNamePrefix returns a LocalTrafficDetector that considers +// traffic from interfaces starting with interfacePrefix to be from a local pod, and +// traffic from other interfaces to be non-local. func NewDetectLocalByInterfaceNamePrefix(interfacePrefix string) (LocalTrafficDetector, error) { if len(interfacePrefix) == 0 { return nil, fmt.Errorf("no interface prefix set") } - return &detectLocalByInterfaceNamePrefix{ + return &detectLocal{ ifLocal: []string{"-i", interfacePrefix + "+"}, ifNotLocal: []string{"!", "-i", interfacePrefix + "+"}, ifLocalNFT: []string{"iif", interfacePrefix + "*"}, ifNotLocalNFT: []string{"iif", "!=", interfacePrefix + "*"}, }, nil } - -func (d *detectLocalByInterfaceNamePrefix) IsImplemented() bool { - return true -} - -func (d *detectLocalByInterfaceNamePrefix) IfLocal() []string { - return d.ifLocal -} - -func (d *detectLocalByInterfaceNamePrefix) IfNotLocal() []string { - return d.ifNotLocal -} - -func (d *detectLocalByInterfaceNamePrefix) IfLocalNFT() []string { - return d.ifLocalNFT -} - -func (d *detectLocalByInterfaceNamePrefix) IfNotLocalNFT() []string { - return d.ifNotLocalNFT -} From 3db434d6be345dea55afd5fc3ac841fcec003020 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sun, 28 Jan 2024 09:38:55 -0500 Subject: [PATCH 3/3] Remove errors from LocalTrafficDetector constructors The constructors only return an error if you pass them invalid data, but we only ever pass them data which has already been validated, making the error checking just annoying. Just make them return garbage output if you give them garbage input. --- cmd/kube-proxy/app/server_linux.go | 50 ++--- cmd/kube-proxy/app/server_linux_test.go | 253 +++++++++--------------- pkg/proxy/iptables/proxier_test.go | 2 +- pkg/proxy/nftables/proxier_test.go | 2 +- pkg/proxy/util/localdetector.go | 30 +-- pkg/proxy/util/localdetector_test.go | 118 +---------- 6 files changed, 122 insertions(+), 333 deletions(-) diff --git a/cmd/kube-proxy/app/server_linux.go b/cmd/kube-proxy/app/server_linux.go index 7e93bbb29a9..0413983390c 100644 --- a/cmd/kube-proxy/app/server_linux.go +++ b/cmd/kube-proxy/app/server_linux.go @@ -174,10 +174,7 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. if dualStack { ipt, _ := getIPTables(s.PrimaryIPFamily) - localDetectors, err = getDualStackLocalDetectorTuple(logger, config.DetectLocalMode, config, s.podCIDRs) - if err != nil { - return nil, fmt.Errorf("unable to create proxier: %v", err) - } + localDetectors = getDualStackLocalDetectorTuple(logger, config.DetectLocalMode, config, s.podCIDRs) // TODO this has side effects that should only happen when Run() is invoked. proxier, err = iptables.NewDualStackProxier( @@ -201,10 +198,7 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. } else { // Create a single-stack proxier if and only if the node does not support dual-stack (i.e, no iptables support). _, iptInterface := getIPTables(s.PrimaryIPFamily) - localDetector, err = getLocalDetector(logger, s.PrimaryIPFamily, config.DetectLocalMode, config, s.podCIDRs) - if err != nil { - return nil, fmt.Errorf("unable to create proxier: %v", err) - } + localDetector = getLocalDetector(logger, s.PrimaryIPFamily, config.DetectLocalMode, config, s.podCIDRs) // TODO this has side effects that should only happen when Run() is invoked. proxier, err = iptables.NewProxier( @@ -244,10 +238,7 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. ipt, _ := getIPTables(s.PrimaryIPFamily) // Always ordered to match []ipt - localDetectors, err = getDualStackLocalDetectorTuple(logger, config.DetectLocalMode, config, s.podCIDRs) - if err != nil { - return nil, fmt.Errorf("unable to create proxier: %v", err) - } + localDetectors = getDualStackLocalDetectorTuple(logger, config.DetectLocalMode, config, s.podCIDRs) proxier, err = ipvs.NewDualStackProxier( ctx, @@ -276,10 +267,7 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. ) } else { _, iptInterface := getIPTables(s.PrimaryIPFamily) - localDetector, err = getLocalDetector(logger, s.PrimaryIPFamily, config.DetectLocalMode, config, s.podCIDRs) - if err != nil { - return nil, fmt.Errorf("unable to create proxier: %v", err) - } + localDetector = getLocalDetector(logger, s.PrimaryIPFamily, config.DetectLocalMode, config, s.podCIDRs) proxier, err = ipvs.NewProxier( ctx, @@ -315,10 +303,7 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. logger.Info("Using nftables Proxier") if dualStack { - localDetectors, err = getDualStackLocalDetectorTuple(logger, config.DetectLocalMode, config, s.podCIDRs) - if err != nil { - return nil, fmt.Errorf("unable to create proxier: %v", err) - } + localDetectors = getDualStackLocalDetectorTuple(logger, config.DetectLocalMode, config, s.podCIDRs) // TODO this has side effects that should only happen when Run() is invoked. proxier, err = nftables.NewDualStackProxier( @@ -338,10 +323,7 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. ) } else { // Create a single-stack proxier if and only if the node does not support dual-stack - localDetector, err = getLocalDetector(logger, s.PrimaryIPFamily, config.DetectLocalMode, config, s.podCIDRs) - if err != nil { - return nil, fmt.Errorf("unable to create proxier: %v", err) - } + localDetector = getLocalDetector(logger, s.PrimaryIPFamily, config.DetectLocalMode, config, s.podCIDRs) // TODO this has side effects that should only happen when Run() is invoked. proxier, err = nftables.NewProxier( @@ -504,7 +486,7 @@ func detectNumCPU() int { return numCPU } -func getLocalDetector(logger klog.Logger, ipFamily v1.IPFamily, mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, nodePodCIDRs []string) (proxyutil.LocalTrafficDetector, error) { +func getLocalDetector(logger klog.Logger, ipFamily v1.IPFamily, mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, nodePodCIDRs []string) proxyutil.LocalTrafficDetector { switch mode { case proxyconfigapi.LocalModeClusterCIDR: // LocalModeClusterCIDR is the default if --detect-local-mode wasn't passed, @@ -538,22 +520,14 @@ func getLocalDetector(logger klog.Logger, ipFamily v1.IPFamily, mode proxyconfig } logger.Info("Defaulting to no-op detect-local") - return proxyutil.NewNoOpLocalDetector(), nil + return proxyutil.NewNoOpLocalDetector() } -func getDualStackLocalDetectorTuple(logger klog.Logger, mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, nodePodCIDRs []string) ([2]proxyutil.LocalTrafficDetector, error) { - var localDetectors [2]proxyutil.LocalTrafficDetector - var err error - - localDetectors[0], err = getLocalDetector(logger, v1.IPv4Protocol, mode, config, nodePodCIDRs) - if err != nil { - return localDetectors, err +func getDualStackLocalDetectorTuple(logger klog.Logger, mode proxyconfigapi.LocalMode, config *proxyconfigapi.KubeProxyConfiguration, nodePodCIDRs []string) [2]proxyutil.LocalTrafficDetector { + return [2]proxyutil.LocalTrafficDetector{ + getLocalDetector(logger, v1.IPv4Protocol, mode, config, nodePodCIDRs), + getLocalDetector(logger, v1.IPv6Protocol, mode, config, nodePodCIDRs), } - localDetectors[1], err = getLocalDetector(logger, v1.IPv6Protocol, mode, config, nodePodCIDRs) - if err != nil { - return localDetectors, err - } - return localDetectors, nil } // platformCleanup removes stale kube-proxy rules that can be safely removed. If diff --git a/cmd/kube-proxy/app/server_linux_test.go b/cmd/kube-proxy/app/server_linux_test.go index 11ebc4bc849..776c5d0fa14 100644 --- a/cmd/kube-proxy/app/server_linux_test.go +++ b/cmd/kube-proxy/app/server_linux_test.go @@ -116,56 +116,49 @@ func Test_getLocalDetector(t *testing.T) { family v1.IPFamily expected proxyutil.LocalTrafficDetector nodePodCIDRs []string - errExpected bool }{ // LocalModeClusterCIDR { - name: "LocalModeClusterCIDR, IPv4 cluster", - mode: proxyconfigapi.LocalModeClusterCIDR, - config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, - family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("10.0.0.0/14")), - errExpected: false, + name: "LocalModeClusterCIDR, IPv4 cluster", + mode: proxyconfigapi.LocalModeClusterCIDR, + config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, + family: v1.IPv4Protocol, + expected: proxyutil.NewDetectLocalByCIDR("10.0.0.0/14"), }, { - name: "LocalModeClusterCIDR, IPv6 cluster", - mode: proxyconfigapi.LocalModeClusterCIDR, - config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, - family: v1.IPv6Protocol, - expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64")), - errExpected: false, + name: "LocalModeClusterCIDR, IPv6 cluster", + mode: proxyconfigapi.LocalModeClusterCIDR, + config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, + family: v1.IPv6Protocol, + expected: proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64"), }, { - name: "LocalModeClusterCIDR, IPv6 cluster with IPv6 config", - mode: proxyconfigapi.LocalModeClusterCIDR, - config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, - family: v1.IPv6Protocol, - expected: proxyutil.NewNoOpLocalDetector(), - errExpected: false, + name: "LocalModeClusterCIDR, IPv6 cluster with IPv4 config", + mode: proxyconfigapi.LocalModeClusterCIDR, + config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, + family: v1.IPv6Protocol, + expected: proxyutil.NewNoOpLocalDetector(), }, { - name: "LocalModeClusterCIDR, IPv4 cluster with IPv6 config", - mode: proxyconfigapi.LocalModeClusterCIDR, - config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, - family: v1.IPv4Protocol, - expected: proxyutil.NewNoOpLocalDetector(), - errExpected: false, + name: "LocalModeClusterCIDR, IPv4 cluster with IPv6 config", + mode: proxyconfigapi.LocalModeClusterCIDR, + config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, + family: v1.IPv4Protocol, + expected: proxyutil.NewNoOpLocalDetector(), }, { - name: "LocalModeClusterCIDR, IPv4 kube-proxy in dual-stack IPv6-primary cluster", - mode: proxyconfigapi.LocalModeClusterCIDR, - config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64,10.0.0.0/14"}, - family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("10.0.0.0/14")), - errExpected: false, + name: "LocalModeClusterCIDR, IPv4 kube-proxy in dual-stack IPv6-primary cluster", + mode: proxyconfigapi.LocalModeClusterCIDR, + config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64,10.0.0.0/14"}, + family: v1.IPv4Protocol, + expected: proxyutil.NewDetectLocalByCIDR("10.0.0.0/14"), }, { - name: "LocalModeClusterCIDR, no ClusterCIDR", - mode: proxyconfigapi.LocalModeClusterCIDR, - config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, - family: v1.IPv4Protocol, - expected: proxyutil.NewNoOpLocalDetector(), - errExpected: false, + name: "LocalModeClusterCIDR, no ClusterCIDR", + mode: proxyconfigapi.LocalModeClusterCIDR, + config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, + family: v1.IPv4Protocol, + expected: proxyutil.NewNoOpLocalDetector(), }, // LocalModeNodeCIDR { @@ -173,18 +166,16 @@ func Test_getLocalDetector(t *testing.T) { mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("10.0.0.0/24")), + expected: proxyutil.NewDetectLocalByCIDR("10.0.0.0/24"), nodePodCIDRs: []string{"10.0.0.0/24"}, - errExpected: false, }, { name: "LocalModeNodeCIDR, IPv6 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, family: v1.IPv6Protocol, - expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), + expected: proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96"), nodePodCIDRs: []string{"2002::1234:abcd:ffff:0:0/96"}, - errExpected: false, }, { name: "LocalModeNodeCIDR, IPv6 cluster with IPv4 config", @@ -193,7 +184,6 @@ func Test_getLocalDetector(t *testing.T) { family: v1.IPv6Protocol, expected: proxyutil.NewNoOpLocalDetector(), nodePodCIDRs: []string{"10.0.0.0/24"}, - errExpected: false, }, { name: "LocalModeNodeCIDR, IPv4 cluster with IPv6 config", @@ -202,16 +192,14 @@ func Test_getLocalDetector(t *testing.T) { family: v1.IPv4Protocol, expected: proxyutil.NewNoOpLocalDetector(), nodePodCIDRs: []string{"2002::1234:abcd:ffff:0:0/96"}, - 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:0:0:1234::/64"}, family: v1.IPv6Protocol, - expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), + expected: proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96"), nodePodCIDRs: []string{"10.0.0.0/24", "2002::1234:abcd:ffff:0:0/96"}, - errExpected: false, }, { name: "LocalModeNodeCIDR, no PodCIDRs", @@ -220,16 +208,14 @@ func Test_getLocalDetector(t *testing.T) { family: v1.IPv4Protocol, expected: proxyutil.NewNoOpLocalDetector(), nodePodCIDRs: []string{}, - errExpected: false, }, // unknown mode { - name: "unknown LocalMode", - mode: proxyconfigapi.LocalMode("abcd"), - config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, - family: v1.IPv4Protocol, - expected: proxyutil.NewNoOpLocalDetector(), - errExpected: false, + name: "unknown LocalMode", + mode: proxyconfigapi.LocalMode("abcd"), + config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, + family: v1.IPv4Protocol, + expected: proxyutil.NewNoOpLocalDetector(), }, // LocalModeBridgeInterface { @@ -238,9 +224,8 @@ func Test_getLocalDetector(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{BridgeInterface: "eth"}, }, - family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByBridgeInterface("eth")), - errExpected: false, + family: v1.IPv4Protocol, + expected: proxyutil.NewDetectLocalByBridgeInterface("eth"), }, { name: "LocalModeBridgeInterface, strange bridge name", @@ -248,9 +233,8 @@ func Test_getLocalDetector(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{BridgeInterface: "1234567890123456789"}, }, - family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByBridgeInterface("1234567890123456789")), - errExpected: false, + family: v1.IPv4Protocol, + expected: proxyutil.NewDetectLocalByBridgeInterface("1234567890123456789"), }, // LocalModeInterfaceNamePrefix { @@ -259,9 +243,8 @@ func Test_getLocalDetector(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{InterfaceNamePrefix: "eth"}, }, - family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByInterfaceNamePrefix("eth")), - errExpected: false, + family: v1.IPv4Protocol, + expected: proxyutil.NewDetectLocalByInterfaceNamePrefix("eth"), }, { name: "LocalModeInterfaceNamePrefix, strange interface name", @@ -269,25 +252,14 @@ func Test_getLocalDetector(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{InterfaceNamePrefix: "1234567890123456789"}, }, - family: v1.IPv4Protocol, - expected: resolveLocalDetector(t)(proxyutil.NewDetectLocalByInterfaceNamePrefix("1234567890123456789")), - errExpected: false, + family: v1.IPv4Protocol, + expected: proxyutil.NewDetectLocalByInterfaceNamePrefix("1234567890123456789"), }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { logger, _ := ktesting.NewTestContext(t) - r, err := getLocalDetector(logger, c.family, c.mode, c.config, c.nodePodCIDRs) - if c.errExpected { - if err == nil { - t.Errorf("Expected error, but succeeded with %v", r) - } - return - } - if err != nil { - t.Errorf("Error resolving detect-local: %v", err) - return - } + r := getLocalDetector(logger, c.family, c.mode, c.config, c.nodePodCIDRs) if !reflect.DeepEqual(r, c.expected) { t.Errorf("Unexpected detect-local implementation, expected: %q, got: %q", c.expected, r) } @@ -302,35 +274,34 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { config *proxyconfigapi.KubeProxyConfiguration expected [2]proxyutil.LocalTrafficDetector nodePodCIDRs []string - errExpected bool }{ // LocalModeClusterCIDR { name: "LocalModeClusterCIDR, dual-stack IPv4-primary cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14,2002:0:0:1234::/64"}, - expected: resolveDualStackLocalDetectors(t)( - proxyutil.NewDetectLocalByCIDR("10.0.0.0/14"))( - proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64")), - errExpected: false, + expected: [2]proxyutil.LocalTrafficDetector{ + proxyutil.NewDetectLocalByCIDR("10.0.0.0/14"), + proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64"), + }, }, { name: "LocalModeClusterCIDR, dual-stack IPv6-primary cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64,10.0.0.0/14"}, - expected: resolveDualStackLocalDetectors(t)( - proxyutil.NewDetectLocalByCIDR("10.0.0.0/14"))( - proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64")), - errExpected: false, + expected: [2]proxyutil.LocalTrafficDetector{ + proxyutil.NewDetectLocalByCIDR("10.0.0.0/14"), + proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64"), + }, }, { name: "LocalModeClusterCIDR, single-stack IPv4 cluster", mode: proxyconfigapi.LocalModeClusterCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, expected: [2]proxyutil.LocalTrafficDetector{ - resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("10.0.0.0/14")), - proxyutil.NewNoOpLocalDetector()}, - errExpected: false, + proxyutil.NewDetectLocalByCIDR("10.0.0.0/14"), + proxyutil.NewNoOpLocalDetector(), + }, }, { name: "LocalModeClusterCIDR, single-stack IPv6 cluster", @@ -338,46 +309,48 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, expected: [2]proxyutil.LocalTrafficDetector{ proxyutil.NewNoOpLocalDetector(), - resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64"))}, - errExpected: false, + proxyutil.NewDetectLocalByCIDR("2002:0:0:1234::/64"), + }, }, { - name: "LocalModeClusterCIDR, no ClusterCIDR", - mode: proxyconfigapi.LocalModeClusterCIDR, - config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, - expected: [2]proxyutil.LocalTrafficDetector{proxyutil.NewNoOpLocalDetector(), proxyutil.NewNoOpLocalDetector()}, - errExpected: false, + name: "LocalModeClusterCIDR, no ClusterCIDR", + mode: proxyconfigapi.LocalModeClusterCIDR, + config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, + expected: [2]proxyutil.LocalTrafficDetector{ + proxyutil.NewNoOpLocalDetector(), + proxyutil.NewNoOpLocalDetector(), + }, }, // LocalModeNodeCIDR { name: "LocalModeNodeCIDR, dual-stack IPv4-primary cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14,2002:0:0:1234::/64"}, - expected: resolveDualStackLocalDetectors(t)( - proxyutil.NewDetectLocalByCIDR("10.0.0.0/24"))( - proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), + expected: [2]proxyutil.LocalTrafficDetector{ + proxyutil.NewDetectLocalByCIDR("10.0.0.0/24"), + proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96"), + }, nodePodCIDRs: []string{"10.0.0.0/24", "2002::1234:abcd:ffff:0:0/96"}, - errExpected: false, }, { name: "LocalModeNodeCIDR, dual-stack IPv6-primary cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64,10.0.0.0/14"}, - expected: resolveDualStackLocalDetectors(t)( - proxyutil.NewDetectLocalByCIDR("10.0.0.0/24"))( - proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96")), + expected: [2]proxyutil.LocalTrafficDetector{ + proxyutil.NewDetectLocalByCIDR("10.0.0.0/24"), + proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96"), + }, nodePodCIDRs: []string{"2002::1234:abcd:ffff:0:0/96", "10.0.0.0/24"}, - errExpected: false, }, { name: "LocalModeNodeCIDR, single-stack IPv4 cluster", mode: proxyconfigapi.LocalModeNodeCIDR, config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "10.0.0.0/14"}, expected: [2]proxyutil.LocalTrafficDetector{ - resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("10.0.0.0/24")), - proxyutil.NewNoOpLocalDetector()}, + proxyutil.NewDetectLocalByCIDR("10.0.0.0/24"), + proxyutil.NewNoOpLocalDetector(), + }, nodePodCIDRs: []string{"10.0.0.0/24"}, - errExpected: false, }, { name: "LocalModeNodeCIDR, single-stack IPv6 cluster", @@ -385,17 +358,19 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: "2002:0:0:1234::/64"}, expected: [2]proxyutil.LocalTrafficDetector{ proxyutil.NewNoOpLocalDetector(), - resolveLocalDetector(t)(proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96"))}, + proxyutil.NewDetectLocalByCIDR("2002::1234:abcd:ffff:0:0/96"), + }, nodePodCIDRs: []string{"2002::1234:abcd:ffff:0:0/96"}, - errExpected: false, }, { - name: "LocalModeNodeCIDR, no PodCIDRs", - mode: proxyconfigapi.LocalModeNodeCIDR, - config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, - expected: [2]proxyutil.LocalTrafficDetector{proxyutil.NewNoOpLocalDetector(), proxyutil.NewNoOpLocalDetector()}, + name: "LocalModeNodeCIDR, no PodCIDRs", + mode: proxyconfigapi.LocalModeNodeCIDR, + config: &proxyconfigapi.KubeProxyConfiguration{ClusterCIDR: ""}, + expected: [2]proxyutil.LocalTrafficDetector{ + proxyutil.NewNoOpLocalDetector(), + proxyutil.NewNoOpLocalDetector(), + }, nodePodCIDRs: []string{}, - errExpected: false, }, // LocalModeBridgeInterface { @@ -404,10 +379,10 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{BridgeInterface: "eth"}, }, - expected: resolveDualStackLocalDetectors(t)( - proxyutil.NewDetectLocalByBridgeInterface("eth"))( - proxyutil.NewDetectLocalByBridgeInterface("eth")), - errExpected: false, + expected: [2]proxyutil.LocalTrafficDetector{ + proxyutil.NewDetectLocalByBridgeInterface("eth"), + proxyutil.NewDetectLocalByBridgeInterface("eth"), + }, }, // LocalModeInterfaceNamePrefix { @@ -416,26 +391,16 @@ func Test_getDualStackLocalDetectorTuple(t *testing.T) { config: &proxyconfigapi.KubeProxyConfiguration{ DetectLocal: proxyconfigapi.DetectLocalConfiguration{InterfaceNamePrefix: "veth"}, }, - expected: resolveDualStackLocalDetectors(t)( - proxyutil.NewDetectLocalByInterfaceNamePrefix("veth"))( - proxyutil.NewDetectLocalByInterfaceNamePrefix("veth")), - errExpected: false, + expected: [2]proxyutil.LocalTrafficDetector{ + proxyutil.NewDetectLocalByInterfaceNamePrefix("veth"), + proxyutil.NewDetectLocalByInterfaceNamePrefix("veth"), + }, }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { logger, _ := ktesting.NewTestContext(t) - r, err := getDualStackLocalDetectorTuple(logger, c.mode, c.config, c.nodePodCIDRs) - if c.errExpected { - if err == nil { - t.Errorf("Expected error, but succeeded with %q", r) - } - return - } - if err != nil { - t.Errorf("Error resolving detect-local: %v", err) - return - } + r := getDualStackLocalDetectorTuple(logger, c.mode, c.config, c.nodePodCIDRs) if !reflect.DeepEqual(r, c.expected) { t.Errorf("Unexpected detect-local implementation, expected: %q, got: %q", c.expected, r) } @@ -455,32 +420,6 @@ func makeNodeWithPodCIDRs(cidrs ...string) *v1.Node { } } -func resolveLocalDetector(t *testing.T) func(proxyutil.LocalTrafficDetector, error) proxyutil.LocalTrafficDetector { - return func(localDetector proxyutil.LocalTrafficDetector, err error) proxyutil.LocalTrafficDetector { - t.Helper() - if err != nil { - t.Fatalf("Error resolving detect-local: %v", err) - } - return localDetector - } -} - -func resolveDualStackLocalDetectors(t *testing.T) func(localDetector proxyutil.LocalTrafficDetector, err1 error) func(proxyutil.LocalTrafficDetector, error) [2]proxyutil.LocalTrafficDetector { - return func(localDetector proxyutil.LocalTrafficDetector, err error) func(proxyutil.LocalTrafficDetector, error) [2]proxyutil.LocalTrafficDetector { - t.Helper() - if err != nil { - t.Fatalf("Error resolving dual stack detect-local: %v", err) - } - return func(otherLocalDetector proxyutil.LocalTrafficDetector, err1 error) [2]proxyutil.LocalTrafficDetector { - t.Helper() - if err1 != nil { - t.Fatalf("Error resolving dual stack detect-local: %v", err) - } - return [2]proxyutil.LocalTrafficDetector{localDetector, otherLocalDetector} - } - } -} - func TestConfigChange(t *testing.T) { setUp := func() (*os.File, string, error) { tempDir, err := os.MkdirTemp("", "kubeproxy-config-change") diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 5ae5788d8af..552764643d6 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -93,7 +93,7 @@ func NewFakeProxier(ipt utiliptables.Interface) *Proxier { ipfamily = v1.IPv6Protocol podCIDR = "fd00:10::/64" } - detectLocal, _ := proxyutil.NewDetectLocalByCIDR(podCIDR) + detectLocal := proxyutil.NewDetectLocalByCIDR(podCIDR) networkInterfacer := proxyutiltest.NewFakeNetwork() itf := net.Interface{Index: 0, MTU: 0, Name: "lo", HardwareAddr: nil, Flags: 0} diff --git a/pkg/proxy/nftables/proxier_test.go b/pkg/proxy/nftables/proxier_test.go index 833aa5798c1..12f40e86aed 100644 --- a/pkg/proxy/nftables/proxier_test.go +++ b/pkg/proxy/nftables/proxier_test.go @@ -84,7 +84,7 @@ func NewFakeProxier(ipFamily v1.IPFamily) (*knftables.Fake, *Proxier) { podCIDR = "fd00:10::/64" serviceCIDRs = "fd00:10:96::/112" } - detectLocal, _ := proxyutil.NewDetectLocalByCIDR(podCIDR) + detectLocal := proxyutil.NewDetectLocalByCIDR(podCIDR) nodePortAddresses := []string{fmt.Sprintf("%s/32", testNodeIP), fmt.Sprintf("%s/128", testNodeIPv6)} networkInterfacer := proxyutiltest.NewFakeNetwork() diff --git a/pkg/proxy/util/localdetector.go b/pkg/proxy/util/localdetector.go index 6f2d77a9b0d..6e296b0c71b 100644 --- a/pkg/proxy/util/localdetector.go +++ b/pkg/proxy/util/localdetector.go @@ -17,8 +17,6 @@ limitations under the License. package util import ( - "fmt" - netutils "k8s.io/utils/net" ) @@ -77,15 +75,11 @@ func NewNoOpLocalDetector() LocalTrafficDetector { } // NewDetectLocalByCIDR returns a LocalTrafficDetector that considers traffic from the -// provided cidr to be from a local pod, and other traffic to be non-local. -func NewDetectLocalByCIDR(cidr string) (LocalTrafficDetector, error) { - _, parsed, err := netutils.ParseCIDRSloppy(cidr) - if err != nil { - return nil, err - } - +// provided cidr to be from a local pod, and other traffic to be non-local. cidr is +// assumed to be valid. +func NewDetectLocalByCIDR(cidr string) LocalTrafficDetector { nftFamily := "ip" - if netutils.IsIPv6CIDR(parsed) { + if netutils.IsIPv6CIDRString(cidr) { nftFamily = "ip6" } @@ -94,35 +88,29 @@ func NewDetectLocalByCIDR(cidr string) (LocalTrafficDetector, error) { ifNotLocal: []string{"!", "-s", cidr}, ifLocalNFT: []string{nftFamily, "saddr", cidr}, ifNotLocalNFT: []string{nftFamily, "saddr", "!=", cidr}, - }, nil + } } // NewDetectLocalByBridgeInterface returns a LocalTrafficDetector that considers traffic // from interfaceName to be from a local pod, and traffic from other interfaces to be // non-local. -func NewDetectLocalByBridgeInterface(interfaceName string) (LocalTrafficDetector, error) { - if len(interfaceName) == 0 { - return nil, fmt.Errorf("no bridge interface name set") - } +func NewDetectLocalByBridgeInterface(interfaceName string) LocalTrafficDetector { return &detectLocal{ ifLocal: []string{"-i", interfaceName}, ifNotLocal: []string{"!", "-i", interfaceName}, ifLocalNFT: []string{"iif", interfaceName}, ifNotLocalNFT: []string{"iif", "!=", interfaceName}, - }, nil + } } // NewDetectLocalByInterfaceNamePrefix returns a LocalTrafficDetector that considers // traffic from interfaces starting with interfacePrefix to be from a local pod, and // traffic from other interfaces to be non-local. -func NewDetectLocalByInterfaceNamePrefix(interfacePrefix string) (LocalTrafficDetector, error) { - if len(interfacePrefix) == 0 { - return nil, fmt.Errorf("no interface prefix set") - } +func NewDetectLocalByInterfaceNamePrefix(interfacePrefix string) LocalTrafficDetector { return &detectLocal{ ifLocal: []string{"-i", interfacePrefix + "+"}, ifNotLocal: []string{"!", "-i", interfacePrefix + "+"}, ifLocalNFT: []string{"iif", interfacePrefix + "*"}, ifNotLocalNFT: []string{"iif", "!=", interfacePrefix + "*"}, - }, nil + } } diff --git a/pkg/proxy/util/localdetector_test.go b/pkg/proxy/util/localdetector_test.go index f21e2014f7e..473aaeb8ce4 100644 --- a/pkg/proxy/util/localdetector_test.go +++ b/pkg/proxy/util/localdetector_test.go @@ -38,46 +38,6 @@ func TestNoOpLocalDetector(t *testing.T) { } } -func TestNewDetectLocalByCIDR(t *testing.T) { - cases := []struct { - cidr string - errExpected bool - }{ - { - cidr: "10.0.0.0/14", - errExpected: false, - }, - { - cidr: "2002:0:0:1234::/64", - errExpected: false, - }, - { - cidr: "10.0.0.0", - errExpected: true, - }, - { - cidr: "2002:0:0:1234::", - errExpected: true, - }, - { - cidr: "", - errExpected: true, - }, - } - for i, c := range cases { - r, err := NewDetectLocalByCIDR(c.cidr) - if c.errExpected { - if err == nil { - t.Errorf("Case[%d] expected error, but succeeded with: %q", i, r) - } - continue - } - if err != nil { - t.Errorf("Case[%d] failed with error: %v", i, err) - } - } -} - func TestDetectLocalByCIDR(t *testing.T) { cases := []struct { cidr string @@ -96,11 +56,7 @@ func TestDetectLocalByCIDR(t *testing.T) { }, } for _, c := range cases { - localDetector, err := NewDetectLocalByCIDR(c.cidr) - if err != nil { - t.Errorf("Error initializing localDetector: %v", err) - continue - } + localDetector := NewDetectLocalByCIDR(c.cidr) if !localDetector.IsImplemented() { t.Error("DetectLocalByCIDR returns false for IsImplemented") } @@ -118,66 +74,6 @@ func TestDetectLocalByCIDR(t *testing.T) { } } -func TestNewDetectLocalByBridgeInterface(t *testing.T) { - cases := []struct { - ifaceName string - errExpected bool - }{ - { - ifaceName: "avz", - errExpected: false, - }, - { - ifaceName: "", - errExpected: true, - }, - } - for i, c := range cases { - r, err := NewDetectLocalByBridgeInterface(c.ifaceName) - if c.errExpected { - if err == nil { - t.Errorf("Case[%d] expected error, but succeeded with: %q", i, r) - } - continue - } - if err != nil { - t.Errorf("Case[%d] failed with error: %v", i, err) - } - } -} - -func TestNewDetectLocalByInterfaceNamePrefix(t *testing.T) { - cases := []struct { - ifacePrefix string - errExpected bool - }{ - { - ifacePrefix: "veth", - errExpected: false, - }, - { - ifacePrefix: "cbr0", - errExpected: false, - }, - { - ifacePrefix: "", - errExpected: true, - }, - } - for i, c := range cases { - r, err := NewDetectLocalByInterfaceNamePrefix(c.ifacePrefix) - if c.errExpected { - if err == nil { - t.Errorf("Case[%d] expected error, but succeeded with: %q", i, r) - } - continue - } - if err != nil { - t.Errorf("Case[%d] failed with error: %v", i, err) - } - } -} - func TestDetectLocalByBridgeInterface(t *testing.T) { cases := []struct { ifaceName string @@ -191,11 +87,7 @@ func TestDetectLocalByBridgeInterface(t *testing.T) { }, } for _, c := range cases { - localDetector, err := NewDetectLocalByBridgeInterface(c.ifaceName) - if err != nil { - t.Errorf("Error initializing localDetector: %v", err) - continue - } + localDetector := NewDetectLocalByBridgeInterface(c.ifaceName) if !localDetector.IsImplemented() { t.Error("DetectLocalByBridgeInterface returns false for IsImplemented") } @@ -228,11 +120,7 @@ func TestDetectLocalByInterfaceNamePrefix(t *testing.T) { }, } for _, c := range cases { - localDetector, err := NewDetectLocalByInterfaceNamePrefix(c.ifacePrefix) - if err != nil { - t.Errorf("Error initializing localDetector: %v", err) - continue - } + localDetector := NewDetectLocalByInterfaceNamePrefix(c.ifacePrefix) if !localDetector.IsImplemented() { t.Error("DetectLocalByInterfaceNamePrefix returns false for IsImplemented") }