From 428a5d9979858c39a24c9128816f4b3f9df0a4cd Mon Sep 17 00:00:00 2001 From: Arvindsrinivasan Lakshmi Narasimhan <55814491+arlakshm@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:20:59 -0800 Subject: [PATCH] [chassis] fix show bgp summary when no neighbors are present on one ASIC (#3158) This PR #3099 fixes the case where on chassis Linecard there are no BGP neighbors. However, if the Linecard has neighbors on one ASIC but not on other, the command show bgp summary displayed no neighbors. This PR fixes this. How I did it Add check in bgp_util to create empty peer list only once Add UT to cover this case --- tests/bgp_commands_test.py | 53 +++++++++-- tests/conftest.py | 23 ++++- .../asic0/show_ip_bgp_summary.json | 92 +++++++++++++++++++ utilities_common/bgp_util.py | 3 +- 4 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 tests/mock_tables/asic0/show_ip_bgp_summary.json diff --git a/tests/bgp_commands_test.py b/tests/bgp_commands_test.py index db0fb6b76..a60ba8c81 100644 --- a/tests/bgp_commands_test.py +++ b/tests/bgp_commands_test.py @@ -280,11 +280,12 @@ Total number of neighbors 23 """ - -SHOW_BGP_SUMMARY_V4_NO_EXT_NEIGHBORS = """ +SHOW_BGP_SUMMARY_V4_NO_EXT_NEIGHBORS_ON_ALL_ASIC = """ IPv4 Unicast Summary: asic0: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 8972 +asic1: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 +BGP table version 8972 RIB entries 0, using 0 bytes of memory Peers 0, using 0 KiB of memory Peer groups 0, using 0 bytes of memory @@ -296,6 +297,24 @@ Total number of neighbors 0 """ +SHOW_BGP_SUMMARY_V4_NO_EXT_NEIGHBORS_ON_ASIC1 = """ +IPv4 Unicast Summary: +asic0: BGP router identifier 192.0.0.6, local AS number 65100 vrf-id 0 +BGP table version 59923 +asic1: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 +BGP table version 8972 +RIB entries 3, using 3 bytes of memory +Peers 3, using 3 KiB of memory +Peer groups 3, using 3 bytes of memory + + +Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName +----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- +10.0.0.1 4 65222 4633 11029 0 0 0 00:18:33 8514 ARISTA01T2 + +Total number of neighbors 1 +""" + SHOW_BGP_SUMMARY_ALL_V4_NO_EXT_NEIGHBORS = """ IPv4 Unicast Summary: asic0: BGP router identifier 192.0.0.6, local AS number 65100 vrf-id 0 @@ -513,13 +532,14 @@ def test_bgp_summary_multi_asic_no_v6_neigh( assert result.exit_code == 0 assert result.output == show_error_no_v6_neighbor_multi_asic - @patch.object(bgp_util, 'get_external_bgp_neighbors_dict', mock.MagicMock(return_value={})) + @patch.object(multi_asic.MultiAsic, 'get_ns_list_based_on_options', mock.Mock(return_value=['asic0', 'asic1'])) @patch.object(multi_asic.MultiAsic, 'get_display_option', mock.MagicMock(return_value=constants.DISPLAY_EXTERNAL)) @pytest.mark.parametrize('setup_multi_asic_bgp_instance', - ['show_bgp_summary_no_ext_neigh_on_all_asic'], indirect=['setup_multi_asic_bgp_instance']) + ['show_bgp_summary_no_ext_neigh_on_all_asic'], + indirect=['setup_multi_asic_bgp_instance']) @patch.object(device_info, 'is_chassis', mock.MagicMock(return_value=True)) - def test_bgp_summary_multi_asic_no_external_neighbor( + def test_bgp_summary_multi_asic_no_external_neighbors_on_all_asic( self, setup_bgp_commands, setup_multi_asic_bgp_instance): @@ -529,8 +549,27 @@ def test_bgp_summary_multi_asic_no_external_neighbor( show.cli.commands["ip"].commands["bgp"].commands["summary"], []) print("{}".format(result.output)) assert result.exit_code == 0 - assert result.output == SHOW_BGP_SUMMARY_V4_NO_EXT_NEIGHBORS - + assert result.output == SHOW_BGP_SUMMARY_V4_NO_EXT_NEIGHBORS_ON_ALL_ASIC + + + @patch.object(multi_asic.MultiAsic, 'get_ns_list_based_on_options', mock.Mock(return_value=['asic0', 'asic1'])) + @patch.object(multi_asic.MultiAsic, 'get_display_option', mock.MagicMock(return_value=constants.DISPLAY_EXTERNAL)) + @pytest.mark.parametrize('setup_multi_asic_bgp_instance', + ['show_bgp_summary_no_ext_neigh_on_asic1'], + indirect=['setup_multi_asic_bgp_instance']) + @patch.object(device_info, 'is_chassis', mock.MagicMock(return_value=True)) + def test_bgp_summary_multi_asic_no_external_neighbor_on_asic1( + self, + setup_bgp_commands, + setup_multi_asic_bgp_instance): + show = setup_bgp_commands + runner = CliRunner() + result = runner.invoke( + show.cli.commands["ip"].commands["bgp"].commands["summary"], []) + print("{}".format(result.output)) + assert result.exit_code == 0 + assert result.output == SHOW_BGP_SUMMARY_V4_NO_EXT_NEIGHBORS_ON_ASIC1 + @pytest.mark.parametrize('setup_multi_asic_bgp_instance', ['show_bgp_summary_no_ext_neigh_on_all_asic'], indirect=['setup_multi_asic_bgp_instance']) diff --git a/tests/conftest.py b/tests/conftest.py index 886c681b0..fd859cccc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -357,8 +357,25 @@ def mock_run_show_summ_bgp_command_no_ext_neigh_on_all_asic(vtysh_cmd, bgp_names return mock_frr_data else: return "" - - + + def mock_run_show_summ_bgp_command_no_ext_neigh_on_asic1(vtysh_cmd, bgp_namespace, vtysh_shell_cmd=constants.VTYSH_COMMAND): + if vtysh_cmd == "show ip bgp summary json": + if bgp_namespace == "asic1": + m_asic_json_file = 'no_ext_bgp_neigh.json' + else: + m_asic_json_file = 'show_ip_bgp_summary.json' + else: + m_asic_json_file = 'device_bgp_info.json' + + bgp_mocked_json = os.path.join( + test_path, 'mock_tables', bgp_namespace, m_asic_json_file) + if os.path.isfile(bgp_mocked_json): + with open(bgp_mocked_json) as json_data: + mock_frr_data = json_data.read() + return mock_frr_data + else: + return "" + _old_run_bgp_command = bgp_util.run_bgp_command if request.param == 'ip_route_for_int_ip': bgp_util.run_bgp_command = mock_run_bgp_command_for_static @@ -366,6 +383,8 @@ def mock_run_show_summ_bgp_command_no_ext_neigh_on_all_asic(vtysh_cmd, bgp_names bgp_util.run_bgp_command = mock_run_show_sum_bgp_command elif request.param == 'show_bgp_summary_no_ext_neigh_on_all_asic': bgp_util.run_bgp_command = mock_run_show_summ_bgp_command_no_ext_neigh_on_all_asic + elif request.param == 'show_bgp_summary_no_ext_neigh_on_asic1': + bgp_util.run_bgp_command = mock_run_show_summ_bgp_command_no_ext_neigh_on_asic1 else: bgp_util.run_bgp_command = mock_run_bgp_command diff --git a/tests/mock_tables/asic0/show_ip_bgp_summary.json b/tests/mock_tables/asic0/show_ip_bgp_summary.json new file mode 100644 index 000000000..5db427287 --- /dev/null +++ b/tests/mock_tables/asic0/show_ip_bgp_summary.json @@ -0,0 +1,92 @@ +{ +"ipv4Unicast":{ + "routerId":"192.0.0.6", + "as":65100, + "vrfId":0, + "vrfName":"default", + "tableVersion":59923, + "ribCount":163, + "ribMemory":29992, + "peerCount":3, + "peerMemory":3704040, + "peerGroupCount":3, + "peerGroupMemory":128, + "peers":{ + "3.3.3.2":{ + "hostname":"svcstr-sonic-lc1-1", + "remoteAs":65100, + "localAs":65100, + "version":4, + "msgRcvd":127, + "msgSent":123, + "tableVersion":0, + "outq":0, + "inq":0, + "peerUptime":"00:05:26", + "peerUptimeMsec":326000, + "peerUptimeEstablishedEpoch":1707332746, + "pfxRcd":16, + "pfxSnt":12, + "state":"Established", + "peerState":"OK", + "connectionsEstablished":1, + "connectionsDropped":0, + "desc":"ASIC1", + "idType":"ipv4" + }, + "3.3.3.8":{ + "hostname":"svcstr-sonic-lc2-1", + "remoteAs":65100, + "localAs":65100, + "version":4, + "msgRcvd":129, + "msgSent":123, + "tableVersion":0, + "outq":0, + "inq":0, + "peerUptime":"00:05:26", + "peerUptimeMsec":326000, + "peerUptimeEstablishedEpoch":1707332746, + "pfxRcd":18, + "pfxSnt":12, + "state":"Established", + "peerState":"OK", + "connectionsEstablished":1, + "connectionsDropped":0, + "desc":"svcstr-sonic-lc2-1-ASIC1", + "idType":"ipv4" + }, + "10.0.0.1":{ + "hostname":"ARISTA01T2", + "remoteAs":65222, + "localAs":65200, + "version":4, + "msgRcvd":4633, + "msgSent":11029, + "tableVersion":0, + "outq":0, + "inq":0, + "peerUptime":"00:18:33", + "peerUptimeMsec":326000, + "peerUptimeEstablishedEpoch":1707332746, + "pfxRcd":8514, + "pfxSnt":12, + "state":"Established", + "peerState":"OK", + "connectionsEstablished":1, + "connectionsDropped":0, + "desc":"ARISTA01T2", + "idType":"ipv4" + } + + }, + "failedPeers":0, + "displayedPeers": 5, + "totalPeers":5, + "dynamicPeers":0, + "bestPath":{ + "multiPathRelax":"true", + "peerTypeRelax":true + } +} +} diff --git a/utilities_common/bgp_util.py b/utilities_common/bgp_util.py index 814cc84b8..64054662e 100644 --- a/utilities_common/bgp_util.py +++ b/utilities_common/bgp_util.py @@ -401,7 +401,8 @@ def process_bgp_summary_json(bgp_summary, cmd_output, device, has_bgp_neighbors= bgp_summary.setdefault('peers', []).append(peers) else: - bgp_summary['peers'] = [] + if 'peers' not in bgp_summary: + bgp_summary['peers'] = [] except KeyError as e: ctx = click.get_current_context() ctx.fail("{} missing in the bgp_summary".format(e.args[0]))