[gNOI-4.1] Fix verification of intf and BGP status after config push#5385
[gNOI-4.1] Fix verification of intf and BGP status after config push#5385sriram-arista wants to merge 1 commit intoopenconfig:mainfrom
Conversation
|
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 #5385 / fa3e734Virtual Devices
Hardware Devices
|
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 improves the robustness of configuration verification tests by shifting from full object comparison to explicit leaf node validation. This change prevents test failures caused by the presence of additional operational or default state nodes in the device response. Necessary infrastructure for handling these implicit defaults has been added via new metadata 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 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 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the interface and BGP configuration verification in osinstall_test.go to use granular GNMI lookups instead of deep equality checks and introduces a new deviation for missing BGP enabled leaves. The review feedback identifies that using gnmi.Get for leaves that may be omitted by the device can cause fatal test failures, recommending gnmi.LookupConfig instead. Additionally, the feedback suggests that the newly added specific BGP deviation is redundant and should be replaced by the existing MissingValueForDefaults deviation to maintain consistency and reduce code duplication.
2bbd257 to
eef720d
Compare
eef720d to
fa3e734
Compare
Issue:
TestPushAndVerifyInterfaceConfigandTestPushAndVerifyBGPConfigpush config via gNMI and then verify the response if it matches with the pushed config. But the received config could contain additional leaf nodes due to the operational / default state.Fix:
Verify only the leaf nodes which are explicitly configured in the test.
neighbor.enabledleaf node has a YANG default of True and is not returned in the response so usedmissing_value_for_defaultsdeviation.bgp_default_vrf_ipv4_unicast_afi_safi_enabled_leaf_missingto get the implicit state when the leaf node (bgp/global/afi-safis[IPV4_Unicast]/state/enabled- for which the YANG default is deleted in Arista implementation) is missing