mirceaulinic commented 7 years ago

Ping @jeffgallagher - would you mind testing this branch?

jeffgallagher commented 7 years ago

@mirceaulinic - From my perspective #138 is good - single int value returned.

139 is fixed - the outer key is now a correct global VRF and not the routing instance.. i.e. inet.0 etc are now considered to be a part of the global vrf.

Both items can be closed from my testing.

This does still leave one item that is likely a separate issue, but I will mention it here since one solution would touch this code and relates somewhat to #139:

In JunOS we now have:

get_bgp_neighbors(): Outer key values: global

get_bgp_neighbors_detail(): Outer key values: global

Where both outer keys are now (correctly) listed as global. (I say correct since the documentation for get_bgp_neighbors indicates that the first (outer) dictionary will be 'global' if there is no VRF. )

However, run the same basic query against a Cisco XR box and you get:

get_bgp_neighbors(): Outer key values: global

get_bgp_neighbors_detail(): Outer key values: default

JunOS returns 'global' for both calls when there is no VRF, however IOSXR returns 'global' for the neighbor() call but 'default' for the neighbor_detail().

Either best to rename/default everything to 'default' or everything to 'global' - probably not wise to return different values for different OS types. I did have a collision on the XR side as well when both values came back different. Relative to #139 one option is to convert all things to 'default' from global.

Perhaps this is better addressed on the Cisco side. Otherwise code runs great.

dbarrosop commented 7 years ago

JunOS returns 'global' for both calls when there is no VRF, however IOSXR returns 'global' for the neighbor() call but 'default' for the neighbor_detail().

@jeffgallagher that is a bug on the IOSXR side, everything should be global for consistency. It's actually reported already: https://github.com/napalm-automation/napalm-iosxr/issues/115

@mirceaulinic looks like CI is broken

mirceaulinic commented 7 years ago

Yes @dbarrosop - I will com back to this and find the cause.