Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eos issue#1743 bug fix get_bgp_neighbors() #1746

Conversation

vaderisback77
Copy link

Testing using updated NEIGHBOR_FILTER value

--snipped output for IPv4 and IPv6 BGP neighbors--


## BEFORE

>>> print(pprint(output_neighbor_cmds))
[{'
            'BGP neighbor is 110.1.1.1, remote AS 60001, external link\n'
            '  BGP version 4, remote router ID 194.0.0.1, VRF VPN-DRG-2555\n'
            '  BGP state is Idle, Could not find interface for peer\n'
            '    IPv4 Unicast:                     0         0              '
            '0                   0\n'
            '    IPv6 Unicast:                     0         0              '
            '0                   0\n'
            'Local AS is 60000, local router ID 12.12.12.12\n'
            'BGP neighbor is 111.1.1.1, remote AS 31898, external link\n'
            '  BGP version 4, remote router ID 111.1.1.1, VRF VPN-DRG-2555\n'
            '  BGP state is Established, up for 1d04h\n'
            '    IPv4 Unicast:                     1         0              '
            '0                   0\n'
            '    IPv6 Unicast:                     0         0              '
            '0                   0\n'
            'Local AS is 60000, local router ID 12.12.12.12\n'
            '  IPv4 Unicast: 111.1.1.0\n'

             'BGP neighbor is 2607:9b80:8000:187::5, remote AS 31898, external '
            'link\n'
            '  BGP version 4, remote router ID 111.1.27.1, VRF VPN-DRG-2581\n'
            '  BGP state is Established, up for 1d04h\n'
            '    IPv4 Unicast:                     0         0              '
            '0                   0\n'
            '    IPv6 Unicast:                     0         0              '
            '0                   0\n'
            'Local AS is 60000, local router ID 111.1.27.0\n'
            '  IPv6 Unicast: 2607:9b80:8000:187::4\n'


## AFTER

>>> pprint(output_neighbor_cmds)
[{'output': 'BGP neighbor is 110.1.1.1, remote AS 60001, external link\n'
            '  BGP version 4, remote router ID 0.0.0.0, VRF VPN-DRG-2555\n'
            '  BGP state is Idle, Could not find interface for peer\n'
            '    IPv4 Unicast:                     0         0              '
            '0                   0\n'
            '    IPv6 Unicast:                     0         0              '
            '0                   0\n'
            'Local AS is 60000, local router ID 12.12.12.12\n'
            'BGP neighbor is 111.1.1.1, remote AS 31898, external link\n'
            '  BGP version 4, remote router ID 111.1.1.1, VRF VPN-DRG-2555\n'
            '  BGP state is Established, up for 00:33:24\n'
            '    IPv4 Unicast:                     1         0              '
            '0                   0\n'
            '    IPv6 Unicast:                     0         0              '
            '0                   0\n'
            'Local AS is 60000, local router ID 12.12.12.12\n'

            'BGP neighbor is 2607:9b80:8000:187::5, remote AS 31898, external '
            'link\n'
            '  BGP version 4, remote router ID 111.1.27.1, VRF VPN-DRG-2581\n'
            '  BGP state is Established, up for 00:33:13\n'
            '    IPv4 Unicast:                     0         0              '
            '0                   0\n'
            '    IPv6 Unicast:                     0         0              '
            '0                   0\n'
            'Local AS is 60000, local router ID 111.1.27.0\n'

@vaderisback77
Copy link
Author

Local pytest checks passing after adding missing mocked data files and dependencies

(napalm-3.9) ➜  napalm git:(vaderisback77/issue-1743/bug-fix/get-bgp-neighbors-7050cx3m) ✗ pytest test
============================================================================================ test session starts ============================================================================================
platform darwin -- Python 3.9.5, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
rootdir: /Users/saurasar/Documents/ocina/git/napalm, configfile: setup.cfg
plugins: pylama-8.2.1, json-0.4.0, cov-3.0.0
collected 591 items                                                                                                                                                                                         

test/base/test_get_network_driver.py .............                                                                                                                                                    [  2%]
test/base/test_helpers.py .............                                                                                                                                                               [  4%]
test/base/test_mock_driver.py ..........                                                                                                                                                              [  6%]
test/base/test_napalm_test_framework.py sssssssssssssssssssssssssssssss                                                                                                                               [ 11%]
test/base/validate/test_unit.py ............................                                                                                                                                          [ 16%]
test/base/validate/test_validate.py ............                                                                                                                                                      [ 18%]
test/eos/test_cli_syntax.py ....                                                                                                                                                                      [ 18%]
test/eos/test_getters.py ...............................ss...........ss.................s.                                                                                                            [ 29%]
test/eos/test_heredoc.py ...                                                                                                                                                                          [ 30%]
test/eos/test_versions.py ..                                                                                                                                                                          [ 30%]
test/ios/test_getters.py .....................................................................s..s...............s..                                                                                  [ 46%]
test/iosxr/test_getters.py ................ss......s...s..s...sss                                                                                                                                     [ 52%]
test/iosxr_netconf/test_getters.py ......................ss......s...s..s.sssss                                                                                                                       [ 59%]
test/junos/test_getters.py ...........................s..........s..............s...                                                                                                                  [ 69%]
test/nxapi_plumbing/test_build_payload.py .....                                                                                                                                                       [ 70%]
test/nxapi_plumbing/test_config.py .......                                                                                                                                                            [ 71%]
test/nxapi_plumbing/test_import.py .                                                                                                                                                                  [ 71%]
test/nxapi_plumbing/test_show_jsonrpc.py ......                                                                                                                                                       [ 72%]
test/nxapi_plumbing/test_show_xml.py .....                                                                                                                                                            [ 73%]
test/nxos/test_getters.py .....s.......ss...ss........ss.ss.....s....s.....                                                                                                                           [ 81%]
test/nxos_ssh/test_getters.py .................ss....s...s.........s.ss........ss............                                                                                                         [ 92%]
test/pyiosxr/test_iosxr.py ............................................                                                                                                                               [100%]

============================================================================================= warnings summary ==============================================================================================
test/iosxr_netconf/test_getters.py::TestGetter::test_get_config[normal]
  /Users/saurasar/Documents/ocina/git/napalm/napalm/iosxr_netconf/iosxr_netconf.py:3142: FutureWarning: The behavior of this method will change in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
    if cli_tree:

-- Docs: https://docs.pytest.org/en/stable/warnings.html
--------------------------------------------------------------- generated json report: /Users/saurasar/Documents/ocina/git/napalm/report.json ---------------------------------------------------------------
================================================================================ 511 passed, 80 skipped, 1 warning in 9.44s =================================================================================
(napalm-3.9) ➜  napalm git:(vaderisback77/issue-1743/bug-fix/get-bgp-neighbors-7050cx3m) ✗ 

Copy link
Member

@bewing bewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know if this is JUST platform-related, or did the output update in other devices running the latest EOS versions?

napalm/eos/eos.py Show resolved Hide resolved
@vaderisback77
Copy link
Author

vaderisback77 commented Sep 19, 2022

Do we know if this is JUST platform-related, or did the output update in other devices running the latest EOS versions?

We noticed this issue during the onboarding of 7050CX3M platform with 4.27.2F-DPE, I am running same tests against few other platforms we have in the lab, and will get back with my findings.
I didn't see this same issue when i earlier tested on 7050TX running 4.24.x

@vaderisback77

This comment was marked as duplicate.

@vaderisback77
Copy link
Author

Do we know if this is JUST platform-related, or did the output update in other devices running the latest EOS versions?

I tested in another platform, DCS-7050SX3-48YC8-F with the same code version(4.27.2F-DPE), and I am able to reproduce the issue. This extra line (' IPv[46] Unicast: 111.1.1.0\n') is getting added for established BGP neighbors

            '  BGP version 4, remote router ID 111.1.1.1, VRF VPN-DRG-2555\n'
            '  BGP state is Established, up for 10d09h\n'
            '    IPv4 Unicast:                     1         0              '
            '0                   0\n'
            '    IPv6 Unicast:                     0         0              '
            '0                   0\n'
            'Local AS is 60000, local router ID 12.12.12.12\n'
            '  IPv4 Unicast: 111.1.1.0\n'
         
 >>> k.get_bgp_neighbors()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/saurasar/Documents/ocina/git/napalm/napalm/eos/eos.py", line 750, in get_bgp_neighbors
    "remote_as": napalm.base.helpers.as_number(neighbor_info.group("as")),
AttributeError: 'NoneType' object has no attribute 'group'

@bewing
Copy link
Member

bewing commented Sep 25, 2022

I did some local testing today, and this change overall looks good and doesn't appear to cause any regressions. However, can you please git rm the old/unused show_ip_bgp_neighbors_vrf_all__include_remote... files from the test cases, as they will no longer be used after this change?

@vaderisback77
Copy link
Author

I did some local testing today, and this change overall looks good and doesn't appear to cause any regressions. However, can you please git rm the old/unused show_ip_bgp_neighbors_vrf_all__include_remote... files from the test cases, as they will no longer be used after this change?

done.

@bewing bewing self-assigned this Sep 26, 2022
Copy link
Member

@bewing bewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants