diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 5499482a739..921ff01f0d1 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -2708,6 +2708,7 @@ func Test_syncService(t *testing.T) { ipset := ipsettest.NewFake(testIPSetVersion) proxier := NewFakeProxier(ipt, ipvs, ipset, nil) + proxier.netlinkHandle.EnsureDummyDevice(DefaultDummyDevice) if testCases[i].oldVirtualServer != nil { if err := proxier.ipvs.AddVirtualServer(testCases[i].oldVirtualServer); err != nil { t.Errorf("Case [%d], unexpected add IPVS virtual server error: %v", i, err) @@ -2906,3 +2907,109 @@ func TestCleanLegacyService(t *testing.T) { } } } + +func TestCleanLegacyBindAddr(t *testing.T) { + ipt := iptablestest.NewFake() + ipvs := ipvstest.NewFake() + ipset := ipsettest.NewFake(testIPSetVersion) + fp := NewFakeProxier(ipt, ipvs, ipset, nil) + + // All ipvs service addresses that were bound to ipvs0 in the latest sync loop. + activeBindAddrs := map[string]bool{"1.2.3.4": true, "1002:ab8::2:1": true} + // All service addresses that were bound to ipvs0 in system + currentBindAddrs := []string{"1.2.3.4", "1.2.3.5", "1.2.3.6", "1002:ab8::2:1", "1002:ab8::2:2"} + + fp.netlinkHandle.EnsureDummyDevice(DefaultDummyDevice) + + for i := range currentBindAddrs { + fp.netlinkHandle.EnsureAddressBind(currentBindAddrs[i], DefaultDummyDevice) + } + fp.cleanLegacyBindAddr(activeBindAddrs, currentBindAddrs) + + remainingAddrs, _ := fp.netlinkHandle.ListBindAddress(DefaultDummyDevice) + // should only remain "1.2.3.4" and "1002:ab8::2:1" + if len(remainingAddrs) != 2 { + t.Errorf("Expected number of remaining bound addrs after cleanup to be %v. Got %v", 2, len(remainingAddrs)) + } + + // check that address "1.2.3.4" and "1002:ab8::2:1" remain + remainingAddrsMap := make(map[string]bool) + for i := range remainingAddrs { + remainingAddrsMap[remainingAddrs[i]] = true + } + if !reflect.DeepEqual(activeBindAddrs, remainingAddrsMap) { + t.Errorf("Expected remainingAddrsMap %v, got %v", activeBindAddrs, remainingAddrsMap) + } +} + +func TestMultiPortServiceBindAddr(t *testing.T) { + ipt := iptablestest.NewFake() + ipvs := ipvstest.NewFake() + ipset := ipsettest.NewFake(testIPSetVersion) + fp := NewFakeProxier(ipt, ipvs, ipset, nil) + + service1 := makeTestService("ns1", "svc1", func(svc *v1.Service) { + svc.Spec.Type = v1.ServiceTypeClusterIP + svc.Spec.ClusterIP = "172.16.55.4" + svc.Spec.Ports = addTestPort(svc.Spec.Ports, "port1", "TCP", 1234, 0, 0) + svc.Spec.Ports = addTestPort(svc.Spec.Ports, "port2", "TCP", 1235, 0, 0) + }) + service2 := makeTestService("ns1", "svc1", func(svc *v1.Service) { + svc.Spec.Type = v1.ServiceTypeClusterIP + svc.Spec.ClusterIP = "172.16.55.4" + svc.Spec.Ports = addTestPort(svc.Spec.Ports, "port1", "TCP", 1234, 0, 0) + }) + service3 := makeTestService("ns1", "svc1", func(svc *v1.Service) { + svc.Spec.Type = v1.ServiceTypeClusterIP + svc.Spec.ClusterIP = "172.16.55.4" + svc.Spec.Ports = addTestPort(svc.Spec.Ports, "port1", "TCP", 1234, 0, 0) + svc.Spec.Ports = addTestPort(svc.Spec.Ports, "port2", "TCP", 1235, 0, 0) + svc.Spec.Ports = addTestPort(svc.Spec.Ports, "port3", "UDP", 1236, 0, 0) + }) + + fp.servicesSynced = true + fp.endpointsSynced = true + + // first, add multi-port service1 + fp.OnServiceAdd(service1) + fp.syncProxyRules() + remainingAddrs, _ := fp.netlinkHandle.ListBindAddress(DefaultDummyDevice) + // should only remain address "172.16.55.4" + if len(remainingAddrs) != 1 { + t.Errorf("Expected number of remaining bound addrs after cleanup to be %v. Got %v", 1, len(remainingAddrs)) + } + if remainingAddrs[0] != "172.16.55.4" { + t.Errorf("Expected remaining address should be %s, got %s", "172.16.55.4", remainingAddrs[0]) + } + + // update multi-port service1 to single-port service2 + fp.OnServiceUpdate(service1, service2) + fp.syncProxyRules() + remainingAddrs, _ = fp.netlinkHandle.ListBindAddress(DefaultDummyDevice) + // should still only remain address "172.16.55.4" + if len(remainingAddrs) != 1 { + t.Errorf("Expected number of remaining bound addrs after cleanup to be %v. Got %v", 1, len(remainingAddrs)) + } else if remainingAddrs[0] != "172.16.55.4" { + t.Errorf("Expected remaining address should be %s, got %s", "172.16.55.4", remainingAddrs[0]) + } + + // update single-port service2 to multi-port service3 + fp.OnServiceUpdate(service2, service3) + fp.syncProxyRules() + remainingAddrs, _ = fp.netlinkHandle.ListBindAddress(DefaultDummyDevice) + // should still only remain address "172.16.55.4" + if len(remainingAddrs) != 1 { + t.Errorf("Expected number of remaining bound addrs after cleanup to be %v. Got %v", 1, len(remainingAddrs)) + } else if remainingAddrs[0] != "172.16.55.4" { + t.Errorf("Expected remaining address should be %s, got %s", "172.16.55.4", remainingAddrs[0]) + } + + // delete multi-port service3 + fp.OnServiceDelete(service3) + fp.syncProxyRules() + remainingAddrs, _ = fp.netlinkHandle.ListBindAddress(DefaultDummyDevice) + // all addresses should be unbound + if len(remainingAddrs) != 0 { + t.Errorf("Expected number of remaining bound addrs after cleanup to be %v. Got %v", 0, len(remainingAddrs)) + } +} diff --git a/pkg/proxy/ipvs/testing/fake.go b/pkg/proxy/ipvs/testing/fake.go index 39fe7dba2d5..1f0a98fdd72 100644 --- a/pkg/proxy/ipvs/testing/fake.go +++ b/pkg/proxy/ipvs/testing/fake.go @@ -39,27 +39,76 @@ func NewFakeNetlinkHandle() *FakeNetlinkHandle { // EnsureAddressBind is a mock implementation func (h *FakeNetlinkHandle) EnsureAddressBind(address, devName string) (exist bool, err error) { + if len(devName) == 0 { + return false, fmt.Errorf("Device name can't be empty") + } + if _, ok := h.localAddresses[devName]; !ok { + return false, fmt.Errorf("Error bind address: %s to a non-exist interface: %s", address, devName) + } + for _, addr := range h.localAddresses[devName] { + if addr == address { + // return true if the address is already bound to device + return true, nil + } + } + h.localAddresses[devName] = append(h.localAddresses[devName], address) return false, nil } // UnbindAddress is a mock implementation func (h *FakeNetlinkHandle) UnbindAddress(address, devName string) error { - return nil + if len(devName) == 0 { + return fmt.Errorf("Device name can't be empty") + } + if _, ok := h.localAddresses[devName]; !ok { + return fmt.Errorf("Error unbind address: %s from a non-exist interface: %s", address, devName) + } + for i, addr := range h.localAddresses[devName] { + if addr == address { + // delete address from slice h.localAddresses[devName] + h.localAddresses[devName] = append(h.localAddresses[devName][:i], h.localAddresses[devName][i+1:]...) + return nil + } + } + // return error message if address is not found in slice h.localAddresses[devName] + return fmt.Errorf("Address: %s is not found in interface: %s", address, devName) } // EnsureDummyDevice is a mock implementation func (h *FakeNetlinkHandle) EnsureDummyDevice(devName string) (bool, error) { - return false, nil + if len(devName) == 0 { + return false, fmt.Errorf("Device name can't be empty") + } + if _, ok := h.localAddresses[devName]; !ok { + // create dummy interface if devName is not found in localAddress map + h.localAddresses[devName] = make([]string, 0) + return false, nil + } + // return true if devName is already created in localAddress map + return true, nil } // DeleteDummyDevice is a mock implementation func (h *FakeNetlinkHandle) DeleteDummyDevice(devName string) error { + if len(devName) == 0 { + return fmt.Errorf("Device name can't be empty") + } + if _, ok := h.localAddresses[devName]; !ok { + return fmt.Errorf("Error deleting a non-exist interface: %s", devName) + } + delete(h.localAddresses, devName) return nil } // ListBindAddress is a mock implementation func (h *FakeNetlinkHandle) ListBindAddress(devName string) ([]string, error) { - return nil, nil + if len(devName) == 0 { + return nil, fmt.Errorf("Device name can't be empty") + } + if _, ok := h.localAddresses[devName]; !ok { + return nil, fmt.Errorf("Error list addresses from a non-exist interface: %s", devName) + } + return h.localAddresses[devName], nil } // GetLocalAddresses is a mock implementation