Fix flaky e2e test TestL7NetworkPolicy (antrea-io#4723)

Fixes antrea-io#4718

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

pkg/ovs/ovsctl/ovsctl.go

@@ -398,7 +398,8 @@ func (c *ovsCtlClient) DumpPortsDesc() ([][]string, error) {
 
 func (c *ovsCtlClient) SetPortNoFlood(ofport int) error {
 	// This command does not have standard output, and only has standard err when running with error.
-	_, err := c.ovsOfctlRunner.RunOfctlCmd("mod-port", strconv.FormatUint(uint64(ofport), 10), "no-flood")
+	// NOTE THAT, THIS CONFIGURATION MUST WORK WITH OpenFlow10.
+	_, err := runOfctlCmd(false, "mod-port", c.bridge, strconv.FormatUint(uint64(ofport), 10), "no-flood")
 	if err != nil {
 		return fmt.Errorf("fail to set no-food config for port %d on bridge %s: %v", ofport, c.bridge, err)
 	}

test/e2e/l7networkpolicy_test.go

@@ -18,12 +18,14 @@ import (
 	"context"
 	"fmt"
 	"net"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"
 
 	crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1"
 	agentconfig "antrea.io/antrea/pkg/config/agent"
@@ -119,6 +121,51 @@ func createL7NetworkPolicy(t *testing.T,
 	assert.NoError(t, err)
 }
 
+func probeL7NetworkPolicy(t *testing.T, data *TestData, serverPodName, clientPodName string, serverIPs []*net.IP, allowHTTPPathHostname, allowHTTPPathClientIP bool) {
+	for _, ip := range serverIPs {
+		baseURL := net.JoinHostPort(ip.String(), "8080")
+
+		// To verify that if path 'clientip' is allowed.
+		assert.NoError(t, wait.PollImmediate(time.Second, 5*time.Second, func() (bool, error) {
+			_, err := probeClientIPFromPod(data, clientPodName, agnhostContainerName, baseURL)
+			if allowHTTPPathClientIP && err != nil || !allowHTTPPathClientIP && err == nil {
+				return false, nil
+			}
+			return true, nil
+		}))
+
+		// To verify that if path 'hostname' is allowed.
+		assert.NoError(t, wait.PollImmediate(time.Second, 5*time.Second, func() (bool, error) {
+			hostname, err := probeHostnameFromPod(data, clientPodName, agnhostContainerName, baseURL)
+			if allowHTTPPathHostname && err != nil || !allowHTTPPathHostname && err == nil {
+				return false, nil
+			}
+			if allowHTTPPathHostname && serverPodName != hostname {
+				return false, nil
+			}
+			return true, nil
+		}))
+
+		// For IPv4, non-HTTP connections should be rejected by Suricata. For IPv6, there is an issue that reject
+		// packet cannot be generated by Suricata and sent back to client.
+		if ip.To4() != nil {
+			cmd := []string{"bash", "-c", fmt.Sprintf("dig @%s google.com a +tcp -p 8080", ip)}
+			assert.NoError(t, wait.PollImmediate(time.Second, 5*time.Second, func() (bool, error) {
+				stdout, _, err := data.RunCommandFromPod(data.testNamespace, clientPodName, agnhostContainerName, cmd)
+				// For the client Pod which is selected by the L7 NetworkPolicy, the expected output returned
+				// from Suricata should contain "connection reset".
+				if err != nil {
+					return false, nil
+				}
+				if !strings.Contains(stdout, fmt.Sprintf("communications error to %s#8080: connection reset", ip)) {
+					return false, nil
+				}
+				return true, nil
+			}))
+		}
+	}
+}
+
 func testL7NetworkPolicyHTTP(t *testing.T, data *TestData) {
 	clientPodName := "test-l7-http-client-selected"
 	clientPodLabels := map[string]string{"test-l7-http-e2e": "client"}
@@ -167,41 +214,6 @@ func testL7NetworkPolicyHTTP(t *testing.T, data *TestData) {
 	policyAllowPathHostname := "test-l7-http-allow-path-hostname"
 	policyAllowAnyPath := "test-l7-http-allow-any-path"
 
-	probeFn := func(allowHTTPPathHostname, allowHTTPPathClientIP bool) {
-		for _, ip := range serverIPs {
-			baseURL := net.JoinHostPort(ip.String(), "8080")
-
-			// To verify that if path 'hostname' is Allowed.
-			hostname, err := probeHostnameFromPod(data, clientPodName, agnhostContainerName, baseURL)
-			if allowHTTPPathHostname {
-				assert.NoError(t, err)
-				assert.Equal(t, serverPodName, hostname)
-			} else {
-				assert.NotNil(t, err)
-			}
-
-			// To verify that if path 'clientip' is Allowted.
-			_, err = probeClientIPFromPod(data, clientPodName, agnhostContainerName, baseURL)
-			if allowHTTPPathClientIP {
-				assert.NoError(t, err)
-			} else {
-				assert.NotNil(t, err)
-			}
-
-			// For IPv4, non-HTTP connections should be rejected by Suricata. For IPv6, there is an issue that reject
-			// packet cannot be generated by Suricata and sent back to client.
-			if ip.To4() != nil {
-				cmd = []string{"bash", "-c", fmt.Sprintf("dig @%s google.com a +tcp -p 8080", ip)}
-				stdout, _, err := data.RunCommandFromPod(data.testNamespace, clientPodName, agnhostContainerName, cmd)
-				// For the client Pod which is selected by the L7 NetworkPolicy, the expected output returned
-				// from Suricata should contain "connection reset".
-				assert.NoError(t, err)
-				assert.Contains(t, stdout, fmt.Sprintf("communications error to %s#8080: connection reset", ip))
-			}
-		}
-
-	}
-
 	t.Run("Ingress", func(t *testing.T) {
 		// Create two L7 NetworkPolicies, one allows HTTP path 'hostname', the other allows any HTTP path. Note that,
 		// the priority of the first one is higher than the second one, and they have the same appliedTo labels and Pod
@@ -215,15 +227,15 @@ func testL7NetworkPolicyHTTP(t *testing.T, data *TestData) {
 		// the first L7 NetworkPolicy has higher priority, matched packets will be only matched by the first L7 NetworkPolicy.
 		// As a result, only HTTP path 'hostname' is allowed by the first L7 NetworkPolicy, other HTTP path like 'clientip'
 		// will be rejected.
-		probeFn(true, false)
+		probeL7NetworkPolicy(t, data, serverPodName, clientPodName, serverIPs, true, false)
 
 		// Delete the first L7 NetworkPolicy that only allows HTTP path 'hostname'.
 		data.crdClient.CrdV1alpha1().NetworkPolicies(data.testNamespace).Delete(context.TODO(), policyAllowPathHostname, metav1.DeleteOptions{})
 		time.Sleep(networkPolicyDelay)
 
 		// Since the fist L7 NetworkPolicy has been deleted, corresponding packets will be matched by the second L7 NetworkPolicy,
 		// and the second L7 NetworkPolicy allows any HTTP path, then both path 'hostname' and 'clientip' are allowed.
-		probeFn(true, true)
+		probeL7NetworkPolicy(t, data, serverPodName, clientPodName, serverIPs, true, true)
 
 		data.crdClient.CrdV1alpha1().NetworkPolicies(data.testNamespace).Delete(context.TODO(), policyAllowAnyPath, metav1.DeleteOptions{})
 	})
@@ -242,14 +254,14 @@ func testL7NetworkPolicyHTTP(t *testing.T, data *TestData) {
 		// the first L7 NetworkPolicy has higher priority, matched packets will be only matched by the first L7 NetworkPolicy.
 		// As a result, only HTTP path 'hostname' is allowed by the first L7 NetworkPolicy, other HTTP path like 'clientip'
 		// will be rejected.
-		probeFn(true, false)
+		probeL7NetworkPolicy(t, data, serverPodName, clientPodName, serverIPs, true, false)
 
 		// Delete the first L7 NetworkPolicy that only allows HTTP path 'hostname'.
 		data.crdClient.CrdV1alpha1().NetworkPolicies(data.testNamespace).Delete(context.TODO(), policyAllowPathHostname, metav1.DeleteOptions{})
 		time.Sleep(networkPolicyDelay)
 
 		// Since the fist L7 NetworkPolicy has been deleted, corresponding packets will be matched by the second L7 NetworkPolicy,
 		// and the second L7 NetworkPolicy allows any HTTP path, then both path 'hostname' and 'clientip' are allowed.
-		probeFn(true, true)
+		probeL7NetworkPolicy(t, data, serverPodName, clientPodName, serverIPs, true, true)
 	})
 }