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.

Sign up for GitHub

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

Jump to bottom

[Draft] Add remote compatible show ip bgp and -n all for show ip bgp network #3413

Closed
wants to merge 16 commits into from
Closed

[Draft] Add remote compatible show ip bgp and -n all for show ip bgp network #3413

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
9579a8c
enable direct exec of show ip bgp on supervisor and enable -n all for…
BYGX-wcr Jul 11, 2024
9cf9ccf
add unit test for remote show ip bgp and show ip bgp -n all
BYGX-wcr Jul 11, 2024
729e715
fix for pre-commit check
BYGX-wcr Jul 11, 2024
a15c3b3
fix test suites
BYGX-wcr Jul 11, 2024
146c6b4
fix for unit test suite
BYGX-wcr Jul 11, 2024
2f101b5
fix for test suites
BYGX-wcr Jul 11, 2024
069e047
fix output mismatch for bgp_v4_network_all_asic test vector
BYGX-wcr Jul 11, 2024
da871c2
fix remote show test
BYGX-wcr Jul 11, 2024
74cd5d9
fix rexec in show ip bgp and the corresponding tests
BYGX-wcr Jul 12, 2024
3d816d8
add mechanism to reset subprocess.run
BYGX-wcr Jul 12, 2024
71c17c0
twist to try
BYGX-wcr Jul 12, 2024
80b57fb
twist expected outputs
BYGX-wcr Jul 12, 2024
2f946f8
increase test coverage
BYGX-wcr Jul 12, 2024
c5a3829
small fix
BYGX-wcr Jul 12, 2024
7352a97
small fix
BYGX-wcr Jul 12, 2024
17bf9d0
remove test for show.main
BYGX-wcr Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,20 @@ A convenient alternative is to let the SONiC build system configure a build envi
```
python3 setup.py bdist_wheel
```
Note: This command by default will not update the wheel package in target/. To specify the destination location of wheel package, use "-d" option.

#### To run unit tests

```
python3 setup.py test
```

#### To install the package on a SONiC machine
```
sudo pip uninstall sonic-utilities
sudo pip install YOUR_WHEEL_PACKAGE
```
Note: Don't use "--force-reinstall".

### sonic-utilities-data

Expand Down
39 changes: 32 additions & 7 deletions show/bgp_frr_v4.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import click
import sys
import subprocess

from sonic_py_common import multi_asic
from sonic_py_common import multi_asic, device_info
from show.main import ip
import utilities_common.bgp_util as bgp_util
import utilities_common.cli as clicommon
import utilities_common.constants as constants
import utilities_common.multi_asic as multi_asic_util
import rcli.rexec

###############################################################################
#
Expand All @@ -17,6 +20,12 @@
@ip.group(cls=clicommon.AliasedGroup)
def bgp():
"""Show IPv4 BGP (Border Gateway Protocol) information"""
if device_info.is_supervisor():
# if the device is a chassis, the command need to be executed by rexec
click.echo("Since the current device is a chassis supervisor, " +
"this command will be executed remotely on all linecards")
proc = subprocess.run(["rexec", "all"] + ["-c", " ".join(sys.argv)])
sys.exit(proc.returncode)
pass


Expand Down Expand Up @@ -102,10 +111,16 @@ def neighbors(ipaddress, info_type, namespace):
def network(ipaddress, info_type, namespace):
"""Show IP (IPv4) BGP network"""

if multi_asic.is_multi_asic() and namespace not in multi_asic.get_namespace_list():
ctx = click.get_current_context()
ctx.fail('-n/--namespace option required. provide namespace from list {}'\
.format(multi_asic.get_namespace_list()))
namespace = namespace.strip()
if multi_asic.is_multi_asic():
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you are just keeping original logic here, that if it is multi-asic, and namespace is not provided, error out.
but I think original logic may not be updated, can you change to: if multi asic, and namespace is not provided, print for all asics?
refer to: #2427
another pros for this is, you are running this cmd on sup, if one chassis has both multi/single-asic LCs, show ip bgp sum will output for all LCs, instead of having multi-asic LC error out, in this case we could have at least cmd working for both multi/single-asic.

