Do not merge test frr encap #4966

dracazacula · 2025-12-31T10:56:54Z

No description provided.

dracazacula · 2025-12-31T10:57:07Z

/fptest virtual

gemini-code-assist · 2025-12-31T10:57:08Z

Summary of Changes

Hello @dracazacula, 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 primarily addresses a logical error in VRF policy creation for IPv6 configurations and refines error reporting in a BGP test case. Additionally, it updates test metadata to acknowledge a specific platform deviation related to ISIS interface settings, ensuring more accurate test execution and reporting.

Highlights

Test Error Handling: Modified a BGP neighbor formation check in a test file to use t.Errorf instead of t.Fatal. This change allows the test to log the error and continue execution, providing more comprehensive reporting before a potential failure.
Test Metadata Update: Added a new platform deviation, missing_isis_interface_afi_safi_enable: true, to the metadata.textproto file. This indicates a known exception for certain platforms regarding ISIS interface configuration within the test environment.
VRF Policy Logic Fix: Corrected a bug in internal/vrfpolicy/vrfpolicy.go where the GetOrCreateIpv4() method was erroneously called within a code block intended for IPv6 processing. This has been fixed to correctly call GetOrCreateIpv6().

🧠 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.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

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. ↩

OpenConfigBot · 2025-12-31T10:57:39Z

Pull Request Functional Test Report for #4966 / `03ed13b`

Virtual Devices

Device	Test Documentation	Job	Raw Log
Arista cEOS	TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios	2dd6efff	Log
Cisco 8000E	TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios	47594117	Log
Cisco XRd	TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios	86249983	Log
Juniper ncPTX	TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios	e31fe353	Log
Nokia SR Linux	TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios	306ab644	Log
Openconfig Lemming	TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios	8e57073d	Log

Hardware Devices

Device	Test	Test Documentation	Raw Log
Arista 7808		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios
Cisco 8808		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios
Juniper PTX10008		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios
Nokia 7250 IXR-10e		TE-16.2: encapsulation FRR scenarios TE-16.3: encapsulation FRR scenarios

Help

gemini-code-assist

Code Review

This pull request includes a bug fix in VRF policy creation, an update to a test file, and a metadata update. My review focuses on two key issues. First, a change in encap_frr_with_reencap_vrf_test.go introduces a critical bug by replacing t.Fatal with t.Errorf, which will lead to a panic. I've recommended reverting this. Second, while a bug was correctly fixed in vrfpolicy.go, the surrounding logic remains susceptible to a panic. I've provided a suggestion to make the code more robust.

gemini-code-assist · 2025-12-31T10:58:57Z

feature/gribi/otg_tests/encap_frr_with_reencap_vrf_test/encap_frr_with_reencap_vrf_test.go

 	if !ok {
 		fptest.LogQuery(t, "BGP reported state", nbrPath.State(), gnmi.Get(t, dut, nbrPath.State()))
-		t.Fatal("No BGP neighbor formed")
+		t.Errorf("No BGP neighbor formed")


Changing t.Fatal to t.Errorf will cause a panic on line 624 if the BGP neighbor fails to establish (!ok). When !ok, the status variable can be nil, and calling status.Val() will result in a nil pointer dereference. t.Fatal was correct here as it stops test execution, preventing the panic. Please revert this change to use t.Fatal to ensure the test exits correctly on failure.

Suggested change

t.Errorf("No BGP neighbor formed")

t.Fatal("No BGP neighbor formed")

coveralls · 2025-12-31T11:01:45Z

Pull Request Test Coverage Report for Build 20662280754

Details

0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 10.03%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
internal/vrfpolicy/vrfpolicy.go	0	1	0.0%

Totals
Change from base Build 20650373581:	0.0%
Covered Lines:	2227
Relevant Lines:	22203

💛 - Coveralls

dracazacula · 2025-12-31T12:00:06Z

/fptest virtual

dracazacula · 2025-12-31T13:51:04Z

/fptest virtual

dracazacula · 2025-12-31T15:50:38Z

/fptest virtual

dracazacula · 2026-01-01T13:30:25Z

/fptest virtual

dracazacula · 2026-01-02T10:19:11Z

/fptest virtual

dracazacula · 2026-01-02T15:50:11Z

/fptest virtual

dracazacula · 2026-01-02T16:19:23Z

/fptest virtual

dracazacula · 2026-01-02T16:44:26Z

/fptest virtual

dracazacula requested review from a team as code owners December 31, 2025 10:56

gemini-code-assist bot reviewed Dec 31, 2025

View reviewed changes

dracazacula force-pushed the do_not_merge_test_frr_encap branch from 236962b to d3130e0 Compare December 31, 2025 11:54

dracazacula force-pushed the do_not_merge_test_frr_encap branch from d3130e0 to 293c6a4 Compare December 31, 2025 13:50

dracazacula force-pushed the do_not_merge_test_frr_encap branch from 293c6a4 to b2aa1ea Compare December 31, 2025 15:50

dracazacula force-pushed the do_not_merge_test_frr_encap branch from b2aa1ea to 5b20cf5 Compare January 1, 2026 13:29

dracazacula added 3 commits January 2, 2026 10:18

Add missing deviation for Nokia ISIS configuration

922ead2

ipv6 rule in VRF selection policy

1eeac05

ignore bgp error

336eba2

dracazacula force-pushed the do_not_merge_test_frr_encap branch from 5b20cf5 to 336eba2 Compare January 2, 2026 10:18

dracazacula force-pushed the do_not_merge_test_frr_encap branch 3 times, most recently from 8747709 to c3653a2 Compare January 2, 2026 15:49

dracazacula force-pushed the do_not_merge_test_frr_encap branch from c3653a2 to e9af621 Compare January 2, 2026 16:17

dracazacula force-pushed the do_not_merge_test_frr_encap branch from e9af621 to 0e12ac4 Compare January 2, 2026 16:42

arista hack

03ed13b

dracazacula force-pushed the do_not_merge_test_frr_encap branch from 0e12ac4 to 03ed13b Compare January 2, 2026 16:43

	t.Errorf("No BGP neighbor formed")
	t.Fatal("No BGP neighbor formed")

Do not merge test frr encap #4966

Are you sure you want to change the base?

Do not merge test frr encap #4966

Conversation

dracazacula commented Dec 31, 2025

Uh oh!

dracazacula commented Dec 31, 2025

Uh oh!

gemini-code-assist bot commented Dec 31, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

OpenConfigBot commented Dec 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Functional Test Report for #4966 / 03ed13b

Virtual Devices

Hardware Devices

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

coveralls commented Dec 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 20662280754

Details

💛 - Coveralls

Uh oh!

dracazacula commented Dec 31, 2025

Uh oh!

dracazacula commented Dec 31, 2025

Uh oh!

dracazacula commented Dec 31, 2025

Uh oh!

dracazacula commented Jan 1, 2026

Uh oh!

dracazacula commented Jan 2, 2026

Uh oh!

dracazacula commented Jan 2, 2026

Uh oh!

dracazacula commented Jan 2, 2026

Uh oh!

dracazacula commented Jan 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

OpenConfigBot commented Dec 31, 2025 •

edited

Loading

Pull Request Functional Test Report for #4966 / `03ed13b`

coveralls commented Dec 31, 2025 •

edited

Loading