fix(e2e): handle neighbor entry GC race in bridge refresh test by fedepaol · Pull Request #339 · openperouter/openperouter

fedepaol · 2026-04-24T14:38:13Z

Is this a BUG FIX or a FEATURE ?:

Uncomment only one, leave it on its own line:

/kind bug
/kind cleanup
/kind feature
/kind design
/kind flake
/kind failing
/kind documentation
/kind regression
/kind example

What this PR does / why we need it:

The IPv6 migration test timed out waiting for a STALE neighbor entry
because the kernel NDP state machine (STALE->DELAY->FAILED->GC) completed
in ~8 seconds, faster than the test polling interval via podman exec.

Introduce ErrNoNeighborEntry sentinel so callers can distinguish
"entry was GC'd" from other failures, and treat a missing entry as
success. Also verify the neighbor entry exists before pod deletion
to confirm it was learned and not simply never created.

Special notes for your reviewer:

Spotted in https://github.com/openperouter/openperouter/actions/runs/24882543505/job/72855481022?pr=310

Release note:

NONE

The IPv6 migration test timed out waiting for a STALE neighbor entry because the kernel NDP state machine (STALE->DELAY->FAILED->GC) completed in ~8 seconds, faster than the test polling interval via podman exec. Introduce ErrNoNeighborEntry sentinel so callers can distinguish "entry was GC'd" from other failures, and treat a missing entry as success. Also verify the neighbor entry exists before pod deletion to confirm it was learned and not simply never created. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

gemini-code-assist · 2026-04-24T14:38:21Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a flaky E2E test in the IPv6 migration workflow. The test was failing due to a race condition where the kernel's neighbor discovery state machine would garbage collect an entry faster than the test could poll for it. By introducing a specific error sentinel and adding pre-migration verification, the test is now more resilient to timing variations in the kernel's neighbor table management.

Highlights

Neighbor Entry Handling: Introduced a new sentinel error 'ErrNoNeighborEntry' to distinguish between missing neighbor entries and other failure states.
Test Robustness: Updated the bridge refresh test to treat a missing neighbor entry as a success during the STALE state wait, preventing race conditions where the entry is garbage collected before the test checks it.
Pre-migration Verification: Added a mandatory check to verify that the neighbor entry exists on the router before initiating pod migration.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

fedepaol · 2026-04-24T14:39:46Z

/gemini review

gemini-code-assist

Code Review

This pull request enhances the BridgeRefresher E2E test by adding a verification step for neighbor entries before pod migration and improving the efficiency of the neighbor stale check by handling missing entries explicitly. Review feedback suggests moving the exported error variable to the top of the file to adhere to the repository's newspaper structure guidelines and using the new sentinel error in the checkNeighborExists function for consistency.

gemini-code-assist · 2026-04-24T14:39:46Z

 	return nil
 }

+var ErrNoNeighborEntry = errors.New("no neighbor entry")


The exported variable ErrNoNeighborEntry should be moved to the top of the file, following the repository's newspaper code structure guidelines (line 33). This ensures that exported symbols are easily discoverable at the beginning of the file.

References

Exported methods and types go at the top of the file ^(link)

gemini-code-assist · 2026-04-24T14:39:46Z

+		return fmt.Errorf("failed to check neighbor on router %s: %w", exec.Name(), err)
+	}
+	if strings.TrimSpace(out) == "" {
+		return fmt.Errorf("no neighbor entry for %s on %s in router %s", podIP, bridgeDev, exec.Name())


For consistency with checkNeighborStale, consider using the ErrNoNeighborEntry sentinel error here as well.

Suggested change

return fmt.Errorf("no neighbor entry for %s on %s in router %s", podIP, bridgeDev, exec.Name())

return fmt.Errorf("%w for %s on %s in router %s", ErrNoNeighborEntry, podIP, bridgeDev, exec.Name())

this could be interesting.

gemini-code-assist

Code Review

This pull request enhances the E2E bridge refresh tests by adding a verification step to ensure neighbor entries exist before pod migration. It introduces a new helper function checkNeighborExists and a specific error ErrNoNeighborEntry. Additionally, the checkNeighborStale logic was updated to treat missing neighbor entries as a successful termination condition for the wait loop, preventing unnecessary timeouts when entries expire early. I have no feedback to provide.

maiqueb

tks

maiqueb · 2026-04-27T11:51:06Z

+					err := checkNeighborStale(cs, migratingPodIPOnly, l2VNI, migratingPod.Spec.NodeName)
+					if errors.Is(err, ErrNoNeighborEntry) { // if the neighbor entry is expired no need to wait
+						return nil
+					}
+					return err
 				}, 3*time.Minute, 2*time.Second).ShouldNot(HaveOccurred())


ok, got it.

can't we move this logic to the matcher instead ? like, the error should either: not have occurred, or match ErrNoNeighborEntry ?

I think it would read better.

maiqueb · 2026-04-27T11:52:32Z

+				By("Verifying neighbor entry exists on the router before migration")
+				Eventually(func() error {
+					return checkNeighborExists(cs, migratingPodIPOnly, l2VNI, migratingPod.Spec.NodeName)
+				}, time.Minute, 2*time.Second).ShouldNot(HaveOccurred())


isn't should succeed the same ? I think it reads better.

maiqueb · 2026-04-27T11:53:13Z

+		return fmt.Errorf("failed to check neighbor on router %s: %w", exec.Name(), err)
+	}
+	if strings.TrimSpace(out) == "" {
+		return fmt.Errorf("no neighbor entry for %s on %s in router %s", podIP, bridgeDev, exec.Name())


this could be interesting.

maiqueb · 2026-04-27T11:53:24Z

 	out = strings.TrimSpace(out)
 	if out == "" {
-		return fmt.Errorf("no neighbor entry for %s on %s in router %s", podIP, bridgeDev, exec.Name())
+		return fmt.Errorf("%w for %s on %s in router %s", ErrNoNeighborEntry, podIP, bridgeDev, exec.Name())


github-actions Bot added the kind/flake label Apr 24, 2026

gemini-code-assist Bot reviewed Apr 24, 2026

View reviewed changes

maiqueb approved these changes Apr 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(e2e): handle neighbor entry GC race in bridge refresh test#339

fix(e2e): handle neighbor entry GC race in bridge refresh test#339
fedepaol wants to merge 1 commit into
openperouter:mainfrom
fedepaol:fix-bridge-refresh-neighbor-race

fedepaol commented Apr 24, 2026

Uh oh!

gemini-code-assist Bot commented Apr 24, 2026

Uh oh!

fedepaol commented Apr 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Apr 24, 2026

Uh oh!

gemini-code-assist Bot Apr 24, 2026

Uh oh!

maiqueb Apr 27, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

maiqueb left a comment

Uh oh!

maiqueb Apr 27, 2026

Uh oh!

maiqueb Apr 27, 2026

Uh oh!

maiqueb Apr 27, 2026

Uh oh!

maiqueb Apr 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	return fmt.Errorf("no neighbor entry for %s on %s in router %s", podIP, bridgeDev, exec.Name())
	return fmt.Errorf("%w for %s on %s in router %s", ErrNoNeighborEntry, podIP, bridgeDev, exec.Name())

Conversation

fedepaol commented Apr 24, 2026

Uh oh!

gemini-code-assist Bot commented Apr 24, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

fedepaol commented Apr 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

maiqueb Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

maiqueb left a comment

Choose a reason for hiding this comment

Uh oh!

maiqueb Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

maiqueb Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

maiqueb Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

maiqueb Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants