From 0c23f5093f64881ffa738717272b6975f54320b1 Mon Sep 17 00:00:00 2001 From: AllenZMC Date: Sun, 1 May 2022 17:37:23 +0800 Subject: [PATCH] Optimize test cases for ipvs --- pkg/proxy/ipvs/proxier_test.go | 143 ++++++++++++++++++--------------- 1 file changed, 80 insertions(+), 63 deletions(-) diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 2f6cbb2f05d..81d6e8b66b4 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -3079,6 +3079,7 @@ func Test_updateEndpointsMap(t *testing.T) { // previousEndpoints and currentEndpoints are used to call appropriate // handlers OnEndpoints* (based on whether corresponding values are nil // or non-nil) and must be of equal length. + name string previousEndpoints []*discovery.EndpointSlice currentEndpoints []*discovery.EndpointSlice oldEndpoints map[proxy.ServicePortName][]*proxy.BaseEndpointInfo @@ -3088,6 +3089,7 @@ func Test_updateEndpointsMap(t *testing.T) { expectedHealthchecks map[types.NamespacedName]int }{{ // Case[0]: nothing + name: "nothing", oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{}, expectedResult: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{}, expectedStaleEndpoints: []proxy.ServiceEndpoint{}, @@ -3095,6 +3097,7 @@ func Test_updateEndpointsMap(t *testing.T) { expectedHealthchecks: map[types.NamespacedName]int{}, }, { // Case[1]: no change, named port, local + name: "no change, named port, local", previousEndpoints: namedPortLocal, currentEndpoints: namedPortLocal, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3114,6 +3117,7 @@ func Test_updateEndpointsMap(t *testing.T) { }, }, { // Case[2]: no change, multiple subsets + name: "no change, multiple subsets", previousEndpoints: multipleSubsets, currentEndpoints: multipleSubsets, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3137,6 +3141,7 @@ func Test_updateEndpointsMap(t *testing.T) { expectedHealthchecks: map[types.NamespacedName]int{}, }, { // Case[3]: no change, multiple subsets, multiple ports, local + name: "no change, multiple subsets, multiple ports, local", previousEndpoints: multipleSubsetsMultiplePortsLocal, currentEndpoints: multipleSubsetsMultiplePortsLocal, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3168,6 +3173,7 @@ func Test_updateEndpointsMap(t *testing.T) { }, }, { // Case[4]: no change, multiple endpoints, subsets, IPs, and ports + name: "no change, multiple endpoints, subsets, IPs, and ports", previousEndpoints: multipleSubsetsIPsPorts, currentEndpoints: multipleSubsetsIPsPorts, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3230,6 +3236,7 @@ func Test_updateEndpointsMap(t *testing.T) { }, }, { // Case[5]: add an Endpoints + name: "add an Endpoints", previousEndpoints: []*discovery.EndpointSlice{nil}, currentEndpoints: namedPortLocal, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{}, @@ -3247,6 +3254,7 @@ func Test_updateEndpointsMap(t *testing.T) { }, }, { // Case[6]: remove an Endpoints + name: "remove an Endpoints", previousEndpoints: namedPortLocal, currentEndpoints: []*discovery.EndpointSlice{nil}, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3263,6 +3271,7 @@ func Test_updateEndpointsMap(t *testing.T) { expectedHealthchecks: map[types.NamespacedName]int{}, }, { // Case[7]: add an IP and port + name: "add an IP and port", previousEndpoints: namedPort, currentEndpoints: namedPortsLocalNoLocal, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3289,6 +3298,7 @@ func Test_updateEndpointsMap(t *testing.T) { }, }, { // Case[8]: remove an IP and port + name: "remove an IP and port", previousEndpoints: namedPortsLocalNoLocal, currentEndpoints: namedPort, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3320,6 +3330,7 @@ func Test_updateEndpointsMap(t *testing.T) { expectedHealthchecks: map[types.NamespacedName]int{}, }, { // Case[9]: add a subset + name: "add a subset", previousEndpoints: []*discovery.EndpointSlice{namedPort[0], nil}, currentEndpoints: multipleSubsetsWithLocal, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3344,6 +3355,7 @@ func Test_updateEndpointsMap(t *testing.T) { }, }, { // Case[10]: remove a subset + name: "remove a subset", previousEndpoints: multipleSubsets, currentEndpoints: []*discovery.EndpointSlice{namedPort[0], nil}, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3367,6 +3379,7 @@ func Test_updateEndpointsMap(t *testing.T) { expectedHealthchecks: map[types.NamespacedName]int{}, }, { // Case[11]: rename a port + name: "rename a port", previousEndpoints: namedPort, currentEndpoints: namedPortRenamed, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3389,6 +3402,7 @@ func Test_updateEndpointsMap(t *testing.T) { expectedHealthchecks: map[types.NamespacedName]int{}, }, { // Case[12]: renumber a port + name: "renumber a port", previousEndpoints: namedPort, currentEndpoints: namedPortRenumbered, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3409,6 +3423,7 @@ func Test_updateEndpointsMap(t *testing.T) { expectedHealthchecks: map[types.NamespacedName]int{}, }, { // Case[13]: complex add and remove + name: "complex add and remove", previousEndpoints: complexBefore, currentEndpoints: complexAfter, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{ @@ -3473,7 +3488,8 @@ func Test_updateEndpointsMap(t *testing.T) { makeNSN("ns4", "ep4"): 1, }, }, { - // Case[15]: change from 0 endpoint address to 1 named port + // Case[14]: change from 0 endpoint address to 1 named port + name: "change from 0 endpoint address to 1 named port", previousEndpoints: emptyEndpointSlices, currentEndpoints: namedPort, oldEndpoints: map[proxy.ServicePortName][]*proxy.BaseEndpointInfo{}, @@ -3491,75 +3507,76 @@ func Test_updateEndpointsMap(t *testing.T) { } for tci, tc := range testCases { - ipt := iptablestest.NewFake() - ipvs := ipvstest.NewFake() - ipset := ipsettest.NewFake(testIPSetVersion) - fp := NewFakeProxier(ipt, ipvs, ipset, nil, nil, v1.IPv4Protocol) - fp.hostname = nodeName + t.Run(tc.name, func(t *testing.T) { + ipt := iptablestest.NewFake() + ipvs := ipvstest.NewFake() + ipset := ipsettest.NewFake(testIPSetVersion) + fp := NewFakeProxier(ipt, ipvs, ipset, nil, nil, v1.IPv4Protocol) + fp.hostname = nodeName - // First check that after adding all previous versions of endpoints, - // the fp.oldEndpoints is as we expect. - for i := range tc.previousEndpoints { - if tc.previousEndpoints[i] != nil { - fp.OnEndpointSliceAdd(tc.previousEndpoints[i]) - } - } - fp.endpointsMap.Update(fp.endpointsChanges) - compareEndpointsMaps(t, tci, fp.endpointsMap, tc.oldEndpoints) - - // Now let's call appropriate handlers to get to state we want to be. - if len(tc.previousEndpoints) != len(tc.currentEndpoints) { - t.Fatalf("[%d] different lengths of previous and current endpoints", tci) - continue - } - - for i := range tc.previousEndpoints { - prev, curr := tc.previousEndpoints[i], tc.currentEndpoints[i] - switch { - case prev == nil: - fp.OnEndpointSliceAdd(curr) - case curr == nil: - fp.OnEndpointSliceDelete(prev) - default: - fp.OnEndpointSliceUpdate(prev, curr) - } - } - result := fp.endpointsMap.Update(fp.endpointsChanges) - newMap := fp.endpointsMap - compareEndpointsMaps(t, tci, newMap, tc.expectedResult) - if len(result.StaleEndpoints) != len(tc.expectedStaleEndpoints) { - t.Errorf("[%d] expected %d staleEndpoints, got %d: %v", tci, len(tc.expectedStaleEndpoints), len(result.StaleEndpoints), result.StaleEndpoints) - } - for _, x := range tc.expectedStaleEndpoints { - found := false - for _, stale := range result.StaleEndpoints { - if stale == x { - found = true - break + // First check that after adding all previous versions of endpoints, + // the fp.oldEndpoints is as we expect. + for i := range tc.previousEndpoints { + if tc.previousEndpoints[i] != nil { + fp.OnEndpointSliceAdd(tc.previousEndpoints[i]) } } - if !found { - t.Errorf("[%d] expected staleEndpoints[%v], but didn't find it: %v", tci, x, result.StaleEndpoints) + fp.endpointsMap.Update(fp.endpointsChanges) + compareEndpointsMaps(t, tci, fp.endpointsMap, tc.oldEndpoints) + + // Now let's call appropriate handlers to get to state we want to be. + if len(tc.previousEndpoints) != len(tc.currentEndpoints) { + t.Fatalf("[%d] different lengths of previous and current endpoints", tci) } - } - if len(result.StaleServiceNames) != len(tc.expectedStaleServiceNames) { - t.Errorf("[%d] expected %d staleServiceNames, got %d: %v", tci, len(tc.expectedStaleServiceNames), len(result.StaleServiceNames), result.StaleServiceNames) - } - for svcName := range tc.expectedStaleServiceNames { - found := false - for _, stale := range result.StaleServiceNames { - if stale == svcName { - found = true - break + + for i := range tc.previousEndpoints { + prev, curr := tc.previousEndpoints[i], tc.currentEndpoints[i] + switch { + case prev == nil: + fp.OnEndpointSliceAdd(curr) + case curr == nil: + fp.OnEndpointSliceDelete(prev) + default: + fp.OnEndpointSliceUpdate(prev, curr) } } - if !found { - t.Errorf("[%d] expected staleServiceNames[%v], but didn't find it: %v", tci, svcName, result.StaleServiceNames) + result := fp.endpointsMap.Update(fp.endpointsChanges) + newMap := fp.endpointsMap + compareEndpointsMaps(t, tci, newMap, tc.expectedResult) + if len(result.StaleEndpoints) != len(tc.expectedStaleEndpoints) { + t.Errorf("[%d] expected %d staleEndpoints, got %d: %v", tci, len(tc.expectedStaleEndpoints), len(result.StaleEndpoints), result.StaleEndpoints) } - } - if !reflect.DeepEqual(result.HCEndpointsLocalIPSize, tc.expectedHealthchecks) { - t.Errorf("[%d] expected healthchecks %v, got %v", tci, tc.expectedHealthchecks, result.HCEndpointsLocalIPSize) - } + for _, x := range tc.expectedStaleEndpoints { + found := false + for _, stale := range result.StaleEndpoints { + if stale == x { + found = true + break + } + } + if !found { + t.Errorf("[%d] expected staleEndpoints[%v], but didn't find it: %v", tci, x, result.StaleEndpoints) + } + } + if len(result.StaleServiceNames) != len(tc.expectedStaleServiceNames) { + t.Errorf("[%d] expected %d staleServiceNames, got %d: %v", tci, len(tc.expectedStaleServiceNames), len(result.StaleServiceNames), result.StaleServiceNames) + } + for svcName := range tc.expectedStaleServiceNames { + found := false + for _, stale := range result.StaleServiceNames { + if stale == svcName { + found = true + break + } + } + if !found { + t.Errorf("[%d] expected staleServiceNames[%v], but didn't find it: %v", tci, svcName, result.StaleServiceNames) + } + } + if !reflect.DeepEqual(result.HCEndpointsLocalIPSize, tc.expectedHealthchecks) { + t.Errorf("[%d] expected healthchecks %v, got %v", tci, tc.expectedHealthchecks, result.HCEndpointsLocalIPSize) + } + }) } }