diff --git a/config/main.py b/config/main.py index 24d87a3737..b13eda85bf 100644 --- a/config/main.py +++ b/config/main.py @@ -767,13 +767,13 @@ def _stop_services(): def _get_sonic_services(): - out = clicommon.run_command("systemctl list-dependencies --plain sonic.target | sed '1d'", return_cmd=True) + out, _ = clicommon.run_command("systemctl list-dependencies --plain sonic.target | sed '1d'", return_cmd=True) return (unit.strip() for unit in out.splitlines()) def _get_delayed_sonic_units(get_timers=False): - rc1 = clicommon.run_command("systemctl list-dependencies --plain sonic-delayed.target | sed '1d'", return_cmd=True) - rc2 = clicommon.run_command("systemctl is-enabled {}".format(rc1.replace("\n", " ")), return_cmd=True) + rc1, _ = clicommon.run_command("systemctl list-dependencies --plain sonic-delayed.target | sed '1d'", return_cmd=True) + rc2, _ = clicommon.run_command("systemctl is-enabled {}".format(rc1.replace("\n", " ")), return_cmd=True) timer = [line.strip() for line in rc1.splitlines()] state = [line.strip() for line in rc2.splitlines()] services = [] @@ -808,16 +808,16 @@ def _restart_services(): def _delay_timers_elapsed(): for timer in _get_delayed_sonic_units(get_timers=True): - out = clicommon.run_command("systemctl show {} --property=LastTriggerUSecMonotonic --value".format(timer), return_cmd=True) + out, _ = clicommon.run_command("systemctl show {} --property=LastTriggerUSecMonotonic --value".format(timer), return_cmd=True) if out.strip() == "0": return False return True def _per_namespace_swss_ready(service_name): - out = clicommon.run_command("systemctl show {} --property ActiveState --value".format(service_name), return_cmd=True) + out, _ = clicommon.run_command("systemctl show {} --property ActiveState --value".format(service_name), return_cmd=True) if out.strip() != "active": return False - out = clicommon.run_command("systemctl show {} --property ActiveEnterTimestampMonotonic --value".format(service_name), return_cmd=True) + out, _ = clicommon.run_command("systemctl show {} --property ActiveEnterTimestampMonotonic --value".format(service_name), return_cmd=True) swss_up_time = float(out.strip())/1000000 now = time.monotonic() if (now - swss_up_time > 120): @@ -842,7 +842,7 @@ def _swss_ready(): return True def _is_system_starting(): - out = clicommon.run_command("sudo systemctl is-system-running", return_cmd=True) + out, _ = clicommon.run_command("sudo systemctl is-system-running", return_cmd=True) return out.strip() == "starting" def interface_is_in_vlan(vlan_member_table, interface_name): @@ -2631,7 +2631,7 @@ def _qos_update_ports(ctx, ports, dry_run, json_data): command = "{} {} {} -t {},config-db -t {},config-db -y {} --print-data".format( SONIC_CFGGEN_PATH, cmd_ns, from_db, buffer_template_file, qos_template_file, sonic_version_file ) - jsonstr = clicommon.run_command(command, display_cmd=False, return_cmd=True) + jsonstr, _ = clicommon.run_command(command, display_cmd=False, return_cmd=True) jsondict = json.loads(jsonstr) port_table = jsondict.get('PORT') diff --git a/config/vlan.py b/config/vlan.py index 60b0d7ea0b..6ea6ee7d65 100644 --- a/config/vlan.py +++ b/config/vlan.py @@ -73,7 +73,7 @@ def restart_ndppd(): ndppd_config_gen_cmd = "sonic-cfggen -d -t /usr/share/sonic/templates/ndppd.conf.j2,/etc/ndppd.conf" ndppd_restart_cmd = "supervisorctl restart ndppd" - output = clicommon.run_command(verify_swss_running_cmd, return_cmd=True) + output, _ = clicommon.run_command(verify_swss_running_cmd, return_cmd=True) if output and output.strip() != "running": click.echo(click.style('SWSS container is not running, changes will take effect the next time the SWSS container starts', fg='red'),) diff --git a/show/kdump.py b/show/kdump.py index 2d93154c64..e5ce7cde74 100644 --- a/show/kdump.py +++ b/show/kdump.py @@ -49,7 +49,7 @@ def get_kdump_oper_mode(): returns "Not Ready"; """ oper_mode = "Not Ready" - command_stdout = clicommon.run_command("/usr/sbin/kdump-config status", return_cmd=True) + command_stdout, _ = clicommon.run_command("/usr/sbin/kdump-config status", return_cmd=True) for line in command_stdout.splitlines(): if ": ready to kdump" in line: @@ -99,7 +99,7 @@ def get_kdump_core_files(): dump_file_list = [] cmd_message = None - command_stdout = clicommon.run_command(find_core_dump_files, return_cmd=True) + command_stdout, _ = clicommon.run_command(find_core_dump_files, return_cmd=True) dump_file_list = command_stdout.splitlines() if not dump_file_list: @@ -123,7 +123,7 @@ def get_kdump_dmesg_files(): dmesg_file_list = [] cmd_message = None - command_stdout = clicommon.run_command(find_dmesg_files, return_cmd=True) + command_stdout, _ = clicommon.run_command(find_dmesg_files, return_cmd=True) dmesg_file_list = command_stdout.splitlines() if not dmesg_file_list: diff --git a/tests/config_test.py b/tests/config_test.py index aa68cce939..a19d16d6f9 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -124,13 +124,13 @@ def mock_run_command_side_effect(*args, **kwargs): if kwargs.get('return_cmd'): if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'": - return 'snmp.timer' + return 'snmp.timer' , 0 elif command == "systemctl list-dependencies --plain sonic.target | sed '1d'": - return 'swss' + return 'swss', 0 elif command == "systemctl is-enabled snmp.timer": - return 'enabled' + return 'enabled', 0 else: - return '' + return '', 0 def mock_run_command_side_effect_disabled_timer(*args, **kwargs): command = args[0] @@ -140,17 +140,17 @@ def mock_run_command_side_effect_disabled_timer(*args, **kwargs): if kwargs.get('return_cmd'): if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'": - return 'snmp.timer' + return 'snmp.timer', 0 elif command == "systemctl list-dependencies --plain sonic.target | sed '1d'": - return 'swss' + return 'swss', 0 elif command == "systemctl is-enabled snmp.timer": - return 'masked' + return 'masked', 0 elif command == "systemctl show swss.service --property ActiveState --value": - return 'active' + return 'active', 0 elif command == "systemctl show swss.service --property ActiveEnterTimestampMonotonic --value": - return '0' + return '0', 0 else: - return '' + return '', 0 def mock_run_command_side_effect_untriggered_timer(*args, **kwargs): command = args[0] @@ -160,15 +160,15 @@ def mock_run_command_side_effect_untriggered_timer(*args, **kwargs): if kwargs.get('return_cmd'): if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'": - return 'snmp.timer' + return 'snmp.timer', 0 elif command == "systemctl list-dependencies --plain sonic.target | sed '1d'": - return 'swss' + return 'swss', 0 elif command == "systemctl is-enabled snmp.timer": - return 'enabled' + return 'enabled', 0 elif command == "systemctl show snmp.timer --property=LastTriggerUSecMonotonic --value": - return '0' + return '0', 0 else: - return '' + return '', 0 # Load sonic-cfggen from source since /usr/local/bin/sonic-cfggen does not have .py extension. sonic_cfggen = load_module_from_source('sonic_cfggen', '/usr/local/bin/sonic-cfggen') diff --git a/tests/conftest.py b/tests/conftest.py index a3bea71f16..f4c29f828b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -145,9 +145,13 @@ def mock_show_bgp_summary( bgp_mocked_json = os.path.join( test_path, 'mock_tables', 'ipv6_bgp_summary_chassis.json') + _old_run_bgp_command = bgp_util.run_bgp_command bgp_util.run_bgp_command = mock.MagicMock( return_value=mock_show_bgp_summary("", "")) + yield + bgp_util.run_bgp_command = _old_run_bgp_command + @pytest.fixture def setup_t1_topo(): @@ -213,6 +217,7 @@ def mock_run_bgp_command(vtysh_cmd, bgp_namespace, vtysh_shell_cmd=constants.RVT else: return "" + _old_run_bgp_command = bgp_util.run_bgp_command if any ([request.param == 'ip_route',\ request.param == 'ip_specific_route', request.param == 'ip_special_route',\ request.param == 'ipv6_route', request.param == 'ipv6_specific_route']): @@ -230,7 +235,6 @@ def mock_run_bgp_command(vtysh_cmd, bgp_namespace, vtysh_shell_cmd=constants.RVT bgp_util.run_bgp_command = mock.MagicMock( return_value=mock_show_bgp_network_single_asic(request)) elif request.param == 'ip_route_for_int_ip': - _old_run_bgp_command = bgp_util.run_bgp_command 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.MagicMock( @@ -241,8 +245,7 @@ def mock_run_bgp_command(vtysh_cmd, bgp_namespace, vtysh_shell_cmd=constants.RVT yield - if request.param == 'ip_route_for_int_ip': - bgp_util.run_bgp_command = _old_run_bgp_command + bgp_util.run_bgp_command = _old_run_bgp_command @pytest.fixture diff --git a/tests/ip_config_test.py b/tests/ip_config_test.py index 24d09c86e3..2bc3525aeb 100644 --- a/tests/ip_config_test.py +++ b/tests/ip_config_test.py @@ -9,6 +9,7 @@ import config.main as config import show.main as show from utilities_common.db import Db +import utilities_common.bgp_util as bgp_util test_path = os.path.dirname(os.path.abspath(__file__)) ip_config_input_path = os.path.join(test_path, "ip_config_input") @@ -30,13 +31,20 @@ """ class TestConfigIP(object): + _old_run_bgp_command = None @classmethod def setup_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "1" + cls._old_run_bgp_command = bgp_util.run_bgp_command + bgp_util.run_bgp_command = mock.MagicMock( + return_value=cls.mock_run_bgp_command()) print("SETUP") ''' Tests for IPv4 ''' + def mock_run_bgp_command(): + return "" + def test_add_del_interface_valid_ipv4(self): db = Db() runner = CliRunner() @@ -284,4 +292,5 @@ def test_intf_unknown_vrf_bind(self): @classmethod def teardown_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "0" + bgp_util.run_bgp_command = cls._old_run_bgp_command print("TEARDOWN") diff --git a/tests/ip_show_routes_test.py b/tests/ip_show_routes_test.py index 1032fc0672..f39ccc52d5 100644 --- a/tests/ip_show_routes_test.py +++ b/tests/ip_show_routes_test.py @@ -3,11 +3,15 @@ from . import show_ip_route_common from click.testing import CliRunner +import mock +import sys test_path = os.path.dirname(os.path.abspath(__file__)) modules_path = os.path.dirname(test_path) scripts_path = os.path.join(modules_path, "scripts") +sys.path.insert(0, test_path) + class TestShowIpRouteCommands(object): @classmethod @@ -18,6 +22,25 @@ def setup_class(cls): os.environ["UTILITIES_UNIT_TESTING"] = "0" os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "" import mock_tables.dbconnector + + def test_show_ip_route_err( + self, + setup_ip_route_commands): + show = setup_ip_route_commands + + def mock_run_bgp_command(*args, **kwargs): + command = args[0] + return "% Unknown command: show ip route unknown", 1 + + with mock.patch('utilities_common.cli.run_command', mock.MagicMock(side_effect=mock_run_bgp_command)) as mock_run_command: + runner = CliRunner() + result = runner.invoke( + show.cli.commands["ip"].commands["route"], ["unknown"]) + print("{}".format(result.output)) + print(result.exit_code) + assert result.exit_code == 1 + assert result.output == "% Unknown command: show ip route unknown" + "\n" + @pytest.mark.parametrize('setup_single_bgp_instance', ['ip_route'], indirect=['setup_single_bgp_instance']) def test_show_ip_route( @@ -117,3 +140,4 @@ def test_show_ipv6_route_err( print("{}".format(result.output)) assert result.exit_code == 0 assert result.output == show_ip_route_common.show_ipv6_route_err_expected_output + "\n" + diff --git a/tests/vlan_test.py b/tests/vlan_test.py index 241cab0c0e..66ec3606cf 100644 --- a/tests/vlan_test.py +++ b/tests/vlan_test.py @@ -8,6 +8,7 @@ import show.main as show from utilities_common.db import Db from importlib import reload +import utilities_common.bgp_util as bgp_util show_vlan_brief_output="""\ +-----------+-----------------+-----------------+----------------+-------------+ @@ -143,16 +144,23 @@ +-----------+-----------------+-----------------+----------------+-------------+ """ class TestVlan(object): + _old_run_bgp_command = None @classmethod def setup_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "1" # ensure that we are working with single asic config + cls._old_run_bgp_command = bgp_util.run_bgp_command + bgp_util.run_bgp_command = mock.MagicMock( + return_value=cls.mock_run_bgp_command()) from .mock_tables import dbconnector from .mock_tables import mock_single_asic reload(mock_single_asic) dbconnector.load_namespace_config() print("SETUP") + def mock_run_bgp_command(): + return "" + def test_show_vlan(self): runner = CliRunner() result = runner.invoke(show.cli.commands["vlan"], []) @@ -579,4 +587,5 @@ def test_config_vlan_add_member_of_portchannel(self): @classmethod def teardown_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "0" + bgp_util.run_bgp_command = cls._old_run_bgp_command print("TEARDOWN") diff --git a/utilities_common/bgp_util.py b/utilities_common/bgp_util.py index 60b7a7a93b..3897d4a103 100644 --- a/utilities_common/bgp_util.py +++ b/utilities_common/bgp_util.py @@ -1,6 +1,7 @@ import ipaddress import json import re +import sys import click import utilities_common.cli as clicommon @@ -185,7 +186,10 @@ def run_bgp_command(vtysh_cmd, bgp_namespace=multi_asic.DEFAULT_NAMESPACE, vtysh cmd = 'sudo {} {} -c "{}"'.format( vtysh_shell_cmd, bgp_instance_id, vtysh_cmd) try: - output = clicommon.run_command(cmd, return_cmd=True) + output, ret = clicommon.run_command(cmd, return_cmd=True) + if ret != 0: + click.echo(output.rstrip('\n')) + sys.exit(ret) except Exception: ctx = click.get_current_context() ctx.fail("Unable to get summary from bgp {}".format(bgp_instance_id)) diff --git a/utilities_common/cli.py b/utilities_common/cli.py index 1845d84e14..ca9e061078 100644 --- a/utilities_common/cli.py +++ b/utilities_common/cli.py @@ -539,7 +539,7 @@ def run_command(command, display_cmd=False, ignore_error=False, return_cmd=False if return_cmd: output = proc.communicate()[0] - return output + return output, proc.returncode if not interactive_mode: (out, err) = proc.communicate()