Added additional( route update check) atomic changes #4968

paramasivamn1 · 2026-01-02T04:53:07Z

Added additional( route update check) atomic changes

gemini-code-assist · 2026-01-02T04:53:23Z

Summary of Changes

Hello @paramasivamn1, 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 refines the testing framework for AFT atomic flag checks and streamlines BGP configuration logic. It modifies the testCase structure to support more precise post-churn verification in tests and updates a BGP helper function to operate directly on the OpenConfig root model, improving modularity and reducing boilerplate.

Highlights

Test Case Structure Refinement: The testCase struct in afts_atomic_test.go was updated to replace a slice of stoppingConditions with a single stoppingCondition and introduce an additionalVerification function, allowing for more targeted post-churn validation.
BGP Configuration Refactoring: The handleMultipathDeviation function in internal/cfgplugins/bgp.go was refactored to directly modify an oc.Root object, removing the dependency on a gnmi.SetBatch and simplifying its integration into BGP configuration.
Enhanced Test Verification Logic: The TestAtomic function now utilizes the new additionalVerification field for specific test scenarios, enabling more flexible and detailed checks after network churn events.

🧠 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 · 2026-01-02T04:54:17Z

Pull Request Functional Test Report for #4968 / `d1fe492`

Virtual Devices

Device	Test Documentation	Job	Raw Log
Arista cEOS	AFT-3.1: AFTs Atomic Flag Check	780e07e4	Log
Cisco 8000E	AFT-3.1: AFTs Atomic Flag Check	14aa87e5	Log
Cisco XRd	AFT-3.1: AFTs Atomic Flag Check	216d5ddb	Log
Juniper ncPTX	AFT-3.1: AFTs Atomic Flag Check	458365d6	Log
Nokia SR Linux	AFT-3.1: AFTs Atomic Flag Check	45401d39	Log
Openconfig Lemming	AFT-3.1: AFTs Atomic Flag Check	c9aeb336	Log

Hardware Devices

Device	Test	Test Documentation	Raw Log
Arista 7808		AFT-3.1: AFTs Atomic Flag Check
Cisco 8808		AFT-3.1: AFTs Atomic Flag Check
Juniper PTX10008		AFT-3.1: AFTs Atomic Flag Check
Nokia 7250 IXR-10e		AFT-3.1: AFTs Atomic Flag Check

Help

gemini-code-assist

Code Review

This pull request refactors the atomic AFT test by changing how test cases are structured and verified. It also includes some refactoring in the BGP configuration plugin. My review has identified a critical issue in internal/cfgplugins/bgp.go where a function signature change leads to a compilation error in a case that was not updated. I've also pointed out some minor issues like commented-out code that should be removed and a typo in a comment. Please address the critical issue to ensure the code compiles and functions as expected.

gemini-code-assist · 2026-01-02T04:54:35Z

internal/cfgplugins/bgp.go

 // at the afisafi level is supported or not. It updates the sb with the necessary
 // configuration.