if namespace == multi_asic.DEFAULT_NAMESPACE:
ctx = click.get_current_context()
ctx.fail('-n/--namespace option required. provide namespace from list {}'
.format(multi_asic.get_namespace_list()))
if namespace != "all" and namespace not in multi_asic.get_namespace_list():
ctx = click.get_current_context()
ctx.fail('invalid namespace {}. provide namespace from list {}'
.format(namespace, multi_asic.get_namespace_list()))

command = 'show ip bgp'
if ipaddress is not None:
Expand All @@ -125,5 +140,15 @@ def network(ipaddress, info_type, namespace):
if info_type is not None:
command += ' {}'.format(info_type)

output = bgp_util.run_bgp_show_command(command, namespace)
click.echo(output.rstrip('\n'))
if namespace == "all":
if multi_asic.is_multi_asic():
for ns in multi_asic.get_namespace_list():
click.echo("\n======== namespace {} ========".format(ns))
output = bgp_util.run_bgp_show_command(command, ns)
click.echo(output.rstrip('\n'))
else:
output = bgp_util.run_bgp_show_command(command, "")
click.echo(output.rstrip('\n'))
else:
output = bgp_util.run_bgp_show_command(command, namespace)
click.echo(output.rstrip('\n'))
6 changes: 5 additions & 1 deletion show/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,11 @@ def protocol(verbose):
ip.add_command(bgp)
from .bgp_frr_v6 import bgp
ipv6.add_command(bgp)

elif device_info.is_supervisor():
wenyiz2021 marked this conversation as resolved.
Show resolved Hide resolved
from .bgp_frr_v4 import bgp
ip.add_command(bgp)
from .bgp_frr_v6 import bgp
ipv6.add_command(bgp)
#
# 'link-local-mode' subcommand ("show ipv6 link-local-mode")
#
Expand Down
126 changes: 126 additions & 0 deletions tests/bgp_commands_input/bgp_network_test_vector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,9 @@
multi_asic_bgp_network_err = \
"""Error: -n/--namespace option required. provide namespace from list ['asic0', 'asic1']"""

multi_asic_bgp_network_err = \
"""Error: invalid namespace asic_unknown. provide namespace from list ['asic0', 'asic1']"""

bgp_v4_network_asic0 = \
"""
BGP table version is 11256, local router ID is 10.1.0.32, vrf id 0
Expand Down Expand Up @@ -311,6 +314,111 @@
Last update: Thu Apr 22 02:13:30 2021
"""

