Skip to content

Commit

Permalink
[chassis] fix show bgp summary when no neighbors are present on one A…
Browse files Browse the repository at this point in the history
…SIC (sonic-net#3158)

This PR sonic-net#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
  • Loading branch information
arlakshm authored and mssonicbld committed Feb 12, 2024
1 parent 54595c1 commit 326a16f
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 10 deletions.
53 changes: 46 additions & 7 deletions tests/bgp_commands_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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'])
Expand Down
23 changes: 21 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,15 +357,34 @@ 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
elif request.param == 'show_bgp_summary_no_neigh':
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

Expand Down
92 changes: 92 additions & 0 deletions tests/mock_tables/asic0/show_ip_bgp_summary.json
Original file line number Diff line number Diff line change
@@ -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
}
}
}
3 changes: 2 additions & 1 deletion utilities_common/bgp_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]))

0 comments on commit 326a16f

Please sign in to comment.