From 326a16f34092dcb9612796df99ad0348f2d3d423 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 db0fb6b761..a60ba8c81f 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 886c681b06..fd859cccce 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 0000000000..5db427287f --- /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 814cc84b83..64054662e3 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]))