TestID-16.4: Fixes for export policy, drain policy, and prefix validation#5468
TestID-16.4: Fixes for export policy, drain policy, and prefix validation#5468nakulnayak wants to merge 2 commits into
Conversation
…in gRIBI to BGP redistribution test - Apply BGP export policy to TEST_VRF instead of DEFAULT network instance, so the policy is attached to the actual BGP neighbor - Use gnmi.Replace instead of gnmi.Update when prepending drain policy to the export chain, since Update merges with the existing leaf-list and preserves the original policy ordering - Match validated prefix against the actual gRIBI route (198.51.100.1/32) instead of the aggregate prefix-set (198.51.100.0/26)
Summary of ChangesHello, I'm Gemini Code Assist1! 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 introduces targeted fixes to the gRIBI to BGP redistribution test suite. The changes ensure that BGP export policies are correctly scoped to the intended VRF, improve the reliability of policy chain updates by using atomic replacements, and enhance the accuracy of prefix validation by checking against specific route definitions. Highlights
Using Gemini Code AssistThe 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
Customization To customize the 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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. 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. Footnotes
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Pull Request Functional Test Report for #5468 / cd58c3bVirtual Devices
Hardware Devices
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the BGP redistribution test to use a specific VRF name instead of the default network instance and changes the export policy update method from gnmi.Update to gnmi.Replace. It also refactors the IPv4 prefix validation logic to parse a combined prefix string. Feedback was provided regarding the efficiency of parsing the prefix string inside a loop, suggesting it be moved outside for better performance.
| parts := strings.Split(routePrefix, "/") | ||
| prefixAddr := parts[0] | ||
| prefixLen, _ := strconv.Atoi(parts[1]) |
There was a problem hiding this comment.
Parsing the routePrefix constant inside the loop is inefficient as it performs string splitting and integer conversion on every iteration. Since routePrefix is a constant, this parsing should be performed once outside the loop to improve performance and readability.
…in gRIBI to BGP redistribution test - Apply BGP export policy to TEST_VRF instead of DEFAULT network instance, so the policy is attached to the actual BGP neighbor - Use gnmi.Replace instead of gnmi.Update when prepending drain policy to the export chain, since Update merges with the existing leaf-list and preserves the original policy ordering - Match validated prefix against the actual gRIBI route (198.51.100.1/32) instead of the aggregate prefix-set (198.51.100.0/26)
2 participants
This PR adds a few fixes for export policy, drain policy, and prefix validation in the gRIBI to BGP redistribution fpt.