Skip to content

Commit

Permalink
Fix L7 NetworkPolicy e2e test failure
Browse files Browse the repository at this point in the history
Fix #6129

In the failure tests, the following function is called to
verify whether a connection should be allowed or denied.
To verify a connection should be denied, it requires 5 seconds.

```go
func probeClientIPFromPod(data *TestData, pod, container string, baseUrl string) (string, error) {
	url := fmt.Sprintf("%s/%s", baseUrl, "clientip")
	hostPort, _, err := data.runWgetCommandFromTestPodWithRetry(pod, data.testNamespace, container, url, 5)
	if err != nil {
		return "", err
	}
	host, _, err := net.SplitHostPort(hostPort)
	return host, err
}
```

Before #5843, these e2e tests utilized the function `PollImmediate`
from `k8s.io/apimachinery/pkg/util/wait`, which immediately calls an
anonymous function including the above function. Since the timeout
is 5 seconds, and the ticker time is 1 second, and the anonymous
function runs immediately, the 5-second timeout is sufficient to
verify the denied state of a connection as mentioned above. However,
after #5843, the function `Eventually` from `github.com/stretchr/testify/assert`
is used with the same parameters, which implies that the anonymous
function runs after the first ticker time, leaving 4 seconds. 4 seconds
are insufficient to verify the denied state of a connection.

To resolve the issue, `RunCommandFromPod` called in
`data.runWgetCommandFromTestPodWithRetry` is called directly in function
 `Eventually` to verify the connection state.

Signed-off-by: Hongliang Liu <[email protected]>
  • Loading branch information
hongliangl committed Mar 25, 2024
1 parent df82b76 commit 708fa66
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 9 deletions.
9 changes: 6 additions & 3 deletions test/e2e/l7networkpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ func probeL7NetworkPolicyHTTP(t *testing.T, data *TestData, serverPodName, clien

// Verify that access to path /clientip is as expected.
assert.Eventually(t, func() bool {
_, err := probeClientIPFromPod(data, clientPodName, agnhostContainerName, baseURL)
cmd := []string{"wget", "-O", "-", fmt.Sprintf("%s/%s", baseURL, "clientip"), "-T", "1"}
_, _, err := data.RunCommandFromPod(data.testNamespace, clientPodName, agnhostContainerName, cmd)
if (allowHTTPPathClientIP && err != nil) || (!allowHTTPPathClientIP && err == nil) {
return false
}
Expand All @@ -138,7 +139,8 @@ func probeL7NetworkPolicyHTTP(t *testing.T, data *TestData, serverPodName, clien

// Verify that access to path /hostname is as expected.
assert.Eventually(t, func() bool {
hostname, err := probeHostnameFromPod(data, clientPodName, agnhostContainerName, baseURL)
cmd := []string{"wget", "-O", "-", fmt.Sprintf("%s/%s", baseURL, "hostname"), "-T", "1"}
hostname, _, err := data.RunCommandFromPod(data.testNamespace, clientPodName, agnhostContainerName, cmd)
if (allowHTTPPathHostname && err != nil) || (!allowHTTPPathHostname && err == nil) {
return false
}
Expand Down Expand Up @@ -171,7 +173,8 @@ func probeL7NetworkPolicyHTTP(t *testing.T, data *TestData, serverPodName, clien
func probeL7NetworkPolicyTLS(t *testing.T, data *TestData, clientPodName string, serverName string, canAccess bool) {
url := fmt.Sprintf("https://%s", serverName)
assert.Eventually(t, func() bool {
stdout, stderr, err := data.runWgetCommandFromTestPodWithRetry(clientPodName, data.testNamespace, agnhostContainerName, url, 5)
cmd := []string{"wget", "-O", "-", url, "-T", "1"}
stdout, stderr, err := data.RunCommandFromPod(data.testNamespace, clientPodName, agnhostContainerName, cmd)
if canAccess && err != nil {
t.Logf("Failed to access %s: %v\nStdout: %s\nStderr: %s\n", url, err, stdout, stderr)
return false
Expand Down
6 changes: 0 additions & 6 deletions test/e2e/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,6 @@ func probeFromPod(data *TestData, pod, container string, url string) error {
return err
}

func probeHostnameFromPod(data *TestData, pod, container string, baseUrl string) (string, error) {
url := fmt.Sprintf("%s/%s", baseUrl, "hostname")
hostname, _, err := data.runWgetCommandFromTestPodWithRetry(pod, data.testNamespace, container, url, 5)
return hostname, err
}

func probeClientIPFromPod(data *TestData, pod, container string, baseUrl string) (string, error) {
url := fmt.Sprintf("%s/%s", baseUrl, "clientip")
hostPort, _, err := data.runWgetCommandFromTestPodWithRetry(pod, data.testNamespace, container, url, 5)
Expand Down

0 comments on commit 708fa66

Please sign in to comment.