bgp_v4_network_all_asic = \
"""
======== namespace asic0 ========

BGP table version is 11256, local router ID is 10.1.0.32, vrf id 0
Default local pref 100, local AS 65100
Status codes: s suppressed, d damped, h history, * valid, > best, = multipath,
i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes: i - IGP, e - EGP, ? - incomplete

Network Next Hop Metric LocPrf Weight Path
* i0.0.0.0/0 10.1.0.2 100 0 65200 6666 6667 i
* i 10.1.0.0 100 0 65200 6666 6667 i
*= 10.0.0.5 0 65200 6666 6667 i
*> 10.0.0.1 0 65200 6666 6667 i
* i8.0.0.0/32 10.1.0.2 0 100 0 i
* i 10.1.0.0 0 100 0 i
* 0.0.0.0 0 32768 ?
*> 0.0.0.0 0 32768 i
*=i8.0.0.1/32 10.1.0.2 0 100 0 i
*>i 10.1.0.0 0 100 0 i
*=i8.0.0.2/32 10.1.0.2 0 100 0 i
*>i 10.1.0.0 0 100 0 i
*=i8.0.0.3/32 10.1.0.2 0 100 0 i
*>i 10.1.0.0 0 100 0 i
*>i8.0.0.4/32 10.1.0.0 0 100 0 i
*>i8.0.0.5/32 10.1.0.2 0 100 0 i
* i10.0.0.0/31 10.1.0.2 0 100 0 ?
* i 10.1.0.0 0 100 0 ?
*> 0.0.0.0 0 32768 ?
* i10.0.0.4/31 10.1.0.2 0 100 0 ?
* i 10.1.0.0 0 100 0 ?
*> 0.0.0.0 0 32768 ?
*=i10.0.0.8/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.12/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.32/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.34/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.36/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.38/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.40/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.42/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.44/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?

======== namespace asic1 ========

BGP table version is 11256, local router ID is 10.1.0.32, vrf id 0
Default local pref 100, local AS 65100
Status codes: s suppressed, d damped, h history, * valid, > best, = multipath,
i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes: i - IGP, e - EGP, ? - incomplete

Network Next Hop Metric LocPrf Weight Path
* i0.0.0.0/0 10.1.0.2 100 0 65200 6666 6667 i
* i 10.1.0.0 100 0 65200 6666 6667 i
*= 10.0.0.5 0 65200 6666 6667 i
*> 10.0.0.1 0 65200 6666 6667 i
* i8.0.0.0/32 10.1.0.2 0 100 0 i
* i 10.1.0.0 0 100 0 i
* 0.0.0.0 0 32768 ?
*> 0.0.0.0 0 32768 i
*=i8.0.0.1/32 10.1.0.2 0 100 0 i
*>i 10.1.0.0 0 100 0 i
*=i8.0.0.2/32 10.1.0.2 0 100 0 i
*>i 10.1.0.0 0 100 0 i
*=i8.0.0.3/32 10.1.0.2 0 100 0 i
*>i 10.1.0.0 0 100 0 i
*>i8.0.0.4/32 10.1.0.0 0 100 0 i
*>i8.0.0.5/32 10.1.0.2 0 100 0 i
* i10.0.0.0/31 10.1.0.2 0 100 0 ?
* i 10.1.0.0 0 100 0 ?
*> 0.0.0.0 0 32768 ?
* i10.0.0.4/31 10.1.0.2 0 100 0 ?
* i 10.1.0.0 0 100 0 ?
*> 0.0.0.0 0 32768 ?
*=i10.0.0.8/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.12/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.32/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.34/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.36/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.38/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.40/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.42/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
*=i10.0.0.44/31 10.1.0.2 0 100 0 ?
*>i 10.1.0.0 0 100 0 ?
"""

