Fixing BGP LLGR test for Nokia

[ram-mac]

BGP Long-Lived Graceful Restart (LLGR) test failures on Nokia devices by addressing multiple issues including configuration timing, graceful restart mode mismatches, and unsupported operations during restart. Skipping the "Configure new BGP Peer" section during graceful restart (due to the new deviation bgp_config_during_graceful_restart_unsupported)

• for Nokia, subinterface 0 is not created but attempted to be bound. This PR fixes to bind actual subinterfaces

• Nokia regarding AFI/SAFI GR content and need to relax GR Mode check was missing, so added deviation "bgp_graceful_restart_under_afi_safi_unsupported", "missing_value_for_defaults" along with above deviations.

• The test expects HELPER_ONLY but Nokia reports BILATERAL. I will update the test to accept both, as BILATERAL is a valid mode for a device supporting full GR.

• Invalid Path: The path afi-safi/.../graceful-restart/enabled is not supported. The deviation bgp_graceful_restart_under_afi_safi_unsupported is used for Nokia to skip this check in the test.

[gemini-code-assist]

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 significantly enhances the stability and compatibility of BGP Long-Lived Graceful Restart (LLGR) tests, specifically for Nokia network devices. By introducing new deviations and refining configuration logic, it ensures that tests correctly account for Nokia's unique behaviors and limitations regarding BGP graceful restart, preventing false negatives and improving the overall reliability of the testing suite.

Highlights

• Nokia BGP LLGR Test Fixes: Addressed multiple issues causing BGP Long-Lived Graceful Restart (LLGR) test failures on Nokia devices, including configuration timing, graceful restart mode mismatches, and unsupported operations during restart.
• Subinterface Binding Correction: Corrected an issue where subinterface 0 was attempted to be bound for Nokia devices, instead ensuring actual subinterfaces are bound.
• New Deviations Introduced: Introduced new deviations for Nokia: 'bgp_graceful_restart_under_afi_safi_unsupported' to handle AFI/SAFI GR content, 'missing_value_for_defaults', and 'bgp_config_during_graceful_restart_unsupported' to skip BGP peer configuration during graceful restart.
• Graceful Restart Mode Flexibility: Updated the graceful restart mode check to accept both 'HELPER_ONLY' and 'BILATERAL' modes, as 'BILATERAL' is a valid mode for devices supporting full GR.
• Retry Mechanisms for gNMI Operations: Implemented retry logic for gNMI Replace, Update, and Delete operations to improve test stability and robustness against transient configuration failures.

Changelog

• feature/bgp/ate_tests/bgp_long_lived_graceful_restart/bgp_long_lived_graceful_restart_test.go
  • Replaced gnmi.Replace with gnmi.Update in configureRoutePolicy.
  • Modified configInterfaceDUT to correctly assign subinterfaces for Nokia devices, including handling RequireRoutedSubinterface0 and iterating through specific subinterfaces.
  • Replaced direct gnmi.Delete and gnmi.Update/gnmi.Replace calls with new deleteWithRetry, updateWithRetry, and replaceWithRetry functions for improved robustness.
  • Adjusted verifyGracefulRestart to conditionally check PeerRestartTime and accept BILATERAL mode for graceful restart, and to skip AFI-SAFI GR state checks based on a new deviation.
  • Added replaceWithRetry, updateWithRetry, and deleteWithRetry helper functions to handle transient gNMI operation failures.
  • Introduced t.Skip conditions for BGP peer and policy configuration/removal during graceful restart based on the BgpConfigDuringGracefulRestartUnsupported deviation.
• feature/bgp/ate_tests/bgp_long_lived_graceful_restart/metadata.textproto
  • Added missing_value_for_defaults, bgp_graceful_restart_under_afi_safi_unsupported, and bgp_config_during_graceful_restart_unsupported deviations for Nokia.
• internal/deviations/deviations.go
  • Added the BgpConfigDuringGracefulRestartUnsupported function to check for device support of BGP configuration during graceful restart.
• proto/metadata.proto
  • Added a new boolean field bgp_config_during_graceful_restart_unsupported to the Metadata.Deviations message.
• proto/metadata_go_proto/metadata.pb.go
  • Updated the generated Go code for Metadata_Deviations to include the BgpConfigDuringGracefulRestartUnsupported field and its corresponding getter method.

Activity

• No human activity (comments, reviews, or progress updates) has been recorded for this pull request yet.

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.

Footnotes

1. 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]

Pull Request Functional Test Report for #5189 / 12b8bde

No tests identified for validation.

Help

[gemini-code-assist]

Code Review

This pull request introduces fixes for the BGP Long-Lived Graceful Restart (LLGR) test, primarily for Nokia devices. The changes include adding new deviations to handle platform-specific behavior, such as unsupported BGP configurations during graceful restart and differences in graceful restart mode reporting. Additionally, the PR introduces retry mechanisms for gNMI operations to enhance test stability and updates interface configuration logic. Overall, the changes are well-aligned with the goal of improving test robustness for Nokia devices. I've included a few suggestions to fix minor bugs in logging.

[AmrNJ]

LGTM.

[singhavnish2516]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces fixes for BGP LLGR tests on Nokia devices, primarily by adding new deviations and making gNMI operations more resilient with retry logic. The changes are well-aligned with the goal of improving test stability. I've identified a couple of areas for improvement: the retry helper functions have significant code duplication that could be refactored, and there's a section with a redundant and inefficient check that could be simplified. Addressing these points would enhance the code's maintainability and performance, while ensuring retry intervals are sufficient for test stability.