[DON'T MERGE] - Add VRF structure in for get_interfaces_ip

[ktbyers]

Add VRF structure for get_interfaces_ip

[ktbyers]

Replaces #815

[ktbyers]

Related to:

#792

[ktbyers]

Items that are needed (at least longer term):

• unit tests
• getters table updated to indicate vrf support
• make sure napalm-validate works properly with getter
• make sure napalm-ansible works with getter
• make sure no issues with Salt.

[ktbyers]

@FloLaco Can you provide more details on what the issue with napalm validate is?

[coveralls]

Coverage remained the same at 0.0% when pulling dcfbb83 on vrf_get_interface_ip into dd68b7f on develop.

[FloLaco]

The issue is how to provide the vrf argument in the validation file ?

[ktbyers]

I think you should be able to do:

    _kwargs:
      vrf: whatever

We will need to verify this though.

[ktbyers]

@bewing Any chance of reviewing this? Thanks.

[FloLaco]

I think you should be able to do:

    _kwargs:
      vrf: whatever

We will need to verify this though.

@ktbyers I tried to add kwargs in 2 places but it does not work.
Maybe I did something wrong ...

    napalm_validate:
      validation_file: "{{ val_dir }}/validate-nxos_{{ site }}.yml"
      [...]
      kwargs:
        - vrf: all

or

- get_ospf_neighbors:
    kwargs:
      - vrf: all
    UNDERLAY:
      global:
        peers:
         10.55.0.1: {}
         10.55.0.3: {}

I also tried _kwargs

[ktbyers]

Put on hold as we are probably going to solve this slightly differently.

[ktbyers]

@FloLaco It should be _kwargs.

Why are you making it a list?

    kwargs:
      - vrf: all

I think it should be:

    _kwargs:
         vrf: whatever

[FloLaco]

@FloLaco It should be _kwargs.

Why are you making it a list?

    kwargs:
      - vrf: all

I think it should be:

    _kwargs:
         vrf: whatever

@ktbyers
I also tried it but it didn't work.
Confirm me, I need to put this in my validation_file like I did on my 2nd example

[ktbyers]

@FloLaco I am pretty much going from this example (and I had checked the source code last week for a different issue). See the 'ping' example.

https://napalm.readthedocs.io/en/latest/validate/

Yes, in your validation file. Here is the relevant area of the source code:

https://github.com/napalm-automation/napalm/blob/develop/napalm/base/validate.py#L192

Note, this would be a separate issue so if there is a problem here you should open a separate issue on it.

[FloLaco]

@ktbyers Thanks for the links. Ok so it should work like you said. I'll try again tomorrow and will open a new issue if it's still not working.
Thanks

[ktbyers]

Yes, sounds good. I haven't tested it so if there is an issue there...just let us know.

[FloLaco]

@ktbyers I've tested and it works.. noob error, I've edited the wrong validation_file ...

[jobec]

For nxos_ssh the below patch should work.
We're using it internally in a custom nxoss_ssh driver.

--- a/napalm/nxos_ssh/nxos_ssh.py
+++ b/napalm/nxos_ssh/nxos_ssh.py
@@ -889,7 +889,7 @@ class NXOSSSHDriver(NXOSDriverBase):
         output = self._send_command(command) # noqa
         return ntp_stats

-    def get_interfaces_ip(self):
+    def get_interfaces_ip(self, vrf=""):
         """
         Get interface IP details. Returns a dictionary of dictionaries.

@@ -920,8 +920,8 @@ class NXOSSSHDriver(NXOSDriverBase):
         }
         """
         interfaces_ip = {}
-        ipv4_command = 'show ip interface vrf default'
-        ipv6_command = 'show ipv6 interface vrf default'
+        ipv4_command = 'show ip interface vrf {}'.format(vrf or "default")
+        ipv6_command = 'show ipv6 interface vrf {}'.format(vrf or "default")
         output_v4 = self._send_command(ipv4_command)
         output_v6 = self._send_command(ipv6_command)

[jobec]

I've implemented the above patch in PR #877

[jobec]

@ktbyers

From base.py:

'vrf' of null-string will default to the global or default VRF. 'vrf' of 'all' will
return the interface IPs for all VRFs. 'vrf' referring to a specific VRF will return
the interface IPs of that specific VRF. In all cases the same data structure is returned
and no reference to the VRF that was used is included in the output.

I would vote to turn this around and make the default to show from all VRFs. E.g. vrf='' equals vrf=all``.
Because:

• It looks like this is already the case for all platforms except for nxos.
• People expect to see everything by default, instead of only a subset on some platforms and all on others.
• It would mean quite some juggling to turn it around for other platforms (e.g. show only global or default VRFs).

[ktbyers]

@jobec Let me review the past history on this issue a bit.

On the face of it that all sounds reasonable as my main concern regarding using 'all' related to backwards compatibility.

[ktbyers]

This is not being actively worked on any longer so I am going to close it.