napalm-automation / napalm

Network Automation and Programmability Abstraction Layer with Multivendor support
Apache License 2.0
2.26k stars 554 forks source link

Handle multiple VRF #792

Open FloLaco opened 6 years ago

FloLaco commented 6 years ago

Hello,

@ktbyers notice me (on that thread : https://networktocode.slack.com/archives/C0EEQTCRH/p1534270593000100) that I should open a new issue to talk about this subjet.

What's the bes way to handle multi VRF ? Do we need to work with vrf all suffix in all OS command, or should we provide the vrf argument in the getter ?

How can we keep compatibility with the compliance_report method, where the YAML file does not take any argument (I think so).

For example, I aws talking about : get_interfaces_ip on NXOS

ktbyers commented 6 years ago

I will try to respond on this in the next couple of days. I would respond quicker, but unfortunately, I think there will be a bit of complexity on the discussion.

ktbyers commented 6 years ago

Sorry for the delay in responding. This is a somewhat hard problem as it has to be implemented for all the core-platforms. Note, this would need to be in the PR for whoever does this (i.e. they would have to do the implementation and testing on all the five core platforms).

You can see an example of it here:

https://github.com/napalm-automation/napalm/pull/512/files#diff-42f76fa4e7c3a361b0ebdd6daa24796eR729

In this solution they added an argument where you could use the getter and specify the VRF. The returned data structure stayed the same.

That is a good way to do it since it doesn't create any backwards compatibility issues.

Also note if you want to do this, we should probably discuss it and come to an agreement on what the method signature and the returned data structure would look like before you do the PR work (given how much work it is).

ktbyers commented 6 years ago

We started working on get_interfaces_ip multiple VRF here:

https://github.com/napalm-automation/napalm/pull/815

We also should probably add VRF support to our getters table so it is clear which getters support VRF and which don't.

FloLaco commented 6 years ago

I agree to add this information into the documentation support matrix. I’ve done some work in my own repo branch PRODC4 (https://github.com/FloLaco/napalm/tree/PRODC4?files=1)

ktbyers commented 6 years ago

Okay, cool. I will see if I can get a PR adding the vrf=None argument in (so that we at least can start adding support for get_interfaces_ip by platform (one at a time).

jobec commented 5 years ago

The VRF argument is also needed for get_arp_table because, on for example nxos_ssh, it gives back only the ARP table of the default VRF. While the MAC address table is across all interface, irrespective of the VRF.

jobec commented 5 years ago

One other thing missing is a get_vrfs() kind of getter. Because you can't use VRFs if you don't know which ones are there on a device.