Shutdown BGP neighbours are not parsed correctly

jemcek commented 7 years ago

Description of Issue/Question

If a BGP neighbour is Admin shutdown, the key "is_enabled" has value True instead of False

Did you follow the steps from https://github.com/napalm-automation/napalm#faq

[* ] Yes
[ ] No

napalm-ios version

napalm-ios==0.6.2

IOS version

Cisco IOS Software, s2t54 Software (s2t54-ADVIPSERVICESK9-M), Version 15.2(1)SY1a, RELEASE SOFTWARE (fc6)

Steps to Reproduce the Issue

In case the bgp neighbour 10.1.1.1 is shutdown, the output from "show bgp all summary" is: .. 10.1.1.1 4 65001 0 0 1 0 0 never Idle (Admin) ... but the "Admin" string is not parsed correctly and the result is:

"10.1.1.1": { ... "is_enabled": true, "is_up": false,

The problem is in the line 1198 inside the ios.py which strips the "Admin" string because it is the 11th element inside the list: fields = line.split()[:10]

and so it never gettes into "state_prefix" variable for later check:

if '(Admin)' in state_prefix: is_enabled = False else: is_enabled = True

if I for example remove the [:10] and concatenate last two strings Idle and (Admin), the it works fine:

fields = line.split() if len(fields) > 10: fields[9] = fields[9] + fields[10] fields = fields[:10]

"10.1.1.1": {... "is_enabled": false, "is_up": false,

ktbyers commented 7 years ago

@jemcek Can you retest this using the develop branch?

get_bgp_neighbors was re-written recently (in this PR):

https://github.com/napalm-automation/napalm-ios/pull/153

jemcek commented 7 years ago

I tested the development branch and I can confirm that this issue is solved.

However, there is another problem now, that is not present int the master branch (because of the different show commands used).

If the bgp configuration is like this :

router bgp XZ
  ...
  address-family ipv4
     neighbour 10.1.1.1 remote-as 10
  address-family ipv4 multicast 
     neighbour 10.1.1.1 remote-as 10

then the command that is used inside the get_bgp_neighbours routine also returns the Multicast peerings detail:

#show bgp ipv4 unicast neighbours
BGP neighbour is 10.1.1.1
...
   For address family: IPv4 Unicast
   ...
   For address family: IPv4 Multicast

In my case this causes the get_bgp_neighbour() to crash here:

else:
   # found previous data for matching remote_addr, but for different afi
   existing = bgp_neighbor_data['global']['peers'][remote_addr]
   assert afi not in existing['address_family']     <----- here it crashes

Probably the multicast part of the response should be checked and skiped earlier.

ktbyers commented 7 years ago

Can you post the output from the following:

show bgp ipv4 unicast neighbors
show bgp all summary

We can then update the unit tests and correct the problem. Thanks.

jemcek commented 7 years ago

Here is the output:

bgp-output.txt.zip

mirceaulinic commented 6 years ago

Hi @jemcek - we are currently in the process of reunification, please check https://napalm-automation.net/reunification/. For the time being, we have moved this issue to https://github.com/napalm-automation/napalm/issues/461 so we can discuss further. Going forward, we'd like to ask you to submit Pull Requests and Issues to the main repository: https://github.com/napalm-automation/napalm

Thanks for understanding!

napalm-automation / napalm-ios