napalm-automation / napalm-base

Apache License 2.0
32 stars 48 forks source link

Add VRF support to base traceroute and ping #185

Closed XioNoX closed 7 years ago

XioNoX commented 7 years ago

Cf. https://github.com/napalm-automation/napalm-ios/pull/102 as well.

Please let me know if anything is wrong.

dbarrosop commented 7 years ago

Two things:

  1. pylama isn not happy. Can you fix it?
  2. We will have to make sure all drivers are updated before accepting this one or things will break.
mirceaulinic commented 7 years ago

JunOS: https://github.com/napalm-automation/napalm-junos/pull/116

mirceaulinic commented 7 years ago

IOS-XR: https://github.com/napalm-automation/napalm-iosxr/pull/97

mirceaulinic commented 7 years ago

eOS: https://github.com/napalm-automation/napalm-eos/pull/125

mirceaulinic commented 7 years ago

Just realised @XioNoX come with the PR for IOS: https://github.com/napalm-automation/napalm-ios/pull/102 The only missing piece is pluribus which doesn't know about VRF, so it will be just a slight adjustment in the header. And then we're ready to get this in!

mirceaulinic commented 7 years ago

And Pluribus: https://github.com/napalm-automation/napalm-pluribus/pull/38

mirceaulinic commented 7 years ago

Actually we're waiting for few more drivers to implement this change: nxos, ros and vyos.

dbarrosop commented 7 years ago

I am a bit lost here. What's the current status? Should we tag all the related issues/PR with a vrf tag to be able to find them out easily?

mirceaulinic commented 7 years ago

ROS: https://github.com/napalm-automation/napalm-ros/pull/42

mirceaulinic commented 7 years ago

Vyos: https://github.com/napalm-automation/napalm-vyos/pull/12

mirceaulinic commented 7 years ago

@dbarrosop @XioNoX as this a API change, the vrf arg has to to added on all drivers implementing ping and/or traceroute:

We're currently waiting for the maintainers of ros and vyos to confirm the changes I have proposed make sense. Afterwards, we can merge this and release a new napalm-base version and one minor release for each driver.