Msec-1.1 : updated test for Juniper devices - #5627
Conversation
Pull Request Functional Test Report for #5627 / ea800d0Virtual 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 extends the existing MACsec test suite to support Juniper devices. It introduces necessary configuration push logic and adapts state retrieval methods to accommodate Juniper's specific gNMI requirements, ensuring parity with existing Arista support. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces support for Juniper devices in MACsec testing, including Juniper-specific CLI configuration, gNMI Subscribe ONCE state retrieval, and helper updates. The review feedback correctly points out that the updated IP addresses (192.168.0.1/30 and 192.168.0.2/30) violate the repository's network assignment guidelines by using a forbidden private range, and suggests using RFC 5737 blocks instead.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for Juniper devices in the MACsec OTG tests, including Juniper-specific CLI configuration, gNMI Subscribe ONCE handling for state data, and updated deviations in metadata.textproto. It also updates test IP addresses to RFC 5737 compliant blocks. Feedback highlights a mismatch where a hardcoded ping destination of '10.0.0.2' remains in the test, and recommends using a cancellable context for the gNMI Subscribe client to prevent resource leaks.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for Juniper devices in the MACsec OTG tests by implementing Juniper-specific configuration and state retrieval via gNMI Subscribe ONCE. It also updates IP addresses to RFC 5737 compliant blocks and introduces a helper to resolve the correct functional translator name per vendor. The feedback points out a bug where ip2 is passed directly to sendGnoiPing with its CIDR mask suffix, which will cause the ping to fail, and suggests stripping the mask first.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
@sudhirakondi - Could you please look into the failing @rohit-rp - Friendly ping! Could you please take a look and provide your final code owner approval once the author fixes the failing check and updates the branch? |
|
@ksgireesha |
This Pull is dependent on Merge of Functional Translator code in openconfig/functional-translators#18. This adds support for translation of Native Juniper Paths to OC for macsec.