From 4f02671b23d3f42aff10ca7274a33bffb0e8bdff Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Thu, 22 Dec 2022 17:04:11 +0100 Subject: [PATCH 01/11] proxy/ipvs: Remove kernel module tests To check kernel modules is a bad way to check functionality. This commit just removes the checks and makes it possible to use a statically linked kernel. Minimal updates to unit-tests are made. --- pkg/proxy/ipvs/proxier.go | 47 +--------------------------------- pkg/proxy/ipvs/proxier_test.go | 24 ----------------- 2 files changed, 1 insertion(+), 70 deletions(-) diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index da96f3da640..38b8a162dca 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -720,53 +720,8 @@ func (handle *LinuxKernelHandler) GetKernelVersion() (string, error) { } // 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. +// Only the ipset version is checked. Necessary kernel functions are assumed to be present. func CanUseIPVSProxier(handle KernelHandler, ipsetver IPSetVersioner, scheduler string) error { - mods, err := handle.GetModules() - if err != nil { - 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 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 fmt.Errorf("error parsing kernel version %q: %v", kernelVersionStr, err) - } - mods = utilipvs.GetRequiredIPVSModules(kernelVersion) - wantModules := sets.NewString() - // We check for the existence of the scheduler mod and will trigger a missingMods error if not found - if scheduler == "" { - scheduler = defaultScheduler - } - schedulerMod := "ip_vs_" + scheduler - mods = append(mods, schedulerMod) - wantModules.Insert(mods...) - - modules := wantModules.Difference(loadModules).UnsortedList() - var missingMods []string - ConntrackiMissingCounter := 0 - for _, mod := range modules { - if strings.Contains(mod, "nf_conntrack") { - ConntrackiMissingCounter++ - } else { - missingMods = append(missingMods, mod) - } - } - if ConntrackiMissingCounter == 2 { - missingMods = append(missingMods, "nf_conntrack_ipv4(or nf_conntrack for Linux kernel 4.19 and later)") - } - - if len(missingMods) != 0 { - 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 { diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 87ffac52225..ba15bac1202 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -307,14 +307,6 @@ func TestCanUseIPVSProxier(t *testing.T) { ipsetVersion: "1.1", ok: false, }, - // case 3, missing required ip_vs_* kernel modules - { - mods: []string{"ip_vs", "a", "bc", "def"}, - scheduler: "sed", - kernelVersion: "4.19", - ipsetVersion: MinIPSetCheckVersion, - ok: false, - }, // case 4, ipset version too low { mods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, @@ -347,14 +339,6 @@ func TestCanUseIPVSProxier(t *testing.T) { ipsetVersion: "6.19", ok: true, }, - // case 8, not ok for sed based IPVS scheduling - { - mods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, - scheduler: "sed", - kernelVersion: "4.19", - ipsetVersion: MinIPSetCheckVersion, - ok: false, - }, // case 9, ok for dh based IPVS scheduling { mods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack", "ip_vs_dh"}, @@ -363,14 +347,6 @@ func TestCanUseIPVSProxier(t *testing.T) { ipsetVersion: MinIPSetCheckVersion, ok: true, }, - // case 10, non-existent scheduler, error due to modules not existing - { - mods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack", "ip_vs_dh"}, - scheduler: "foobar", - kernelVersion: "4.19", - ipsetVersion: MinIPSetCheckVersion, - ok: false, - }, } for i := range testCases { From cd15ca054873f6e75faf21a44a01e797f07249e7 Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Thu, 22 Dec 2022 20:36:53 +0100 Subject: [PATCH 02/11] proxy/ipvs: Check that a dummy virtual server can be added This tests both ipvs and the configured scheduler --- cmd/kube-proxy/app/server_others.go | 4 +- pkg/proxy/ipvs/proxier.go | 56 ++++++++++- pkg/proxy/ipvs/proxier_test.go | 140 +++++++++++++++------------- 3 files changed, 130 insertions(+), 70 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index 9836584bfa4..c04fbc03c00 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -240,10 +240,10 @@ func newProxyServer( } else if proxyMode == proxyconfigapi.ProxyModeIPVS { kernelHandler := ipvs.NewLinuxKernelHandler() ipsetInterface = utilipset.New(execer) - if err := ipvs.CanUseIPVSProxier(kernelHandler, ipsetInterface, config.IPVS.Scheduler); err != nil { + ipvsInterface = utilipvs.New() + if err := ipvs.CanUseIPVSProxier(ipvsInterface, ipsetInterface, config.IPVS.Scheduler); err != nil { return nil, fmt.Errorf("can't use the IPVS proxier: %v", err) } - ipvsInterface = utilipvs.New() klog.InfoS("Using ipvs Proxier") if dualStack { diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 38b8a162dca..1867f0d4777 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -720,8 +720,60 @@ func (handle *LinuxKernelHandler) GetKernelVersion() (string, error) { } // CanUseIPVSProxier checks if we can use the ipvs Proxier. -// Only the ipset version is checked. Necessary kernel functions are assumed to be present. -func CanUseIPVSProxier(handle KernelHandler, ipsetver IPSetVersioner, scheduler string) error { +// If the ipvs already has virtual servers (VS) we assume this is a re-start and +// skip the tests. If the ipvs is empty we try to add a VS with the passed scheduler. +// If that works we assume that ipvs and the passed scheduler are supported. +// The ipset version is also checked. +func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, scheduler string) error { + // Check is any virtual servers (vs) are configured. If any, we assume + // that this is a kube-proxy re-start and skip the checks. + vservers, err := ipvs.GetVirtualServers() + if err != nil { + klog.ErrorS(err, "Can't read the ipvs") + return err + } + klog.V(5).InfoS("Virtual Servers", "count", len(vservers)) + if len(vservers) > 0 { + klog.InfoS("Assuming kube-proxy re-start", "virtual-servers", len(vservers)) + return nil + } + + // BUG: If ipvs is not compiled into the kernel ipvs.GetVirtualServers + // does not return an error. Instead also ipvs.AddVirtualServer returns OK + // (no error)! + + // This is the first time kube-proxy is loaded on the node. Try to + // insert a dummy VS with the passed scheduler. The VIP address can be + // anything since no traffic will be routed to ipvs yet. + vs := utilipvs.VirtualServer{ + Address: net.ParseIP("10.0.0.1"), + Protocol: "TCP", + Port: 20000, + Scheduler: scheduler, + } + if err := ipvs.AddVirtualServer(&vs); err != nil { + klog.ErrorS(err, "Could not create dummy VS", "scheduler", scheduler) + return err + } + + // To overcome the BUG described above we check that the VS is *really* added. + vservers, err = ipvs.GetVirtualServers() + if err != nil { + klog.ErrorS(err, "ipvs.GetVirtualServers") + return err + } + klog.V(5).InfoS("Virtual Servers after adding dummy", "count", len(vservers)) + if len(vservers) == 0 { + klog.InfoS("Dummy VS not created", "scheduler", scheduler) + return fmt.Errorf("Ipvs not supported") + } + + klog.V(5).InfoS("Dummy VS created", "vs", vs) + if err := ipvs.DeleteVirtualServer(&vs); err != nil { + klog.ErrorS(err, "Could not delete dummy VS") + return err + } + // Check ipset version versionString, err := ipsetver.GetVersion() if err != nil { diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index ba15bac1202..9e228c8eabe 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -75,21 +75,60 @@ func (f *fakeIPGetter) BindedIPs() (sets.String, error) { return f.bindedIPs, nil } -// fakeKernelHandler implements KernelHandler. -type fakeKernelHandler struct { - modules []string - kernelVersion string +// fakeIpvs implements utilipvs.Interface +type fakeIpvs struct { + ipvsErr string + vsCreated bool +} +func (f *fakeIpvs) Flush() error { + return nil +} +func (f *fakeIpvs) AddVirtualServer(*utilipvs.VirtualServer) error { + if f.ipvsErr == "AddVirtualServer" { + return fmt.Errorf("oops") + } + f.vsCreated = true + return nil +} +func (f *fakeIpvs) UpdateVirtualServer(*utilipvs.VirtualServer) error { + return nil +} +func (f *fakeIpvs) DeleteVirtualServer(*utilipvs.VirtualServer) error { + if f.ipvsErr == "DeleteVirtualServer" { + return fmt.Errorf("oops") + } + return nil +} +func (f *fakeIpvs) GetVirtualServer(*utilipvs.VirtualServer) (*utilipvs.VirtualServer, error) { + return nil, nil +} +func (f *fakeIpvs) GetVirtualServers() ([]*utilipvs.VirtualServer, error) { + if f.ipvsErr == "GetVirtualServers" { + return nil, fmt.Errorf("oops") + } + if f.vsCreated { + vs := []*utilipvs.VirtualServer{&utilipvs.VirtualServer{}} + return vs, nil + } + return nil, nil +} +func (f *fakeIpvs) AddRealServer(*utilipvs.VirtualServer, *utilipvs.RealServer) error { + return nil +} +func (f *fakeIpvs) GetRealServers(*utilipvs.VirtualServer) ([]*utilipvs.RealServer, error) { + return nil, nil +} +func (f *fakeIpvs) DeleteRealServer(*utilipvs.VirtualServer, *utilipvs.RealServer) error { + return nil +} +func (f *fakeIpvs) UpdateRealServer(*utilipvs.VirtualServer, *utilipvs.RealServer) error { + return nil +} +func (f *fakeIpvs) ConfigureTimeouts(time.Duration, time.Duration, time.Duration) error { + return nil } -func (fake *fakeKernelHandler) GetModules() ([]string, error) { - return fake.modules, nil -} - -func (fake *fakeKernelHandler) GetKernelVersion() (string, error) { - return fake.kernelVersion, nil -} - -// fakeKernelHandler implements KernelHandler. +// fakeIPSetVersioner implements IPSetVersioner. type fakeIPSetVersioner struct { version string err error @@ -273,88 +312,57 @@ func TestCleanupLeftovers(t *testing.T) { func TestCanUseIPVSProxier(t *testing.T) { testCases := []struct { - mods []string + name string scheduler string - kernelVersion string - kernelErr error ipsetVersion string ipsetErr error + ipvsErr string ok bool }{ - // case 0, kernel error { - mods: []string{"foo", "bar", "baz"}, - scheduler: "", - kernelVersion: "4.19", - kernelErr: fmt.Errorf("oops"), - ipsetVersion: "0.0", - ok: false, + name: "happy days", + ipsetVersion: MinIPSetCheckVersion, + ok: true, }, - // case 1, ipset error { - mods: []string{"foo", "bar", "baz"}, + name: "ipset error", scheduler: "", - kernelVersion: "4.19", ipsetVersion: MinIPSetCheckVersion, ipsetErr: fmt.Errorf("oops"), ok: false, }, - // case 2, missing required kernel modules and ipset version too low { - mods: []string{"foo", "bar", "baz"}, + name: "ipset version too low", scheduler: "rr", - kernelVersion: "4.19", - ipsetVersion: "1.1", - ok: false, - }, - // case 4, ipset version too low - { - mods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, - scheduler: "rr", - kernelVersion: "4.19", ipsetVersion: "4.3.0", ok: false, }, - // case 5, ok for linux kernel 4.19 { - mods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, - scheduler: "rr", - kernelVersion: "4.19", + name: "GetVirtualServers fail", ipsetVersion: MinIPSetCheckVersion, - ok: true, + ipvsErr: "GetVirtualServers", + ok: false, }, - // case 6, ok for linux kernel 4.18 { - mods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack_ipv4"}, - scheduler: "rr", - kernelVersion: "4.18", + name: "AddVirtualServer fail", ipsetVersion: MinIPSetCheckVersion, - ok: true, + ipvsErr: "AddVirtualServer", + ok: false, }, - // case 7. ok when module list has extra modules { - mods: []string{"foo", "ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack", "bar"}, - scheduler: "rr", - kernelVersion: "4.19", - ipsetVersion: "6.19", - ok: true, - }, - // case 9, ok for dh based IPVS scheduling - { - mods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack", "ip_vs_dh"}, - scheduler: "dh", - kernelVersion: "4.19", + name: "DeleteVirtualServer fail", ipsetVersion: MinIPSetCheckVersion, - ok: true, + ipvsErr: "DeleteVirtualServer", + ok: false, }, } - for i := range testCases { - handle := &fakeKernelHandler{modules: testCases[i].mods, kernelVersion: testCases[i].kernelVersion} - versioner := &fakeIPSetVersioner{version: testCases[i].ipsetVersion, err: testCases[i].ipsetErr} - 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) + for _, tc := range testCases { + ipvs := &fakeIpvs{tc.ipvsErr, false} + versioner := &fakeIPSetVersioner{version: tc.ipsetVersion, err: tc.ipsetErr} + err := CanUseIPVSProxier(ipvs, versioner, tc.scheduler) + if (err == nil) != tc.ok { + t.Errorf("Case [%s], expect %v, got err: %v", tc.name, tc.ok, err) } } } From b169f22eb8b3df3582028f0a37b79e9c8a02caef Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Fri, 23 Dec 2022 10:11:15 +0100 Subject: [PATCH 03/11] Updates after rewiew To update the scheduler without node reboot now works. The address for the probe VS is now 198.51.100.0 which is reseved for documentation, please see rfc5737. The comment about this is extended. --- pkg/proxy/ipvs/proxier.go | 54 +++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 1867f0d4777..e326078b46a 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -720,11 +720,20 @@ func (handle *LinuxKernelHandler) GetKernelVersion() (string, error) { } // CanUseIPVSProxier checks if we can use the ipvs Proxier. -// If the ipvs already has virtual servers (VS) we assume this is a re-start and -// skip the tests. If the ipvs is empty we try to add a VS with the passed scheduler. -// If that works we assume that ipvs and the passed scheduler are supported. -// The ipset version is also checked. +// The ipset version and the scheduler are checked. If any virtual servers (VS) +// already exist with the configured scheduler, we just return. Otherwise +// we check if a dummy VS can be configured with the configured scheduler. +// Kernel modules will be loaded automatically if necessary. func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, scheduler string) error { + // Check ipset version + versionString, err := ipsetver.GetVersion() + if err != nil { + return fmt.Errorf("error getting ipset version, error: %v", err) + } + if !checkMinVersion(versionString) { + return fmt.Errorf("ipset version: %s is less than min required version: %s", versionString, MinIPSetCheckVersion) + } + // Check is any virtual servers (vs) are configured. If any, we assume // that this is a kube-proxy re-start and skip the checks. vservers, err := ipvs.GetVirtualServers() @@ -734,19 +743,34 @@ func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, schedul } klog.V(5).InfoS("Virtual Servers", "count", len(vservers)) if len(vservers) > 0 { - klog.InfoS("Assuming kube-proxy re-start", "virtual-servers", len(vservers)) - return nil + klog.V(5).InfoS("Virtual server exists", "count", len(vservers)) + // This is most likely a kube-proxy re-start. We know that ipvs works + // and if any VS uses the configured scheduler, we are done. + for _, vs := range vservers { + if vs.Scheduler == scheduler { + return nil + } + } + klog.V(5).InfoS("No existing VS uses the configured scheduler", "scheduler", scheduler) } // BUG: If ipvs is not compiled into the kernel ipvs.GetVirtualServers // does not return an error. Instead also ipvs.AddVirtualServer returns OK // (no error)! - // This is the first time kube-proxy is loaded on the node. Try to - // insert a dummy VS with the passed scheduler. The VIP address can be - // anything since no traffic will be routed to ipvs yet. + // Try to insert a dummy VS with the passed scheduler. + // We should use a VIP address that is not used on the node. + // An address "198.51.100.0" from the TEST-NET-2 rage in https://datatracker.ietf.org/doc/html/rfc5737 + // is used. These addresses are reserved for documentation. If the user is using + // this address for a VS anyway we *will* mess up, but that would be an invalid configuration. + // If the user have configured the address to an interface on the node (but not a VS) + // then traffic will temporary be routed to ipvs during the probe and dropped. + // The later case is also and invalid configuration, but the traffic impact will be minor. + // This should not be a problem if users honors reserved addresses, but cut/paste + // from documentation is not unheard of, so the restricion to not use the TEST-NET-2 range + // must be documented. vs := utilipvs.VirtualServer{ - Address: net.ParseIP("10.0.0.1"), + Address: net.ParseIP("198.51.100.0"), Protocol: "TCP", Port: 20000, Scheduler: scheduler, @@ -765,7 +789,7 @@ func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, schedul klog.V(5).InfoS("Virtual Servers after adding dummy", "count", len(vservers)) if len(vservers) == 0 { klog.InfoS("Dummy VS not created", "scheduler", scheduler) - return fmt.Errorf("Ipvs not supported") + return fmt.Errorf("Ipvs not supported") // This is a BUG work-around } klog.V(5).InfoS("Dummy VS created", "vs", vs) @@ -774,14 +798,6 @@ func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, schedul return err } - // Check ipset version - versionString, err := ipsetver.GetVersion() - if err != nil { - return fmt.Errorf("error getting ipset version, error: %v", err) - } - if !checkMinVersion(versionString) { - return fmt.Errorf("ipset version: %s is less than min required version: %s", versionString, MinIPSetCheckVersion) - } return nil } From 3bd3759424bc8bd7b0e42a8ee639897d110e256a Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Fri, 23 Dec 2022 10:14:47 +0100 Subject: [PATCH 04/11] gofmt --- pkg/proxy/ipvs/proxier.go | 6 ++-- pkg/proxy/ipvs/proxier_test.go | 65 +++++++++++++++++----------------- 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index e326078b46a..f96efa85057 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -770,9 +770,9 @@ func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, schedul // from documentation is not unheard of, so the restricion to not use the TEST-NET-2 range // must be documented. vs := utilipvs.VirtualServer{ - Address: net.ParseIP("198.51.100.0"), - Protocol: "TCP", - Port: 20000, + Address: net.ParseIP("198.51.100.0"), + Protocol: "TCP", + Port: 20000, Scheduler: scheduler, } if err := ipvs.AddVirtualServer(&vs); err != nil { diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 9e228c8eabe..203f0ffb0c3 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -77,9 +77,10 @@ func (f *fakeIPGetter) BindedIPs() (sets.String, error) { // fakeIpvs implements utilipvs.Interface type fakeIpvs struct { - ipvsErr string + ipvsErr string vsCreated bool } + func (f *fakeIpvs) Flush() error { return nil } @@ -107,7 +108,7 @@ func (f *fakeIpvs) GetVirtualServers() ([]*utilipvs.VirtualServer, error) { return nil, fmt.Errorf("oops") } if f.vsCreated { - vs := []*utilipvs.VirtualServer{&utilipvs.VirtualServer{}} + vs := []*utilipvs.VirtualServer{{}} return vs, nil } return nil, nil @@ -312,48 +313,48 @@ func TestCleanupLeftovers(t *testing.T) { func TestCanUseIPVSProxier(t *testing.T) { testCases := []struct { - name string - scheduler string - ipsetVersion string - ipsetErr error - ipvsErr string - ok bool + name string + scheduler string + ipsetVersion string + ipsetErr error + ipvsErr string + ok bool }{ { - name: "happy days", - ipsetVersion: MinIPSetCheckVersion, - ok: true, + name: "happy days", + ipsetVersion: MinIPSetCheckVersion, + ok: true, }, { - name: "ipset error", - scheduler: "", - ipsetVersion: MinIPSetCheckVersion, - ipsetErr: fmt.Errorf("oops"), - ok: false, + name: "ipset error", + scheduler: "", + ipsetVersion: MinIPSetCheckVersion, + ipsetErr: fmt.Errorf("oops"), + ok: false, }, { - name: "ipset version too low", - scheduler: "rr", - ipsetVersion: "4.3.0", - ok: false, + name: "ipset version too low", + scheduler: "rr", + ipsetVersion: "4.3.0", + ok: false, }, { - name: "GetVirtualServers fail", - ipsetVersion: MinIPSetCheckVersion, - ipvsErr: "GetVirtualServers", - ok: false, + name: "GetVirtualServers fail", + ipsetVersion: MinIPSetCheckVersion, + ipvsErr: "GetVirtualServers", + ok: false, }, { - name: "AddVirtualServer fail", - ipsetVersion: MinIPSetCheckVersion, - ipvsErr: "AddVirtualServer", - ok: false, + name: "AddVirtualServer fail", + ipsetVersion: MinIPSetCheckVersion, + ipvsErr: "AddVirtualServer", + ok: false, }, { - name: "DeleteVirtualServer fail", - ipsetVersion: MinIPSetCheckVersion, - ipvsErr: "DeleteVirtualServer", - ok: false, + name: "DeleteVirtualServer fail", + ipsetVersion: MinIPSetCheckVersion, + ipvsErr: "DeleteVirtualServer", + ok: false, }, } From cf214d07385ddcf612b432136e0ec799583e03f0 Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Fri, 23 Dec 2022 10:54:51 +0100 Subject: [PATCH 05/11] Clean-up un-used code --- pkg/proxy/ipvs/proxier.go | 69 --------------------------------------- pkg/util/ipvs/ipvs.go | 29 ---------------- 2 files changed, 98 deletions(-) diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index f96efa85057..a4ceb050da4 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -24,7 +24,6 @@ import ( "net" "os" "reflect" - "regexp" "strconv" "strings" "sync" @@ -606,7 +605,6 @@ func newServiceInfo(port *v1.ServicePort, service *v1.Service, bsvcPortInfo *pro // KernelHandler can handle the current installed kernel modules. type KernelHandler interface { - GetModules() ([]string, error) GetKernelVersion() (string, error) } @@ -622,73 +620,6 @@ func NewLinuxKernelHandler() *LinuxKernelHandler { } } -// GetModules returns all installed kernel modules. -func (handle *LinuxKernelHandler) GetModules() ([]string, error) { - // Check whether IPVS required kernel modules are built-in - kernelVersionStr, err := handle.GetKernelVersion() - if err != nil { - return nil, err - } - kernelVersion, err := version.ParseGeneric(kernelVersionStr) - if err != nil { - return nil, fmt.Errorf("error parsing kernel version %q: %v", kernelVersionStr, err) - } - ipvsModules := utilipvs.GetRequiredIPVSModules(kernelVersion) - - var bmods, lmods []string - - // Find out loaded kernel modules. If this is a full static kernel it will try to verify if the module is compiled using /boot/config-KERNELVERSION - modulesFile, err := os.Open("/proc/modules") - if err == os.ErrNotExist { - klog.ErrorS(err, "Failed to read file /proc/modules, assuming this is a kernel without loadable modules support enabled") - kernelConfigFile := fmt.Sprintf("/boot/config-%s", kernelVersionStr) - kConfig, err := os.ReadFile(kernelConfigFile) - if err != nil { - return nil, fmt.Errorf("failed to read Kernel Config file %s with error %w", kernelConfigFile, err) - } - for _, module := range ipvsModules { - if match, _ := regexp.Match("CONFIG_"+strings.ToUpper(module)+"=y", kConfig); match { - bmods = append(bmods, module) - } - } - return bmods, nil - } - if err != nil { - return nil, fmt.Errorf("failed to read file /proc/modules with error %w", err) - } - defer modulesFile.Close() - - mods, err := getFirstColumn(modulesFile) - if err != nil { - return nil, fmt.Errorf("failed to find loaded kernel modules: %v", err) - } - - builtinModsFilePath := fmt.Sprintf("/lib/modules/%s/modules.builtin", kernelVersionStr) - b, err := os.ReadFile(builtinModsFilePath) - if err != nil { - klog.ErrorS(err, "Failed to read builtin modules file, you can ignore this message when kube-proxy is running inside container without mounting /lib/modules", "filePath", builtinModsFilePath) - } - - for _, module := range ipvsModules { - if match, _ := regexp.Match(module+".ko", b); match { - bmods = append(bmods, module) - } else { - // Try to load the required IPVS kernel modules if not built in - err := handle.executor.Command("modprobe", "--", module).Run() - if err != nil { - klog.InfoS("Failed to load kernel module with modprobe, "+ - "you can ignore this message when kube-proxy is running inside container without mounting /lib/modules", "moduleName", module) - } else { - lmods = append(lmods, module) - } - } - } - - mods = append(mods, bmods...) - mods = append(mods, lmods...) - return mods, nil -} - // getFirstColumn reads all the content from r into memory and return a // slice which consists of the first word from each line. func getFirstColumn(r io.Reader) ([]string, error) { diff --git a/pkg/util/ipvs/ipvs.go b/pkg/util/ipvs/ipvs.go index 419c5f46af0..2bb08e430a5 100644 --- a/pkg/util/ipvs/ipvs.go +++ b/pkg/util/ipvs/ipvs.go @@ -21,8 +21,6 @@ import ( "strconv" "strings" "time" - - "k8s.io/apimachinery/pkg/util/version" ) // Interface is an injectable interface for running ipvs commands. Implementations must be goroutine-safe. @@ -71,22 +69,6 @@ const ( FlagHashed = 0x2 ) -// IPVS required kernel modules. -const ( - // KernelModuleIPVS is the kernel module "ip_vs" - KernelModuleIPVS string = "ip_vs" - // KernelModuleIPVSRR is the kernel module "ip_vs_rr" - KernelModuleIPVSRR string = "ip_vs_rr" - // KernelModuleIPVSWRR is the kernel module "ip_vs_wrr" - KernelModuleIPVSWRR string = "ip_vs_wrr" - // KernelModuleIPVSSH is the kernel module "ip_vs_sh" - KernelModuleIPVSSH string = "ip_vs_sh" - // KernelModuleNfConntrackIPV4 is the module "nf_conntrack_ipv4" - KernelModuleNfConntrackIPV4 string = "nf_conntrack_ipv4" - // KernelModuleNfConntrack is the kernel module "nf_conntrack" - KernelModuleNfConntrack string = "nf_conntrack" -) - // Equal check the equality of virtual server. // We don't use struct == since it doesn't work because of slice. func (svc *VirtualServer) Equal(other *VirtualServer) bool { @@ -122,17 +104,6 @@ func (rs *RealServer) Equal(other *RealServer) bool { rs.Port == other.Port } -// GetRequiredIPVSModules returns the required ipvs modules for the given linux kernel version. -func GetRequiredIPVSModules(kernelVersion *version.Version) []string { - // "nf_conntrack_ipv4" has been removed since v4.19 - // see https://github.com/torvalds/linux/commit/a0ae2562c6c4b2721d9fddba63b7286c13517d9f - if kernelVersion.LessThan(version.MustParseGeneric("4.19")) { - return []string{KernelModuleIPVS, KernelModuleIPVSRR, KernelModuleIPVSWRR, KernelModuleIPVSSH, KernelModuleNfConntrackIPV4} - } - return []string{KernelModuleIPVS, KernelModuleIPVSRR, KernelModuleIPVSWRR, KernelModuleIPVSSH, KernelModuleNfConntrack} - -} - // IsRsGracefulTerminationNeeded returns true if protocol requires graceful termination for the stale connections func IsRsGracefulTerminationNeeded(proto string) bool { return !strings.EqualFold(proto, "UDP") && !strings.EqualFold(proto, "SCTP") From 4adc687275e94a3bf610dd1719524efb1d6395e3 Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Fri, 23 Dec 2022 11:13:55 +0100 Subject: [PATCH 06/11] Fixed typo --- pkg/proxy/ipvs/proxier.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index a4ceb050da4..1562f3e6729 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -698,7 +698,7 @@ func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, schedul // then traffic will temporary be routed to ipvs during the probe and dropped. // The later case is also and invalid configuration, but the traffic impact will be minor. // This should not be a problem if users honors reserved addresses, but cut/paste - // from documentation is not unheard of, so the restricion to not use the TEST-NET-2 range + // from documentation is not unheard of, so the restriction to not use the TEST-NET-2 range // must be documented. vs := utilipvs.VirtualServer{ Address: net.ParseIP("198.51.100.0"), From 4ef7726c4e48332f0fad2a21af9203f5a8a2cf34 Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Fri, 23 Dec 2022 11:46:45 +0100 Subject: [PATCH 07/11] Remove obsolete test case --- pkg/util/ipvs/ipvs_test.go | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/pkg/util/ipvs/ipvs_test.go b/pkg/util/ipvs/ipvs_test.go index 64a10b2a5cd..e475c7f4141 100644 --- a/pkg/util/ipvs/ipvs_test.go +++ b/pkg/util/ipvs/ipvs_test.go @@ -17,11 +17,8 @@ limitations under the License. package ipvs import ( - "reflect" - "sort" "testing" - "k8s.io/apimachinery/pkg/util/version" netutils "k8s.io/utils/net" ) @@ -385,31 +382,3 @@ func TestFrontendDestinationString(t *testing.T) { } } -func TestGetRequiredIPVSModules(t *testing.T) { - Tests := []struct { - name string - kernelVersion *version.Version - want []string - }{ - { - name: "kernel version < 4.19", - kernelVersion: version.MustParseGeneric("4.18"), - want: []string{KernelModuleIPVS, KernelModuleIPVSRR, KernelModuleIPVSWRR, KernelModuleIPVSSH, KernelModuleNfConntrackIPV4}, - }, - { - name: "kernel version 4.19", - kernelVersion: version.MustParseGeneric("4.19"), - want: []string{KernelModuleIPVS, KernelModuleIPVSRR, KernelModuleIPVSWRR, KernelModuleIPVSSH, KernelModuleNfConntrack}, - }, - } - for _, test := range Tests { - t.Run(test.name, func(t *testing.T) { - got := GetRequiredIPVSModules(test.kernelVersion) - sort.Strings(got) - sort.Strings(test.want) - if !reflect.DeepEqual(got, test.want) { - t.Errorf("GetRequiredIPVSMods() = %v for kenel version: %s, want %v", got, test.kernelVersion, test.want) - } - }) - } -} From 90c03dcf9a38206eef5875cf3d19283b5d4cac80 Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Fri, 23 Dec 2022 12:34:31 +0100 Subject: [PATCH 08/11] gofmt --- pkg/util/ipvs/ipvs_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/util/ipvs/ipvs_test.go b/pkg/util/ipvs/ipvs_test.go index e475c7f4141..56c070e9449 100644 --- a/pkg/util/ipvs/ipvs_test.go +++ b/pkg/util/ipvs/ipvs_test.go @@ -381,4 +381,3 @@ func TestFrontendDestinationString(t *testing.T) { } } } - From dc86bdc3aae34707db63ab1e622a4140f8457167 Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Fri, 23 Dec 2022 13:23:02 +0100 Subject: [PATCH 09/11] Handle an empty scheduler ("") --- pkg/proxy/ipvs/proxier.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 1562f3e6729..6fe7aabc81a 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -665,6 +665,10 @@ func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, schedul return fmt.Errorf("ipset version: %s is less than min required version: %s", versionString, MinIPSetCheckVersion) } + if scheduler == "" { + scheduler = defaultScheduler + } + // Check is any virtual servers (vs) are configured. If any, we assume // that this is a kube-proxy re-start and skip the checks. vservers, err := ipvs.GetVirtualServers() From 68d78c89eccf2d2f381033c836472d53f3ba41c9 Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Fri, 23 Dec 2022 14:19:28 +0100 Subject: [PATCH 10/11] use netutils.ParseIPSloppy --- pkg/proxy/ipvs/proxier.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 6fe7aabc81a..d12bccf4f1e 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -705,7 +705,7 @@ func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, schedul // from documentation is not unheard of, so the restriction to not use the TEST-NET-2 range // must be documented. vs := utilipvs.VirtualServer{ - Address: net.ParseIP("198.51.100.0"), + Address: netutils.ParseIPSloppy("198.51.100.0"), Protocol: "TCP", Port: 20000, Scheduler: scheduler, From 5ff705fd77fff7714959cd6451c5633353880b14 Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Sat, 24 Dec 2022 10:21:27 +0100 Subject: [PATCH 11/11] proxy/ipvs: Describe and handle a bug in moby/ipvs Handle https://github.com/moby/ipvs/issues/27 A work-around was already in place, but a segv would occur when the bug is fixed. That will not happen now. --- pkg/proxy/ipvs/proxier.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index d12bccf4f1e..fbe0e7876d4 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -656,6 +656,15 @@ func (handle *LinuxKernelHandler) GetKernelVersion() (string, error) { // we check if a dummy VS can be configured with the configured scheduler. // Kernel modules will be loaded automatically if necessary. func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, scheduler string) error { + // BUG: https://github.com/moby/ipvs/issues/27 + // If ipvs is not compiled into the kernel no error is returned and handle==nil. + // This in turn causes ipvs.GetVirtualServers and ipvs.AddVirtualServer + // to return ok (err==nil). If/when this bug is fixed parameter "ipvs" will be nil + // if ipvs is not supported by the kernel. Until then a re-read work-around is used. + if ipvs == nil { + return fmt.Errorf("Ipvs not supported by the kernel") + } + // Check ipset version versionString, err := ipsetver.GetVersion() if err != nil { @@ -669,8 +678,7 @@ func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, schedul scheduler = defaultScheduler } - // Check is any virtual servers (vs) are configured. If any, we assume - // that this is a kube-proxy re-start and skip the checks. + // If any virtual server (VS) using the scheduler exist we skip the checks. vservers, err := ipvs.GetVirtualServers() if err != nil { klog.ErrorS(err, "Can't read the ipvs") @@ -678,21 +686,17 @@ func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, schedul } klog.V(5).InfoS("Virtual Servers", "count", len(vservers)) if len(vservers) > 0 { - klog.V(5).InfoS("Virtual server exists", "count", len(vservers)) // This is most likely a kube-proxy re-start. We know that ipvs works // and if any VS uses the configured scheduler, we are done. for _, vs := range vservers { if vs.Scheduler == scheduler { + klog.V(5).InfoS("VS exist, Skipping checks") return nil } } klog.V(5).InfoS("No existing VS uses the configured scheduler", "scheduler", scheduler) } - // BUG: If ipvs is not compiled into the kernel ipvs.GetVirtualServers - // does not return an error. Instead also ipvs.AddVirtualServer returns OK - // (no error)! - // Try to insert a dummy VS with the passed scheduler. // We should use a VIP address that is not used on the node. // An address "198.51.100.0" from the TEST-NET-2 rage in https://datatracker.ietf.org/doc/html/rfc5737 @@ -726,8 +730,8 @@ func CanUseIPVSProxier(ipvs utilipvs.Interface, ipsetver IPSetVersioner, schedul klog.InfoS("Dummy VS not created", "scheduler", scheduler) return fmt.Errorf("Ipvs not supported") // This is a BUG work-around } - klog.V(5).InfoS("Dummy VS created", "vs", vs) + if err := ipvs.DeleteVirtualServer(&vs); err != nil { klog.ErrorS(err, "Could not delete dummy VS") return err