bgp_v6_network_asic0 = \
"""
BGP table version is 12849, local router ID is 10.1.0.32, vrf id 0
Expand Down Expand Up @@ -429,6 +537,9 @@ def mock_show_bgp_network_multi_asic(param):
return bgp_v6_network_ip_address_asic0
elif param == 'bgp_v6_network_bestpath_asic0':
return bgp_v6_network_ip_address_asic0_bestpath
elif param == "bgp_v4_network_all_asic":
# this is mocking the output of a single LC
return bgp_v4_network_asic0
else:
return ''

Expand All @@ -454,6 +565,11 @@ def mock_show_bgp_network_multi_asic(param):
'rc': 1,
'rc_output': bgp_v4_network_longer_prefixes_error
},
'bgp_v4_network_all_asic_on_single_asic': {
'args': ['-nall'],
'rc': 0,
'rc_output': bgp_v4_network
},
'bgp_v6_network': {
'args': [],
'rc': 0,
Expand Down Expand Up @@ -499,6 +615,16 @@ def mock_show_bgp_network_multi_asic(param):
'rc': 0,
'rc_output': bgp_v4_network_bestpath_asic0
},
'bgp_v4_network_all_asic': {
'args': ['-nall'],
'rc': 0,
'rc_output': bgp_v4_network_all_asic
},
'bgp_v4_network_asic_unknown': {
'args': ['-nasic_unkown'],
'rc': 2,
'rc_err_msg': multi_asic_bgp_network_err
},
'bgp_v6_network_multi_asic': {
'args': [],
'rc': 2,
Expand Down
7 changes: 7 additions & 0 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,13 @@ def mock_run_show_summ_bgp_command_no_ext_neigh_on_asic1(
else:
return ""

def mock_multi_asic_list():
return ["asic0", "asic1"]

# mock multi-asic list
if request.param == "bgp_v4_network_all_asic":
multi_asic.get_namespace_list = mock_multi_asic_list

_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
Expand Down
63 changes: 63 additions & 0 deletions tests/remote_show_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import mock
import subprocess
from io import BytesIO
from click.testing import CliRunner


def mock_rexec_command(*args):
mock_stdout = BytesIO(b"""hello world""")
print(mock_stdout.getvalue().decode())
return subprocess.CompletedProcess(args=[], returncode=0, stdout=mock_stdout, stderr=BytesIO())


def mock_rexec_error_cmd(*args):
mock_stderr = BytesIO(b"""Error""")
print(mock_stderr.getvalue().decode())
return subprocess.CompletedProcess(args=[], returncode=1, stdout=BytesIO(), stderr=mock_stderr)


MULTI_LC_REXEC_OUTPUT = '''Since the current device is a chassis supervisor, this command will be executed remotely on all linecards
hello world
'''

MULTI_LC_ERR_OUTPUT = '''Since the current device is a chassis supervisor, this command will be executed remotely on all linecards
Error
'''

class TestRexecBgpNetwork(object):
@classmethod
def setup_class(cls):
pass

@mock.patch("sonic_py_common.device_info.is_supervisor", mock.MagicMock(return_value=True))
@mock.patch("show.main.get_routing_stack", mock.MagicMock(return_value=""))
def test_bgp_setup_in_show_main(self):
import show.main as main
assert main.ip.commands["bgp"] is not None
assert main.ipv6.commands["bgp"] is not None

@mock.patch("sonic_py_common.device_info.is_supervisor", mock.MagicMock(return_value=True))
def test_show_ip_bgp_rexec(self, setup_bgp_commands):
show = setup_bgp_commands
runner = CliRunner()

_old_subprocess_run = subprocess.run
subprocess.run = mock_rexec_command
result = runner.invoke(show.cli.commands["ip"].commands["bgp"], args=["summary"])
print(result.output)
subprocess.run = _old_subprocess_run
assert result.exit_code == 0
assert MULTI_LC_REXEC_OUTPUT == result.output

@mock.patch("sonic_py_common.device_info.is_supervisor", mock.MagicMock(return_value=True))
def test_show_ip_bgp_error_rexec(self, setup_bgp_commands):
show = setup_bgp_commands
runner = CliRunner()

_old_subprocess_run = subprocess.run
subprocess.run = mock_rexec_error_cmd
result = runner.invoke(show.cli.commands["ip"].commands["bgp"], args=["summary"])
print(result.output)
subprocess.run = _old_subprocess_run
assert result.exit_code == 1
assert MULTI_LC_ERR_OUTPUT == result.output
7 changes: 5 additions & 2 deletions tests/show_bgp_network_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ def setup_class(cls):
('bgp_v4_network_bestpath', 'bgp_v4_network_bestpath'),
('bgp_v6_network_longer_prefixes', 'bgp_v6_network_longer_prefixes'),
('bgp_v4_network', 'bgp_v4_network_longer_prefixes_error'),
('bgp_v4_network', 'bgp_v6_network_longer_prefixes_error')],
('bgp_v4_network', 'bgp_v6_network_longer_prefixes_error'),
('bgp_v4_network', 'bgp_v4_network_all_asic_on_single_asic')],
indirect=['setup_single_bgp_instance'])
def test_bgp_network(self, setup_bgp_commands, test_vector,
setup_single_bgp_instance):
Expand All @@ -84,7 +85,9 @@ def setup_class(cls):
('bgp_v4_network_bestpath_asic0', 'bgp_v4_network_bestpath_asic0'),
('bgp_v6_network_asic0', 'bgp_v6_network_asic0'),
('bgp_v6_network_ip_address_asic0', 'bgp_v6_network_ip_address_asic0'),
('bgp_v6_network_bestpath_asic0', 'bgp_v6_network_bestpath_asic0')],
('bgp_v6_network_bestpath_asic0', 'bgp_v6_network_bestpath_asic0'),
('bgp_v4_network_all_asic', 'bgp_v4_network_all_asic'),
('bgp_v4_network', 'bgp_v4_network_asic_unknown')],
indirect=['setup_multi_asic_bgp_instance'])
def test_bgp_network(self, setup_bgp_commands, test_vector,
setup_multi_asic_bgp_instance):
Expand Down
Loading