Skip to content

fix(e2e): wait for flow cleanup before checking service after deletion in metallb test #6359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
oilbeater merged 1 commit into from
Feb 27, 2026
Merged

fix(e2e): wait for flow cleanup before checking service after deletion in metallb test #6359

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions test/e2e/metallb/e2e_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,11 @@ var _ = framework.Describe("[group:metallb]", func() {
ginkgo.By("Deleting the first service")
serviceClient.DeleteSync(serviceName)

ginkgo.By("Waiting for first service's underlay OpenFlow rules to be cleaned up")
for _, ingress := range service.Status.LoadBalancer.Ingress {
waitUnderlayServiceFlowCleaned(nodeNames, providerNetworkName, ingress.IP, curlListenPort, 15*time.Second)
}

ginkgo.By("Checking the second service is still reachable after first service deletion")
for i, ingress := range service2.Status.LoadBalancer.Ingress {
lbsvcIP2 := ingress.IP
Expand Down Expand Up @@ -614,3 +619,21 @@ func waitUnderlayServiceFlow(nodeName, providerNetworkName, serviceIP string, se

return flowFound
}

func waitUnderlayServiceFlowCleaned(nodeNames []string, providerNetworkName, serviceIP string, servicePort int32, timeout time.Duration) {
ginkgo.GinkgoHelper()

bridgeName := util.ExternalBridgeName(providerNetworkName)
matchPort := fmt.Sprintf("tp_dst=%d", servicePort)

framework.WaitUntil(1*time.Second, timeout, func(_ context.Context) (bool, error) {
for _, nodeName := range nodeNames {
cmd := fmt.Sprintf("kubectl ko ofctl %s dump-flows %s | grep -w %s | grep -w %s",
nodeName, bridgeName, serviceIP, matchPort)
if _, err := exec.Command("bash", "-c", cmd).CombinedOutput(); err == nil {
return false, nil // flow still exists on this node
}
}
return true, nil // flow cleaned from all nodes
}, fmt.Sprintf("underlay service flow for %s should be cleaned up", serviceIP))
}
Comment on lines +623 to +639
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The use of grep -w to match an IP address is not reliable. The grep utility considers words to be sequences of letters, digits, and underscores. An IP address string containing dots is not treated as a single word, so grep -w will not match it as intended. This can cause the check to fail incorrectly.

Additionally, without the -F flag, grep treats the pattern as a regular expression, where . is a wildcard for any character. This could lead to incorrect matches (e.g., 1.2.3.4 could match 1a2b3c4).

To make the check more robust and correct, I suggest using grep -F for fixed-string matching and constructing a more specific pattern that includes the OpenFlow field name, such as nw_dst=<IP> or ipv6_dst=<IP>. This avoids both the word-boundary issue and the regex wildcard issue.

func waitUnderlayServiceFlowCleaned(nodeNames []string, providerNetworkName, serviceIP string, servicePort int32, timeout time.Duration) {
	ginkgo.GinkgoHelper()

	bridgeName := util.ExternalBridgeName(providerNetworkName)
	matchPort := fmt.Sprintf("tp_dst=%d", servicePort)

	var ipMatch string
	if strings.Contains(serviceIP, ":") {
		ipMatch = fmt.Sprintf("ipv6_dst=%s", serviceIP)
	} else {
		ipMatch = fmt.Sprintf("nw_dst=%s", serviceIP)
	}

	framework.WaitUntil(1*time.Second, timeout, func(_ context.Context) (bool, error) {
		for _, nodeName := range nodeNames {
			cmd := fmt.Sprintf("kubectl ko ofctl %s dump-flows %s | grep -F -- %q | grep -F -- %q",
				nodeName, bridgeName, ipMatch, matchPort)
			if _, err := exec.Command("bash", "-c", cmd).CombinedOutput(); err == nil {
				return false, nil // flow still exists on this node
			}
		}
		return true, nil // flow cleaned from all nodes
	}, fmt.Sprintf("underlay service flow for %s should be cleaned up", serviceIP))
}

Loading