-func handleMultipathDeviation(t *testing.T, dut *ondatra.DUTDevice, sb *gnmi.SetBatch, cfg BGPNeighborsConfig) error {
+func handleMultipathDeviation(t *testing.T, dut *ondatra.DUTDevice, root *oc.Root, cfg BGPNeighborsConfig) error {


Changing the function signature to remove sb *gnmi.SetBatch introduces a compilation error. The case ondatra.JUNIPER: block within this function (on line 706) still attempts to use sb in a gnmi.BatchUpdate call, but sb is now undefined in this scope. To fix this, you should remove the gnmi.BatchUpdate call from the JUNIPER case, similar to how it was removed from the general case. The caller, CreateBGPNeighbors, is already responsible for batching the updates to the root object.

gemini-code-assist · 2026-01-02T04:54:35Z

feature/afts/otg_tests/afts_atomic/afts_atomic_test.go

+	// oneLinkDownBGP := aftcache.InitialSyncStoppingCondition(t, dut, bgpPrefixes, ipv4OneNH, ipv6OneNH)
+	// if dut.Vendor() == ondatra.CISCO {
+	// 	ipv6OneNH = map[string]bool{"fe80::200:2ff:fe02:202": true}
+	// 	oneLinkDownBGP = aftcache.InitialSyncStoppingCondition(t, dut, bgpPrefixes, ipv4OneNH, ipv6OneNH)
+	// }


This commented-out code should be removed before merging. If it's for future work or debugging, it should not be part of the final commit. Please remove these lines.

gemini-code-assist · 2026-01-02T04:54:35Z

internal/cfgplugins/bgp.go

 }

-// handleMultipathDeviation implements the deviation logic whether multipath config
+f// handleMultipathDeviation implements the deviation logic whether multipath config


There's a typo in this comment. It should start with //.

Suggested change

f// handleMultipathDeviation implements the deviation logic whether multipath config

// handleMultipathDeviation implements the deviation logic whether multipath config

coveralls · 2026-01-03T10:40:48Z

Pull Request Test Coverage Report for Build 20772734558

Details

0 of 8 (0.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.002%) to 10.017%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
internal/cfgplugins/bgp.go	0	8	0.0%

Totals
Change from base Build 20770081068:	-0.002%
Covered Lines:	2227
Relevant Lines:	22232

💛 - Coveralls

updating the isissession typo.

typo

feature/afts/otg_tests/afts_atomic/afts_atomic_test.go

ElodinLaarz · 2026-01-05T21:50:56Z

feature/afts/otg_tests/afts_atomic/afts_atomic_test.go

-	dutPort1 := dut.Port(t, port1Name).Name()
-	dutIntf1 := dutP1.NewOCInterface(dutPort1, dut)
-	gnmi.Replace(t, dut, gnmi.OC().Interface(dutPort1).Config(), dutIntf1)
+	p1 := dut.Port(t, port1Name).Name()
+	i1 := dutP1.NewOCInterface(p1, dut)
+	gnmi.Update(t, dut, gnmi.OC().Interface(p1).Config(), i1)


Some of these changes don't seem totally necessary? Seems like it was just copied from another test? Can we not make these naming changes? (The new names are less descriptive, anyway.)

ElodinLaarz · 2026-01-08T17:33:52Z

internal/cfgplugins/bgp.go


-	if err := handleMultipathDeviation(t, dut, sb, cfg); err != nil {
-		return err
+	// Configure MaximumPaths
+	if deviations.EnableMultipathUnderAfiSafi(dut) {
+		global.GetOrCreateAfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV4_UNICAST).GetOrCreateUseMultiplePaths().GetOrCreateEbgp().MaximumPaths = ygot.Uint32(2)
+		global.GetOrCreateAfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV6_UNICAST).GetOrCreateUseMultiplePaths().GetOrCreateEbgp().MaximumPaths = ygot.Uint32(2)
+	} else {
+		if err := handleMultipathDeviation(t, dut, sb, cfg); err != nil {
+			return err
+		}
 	}


Can you not do this inline? Break it out into a function similar to how it was before and condition on the vendor(s) you expect to be present.

atomic changes

d97134c

paramasivamn1 requested review from a team as code owners January 2, 2026 04:53

paramasivamn1 requested review from ElodinLaarz and singhavnish2516 January 2, 2026 04:53

paramasivamn1 assigned singhavnish2516 Jan 2, 2026

paramasivamn1 assigned ElodinLaarz Jan 2, 2026

gemini-code-assist bot reviewed Jan 2, 2026

View reviewed changes

paramasivamn1 added 2 commits January 2, 2026 05:10

typo fix

a7cacc2

removed bgp changes and modified the atomic_test

7a5e97e

paramasivamn1 added 4 commits January 3, 2026 16:13

Update bgp.go

767178a

Update afts_atomic_test.go

5236cec

updating the isissession typo.

Merge branch 'main' into paramasivamn1-atomic

21194a8

Update bgp.go

2b6402f

typo

ElodinLaarz requested changes Jan 5, 2026

View reviewed changes

paramasivamn1 added 4 commits January 7, 2026 11:20

reverting afts_atomic_test.go changes

47f7854

gofmt

0e7cc2e

adding multipath fixes

2db406e

Update afts_atomic_test.go

dc0f75e

paramasivamn1 requested a review from ram-mac January 7, 2026 06:11

Merge branch 'main' into paramasivamn1-atomic

d1fe492

ElodinLaarz reviewed Jan 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Added additional( route update check) atomic changes #4968

Added additional( route update check) atomic changes #4968

Uh oh!

paramasivamn1 commented Jan 2, 2026

Uh oh!

gemini-code-assist bot commented Jan 2, 2026

Uh oh!

OpenConfigBot commented Jan 2, 2026 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jan 2, 2026

Uh oh!

gemini-code-assist bot Jan 2, 2026

Uh oh!

gemini-code-assist bot Jan 2, 2026

Uh oh!

coveralls commented Jan 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

ElodinLaarz Jan 5, 2026

Uh oh!

ElodinLaarz Jan 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	f// handleMultipathDeviation implements the deviation logic whether multipath config
	// handleMultipathDeviation implements the deviation logic whether multipath config

Added additional( route update check) atomic changes #4968

Are you sure you want to change the base?

Added additional( route update check) atomic changes #4968

Uh oh!

Conversation

paramasivamn1 commented Jan 2, 2026

Uh oh!

gemini-code-assist bot commented Jan 2, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

OpenConfigBot commented Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Functional Test Report for #4968 / d1fe492

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 Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 20772734558

Details

💛 - Coveralls

Uh oh!

Uh oh!

ElodinLaarz Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

ElodinLaarz Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

OpenConfigBot commented Jan 2, 2026 •

edited

Loading

Pull Request Functional Test Report for #4968 / `d1fe492`

coveralls commented Jan 3, 2026 •

edited

Loading