From 1609017f2b5911356656e9edabeac3fb44fca1e5 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 11 Aug 2022 12:26:23 -0400 Subject: [PATCH] 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) } } }