From cd15ca054873f6e75faf21a44a01e797f07249e7 Mon Sep 17 00:00:00 2001 From: Lars Ekman Date: Thu, 22 Dec 2022 20:36:53 +0100 Subject: [PATCH] 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) } } }