From 477d14e53b3441898a0af235ce55e60a495391b9 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 11 Aug 2022 12:02:07 -0400 Subject: [PATCH 1/4] Reorganize "kube-proxy --cleanup-and-exit" This was implemented partly in server.go and partly in server_others.go even though even the parts in server.go were totally linux-specific. Simplify things by putting it all in server_others.go and get rid of some unnecessary abstraction. --- cmd/kube-proxy/app/server.go | 34 +++---------------------- cmd/kube-proxy/app/server_others.go | 38 ++++++++++++++++++++-------- cmd/kube-proxy/app/server_windows.go | 14 +++++----- 3 files changed, 38 insertions(+), 48 deletions(-) diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index 7a5b0c861c5..59aa34e5a97 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -19,7 +19,6 @@ limitations under the License. package app import ( - "errors" goflag "flag" "fmt" "net" @@ -75,9 +74,6 @@ import ( "k8s.io/kubernetes/pkg/proxy/apis/config/validation" "k8s.io/kubernetes/pkg/proxy/config" "k8s.io/kubernetes/pkg/proxy/healthcheck" - "k8s.io/kubernetes/pkg/proxy/iptables" - "k8s.io/kubernetes/pkg/proxy/ipvs" - "k8s.io/kubernetes/pkg/proxy/userspace" proxyutil "k8s.io/kubernetes/pkg/proxy/util" "k8s.io/kubernetes/pkg/util/filesystem" utilflag "k8s.io/kubernetes/pkg/util/flag" @@ -100,7 +96,6 @@ const ( // proxyRun defines the interface to run a specified ProxyServer type proxyRun interface { Run() error - CleanupAndExit() error } // Options contains everything necessary to create and run a proxy server. @@ -314,15 +309,15 @@ func (o *Options) Run() error { return o.writeConfigFile() } + if o.CleanupAndExit { + return cleanupAndExit() + } + proxyServer, err := NewProxyServer(o) if err != nil { return err } - if o.CleanupAndExit { - return proxyServer.CleanupAndExit() - } - o.proxyServer = proxyServer return o.runLoop() } @@ -815,27 +810,6 @@ func getConntrackMax(config kubeproxyconfig.KubeProxyConntrackConfiguration) (in return 0, nil } -// CleanupAndExit remove iptables rules and ipset/ipvs rules in ipvs proxy mode -// and exit if success return nil -func (s *ProxyServer) CleanupAndExit() error { - // cleanup IPv6 and IPv4 iptables rules - ipts := []utiliptables.Interface{ - utiliptables.New(s.execer, utiliptables.ProtocolIPv4), - utiliptables.New(s.execer, utiliptables.ProtocolIPv6), - } - var encounteredError bool - for _, ipt := range ipts { - encounteredError = userspace.CleanupLeftovers(ipt) || encounteredError - encounteredError = iptables.CleanupLeftovers(ipt) || encounteredError - encounteredError = ipvs.CleanupLeftovers(s.IpvsInterface, ipt, s.IpsetInterface) || encounteredError - } - if encounteredError { - return errors.New("encountered an error while tearing down rules") - } - - return nil -} - // detectNodeIP returns the nodeIP used by the proxier // The order of precedence is: // 1. config.bindAddress if bindAddress is not 0.0.0.0 or :: diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index 8a5d580f93d..f0ac6210e5c 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -74,12 +74,11 @@ var timeoutForNodePodCIDR = 5 * time.Minute // NewProxyServer returns a new ProxyServer. func NewProxyServer(o *Options) (*ProxyServer, error) { - return newProxyServer(o.config, o.CleanupAndExit, o.master) + return newProxyServer(o.config, o.master) } func newProxyServer( config *proxyconfigapi.KubeProxyConfiguration, - cleanupAndExit bool, master string) (*ProxyServer, error) { if config == nil { @@ -111,15 +110,6 @@ func newProxyServer( ipvsInterface = utilipvs.New() } - // We omit creation of pretty much everything if we run in cleanup mode - if cleanupAndExit { - return &ProxyServer{ - execer: execer, - IpvsInterface: ipvsInterface, - IpsetInterface: ipsetInterface, - }, nil - } - if len(config.ShowHiddenMetricsForVersion) > 0 { metrics.SetShowHidden() } @@ -603,3 +593,29 @@ func tryIPTablesProxy(kcompat iptables.KernelCompatTester) string { klog.V(1).InfoS("Can't use iptables proxy, using userspace proxier") return proxyModeUserspace } + +// cleanupAndExit remove iptables rules and ipset/ipvs rules +func cleanupAndExit() error { + execer := exec.New() + + // cleanup IPv6 and IPv4 iptables rules, regardless of current configuration + ipts := []utiliptables.Interface{ + utiliptables.New(execer, utiliptables.ProtocolIPv4), + utiliptables.New(execer, utiliptables.ProtocolIPv6), + } + + ipsetInterface := utilipset.New(execer) + ipvsInterface := utilipvs.New() + + var encounteredError bool + for _, ipt := range ipts { + encounteredError = userspace.CleanupLeftovers(ipt) || encounteredError + encounteredError = iptables.CleanupLeftovers(ipt) || encounteredError + encounteredError = ipvs.CleanupLeftovers(ipvsInterface, ipt, ipsetInterface) || encounteredError + } + if encounteredError { + return errors.New("encountered an error while tearing down rules") + } + + return nil +} diff --git a/cmd/kube-proxy/app/server_windows.go b/cmd/kube-proxy/app/server_windows.go index b26f8e328e2..5ba57692429 100644 --- a/cmd/kube-proxy/app/server_windows.go +++ b/cmd/kube-proxy/app/server_windows.go @@ -52,10 +52,10 @@ import ( // NewProxyServer returns a new ProxyServer. func NewProxyServer(o *Options) (*ProxyServer, error) { - return newProxyServer(o.config, o.CleanupAndExit, o.master) + return newProxyServer(o.config, o.master) } -func newProxyServer(config *proxyconfigapi.KubeProxyConfiguration, cleanupAndExit bool, master string) (*ProxyServer, error) { +func newProxyServer(config *proxyconfigapi.KubeProxyConfiguration, master string) (*ProxyServer, error) { if config == nil { return nil, errors.New("config is required") } @@ -66,11 +66,6 @@ func newProxyServer(config *proxyconfigapi.KubeProxyConfiguration, cleanupAndExi return nil, fmt.Errorf("unable to register configz: %s", err) } - // We omit creation of pretty much everything if we run in cleanup mode - if cleanupAndExit { - return &ProxyServer{}, nil - } - if len(config.ShowHiddenMetricsForVersion) > 0 { metrics.SetShowHidden() } @@ -225,3 +220,8 @@ func tryWinKernelSpaceProxy(kcompat winkernel.KernelCompatTester) string { klog.V(1).InfoS("Can't use winkernel proxy, using userspace proxier") return proxyModeUserspace } + +// cleanupAndExit cleans up after a previous proxy run +func cleanupAndExit() error { + return errors.New("--cleanup-and-exit is not implemented on Windows") +} From 9f69a3a9d4d5af4ddfd8802dc311d5d0205663a7 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 11 Aug 2022 12:11:43 -0400 Subject: [PATCH 2/4] kube-proxy: remove iptables-to-userspace fallback Back when iptables was first made the default, there were theoretically some users who wouldn't have been able to support it due to having an old /sbin/iptables. But kube-proxy no longer does the things that didn't work with old iptables, and we removed that check a long time ago. There is also a check for a new-enough kernel version, but it's checking for a feature which was added in kernel 3.6, and no one could possibly be running Kubernetes with a kernel that old. So the fallback code now never actually falls back, so it should just be removed. --- cmd/kube-proxy/app/server_others.go | 31 +++++++---------------------- pkg/proxy/iptables/proxier.go | 28 -------------------------- 2 files changed, 7 insertions(+), 52 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index f0ac6210e5c..b64f25efe00 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -43,7 +43,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" utilnet "k8s.io/apimachinery/pkg/util/net" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientset "k8s.io/client-go/kubernetes" toolswatch "k8s.io/client-go/tools/watch" "k8s.io/component-base/configz" @@ -146,7 +145,7 @@ func newProxyServer( var proxier proxy.Provider var detectLocalMode proxyconfigapi.LocalMode - proxyMode := getProxyMode(string(config.Mode), canUseIPVS, iptables.LinuxKernelCompatTester{}) + proxyMode := getProxyMode(string(config.Mode), canUseIPVS) detectLocalMode, err = getDetectLocalMode(config) if err != nil { return nil, fmt.Errorf("cannot determine detect-local-mode: %v", err) @@ -556,42 +555,26 @@ func cidrTuple(cidrList string) [2]string { return cidrs } -func getProxyMode(proxyMode string, canUseIPVS bool, kcompat iptables.KernelCompatTester) string { +func getProxyMode(proxyMode string, canUseIPVS bool) string { switch proxyMode { case proxyModeUserspace: return proxyModeUserspace case proxyModeIPTables: - return tryIPTablesProxy(kcompat) + return proxyModeIPTables case proxyModeIPVS: - return tryIPVSProxy(canUseIPVS, kcompat) + return tryIPVSProxy(canUseIPVS) } klog.InfoS("Unknown proxy mode, assuming iptables proxy", "proxyMode", proxyMode) - return tryIPTablesProxy(kcompat) + return proxyModeIPTables } -func tryIPVSProxy(canUseIPVS bool, kcompat iptables.KernelCompatTester) string { +func tryIPVSProxy(canUseIPVS bool) string { if canUseIPVS { return proxyModeIPVS } - // Try to fallback to iptables before falling back to userspace klog.V(1).InfoS("Can't use ipvs proxier, trying iptables proxier") - return tryIPTablesProxy(kcompat) -} - -func tryIPTablesProxy(kcompat iptables.KernelCompatTester) string { - // guaranteed false on error, error only necessary for debugging - useIPTablesProxy, err := iptables.CanUseIPTablesProxier(kcompat) - if err != nil { - utilruntime.HandleError(fmt.Errorf("can't determine whether to use iptables proxy, using userspace proxier: %v", err)) - return proxyModeUserspace - } - if useIPTablesProxy { - return proxyModeIPTables - } - // Fallback. - klog.V(1).InfoS("Can't use iptables proxy, using userspace proxier") - return proxyModeUserspace + return proxyModeIPTables } // cleanupAndExit remove iptables rules and ipset/ipvs rules diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index a5c9f695639..1cc9dfb5055 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -85,34 +85,6 @@ const ( largeClusterEndpointsThreshold = 1000 ) -// KernelCompatTester tests whether the required kernel capabilities are -// present to run the iptables proxier. -type KernelCompatTester interface { - IsCompatible() error -} - -// CanUseIPTablesProxier returns true if we should use the iptables Proxier -// instead of the "classic" userspace Proxier. -func CanUseIPTablesProxier(kcompat KernelCompatTester) (bool, error) { - if err := kcompat.IsCompatible(); err != nil { - return false, err - } - return true, nil -} - -var _ KernelCompatTester = LinuxKernelCompatTester{} - -// LinuxKernelCompatTester is the Linux implementation of KernelCompatTester -type LinuxKernelCompatTester struct{} - -// IsCompatible checks for the required sysctls. We don't care about the value, just -// that it exists. If this Proxier is chosen, we'll initialize it as we -// need. -func (lkct LinuxKernelCompatTester) IsCompatible() error { - _, err := utilsysctl.New().GetSysctl(sysctlRouteLocalnet) - return err -} - const sysctlRouteLocalnet = "net/ipv4/conf/all/route_localnet" const sysctlBridgeCallIPTables = "net/bridge/bridge-nf-call-iptables" From 1609017f2b5911356656e9edabeac3fb44fca1e5 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 11 Aug 2022 12:26:23 -0400 Subject: [PATCH 3/4] kube-proxy: remove ipvs-to-iptables fallback If the user passes "--proxy-mode ipvs", and it is not possible to use IPVS, then error out rather than falling back to iptables. There was never any good reason to be doing fallback; this was presumably erroneously added to parallel the iptables-to-userspace fallback (which only existed because we had wanted iptables to be the default but not all systems could support it). In particular, if the user passed configuration options for ipvs, then they presumably *didn't* pass configuration options for iptables, and so even if the iptables proxy is able to run, it is likely to be misconfigured. --- cmd/kube-proxy/app/server_others.go | 51 +++----- cmd/kube-proxy/app/server_others_test.go | 144 ----------------------- pkg/proxy/ipvs/proxier.go | 18 +-- pkg/proxy/ipvs/proxier_test.go | 6 +- 4 files changed, 27 insertions(+), 192 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index b64f25efe00..a7ca88d1a27 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -90,25 +90,9 @@ func newProxyServer( return nil, fmt.Errorf("unable to register configz: %s", err) } - var iptInterface utiliptables.Interface var ipvsInterface utilipvs.Interface - var kernelHandler ipvs.KernelHandler var ipsetInterface utilipset.Interface - // Create a iptables utils. - execer := exec.New() - - kernelHandler = ipvs.NewLinuxKernelHandler() - ipsetInterface = utilipset.New(execer) - canUseIPVS, err := ipvs.CanUseIPVSProxier(kernelHandler, ipsetInterface, config.IPVS.Scheduler) - if string(config.Mode) == proxyModeIPVS && err != nil { - klog.ErrorS(err, "Can't use the IPVS proxier") - } - - if canUseIPVS { - ipvsInterface = utilipvs.New() - } - if len(config.ShowHiddenMetricsForVersion) > 0 { metrics.SetShowHidden() } @@ -145,7 +129,7 @@ func newProxyServer( var proxier proxy.Provider var detectLocalMode proxyconfigapi.LocalMode - proxyMode := getProxyMode(string(config.Mode), canUseIPVS) + proxyMode := getProxyMode(string(config.Mode)) detectLocalMode, err = getDetectLocalMode(config) if err != nil { return nil, fmt.Errorf("cannot determine detect-local-mode: %v", err) @@ -167,7 +151,8 @@ func newProxyServer( if netutils.IsIPv6(nodeIP) { primaryProtocol = utiliptables.ProtocolIPv6 } - iptInterface = utiliptables.New(execer, primaryProtocol) + execer := exec.New() + iptInterface := utiliptables.New(execer, primaryProtocol) var ipt [2]utiliptables.Interface dualStack := true // While we assume that node supports, we do further checks below @@ -255,6 +240,13 @@ func newProxyServer( } proxymetrics.RegisterMetrics() } else if proxyMode == proxyModeIPVS { + kernelHandler := ipvs.NewLinuxKernelHandler() + ipsetInterface = utilipset.New(execer) + if err := ipvs.CanUseIPVSProxier(kernelHandler, ipsetInterface, config.IPVS.Scheduler); err != nil { + return nil, fmt.Errorf("can't use the IPVS proxier: %v", err) + } + ipvsInterface = utilipvs.New() + klog.V(0).InfoS("Using ipvs Proxier") if dualStack { klog.V(0).InfoS("Creating dualStackProxier for ipvs") @@ -555,26 +547,13 @@ func cidrTuple(cidrList string) [2]string { return cidrs } -func getProxyMode(proxyMode string, canUseIPVS bool) string { - switch proxyMode { - case proxyModeUserspace: - return proxyModeUserspace - case proxyModeIPTables: +func getProxyMode(proxyMode string) string { + if proxyMode == "" { + klog.InfoS("Using iptables proxy") return proxyModeIPTables - case proxyModeIPVS: - return tryIPVSProxy(canUseIPVS) + } else { + return proxyMode } - klog.InfoS("Unknown proxy mode, assuming iptables proxy", "proxyMode", proxyMode) - return proxyModeIPTables -} - -func tryIPVSProxy(canUseIPVS bool) string { - if canUseIPVS { - return proxyModeIPVS - } - - klog.V(1).InfoS("Can't use ipvs proxier, trying iptables proxier") - return proxyModeIPTables } // cleanupAndExit remove iptables rules and ipset/ipvs rules diff --git a/cmd/kube-proxy/app/server_others_test.go b/cmd/kube-proxy/app/server_others_test.go index 9e20d10d786..fcaffde4ce3 100644 --- a/cmd/kube-proxy/app/server_others_test.go +++ b/cmd/kube-proxy/app/server_others_test.go @@ -20,7 +20,6 @@ limitations under the License. package app import ( - "fmt" "net" "reflect" "testing" @@ -32,154 +31,11 @@ import ( clientsetfake "k8s.io/client-go/kubernetes/fake" proxyconfigapi "k8s.io/kubernetes/pkg/proxy/apis/config" - "k8s.io/kubernetes/pkg/proxy/ipvs" proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" utiliptables "k8s.io/kubernetes/pkg/util/iptables" utiliptablestest "k8s.io/kubernetes/pkg/util/iptables/testing" ) -type fakeIPSetVersioner struct { - version string // what to return - err error // what to return -} - -func (fake *fakeIPSetVersioner) GetVersion() (string, error) { - return fake.version, fake.err -} - -type fakeKernelCompatTester struct { - ok bool -} - -func (fake *fakeKernelCompatTester) IsCompatible() error { - if !fake.ok { - return fmt.Errorf("error") - } - return nil -} - -// fakeKernelHandler implements KernelHandler. -type fakeKernelHandler struct { - modules []string - kernelVersion string -} - -func (fake *fakeKernelHandler) GetModules() ([]string, error) { - return fake.modules, nil -} - -func (fake *fakeKernelHandler) GetKernelVersion() (string, error) { - return fake.kernelVersion, nil -} - -func Test_getProxyMode(t *testing.T) { - var cases = []struct { - flag string - ipsetVersion string - kmods []string - kernelVersion string - kernelCompat bool - ipsetError error - expected string - scheduler string - }{ - { // flag says userspace - flag: "userspace", - expected: proxyModeUserspace, - }, - { // flag says iptables, kernel not compatible - flag: "iptables", - kernelCompat: false, - expected: proxyModeUserspace, - }, - { // flag says iptables, kernel is compatible - flag: "iptables", - kernelCompat: true, - expected: proxyModeIPTables, - }, - { // detect, kernel not compatible - flag: "", - kernelCompat: false, - expected: proxyModeUserspace, - }, - { // detect, kernel is compatible - flag: "", - kernelCompat: true, - expected: proxyModeIPTables, - }, - { // flag says ipvs, ipset version ok, kernel modules installed for linux kernel before 4.19 - flag: "ipvs", - kmods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack_ipv4"}, - kernelVersion: "4.18", - ipsetVersion: ipvs.MinIPSetCheckVersion, - expected: proxyModeIPVS, - scheduler: "rr", - }, - { // flag says ipvs, ipset version ok, kernel modules installed for linux kernel 4.19 - flag: "ipvs", - kmods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, - kernelVersion: "4.19", - ipsetVersion: ipvs.MinIPSetCheckVersion, - expected: proxyModeIPVS, - scheduler: "rr", - }, - { // flag says ipvs, ipset version too low, fallback on iptables mode - flag: "ipvs", - kmods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, - kernelVersion: "4.19", - ipsetVersion: "0.0", - kernelCompat: true, - expected: proxyModeIPTables, - }, - { // flag says ipvs, bad ipset version, fallback on iptables mode - flag: "ipvs", - kmods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, - kernelVersion: "4.19", - ipsetVersion: "a.b.c", - kernelCompat: true, - expected: proxyModeIPTables, - }, - { // flag says ipvs, required kernel modules are not installed, fallback on iptables mode - flag: "ipvs", - kmods: []string{"foo", "bar", "baz"}, - kernelVersion: "4.19", - ipsetVersion: ipvs.MinIPSetCheckVersion, - kernelCompat: true, - expected: proxyModeIPTables, - }, - { // flag says ipvs, ipset version ok, kernel modules installed for sed scheduler - flag: "ipvs", - kmods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack", "ip_vs_sed"}, - kernelVersion: "4.19", - ipsetVersion: ipvs.MinIPSetCheckVersion, - expected: proxyModeIPVS, - scheduler: "sed", - }, - { // flag says ipvs, kernel modules not installed for sed scheduler, fallback to iptables - flag: "ipvs", - kmods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, - kernelVersion: "4.19", - ipsetVersion: ipvs.MinIPSetCheckVersion, - expected: proxyModeIPTables, - kernelCompat: true, - scheduler: "sed", - }, - } - for i, c := range cases { - kcompater := &fakeKernelCompatTester{c.kernelCompat} - ipsetver := &fakeIPSetVersioner{c.ipsetVersion, c.ipsetError} - khandler := &fakeKernelHandler{ - modules: c.kmods, - kernelVersion: c.kernelVersion, - } - canUseIPVS, _ := ipvs.CanUseIPVSProxier(khandler, ipsetver, cases[i].scheduler) - r := getProxyMode(c.flag, canUseIPVS, kcompater) - if r != c.expected { - t.Errorf("Case[%d] Expected %q, got %q", i, c.expected, r) - } - } -} - func Test_getDetectLocalMode(t *testing.T) { cases := []struct { detectLocal string diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index de9348e5ec4..be72cc9ce2f 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -706,25 +706,25 @@ func (handle *LinuxKernelHandler) GetKernelVersion() (string, error) { return strings.TrimSpace(string(fileContent)), nil } -// CanUseIPVSProxier returns true if we can use the ipvs Proxier. +// CanUseIPVSProxier checks if we can use the ipvs Proxier. // This is determined by checking if all the required kernel modules can be loaded. It may // return an error if it fails to get the kernel modules information without error, in which // case it will also return false. -func CanUseIPVSProxier(handle KernelHandler, ipsetver IPSetVersioner, scheduler string) (bool, error) { +func CanUseIPVSProxier(handle KernelHandler, ipsetver IPSetVersioner, scheduler string) error { mods, err := handle.GetModules() if err != nil { - return false, fmt.Errorf("error getting installed ipvs required kernel modules: %v", err) + return fmt.Errorf("error getting installed ipvs required kernel modules: %v", err) } loadModules := sets.NewString() loadModules.Insert(mods...) kernelVersionStr, err := handle.GetKernelVersion() if err != nil { - return false, fmt.Errorf("error determining kernel version to find required kernel modules for ipvs support: %v", err) + return fmt.Errorf("error determining kernel version to find required kernel modules for ipvs support: %v", err) } kernelVersion, err := version.ParseGeneric(kernelVersionStr) if err != nil { - return false, fmt.Errorf("error parsing kernel version %q: %v", kernelVersionStr, err) + return fmt.Errorf("error parsing kernel version %q: %v", kernelVersionStr, err) } mods = utilipvs.GetRequiredIPVSModules(kernelVersion) wantModules := sets.NewString() @@ -751,18 +751,18 @@ func CanUseIPVSProxier(handle KernelHandler, ipsetver IPSetVersioner, scheduler } if len(missingMods) != 0 { - return false, fmt.Errorf("IPVS proxier will not be used because the following required kernel modules are not loaded: %v", missingMods) + return fmt.Errorf("IPVS proxier will not be used because the following required kernel modules are not loaded: %v", missingMods) } // Check ipset version versionString, err := ipsetver.GetVersion() if err != nil { - return false, fmt.Errorf("error getting ipset version, error: %v", err) + return fmt.Errorf("error getting ipset version, error: %v", err) } if !checkMinVersion(versionString) { - return false, fmt.Errorf("ipset version: %s is less than min required version: %s", versionString, MinIPSetCheckVersion) + return fmt.Errorf("ipset version: %s is less than min required version: %s", versionString, MinIPSetCheckVersion) } - return true, nil + return nil } // CleanupIptablesLeftovers removes all iptables rules and chains created by the Proxier diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 5a149d0a8d3..06690066852 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -364,9 +364,9 @@ func TestCanUseIPVSProxier(t *testing.T) { for i := range testCases { handle := &fakeKernelHandler{modules: testCases[i].mods, kernelVersion: testCases[i].kernelVersion} versioner := &fakeIPSetVersioner{version: testCases[i].ipsetVersion, err: testCases[i].ipsetErr} - ok, err := CanUseIPVSProxier(handle, versioner, testCases[i].scheduler) - if ok != testCases[i].ok { - t.Errorf("Case [%d], expect %v, got %v: err: %v", i, testCases[i].ok, ok, err) + err := CanUseIPVSProxier(handle, versioner, testCases[i].scheduler) + if (err == nil) != testCases[i].ok { + t.Errorf("Case [%d], expect %v, got err: %v", i, testCases[i].ok, err) } } } From 946ce55b04fae49f6cc882cb43603801619fc771 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 16 Aug 2022 09:23:20 -0400 Subject: [PATCH 4/4] kube-proxy: use API constants for proxy modes rather than local redefinitions --- cmd/kube-proxy/app/server.go | 11 ++--------- cmd/kube-proxy/app/server_others.go | 14 +++++++------- cmd/kube-proxy/app/server_windows.go | 20 ++++++++++---------- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index 59aa34e5a97..a173b23cab0 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -86,13 +86,6 @@ import ( utilpointer "k8s.io/utils/pointer" ) -const ( - proxyModeUserspace = "userspace" - proxyModeIPTables = "iptables" - proxyModeIPVS = "ipvs" - proxyModeKernelspace = "kernelspace" //nolint:deadcode,varcheck -) - // proxyRun defines the interface to run a specified ProxyServer type proxyRun interface { Run() error @@ -540,7 +533,7 @@ type ProxyServer struct { Recorder events.EventRecorder ConntrackConfiguration kubeproxyconfig.KubeProxyConntrackConfiguration Conntracker Conntracker // if nil, ignored - ProxyMode string + ProxyMode kubeproxyconfig.ProxyMode NodeRef *v1.ObjectReference MetricsBindAddress string BindAddressHardFail bool @@ -611,7 +604,7 @@ func serveHealthz(hz healthcheck.ProxierHealthUpdater, errCh chan error) { go wait.Until(fn, 5*time.Second, wait.NeverStop) } -func serveMetrics(bindAddress, proxyMode string, enableProfiling bool, errCh chan error) { +func serveMetrics(bindAddress string, proxyMode kubeproxyconfig.ProxyMode, enableProfiling bool, errCh chan error) { if len(bindAddress) == 0 { return } diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index a7ca88d1a27..f203f87a738 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -129,7 +129,7 @@ func newProxyServer( var proxier proxy.Provider var detectLocalMode proxyconfigapi.LocalMode - proxyMode := getProxyMode(string(config.Mode)) + proxyMode := getProxyMode(config.Mode) detectLocalMode, err = getDetectLocalMode(config) if err != nil { return nil, fmt.Errorf("cannot determine detect-local-mode: %v", err) @@ -157,7 +157,7 @@ func newProxyServer( var ipt [2]utiliptables.Interface dualStack := true // While we assume that node supports, we do further checks below - if proxyMode != proxyModeUserspace { + if proxyMode != proxyconfigapi.ProxyModeUserspace { // Create iptables handlers for both families, one is already created // Always ordered as IPv4, IPv6 if primaryProtocol == utiliptables.ProtocolIPv4 { @@ -176,7 +176,7 @@ func newProxyServer( } } - if proxyMode == proxyModeIPTables { + if proxyMode == proxyconfigapi.ProxyModeIPTables { klog.V(0).InfoS("Using iptables Proxier") if config.IPTables.MasqueradeBit == nil { // MasqueradeBit must be specified or defaulted. @@ -239,7 +239,7 @@ func newProxyServer( return nil, fmt.Errorf("unable to create proxier: %v", err) } proxymetrics.RegisterMetrics() - } else if proxyMode == proxyModeIPVS { + } else if proxyMode == proxyconfigapi.ProxyModeIPVS { kernelHandler := ipvs.NewLinuxKernelHandler() ipsetInterface = utilipset.New(execer) if err := ipvs.CanUseIPVSProxier(kernelHandler, ipsetInterface, config.IPVS.Scheduler); err != nil { @@ -342,7 +342,7 @@ func newProxyServer( } useEndpointSlices := true - if proxyMode == proxyModeUserspace { + if proxyMode == proxyconfigapi.ProxyModeUserspace { // userspace mode doesn't support endpointslice. useEndpointSlices = false } @@ -547,10 +547,10 @@ func cidrTuple(cidrList string) [2]string { return cidrs } -func getProxyMode(proxyMode string) string { +func getProxyMode(proxyMode proxyconfigapi.ProxyMode) proxyconfigapi.ProxyMode { if proxyMode == "" { klog.InfoS("Using iptables proxy") - return proxyModeIPTables + return proxyconfigapi.ProxyModeIPTables } else { return proxyMode } diff --git a/cmd/kube-proxy/app/server_windows.go b/cmd/kube-proxy/app/server_windows.go index 5ba57692429..836b95a0f25 100644 --- a/cmd/kube-proxy/app/server_windows.go +++ b/cmd/kube-proxy/app/server_windows.go @@ -102,9 +102,9 @@ func newProxyServer(config *proxyconfigapi.KubeProxyConfiguration, master string } var proxier proxy.Provider - proxyMode := getProxyMode(string(config.Mode), winkernel.WindowsKernelCompatTester{}) + proxyMode := getProxyMode(config.Mode, winkernel.WindowsKernelCompatTester{}) dualStackMode := getDualStackMode(config.Winkernel.NetworkName, winkernel.DualStackCompatTester{}) - if proxyMode == proxyModeKernelspace { + if proxyMode == proxyconfigapi.ProxyModeKernelspace { klog.V(0).InfoS("Using Kernelspace Proxier.") if dualStackMode { klog.V(0).InfoS("Creating dualStackProxier for Windows kernel.") @@ -166,7 +166,7 @@ func newProxyServer(config *proxyconfigapi.KubeProxyConfiguration, master string } } useEndpointSlices := true - if proxyMode == proxyModeUserspace { + if proxyMode == proxyconfigapi.ProxyModeUserspace { // userspace mode doesn't support endpointslice. useEndpointSlices = false } @@ -192,18 +192,18 @@ func getDualStackMode(networkname string, compatTester winkernel.StackCompatTest return compatTester.DualStackCompatible(networkname) } -func getProxyMode(proxyMode string, kcompat winkernel.KernelCompatTester) string { - if proxyMode == proxyModeKernelspace { +func getProxyMode(proxyMode proxyconfigapi.ProxyMode, kcompat winkernel.KernelCompatTester) proxyconfigapi.ProxyMode { + if proxyMode == proxyconfigapi.ProxyModeKernelspace { return tryWinKernelSpaceProxy(kcompat) } - return proxyModeUserspace + return proxyconfigapi.ProxyModeUserspace } func detectNumCPU() int { return goruntime.NumCPU() } -func tryWinKernelSpaceProxy(kcompat winkernel.KernelCompatTester) string { +func tryWinKernelSpaceProxy(kcompat winkernel.KernelCompatTester) proxyconfigapi.ProxyMode { // Check for Windows Kernel Version if we can support Kernel Space proxy // Check for Windows Version @@ -211,14 +211,14 @@ func tryWinKernelSpaceProxy(kcompat winkernel.KernelCompatTester) string { useWinKernelProxy, err := winkernel.CanUseWinKernelProxier(kcompat) if err != nil { klog.ErrorS(err, "Can't determine whether to use windows kernel proxy, using userspace proxier") - return proxyModeUserspace + return proxyconfigapi.ProxyModeUserspace } if useWinKernelProxy { - return proxyModeKernelspace + return proxyconfigapi.ProxyModeKernelspace } // Fallback. klog.V(1).InfoS("Can't use winkernel proxy, using userspace proxier") - return proxyModeUserspace + return proxyconfigapi.ProxyModeUserspace } // cleanupAndExit cleans up after a previous proxy run