From a5fb9bbd354608a3b353baa02483d272c72f5941 Mon Sep 17 00:00:00 2001 From: AllenZMC Date: Sun, 1 May 2022 17:58:41 +0800 Subject: [PATCH] Optimize test cases for iptables --- pkg/proxy/iptables/proxier_test.go | 135 ++++++++++++++++------------- 1 file changed, 76 insertions(+), 59 deletions(-) diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index c2c94981d08..988c598e06e 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -3319,6 +3319,7 @@ func TestUpdateEndpointsMap(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][]*endpointsInfo @@ -3328,6 +3329,7 @@ func TestUpdateEndpointsMap(t *testing.T) { expectedHealthchecks map[types.NamespacedName]int }{{ // Case[0]: nothing + name: "nothing", oldEndpoints: map[proxy.ServicePortName][]*endpointsInfo{}, expectedResult: map[proxy.ServicePortName][]*endpointsInfo{}, expectedStaleEndpoints: []proxy.ServiceEndpoint{}, @@ -3335,6 +3337,7 @@ func TestUpdateEndpointsMap(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][]*endpointsInfo{ @@ -3354,6 +3357,7 @@ func TestUpdateEndpointsMap(t *testing.T) { }, }, { // Case[2]: no change, multiple subsets + name: "no change, multiple subsets", previousEndpoints: multipleSubsets, currentEndpoints: multipleSubsets, oldEndpoints: map[proxy.ServicePortName][]*endpointsInfo{ @@ -3377,6 +3381,7 @@ func TestUpdateEndpointsMap(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][]*endpointsInfo{ @@ -3408,6 +3413,7 @@ func TestUpdateEndpointsMap(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][]*endpointsInfo{ @@ -3470,6 +3476,7 @@ func TestUpdateEndpointsMap(t *testing.T) { }, }, { // Case[5]: add an Endpoints + name: "add an Endpoints", previousEndpoints: []*discovery.EndpointSlice{nil}, currentEndpoints: namedPortLocal, oldEndpoints: map[proxy.ServicePortName][]*endpointsInfo{}, @@ -3487,6 +3494,7 @@ func TestUpdateEndpointsMap(t *testing.T) { }, }, { // Case[6]: remove an Endpoints + name: "remove an Endpoints", previousEndpoints: namedPortLocal, currentEndpoints: []*discovery.EndpointSlice{nil}, oldEndpoints: map[proxy.ServicePortName][]*endpointsInfo{ @@ -3503,6 +3511,7 @@ func TestUpdateEndpointsMap(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][]*endpointsInfo{ @@ -3529,6 +3538,7 @@ func TestUpdateEndpointsMap(t *testing.T) { }, }, { // Case[8]: remove an IP and port + name: "remove an IP and port", previousEndpoints: namedPortsLocalNoLocal, currentEndpoints: namedPort, oldEndpoints: map[proxy.ServicePortName][]*endpointsInfo{ @@ -3560,6 +3570,7 @@ func TestUpdateEndpointsMap(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][]*endpointsInfo{ @@ -3584,6 +3595,7 @@ func TestUpdateEndpointsMap(t *testing.T) { }, }, { // Case[10]: remove a subset + name: "remove a subset", previousEndpoints: multipleSubsets, currentEndpoints: []*discovery.EndpointSlice{namedPort[0], nil}, oldEndpoints: map[proxy.ServicePortName][]*endpointsInfo{ @@ -3607,6 +3619,7 @@ func TestUpdateEndpointsMap(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][]*endpointsInfo{ @@ -3629,6 +3642,7 @@ func TestUpdateEndpointsMap(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][]*endpointsInfo{ @@ -3649,6 +3663,7 @@ func TestUpdateEndpointsMap(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][]*endpointsInfo{ @@ -3714,6 +3729,7 @@ func TestUpdateEndpointsMap(t *testing.T) { }, }, { // Case[14]: change from 0 endpoint address to 1 unnamed port + name: "change from 0 endpoint address to 1 unnamed port", previousEndpoints: emptyEndpointSlices, currentEndpoints: namedPort, oldEndpoints: map[proxy.ServicePortName][]*endpointsInfo{}, @@ -3731,72 +3747,73 @@ func TestUpdateEndpointsMap(t *testing.T) { } for tci, tc := range testCases { - ipt := iptablestest.NewFake() - fp := NewFakeProxier(ipt) - fp.hostname = nodeName + t.Run(tc.name, func(t *testing.T) { + ipt := iptablestest.NewFake() + fp := NewFakeProxier(ipt) + 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) - compareEndpointsMapsExceptChainName(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 - compareEndpointsMapsExceptChainName(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) + compareEndpointsMapsExceptChainName(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 + + 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 + compareEndpointsMapsExceptChainName(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 + } + } + 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) + } + }) } }