Add Dell Enterprise SONiC 'ipv6_router_advertisement' module#497
Conversation
|
Overall, the code proposed in this PR looks very good. The code structure is great. The logic all looks correct and complete. I am posting a few minor comments, plus the following question on the regression test results: For test case 4 (deleted), there is a deletion of the "1001::2000" prefix for Vlan101, but the command doesn't modify the other prefix ("1001::1000"). For that prefix, the "after", "after(Generated)" and diff show two (default?) options that aren't present in the "before". |
kerry-meyer
left a comment
There was a problem hiding this comment.
Overall, the code proposed in this PR looks very good. The code structure is great. The logic all looks correct and complete. I am posting a few minor comments and have separately posted a question on the regression results.
...ins/module_utils/network/sonic/config/ipv6_router_advertisement/ipv6_router_advertisement.py
Outdated
Show resolved
Hide resolved
...ins/module_utils/network/sonic/config/ipv6_router_advertisement/ipv6_router_advertisement.py
Outdated
Show resolved
Hide resolved
...ins/module_utils/network/sonic/config/ipv6_router_advertisement/ipv6_router_advertisement.py
Show resolved
Hide resolved
kerry-meyer
left a comment
There was a problem hiding this comment.
Thanks for addressing the posted review comments and questions.
All current proposed code changes and corresponding test results look good.
Thank you for providing the enterprise_sonic Ansible support for IPv6 neighbor discovery.
Approved.
SUMMARY
Model PR: ansible-network/resource_module_models#290
GitHub Issues
List the GitHub issues impacted by this PR. If no Github issues are affected, please indicate this with "N/A".
ISSUE TYPE
COMPONENT NAME
OUTPUT
ADDITIONAL INFORMATION
Checklist:
How Has This Been Tested?
Regression report: ipv6_router_advertisement_regression-2025-05-08-16-28-41.zip
ansible-lint logs: