Adding AFT-6.4: AFT Prefix Filtering Dynamic Updates Test#5573
Adding AFT-6.4: AFT Prefix Filtering Dynamic Updates Test#5573keysightgems wants to merge 7 commits into
Conversation
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 a new feature profile test for AFT Prefix Filtering Dynamic Updates (AFT-6.4). It includes the necessary test logic to verify that prefix filtering policies correctly handle dynamic additions, deletions, and atomic swaps of routes. To support this, the PR also enhances the internal configuration plugins for routing policies and updates the test framework to better handle gNMI notification monitoring and platform-specific deviations. 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
|
![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 introduces a new dynamic AFT prefix filtering test (AFT-6.4) along with associated configuration plugins, deviations, and telemetry cache helpers to support waiting for update and delete notifications. The code review feedback highlights several critical improvements: using a cancellable context to prevent resource leaks in the test, returning false, nil instead of an error in PeriodicFunc to allow the cache collector to wait properly without failing immediately, removing redundant vendor checks when deviation checks are present, and adding a tracking issue URL to the new deviation accessor comment as required by the style guide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| return nil | ||
| } else { | ||
| ruleSeq := uint32(1) | ||
| for i := 1; i <= encapparams.Count; i++ { |
There was a problem hiding this comment.
please address Static analysis
There was a problem hiding this comment.
This is pre-existing code and is outside the scope of the current PR. The changes may affect other tests, is it required to fix? if yes, can it be handle separately?
|
|
||
| noIPv4NHValidation := len(wantIPV4NHs) == 0 | ||
| noIPv6NHValidation := len(wantIPV6NHs) == 0 | ||
| if noIPv4NHValidation && noIPv6NHValidation { |
There was a problem hiding this comment.
Why are we skipping the NH validation here. Can you elobarate.
There was a problem hiding this comment.
NH validation is skipped here because the test README does not require next-hop validation, the test scope is limited to prefix filtering and AFT notification verification.
The current implementation also serves as a temporary workaround for an existing issue affecting some related test cases. I added this logic to avoid false failures until the issue is fixed (https://partnerissuetracker.corp.google.com/u/1/issues/517809756). Once that is resolved I will remove this code and validate NH.
3 participants
Readme: https://github.com/openconfig/featureprofiles/blob/main/feature/afts/filtered_streaming/otg_tests/afts_prefix_filtering_dynamic/README.md
Test cases failing due to the defect: https://partnerissuetracker.corp.google.com/u/1/issues/517809756
Attached logs here: https://partnerissuetracker.corp.google.com/u/2/issues/523412574