Merge pull request #66057 from xujieasd/cleanLeacyBindAddr-test

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))
+	}
+}

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