Reset pipelines after pipelines are generated in every unit test (#4325)

When generating pipelines, every table ID is allocated by calling
`binding.NextTableID()`. Note that, every time the function is called,
its return value (uint8) is increased by 1. After each unit test,
if not calling `binding.ResetTableID()` to reset the return value,
there is a chance that the return value overflows when function to
generate pipelines is called enough times. When overflowing, two
consecutive return values are 255 and 0, causing test TestBuildPipeline
to fail since 255 is not less than 0 (in TestBuildPipeline, there is
an asserting about the table of previous table should be less than
that of the current table).

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

pkg/agent/openflow/client_test.go

@@ -119,6 +119,7 @@ func TestIdempotentFlowInstallation(t *testing.T) {
 			client.serviceConfig = serviceConfig
 			client.ipProtocols = []binding.Protocol{binding.ProtocolIP}
 			client.generatePipelines()
+			defer resetPipelines()
 
 			m.EXPECT().AddAll(gomock.Any()).Return(nil).Times(1)
 			// Installing the flows should succeed, and all the flows should be added into the cache.
@@ -152,6 +153,7 @@ func TestIdempotentFlowInstallation(t *testing.T) {
 			client.serviceConfig = serviceConfig
 			client.ipProtocols = []binding.Protocol{binding.ProtocolIP}
 			client.generatePipelines()
+			defer resetPipelines()
 
 			errorCall := m.EXPECT().AddAll(gomock.Any()).Return(errors.New("Bundle error")).Times(1)
 			m.EXPECT().AddAll(gomock.Any()).Return(nil).After(errorCall)
@@ -198,6 +200,7 @@ func TestFlowInstallationFailed(t *testing.T) {
 			client.serviceConfig = serviceConfig
 			client.ipProtocols = []binding.Protocol{binding.ProtocolIP}
 			client.generatePipelines()
+			defer resetPipelines()
 
 			// We generate an error for AddAll call.
 			m.EXPECT().AddAll(gomock.Any()).Return(errors.New("Bundle error"))
@@ -237,6 +240,7 @@ func TestConcurrentFlowInstallation(t *testing.T) {
 			client.serviceConfig = serviceConfig
 			client.ipProtocols = []binding.Protocol{binding.ProtocolIP}
 			client.generatePipelines()
+			defer resetPipelines()
 
 			var concurrentCalls atomic.Value // set to true if we observe concurrent calls
 			timeoutCh := make(chan struct{})
@@ -434,6 +438,7 @@ func prepareTraceflowFlow(ctrl *gomock.Controller) *client {
 	c.serviceConfig = serviceConfig
 	c.ipProtocols = []binding.Protocol{binding.ProtocolIP}
 	c.generatePipelines()
+	defer resetPipelines()
 
 	m.EXPECT().AddAll(gomock.Any()).Return(nil).Times(1)
 	c.bridge = ovsoftest.NewMockBridge(ctrl)
@@ -564,6 +569,7 @@ func TestMulticlusterFlowsInstallation(t *testing.T) {
 	client.serviceConfig = serviceConfig
 	client.ipProtocols = []binding.Protocol{binding.ProtocolIP}
 	client.generatePipelines()
+	defer resetPipelines()
 
 	m.EXPECT().AddAll(gomock.Any()).Return(nil).Times(1)
 	clusterID := "cluster-a"
@@ -600,6 +606,7 @@ func TestServiceGroupInstallAndUninstall(t *testing.T) {
 	client.serviceConfig = serviceConfig
 	client.ipProtocols = []binding.Protocol{binding.ProtocolIP}
 	client.generatePipelines()
+	defer resetPipelines()
 	endpoints := []k8sproxy.Endpoint{
 		&k8sproxy.BaseEndpointInfo{
 			Endpoint: net.JoinHostPort("192.168.1.2", "8081"),
@@ -666,6 +673,7 @@ func TestMulticastGroupInstallAndUninstall(t *testing.T) {
 	client.serviceConfig = serviceConfig
 	client.ipProtocols = []binding.Protocol{binding.ProtocolIP}
 	client.generatePipelines()
+	defer resetPipelines()
 	localReceivers := []uint32{101, 102}
 	remoteIPs := []net.IP{
 		net.ParseIP("1.1.1.2"),

pkg/agent/openflow/network_policy_test.go

@@ -89,6 +89,7 @@ func TestPolicyRuleConjunction(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 	preparePipelines()
+	defer resetPipelines()
 	c = prepareClient(ctrl, false)
 
 	ruleID1 := uint32(1001)
@@ -163,6 +164,7 @@ func TestInstallPolicyRuleFlows(t *testing.T) {
 	defer ctrl.Finish()
 
 	preparePipelines()
+	defer resetPipelines()
 	c = prepareClient(ctrl, false)
 	c.nodeConfig = &config.NodeConfig{PodIPv4CIDR: podIPv4CIDR, PodIPv6CIDR: nil}
 	c.networkConfig = &config.NetworkConfig{}
@@ -308,6 +310,7 @@ func TestInstallPolicyRuleFlows(t *testing.T) {
 
 func TestBatchInstallPolicyRuleFlows(t *testing.T) {
 	preparePipelines()
+	defer resetPipelines()
 	tests := []struct {
 		name            string
 		rules           []*types.PolicyRule
@@ -582,6 +585,7 @@ func BenchmarkBatchInstallPolicyRuleFlows(b *testing.B) {
 	defer ctrl.Finish()
 
 	preparePipelines()
+	defer resetPipelines()
 	c = prepareClient(ctrl, false)
 	// Make it return error so no change gets committed to cache.
 	mockOperations := oftest.NewMockOFEntryOperations(ctrl)
@@ -627,6 +631,7 @@ func TestConjMatchFlowContextKeyConflict(t *testing.T) {
 	defer ctrl.Finish()
 
 	preparePipelines()
+	defer resetPipelines()
 	c = prepareClient(ctrl, false)
 	mockEgressDefaultTable.EXPECT().BuildFlow(gomock.Any()).Return(newMockDropFlowBuilder(ctrl)).AnyTimes()
 	mockEgressRuleTable.EXPECT().BuildFlow(gomock.Any()).Return(newMockRuleFlowBuilder(ctrl)).AnyTimes()
@@ -672,6 +677,7 @@ func TestInstallPolicyRuleFlowsInDualStackCluster(t *testing.T) {
 	defer ctrl.Finish()
 
 	preparePipelines()
+	defer resetPipelines()
 	c = prepareClient(ctrl, true)
 	c.nodeConfig = &config.NodeConfig{PodIPv4CIDR: podIPv4CIDR, PodIPv6CIDR: podIPv6CIDR}
 	c.networkConfig = &config.NetworkConfig{IPv4Enabled: true, IPv6Enabled: true}
@@ -1160,6 +1166,7 @@ func TestNetworkPolicyMetrics(t *testing.T) {
 			defer ctrl.Finish()
 
 			preparePipelines()
+			defer resetPipelines()
 			c = prepareClient(ctrl, false)
 			mockOVSClient := ovsctltest.NewMockOVSCtlClient(ctrl)
 			c.ovsctlClient = mockOVSClient
@@ -1177,6 +1184,7 @@ func TestGetMatchFlowUpdates(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 	preparePipelines()
+	defer resetPipelines()
 	c = prepareClient(ctrl, false)
 	c.nodeConfig = &config.NodeConfig{PodIPv4CIDR: podIPv4CIDR, PodIPv6CIDR: nil}
 	c.networkConfig = &config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeEncap, IPv4Enabled: true}
@@ -1268,6 +1276,7 @@ func TestClient_GetPolicyInfoFromConjunction(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 	preparePipelines()
+	defer resetPipelines()
 	c = prepareClient(ctrl, false)
 
 	ruleID1 := uint32(101)

pkg/agent/openflow/pipeline_test.go

@@ -307,11 +307,11 @@ func TestBuildPipeline(t *testing.T) {
 			}
 			require.NotNil(t, tables[len(tables)-1].ofTable, "table %q should be initialized", tables[len(tables)-1].name)
 		}
-		reset()
+		resetPipelines()
 	}
 }
 
-func reset() {
+func resetPipelines() {
 	objs := tableCache.List()
 	for i := 0; i < len(objs); i++ {
 		tableCache.Delete(objs[i])