From 1a2a9a34894ad682351e3b02f0a084780290532b Mon Sep 17 00:00:00 2001 From: Sangita Maity Date: Thu, 23 Dec 2021 09:02:22 -0800 Subject: [PATCH] [config] Fix 'config reload -l' command to get filename by default (#1611) Fix Azure/sonic-buildimage#7433 Right now config reload -l is getting failed due to an error. I guess the problem is here in sonic-utilities repo. If user does not provide filename with config reload -l, command = "{} -j {} -v DEVICE_METADATA.localhost.hwsku".format(SONIC_CFGGEN_PATH, filename) will not provide cfg_hwsku i.e. hwsku parameter as it should which will later cause problem here command = "{} -H -k {} --write-to-db".format(SONIC_CFGGEN_PATH, cfg_hwsku) as hwsku is not available around that time. that's why we notice errors like No such file or directory: 'None' as pasted in this issue. - How I did it To Fix the issue, moved the part where the code gets cfg_hwsku command = "{} -j {} -v DEVICE_METADATA.localhost.hwsku".format(SONIC_CFGGEN_PATH, file) to the same location it needed as we get filename by default. - How to verify it 'sudo config reload -l' Added test cases. Signed-off-by: Sangita Maity --- config/main.py | 29 ++++++---- tests/config_reload_input/config_db.json | 62 ++++++++++++++++++++ tests/config_reload_input/init_cfg.json | 68 ++++++++++++++++++++++ tests/config_test.py | 72 ++++++++++++++++++++++++ 4 files changed, 221 insertions(+), 10 deletions(-) create mode 100644 tests/config_reload_input/config_db.json create mode 100644 tests/config_reload_input/init_cfg.json diff --git a/config/main.py b/config/main.py index 5046275f6e3d..49ddd5fe6316 100644 --- a/config/main.py +++ b/config/main.py @@ -1376,16 +1376,6 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, disable_arp_cach click.echo("Input {} config file(s) separated by comma for multiple files ".format(num_cfg_file)) return - if load_sysinfo: - command = "{} -j {} -v DEVICE_METADATA.localhost.hwsku".format(SONIC_CFGGEN_PATH, filename) - proc = subprocess.Popen(command, shell=True, text=True, stdout=subprocess.PIPE) - cfg_hwsku, err = proc.communicate() - if err: - click.echo("Could not get the HWSKU from config file, exiting") - sys.exit(1) - else: - cfg_hwsku = cfg_hwsku.strip() - # For dual ToR devices, cache ARP and FDB info localhost_metadata = db.cfgdb.get_table('DEVICE_METADATA')['localhost'] cache_arp_table = not disable_arp_cache and 'subtype' in localhost_metadata and localhost_metadata['subtype'].lower() == 'dualtor' @@ -1427,6 +1417,25 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, disable_arp_cach click.echo("The config file {} doesn't exist".format(file)) continue + if load_sysinfo: + try: + command = "{} -j {} -v DEVICE_METADATA.localhost.hwsku".format(SONIC_CFGGEN_PATH, file) + proc = subprocess.Popen(command, shell=True, text=True, stdout=subprocess.PIPE) + output, err = proc.communicate() + + except FileNotFoundError as e: + click.echo("{}".format(str(e)), err=True) + raise click.Abort() + except Exception as e: + click.echo("{}\n{}".format(type(e), str(e)), err=True) + raise click.Abort() + + if not output: + click.secho("Could not get the HWSKU from config file, Exiting!!!", fg='magenta') + sys.exit(1) + + cfg_hwsku = output.strip() + if namespace is None: config_db = ConfigDBConnector() else: diff --git a/tests/config_reload_input/config_db.json b/tests/config_reload_input/config_db.json new file mode 100644 index 000000000000..4b6a0ad405eb --- /dev/null +++ b/tests/config_reload_input/config_db.json @@ -0,0 +1,62 @@ +{ + "DEVICE_METADATA": { + "localhost": { + "docker_routing_config_mode": "split", + "hostname": "sonic", + "hwsku": "Seastone-DX010-25-50", + "mac": "00:e0:ec:89:6e:48", + "platform": "x86_64-cel_seastone-r0", + "type": "ToRRouter" + } + }, + "VLAN_MEMBER": { + "Vlan1000|Ethernet0": { + "tagging_mode": "untagged" + }, + "Vlan1000|Ethernet4": { + "tagging_mode": "untagged" + }, + "Vlan1000|Ethernet8": { + "tagging_mode": "untagged" + } + }, + "VLAN": { + "Vlan1000": { + "vlanid": "1000", + "dhcp_servers": [ + "192.0.0.1", + "192.0.0.2", + "192.0.0.3", + "192.0.0.4" + ] + } + }, + "PORT": { + "Ethernet0": { + "alias": "Eth1", + "lanes": "65, 66, 67, 68", + "description": "Ethernet0 100G link", + "speed": "100000" + }, + "Ethernet4": { + "admin_status": "up", + "alias": "fortyGigE0/4", + "description": "Servers0:eth0", + "index": "1", + "lanes": "29,30,31,32", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000" + }, + "Ethernet8": { + "admin_status": "up", + "alias": "fortyGigE0/8", + "description": "Servers1:eth0", + "index": "2", + "lanes": "33,34,35,36", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000" + } + } +} diff --git a/tests/config_reload_input/init_cfg.json b/tests/config_reload_input/init_cfg.json new file mode 100644 index 000000000000..968db5b8f8e3 --- /dev/null +++ b/tests/config_reload_input/init_cfg.json @@ -0,0 +1,68 @@ +{ + "DEVICE_METADATA": { + "localhost": { + "buffer_model": "traditional", + "default_bgp_status": "up", + "default_pfcwd_status": "disable" + } + }, + "CRM": { + "Config": { + "polling_interval": "300", + "ipv4_route_threshold_type": "percentage", + "ipv4_route_low_threshold": "70", + "ipv4_route_high_threshold": "85", + "ipv6_route_threshold_type": "percentage", + "ipv6_route_low_threshold": "70", + "ipv6_route_high_threshold": "85", + "ipv4_nexthop_threshold_type": "percentage", + "ipv4_nexthop_low_threshold": "70", + "ipv4_nexthop_high_threshold": "85", + "ipv6_nexthop_threshold_type": "percentage", + "ipv6_nexthop_low_threshold": "70", + "ipv6_nexthop_high_threshold": "85", + "ipv4_neighbor_threshold_type": "percentage", + "ipv4_neighbor_low_threshold": "70", + "ipv4_neighbor_high_threshold": "85", + "ipv6_neighbor_threshold_type": "percentage", + "ipv6_neighbor_low_threshold": "70", + "ipv6_neighbor_high_threshold": "85", + "nexthop_group_member_threshold_type": "percentage", + "nexthop_group_member_low_threshold": "70", + "nexthop_group_member_high_threshold": "85", + "nexthop_group_threshold_type": "percentage", + "nexthop_group_low_threshold": "70", + "nexthop_group_high_threshold": "85", + "acl_table_threshold_type": "percentage", + "acl_table_low_threshold": "70", + "acl_table_high_threshold": "85", + "acl_group_threshold_type": "percentage", + "acl_group_low_threshold": "70", + "acl_group_high_threshold": "85", + "acl_entry_threshold_type": "percentage", + "acl_entry_low_threshold": "70", + "acl_entry_high_threshold": "85", + "acl_counter_threshold_type": "percentage", + "acl_counter_low_threshold": "70", + "acl_counter_high_threshold": "85", + "fdb_entry_threshold_type": "percentage", + "fdb_entry_low_threshold": "70", + "fdb_entry_high_threshold": "85", + "snat_entry_threshold_type": "percentage", + "snat_entry_low_threshold": "70", + "snat_entry_high_threshold": "85", + "dnat_entry_threshold_type": "percentage", + "dnat_entry_low_threshold": "70", + "dnat_entry_high_threshold": "85", + "ipmc_entry_threshold_type": "percentage", + "ipmc_entry_low_threshold": "70", + "ipmc_entry_high_threshold": "85", + "mpls_inseg_threshold_type": "percentage", + "mpls_inseg_low_threshold": "70", + "mpls_inseg_high_threshold": "85", + "mpls_nexthop_threshold_type": "percentage", + "mpls_nexthop_low_threshold": "70", + "mpls_nexthop_high_threshold": "85" + } + } +} diff --git a/tests/config_test.py b/tests/config_test.py index 873f9889b44d..58360703f8be 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -13,11 +13,25 @@ from sonic_py_common import device_info from utilities_common.db import Db +from utilities_common.general import load_module_from_source from generic_config_updater.generic_updater import ConfigFormat import config.main as config +# Add Test, module and script path. +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) +sys.path.insert(0, modules_path) +sys.path.insert(0, scripts_path) +os.environ["PATH"] += os.pathsep + scripts_path + +# Config Reload input Path +mock_db_path = os.path.join(test_path, "config_reload_input") + + load_minigraph_command_output="""\ Stopping SONiC target ... Running command: /usr/local/bin/sonic-cfggen -H -m --write-to-db @@ -55,6 +69,10 @@ Reloading Monit configuration ... """ +reload_config_with_sys_info_command_output="""\ +Running command: rm -rf /tmp/dropstat-* +Running command: /usr/local/bin/sonic-cfggen -H -k Seastone-DX010-25-50 --write-to-db""" + def mock_run_command_side_effect(*args, **kwargs): command = args[0] @@ -70,6 +88,60 @@ def mock_run_command_side_effect(*args, **kwargs): return '' +# 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') + + +class TestConfigReload(object): + @classmethod + def setup_class(cls): + os.environ['UTILITIES_UNIT_TESTING'] = "1" + print("SETUP") + + from .mock_tables import mock_single_asic + importlib.reload(mock_single_asic) + + import config.main + importlib.reload(config.main) + + def test_config_reload(self, get_cmd_module, setup_single_broadcom_asic): + with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command: + (config, show) = get_cmd_module + + jsonfile_config = os.path.join(mock_db_path, "config_db.json") + jsonfile_init_cfg = os.path.join(mock_db_path, "init_cfg.json") + + # create object + config.INIT_CFG_FILE = jsonfile_init_cfg + config.DEFAULT_CONFIG_DB_FILE = jsonfile_config + + db = Db() + runner = CliRunner() + obj = {'config_db': db.cfgdb} + + # simulate 'config reload' to provoke load_sys_info option + result = runner.invoke(config.config.commands["reload"], ["-l", "-n", "-y"], obj=obj) + + print(result.exit_code) + print(result.output) + traceback.print_tb(result.exc_info[2]) + + assert result.exit_code == 0 + + assert "\n".join([l.rstrip() for l in result.output.split('\n')][:2]) == reload_config_with_sys_info_command_output + + @classmethod + def teardown_class(cls): + print("TEARDOWN") + os.environ['UTILITIES_UNIT_TESTING'] = "0" + + # change back to single asic config + from .mock_tables import dbconnector + from .mock_tables import mock_single_asic + importlib.reload(mock_single_asic) + dbconnector.load_namespace_config() + + class TestLoadMinigraph(object): @classmethod def setup_class(cls):