feat(device-discovery): vrf discovery for iosxr, iosxr_netconf, nokia_sros, nokia_srl#453
Conversation
…_sros, nokia_srl First custom-driver batch implementing get_network_instances(): - iosxr: "show vrf all detail" via pyIOSXR, parsed with the cisco_xr ntc-template; the literal "not set" RD normalizes to empty. - iosxr_netconf: single subtree-filtered NETCONF get on the Cisco-IOS-XR-mpls-vpn-oper l3vpn/vrfs model — the same data source the CLI command renders. - nokia_sros: VPRN services as L3VRFs via one NETCONF get on configure/service/vprn (service-name, bgp-ipvpn/mpls route-distinguisher, interface list); the Base router is the DEFAULT_INSTANCE. - nokia_srl: three targeted "info from state network-instance" CLI calls (type, interface membership, bgp-vpn route distinguisher) parsed with a block-header line parser; ip-vrf maps to L3VRF, mac-vrf to L2VSI. All four emit the NAPALM OpenConfig shape consumed by discover_vrfs. Default-instance interface membership is intentionally left empty — the discovery pipeline only consumes VRF memberships. Adds a generic shape-validating test_get_network_instances to BaseDriverTest (auto-skips drivers without fixtures) with per-driver scenarios, and extends the runner-dispatch pin test to the nine supported drivers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
Adds VRF discovery support for additional custom NAPALM drivers by implementing get_network_instances() in the OpenConfig “network-instances” shape, plus fixtures and test coverage to validate payload shape and driver support across the runner dispatch path.
Changes:
- Implement
get_network_instances()foriosxr,iosxr_netconf,nokia_sros, andnokia_srlcustom drivers. - Add/extend tests to validate getter shape, scenarios/fixtures, and pin the supported-driver matrix for VRF discovery dispatch.
- Add driver-specific mocked device outputs and expected OC-shaped results for multiple scenarios (normal/empty/default-row/template edge cases).
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| device-discovery/tests/test_runner_vrf_dispatch.py | Extends the driver pin/matrix test to include the new drivers that implement get_network_instances(). |
| device-discovery/tests/custom_drivers/base_test.py | Adds a generic shape-validating (and fixture-comparing) test_get_network_instances to the custom-driver base test suite. |
| device-discovery/custom_napalm/iosxr.py | Implements IOS-XR VRF discovery by parsing show vrf all detail into the NAPALM OC network-instances shape. |
| device-discovery/custom_napalm/iosxr_netconf.py | Implements IOS-XR NETCONF VRF discovery via a subtree-filtered Cisco-IOS-XR-mpls-vpn-oper query and safe XML parsing. |
| device-discovery/custom_napalm/nokia_sros.py | Implements SR-OS VRF discovery via NETCONF configure/service/vprn (VPRN→VRF) parsing into OC shape. |
| device-discovery/custom_napalm/nokia_srl.py | Implements SR Linux network-instance discovery via targeted CLI info from state ... calls and block parsing. |
| device-discovery/tests/custom_drivers/nokia_sros/mock_data/test_get_network_instances/normal/response.xml | Mock NETCONF response for SR-OS VPRN discovery (normal scenario). |
| device-discovery/tests/custom_drivers/nokia_sros/mock_data/test_get_network_instances/normal/expected_result.json | Expected OC-shaped SR-OS network-instances for normal scenario. |
| device-discovery/tests/custom_drivers/nokia_sros/mock_data/test_get_network_instances/no_vprns/response.xml | Mock NETCONF response for SR-OS with no VPRNs. |
| device-discovery/tests/custom_drivers/nokia_sros/mock_data/test_get_network_instances/no_vprns/expected_result.json | Expected OC-shaped SR-OS network-instances when no VPRNs exist. |
| device-discovery/tests/custom_drivers/nokia_srl/mock_data/test_get_network_instances/normal/info_from_state_network-instance_type.txt | Mock SRL type output for network-instance discovery. |
| device-discovery/tests/custom_drivers/nokia_srl/mock_data/test_get_network_instances/normal/info_from_state_network-instance_interface_name.txt | Mock SRL interface membership output for network-instance discovery. |
| device-discovery/tests/custom_drivers/nokia_srl/mock_data/test_get_network_instances/normal/info_from_state_network-instance_protocols_bgp-vpn_bgp-instance_route-distinguisher_rd.txt | Mock SRL RD output for network-instance discovery. |
| device-discovery/tests/custom_drivers/nokia_srl/mock_data/test_get_network_instances/normal/expected_result.json | Expected OC-shaped SRL network-instances for normal scenario. |
| device-discovery/tests/custom_drivers/nokia_srl/mock_data/test_get_network_instances/empty/expected_result.json | Expected OC-shaped SRL network-instances for empty scenario. |
| device-discovery/tests/custom_drivers/iosxr/mock_data/test_get_network_instances/normal/show_vrf_all_detail.txt | Mock IOS-XR CLI output for normal VRF parsing scenario. |
| device-discovery/tests/custom_drivers/iosxr/mock_data/test_get_network_instances/normal/expected_result.json | Expected OC-shaped IOS-XR network-instances for normal scenario. |
| device-discovery/tests/custom_drivers/iosxr/mock_data/test_get_network_instances/missing_af_section/show_vrf_all_detail.txt | Mock IOS-XR CLI output for “missing AF section” edge case. |
| device-discovery/tests/custom_drivers/iosxr/mock_data/test_get_network_instances/missing_af_section/expected_result.json | Expected OC-shaped IOS-XR output for “missing AF section” scenario. |
| device-discovery/tests/custom_drivers/iosxr/mock_data/test_get_network_instances/route_policy_attached/show_vrf_all_detail.txt | Mock IOS-XR CLI output for “route policy attached” edge case. |
| device-discovery/tests/custom_drivers/iosxr/mock_data/test_get_network_instances/route_policy_attached/expected_result.json | Expected OC-shaped IOS-XR output for “route policy attached” scenario. |
| device-discovery/tests/custom_drivers/iosxr/mock_data/test_get_network_instances/no_vrfs/expected_result.json | Expected OC-shaped IOS-XR output when no VRFs exist. |
| device-discovery/tests/custom_drivers/iosxr_netconf/mock_data/test_get_network_instances/normal/response.xml | Mock IOS-XR NETCONF response for normal VRF discovery scenario. |
| device-discovery/tests/custom_drivers/iosxr_netconf/mock_data/test_get_network_instances/normal/expected_result.json | Expected OC-shaped IOS-XR NETCONF output for normal scenario. |
| device-discovery/tests/custom_drivers/iosxr_netconf/mock_data/test_get_network_instances/no_vrfs/response.xml | Mock IOS-XR NETCONF response when no VRFs exist. |
| device-discovery/tests/custom_drivers/iosxr_netconf/mock_data/test_get_network_instances/no_vrfs/expected_result.json | Expected OC-shaped IOS-XR NETCONF output when no VRFs exist. |
| device-discovery/tests/custom_drivers/iosxr_netconf/mock_data/test_get_network_instances/default_row/response.xml | Mock IOS-XR NETCONF response containing a vrf-name=default row. |
| device-discovery/tests/custom_drivers/iosxr_netconf/mock_data/test_get_network_instances/default_row/expected_result.json | Expected OC-shaped IOS-XR NETCONF output ensuring the seeded DEFAULT_INSTANCE isn’t overwritten. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…cs in vrf getters Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e default instance Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2600d22b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…; pin getter ownership The SR Linux getter populated interface membership for the default network instance, contradicting the batch-wide design where default instances carry no membership (the pipeline only consumes VRF memberships). Emit it empty like the other three drivers. Strengthen the driver-pin test to assert which package owns get_network_instances (upstream napalm vs custom_napalm) instead of just "not the base-class stub" — also catches upstream growing its own NotImplementedError-raising override. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… in iosxr_netconf Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
First custom-driver batch implementing
get_network_instances()for VRF discovery (follow-up to #452, which works out of the box on ios/eos/junos/nxos/nxos_ssh via upstream NAPALM). All four return the NAPALM OpenConfig network-instances shape consumed byoptions.discover_vrfs.Per driver
iosxrshow vrf all detailvia pyIOSXRmissing_af_section,route_policy_attached). RDnot set→ absent.iosxr_netconfCisco-IOS-XR-mpls-vpn-operl3vpn/vrfsnokia_srosconfigure/service/vprn<service>/<interface>to matchget_interfaces_ip()naming so downstream VRF attachment joins correctly. RD frombgp-ipvpn/mpls. Base router emitted as DEFAULT_INSTANCE.nokia_srlinfo from state network-instanceCLI callsip-vrf→ L3VRF,mac-vrf→ L2VSI,default→ DEFAULT_INSTANCE; module-prefixed identityrefs tolerated.Common semantics: default instances carry no interface membership by design (the pipeline only consumes VRF memberships); the NAPALM
namefilter is honored on every return path including degraded ones; rows nameddefaultcan never overwrite the seeded DEFAULT_INSTANCE.Tests
test_get_network_instancesinBaseDriverTest(auto-skips drivers on the base-class stub or without fixtures)Review trail
Codex review (caught SROS interface-key mismatch with
get_interfaces_ip()naming + XR default-row overwrite) and two adversarial Claude review rounds (caught the ntc-template stuck-state misattribution — fixed by the driver-local parser — and the name-filter contract gaps on degraded paths). Final round: APPROVED.🤖 Generated with Claude Code