napalm-automation / napalm-junos

Apache License 2.0
22 stars 42 forks source link

fix sent_prefixes if not advertising using send-state #209

Closed ckishimo closed 6 years ago

ckishimo commented 6 years ago

While running get_bgp_neighbors() I was getting the following error:

Traceback (most recent call last):
  File "client_junos_bgp.py", line 21, in <module>
    res = dev.get_bgp_neighbors()
  File "/home/ckishimo/python/napalm-junos/napalm_junos/junos.py", line 595, in get_bgp_neighbors
    uptime_table_items=uptime_table_items)
  File "/home/ckishimo/python/napalm-junos/napalm_junos/junos.py", line 561, in _get_bgp_neighbors_core
    peer['address_family'] = self._parse_route_stats(neighbor_details)
  File "/home/ckishimo/python/napalm-junos/napalm_junos/junos.py", line 475, in _parse_route_stats
    data[family]['sent_prefixes'] = neighbor['sent_prefixes'][idx]
IndexError: list index out of range

It seems not all ribs have the node <advertised-prefix-count>. The only way I found to fix that was to use the send-state node which can have as a value in sync or not advertising. So if send-state == 'not advertising' the <advertised-prefix-count> may not exist, so we can count it as zero. Example:

             <bgp-rib junos:style="detail">
                <name>bgp.evpn.0</name>
                <bgp-rib-state>BGP restart is complete</bgp-rib-state>
                <vpn-rib-state>VPN restart is complete</vpn-rib-state>
                <send-state>not advertising</send-state>
                <active-prefix-count>1</active-prefix-count>
                <received-prefix-count>1</received-prefix-count>
                <accepted-prefix-count>1</accepted-prefix-count>
                <suppressed-prefix-count>0</suppressed-prefix-count>
            </bgp-rib>

Please let me know if that makes sense

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.2%) to 78.571% when pulling 246c0f9c21239f3c551178d18ff003c7aecf7a1c on ckishimo:devel-bgp into 4275c95ca9884beebfb6fa6bfc0d8e0496de86bd on napalm-automation:develop.

mirceaulinic commented 6 years ago

@ckishimo would you be able to provide a test case for this? And does this solve https://github.com/napalm-automation/napalm-junos/issues/177?

ckishimo commented 6 years ago

@mirceaulinic for the test case is it enough to create a new dir in mocked_data/test_get_bgp_neighbors with the xml file and the expected_result.json ? I'll have a look to #177 Thanks!

mirceaulinic commented 6 years ago

Yes @ckishimo - you have to create a new dir under mocked_data/test_get_bgp_neighbors, where you put the expected_results.json, and a file with explicit facts.yml (as the output may be different across versions and platforms) + a couple of XML documents, as in https://github.com/napalm-automation/napalm-junos/tree/develop/test/unit/mocked_data/test_get_bgp_neighbors/normal.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.05%) to 78.689% when pulling 2a2ab9ff3bfb280306548bf8101263e99b523798 on ckishimo:devel-bgp into 4275c95ca9884beebfb6fa6bfc0d8e0496de86bd on napalm-automation:develop.

mirceaulinic commented 6 years ago

One (or maybe few more) @ckishimo - get-bgp-neighbor-informationmaster.xml this time; but have a look at https://github.com/napalm-automation/napalm-junos/tree/develop/test/unit/mocked_data/test_get_bgp_neighbors/normal and make sure you have the same files (with the content you want to provide).

ckishimo commented 6 years ago

@mirceaulinic sorry I thought not all files were needed. I will push same files as in the normal directory

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.4%) to 79.141% when pulling 96baf37a3f81d4d7aef4150c440de0b77c9ed228 on ckishimo:devel-bgp into 4275c95ca9884beebfb6fa6bfc0d8e0496de86bd on napalm-automation:develop.

mirceaulinic commented 6 years ago

Thanks @ckishimo