From 46d302497509e62a98b10e78aaa9bf30dbc0ace5 Mon Sep 17 00:00:00 2001 From: cchoate Date: Mon, 28 Nov 2022 11:27:49 -0800 Subject: [PATCH 1/8] sonic-utilities: Update config reload() to verify formatting of an input file. sonic-utilities: bugfix-9499 * Update config/main.py to verify if a config input file is properly formatted before writing it into confib_db * Update tests/config_test.py to include test cases for invalid input file * Add tests/config_reload_input/config_db_invalid.json as invalid input file used in tests/config_test.py Signed-off-by: cchoate54@gmail.com --- config/main.py | 9 +++ .../config_db_invalid.json | 62 +++++++++++++++++++ tests/config_test.py | 60 +++++++++++++++++- 3 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 tests/config_reload_input/config_db_invalid.json diff --git a/config/main.py b/config/main.py index 004b40bdb4..09c0138c4f 100644 --- a/config/main.py +++ b/config/main.py @@ -1547,6 +1547,15 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form if len(cfg_files) != num_cfg_file: click.echo("Input {} config file(s) separated by comma for multiple files ".format(num_cfg_file)) return + + # Check if the file is properly formatted before proceeding. + for file in cfg_files: + try: + config_input = read_json_file(file) + except Exception as e: + click.secho("Bad format: json file broken. {}".format(str(e)), + fg='magenta') + sys.exit(1) #Stop services before config push if not no_service_restart: diff --git a/tests/config_reload_input/config_db_invalid.json b/tests/config_reload_input/config_db_invalid.json new file mode 100644 index 0000000000..cb394023d8 --- /dev/null +++ b/tests/config_reload_input/config_db_invalid.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_test.py b/tests/config_test.py index 58b0ac9723..801c8b3938 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -4,6 +4,7 @@ import traceback import json import jsonpatch +import shutil import sys import unittest import ipaddress @@ -88,6 +89,10 @@ Reloading Monit configuration ... """ +RELOAD_CONFIG_DB_OUTPUT_INVALID = """\ +Bad format: json file broken. Expecting ',' delimiter: line 12 column 5 (char 321) +""" + RELOAD_YANG_CFG_OUTPUT = """\ Stopping SONiC target ... Running command: /usr/local/bin/sonic-cfggen -Y /tmp/config.json --write-to-db @@ -104,6 +109,10 @@ Reloading Monit configuration ... """ +RELOAD_MASIC_CONFIG_DB_OUTPUT_INVALID = """\ +Bad format: json file broken. Expecting ',' delimiter: line 12 column 5 (char 321) +""" + reload_config_with_sys_info_command_output="""\ Running command: /usr/local/bin/sonic-cfggen -H -k Seastone-DX010-25-50 --write-to-db""" @@ -195,6 +204,7 @@ def mock_run_command_side_effect_gnmi(*args, **kwargs): class TestConfigReload(object): dummy_cfg_file = os.path.join(os.sep, "tmp", "config.json") + dummy_cfg_file_contents = os.path.join(mock_db_path, "config_db.json") @classmethod def setup_class(cls): @@ -206,7 +216,8 @@ def setup_class(cls): import config.main importlib.reload(config.main) - open(cls.dummy_cfg_file, 'w').close() + shutil.copyfile(cls.dummy_cfg_file_contents, cls.dummy_cfg_file) + open(cls.dummy_cfg_file, 'r').close() 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: @@ -479,6 +490,8 @@ def teardown_class(cls): class TestReloadConfig(object): dummy_cfg_file = os.path.join(os.sep, "tmp", "config.json") + dummy_cfg_file_contents = os.path.join(mock_db_path, "config_db.json") + dummy_cfg_file_invalid = os.path.join(mock_db_path, "config_db_invalid.json") @classmethod def setup_class(cls): @@ -486,7 +499,8 @@ def setup_class(cls): print("SETUP") import config.main importlib.reload(config.main) - open(cls.dummy_cfg_file, 'w').close() + shutil.copyfile(cls.dummy_cfg_file_contents, cls.dummy_cfg_file) + open(cls.dummy_cfg_file, 'r').close() def test_reload_config(self, get_cmd_module, setup_single_broadcom_asic): with mock.patch( @@ -507,6 +521,25 @@ def test_reload_config(self, get_cmd_module, setup_single_broadcom_asic): assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \ == RELOAD_CONFIG_DB_OUTPUT + def test_reload_config_invalid_config_file(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 + runner = CliRunner() + + result = runner.invoke( + config.config.commands["reload"], + [self.dummy_cfg_file_invalid, '-y', '-f', "--disable_arp_cache"]) + + print(result.exit_code) + print(result.output) + traceback.print_tb(result.exc_info[2]) + assert result.exit_code == 1 + assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \ + == RELOAD_CONFIG_DB_OUTPUT_INVALID + def test_config_reload_disabled_service(self, get_cmd_module, setup_single_broadcom_asic): with mock.patch( "utilities_common.cli.run_command", @@ -549,6 +582,29 @@ def test_reload_config_masic(self, get_cmd_module, setup_multi_broadcom_masic): assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \ == RELOAD_MASIC_CONFIG_DB_OUTPUT + def test_reload_config_masic_invalid(self, get_cmd_module, setup_multi_broadcom_masic): + 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 + runner = CliRunner() + # 3 config files: 1 for host and 2 for asic + cfg_files = "{},{},{}".format( + self.dummy_cfg_file, + self.dummy_cfg_file_invalid, + self.dummy_cfg_file) + result = runner.invoke( + config.config.commands["reload"], + [cfg_files, '-y', '-f', "--disable_arp_cache"]) + + print(result.exit_code) + print(result.output) + traceback.print_tb(result.exc_info[2]) + assert result.exit_code == 1 + assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \ + == RELOAD_MASIC_CONFIG_DB_OUTPUT_INVALID + def test_reload_yang_config(self, get_cmd_module, setup_single_broadcom_asic): with mock.patch( From de52bddb968abdfb09eb2563e5680a052119ced5 Mon Sep 17 00:00:00 2001 From: cchoate Date: Mon, 28 Nov 2022 13:04:56 -0800 Subject: [PATCH 2/8] Fix LGTM --- config/main.py | 2 +- tests/config_test.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/main.py b/config/main.py index 09c0138c4f..34b7b125ba 100644 --- a/config/main.py +++ b/config/main.py @@ -1551,7 +1551,7 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form # Check if the file is properly formatted before proceeding. for file in cfg_files: try: - config_input = read_json_file(file) + read_json_file(file) except Exception as e: click.secho("Bad format: json file broken. {}".format(str(e)), fg='magenta') diff --git a/tests/config_test.py b/tests/config_test.py index 801c8b3938..1d61a23f54 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -531,7 +531,7 @@ def test_reload_config_invalid_config_file(self, get_cmd_module, setup_single_br result = runner.invoke( config.config.commands["reload"], - [self.dummy_cfg_file_invalid, '-y', '-f', "--disable_arp_cache"]) + [self.dummy_cfg_file_invalid, '-y', '-f']) print(result.exit_code) print(result.output) @@ -596,7 +596,7 @@ def test_reload_config_masic_invalid(self, get_cmd_module, setup_multi_broadcom_ self.dummy_cfg_file) result = runner.invoke( config.config.commands["reload"], - [cfg_files, '-y', '-f', "--disable_arp_cache"]) + [cfg_files, '-y', '-f']) print(result.exit_code) print(result.output) From 96e4a811f5f66522b31deccf377f146bb591ed95 Mon Sep 17 00:00:00 2001 From: cchoate Date: Wed, 30 Nov 2022 14:27:07 -0800 Subject: [PATCH 3/8] sonic-utilities: Update config reload() to verify formatting of all files used to write to Config DB. sonic-utilities: Address PR feedaback * Update config/main.py to verify the format for any file used to write to Config DB * Update tests/config_test.py to include test cases for a yang config file Signed-off-by: cchoate54@gmail.com --- config/main.py | 37 ++++++++++++++++++++++++++++--------- tests/config_test.py | 28 ++++++++++++++++++++++------ 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/config/main.py b/config/main.py index 34b7b125ba..e00e7b11c1 100644 --- a/config/main.py +++ b/config/main.py @@ -1547,15 +1547,34 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form if len(cfg_files) != num_cfg_file: click.echo("Input {} config file(s) separated by comma for multiple files ".format(num_cfg_file)) return - - # Check if the file is properly formatted before proceeding. - for file in cfg_files: - try: - read_json_file(file) - except Exception as e: - click.secho("Bad format: json file broken. {}".format(str(e)), - fg='magenta') - sys.exit(1) + + for inst in range(-1, num_cfg_file-1): + # Get the namespace name, for linux host it is None + if inst == -1: + namespace = None + else: + namespace = "{}{}".format(NAMESPACE_PREFIX, inst) + + # Get the file from user input, else take the default file /etc/sonic/config_db{NS_id}.json + if cfg_files: + file = cfg_files[inst+1] + else: + if file_format == 'config_db': + if namespace is None: + file = DEFAULT_CONFIG_DB_FILE + else: + file = "/etc/sonic/config_db{}.json".format(inst) + else: + file = DEFAULT_CONFIG_YANG_FILE + + # Check the file is properly formatted before proceeding. + try: + # Load golden config json + read_json_file(file) + except Exception as e: + click.secho("Bad format: json file '{}' broken.\n{}".format(file, str(e)), + fg='magenta') + sys.exit(1) #Stop services before config push if not no_service_restart: diff --git a/tests/config_test.py b/tests/config_test.py index 1d61a23f54..8d6c959ff3 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -90,7 +90,8 @@ """ RELOAD_CONFIG_DB_OUTPUT_INVALID = """\ -Bad format: json file broken. Expecting ',' delimiter: line 12 column 5 (char 321) +Bad format: json file '/sonic/src/sonic-utilities/tests/config_reload_input/config_db_invalid.json' broken. +Expecting ',' delimiter: line 12 column 5 (char 321) """ RELOAD_YANG_CFG_OUTPUT = """\ @@ -109,10 +110,6 @@ Reloading Monit configuration ... """ -RELOAD_MASIC_CONFIG_DB_OUTPUT_INVALID = """\ -Bad format: json file broken. Expecting ',' delimiter: line 12 column 5 (char 321) -""" - reload_config_with_sys_info_command_output="""\ Running command: /usr/local/bin/sonic-cfggen -H -k Seastone-DX010-25-50 --write-to-db""" @@ -603,7 +600,7 @@ def test_reload_config_masic_invalid(self, get_cmd_module, setup_multi_broadcom_ traceback.print_tb(result.exc_info[2]) assert result.exit_code == 1 assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \ - == RELOAD_MASIC_CONFIG_DB_OUTPUT_INVALID + == RELOAD_CONFIG_DB_OUTPUT_INVALID def test_reload_yang_config(self, get_cmd_module, setup_single_broadcom_asic): @@ -624,6 +621,25 @@ def test_reload_yang_config(self, get_cmd_module, assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \ == RELOAD_YANG_CFG_OUTPUT + def test_reload_yang_config_invalid(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 + runner = CliRunner() + + result = runner.invoke(config.config.commands["reload"], + [self.dummy_cfg_file_invalid, '-y', '-f', '-t', 'config_yang']) + + print(result.exit_code) + print(result.output) + traceback.print_tb(result.exc_info[2]) + assert result.exit_code == 1 + assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \ + == RELOAD_CONFIG_DB_OUTPUT_INVALID + @classmethod def teardown_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "0" From 5df7003431b8084523160c59f1b62a7451155c89 Mon Sep 17 00:00:00 2001 From: cchoate Date: Wed, 30 Nov 2022 15:17:45 -0800 Subject: [PATCH 4/8] sonic-utilities: * Update tests/config_test.py to implement platform independent paths for new test cases --- tests/config_test.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/tests/config_test.py b/tests/config_test.py index 8d6c959ff3..7352d779bb 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -89,10 +89,11 @@ Reloading Monit configuration ... """ -RELOAD_CONFIG_DB_OUTPUT_INVALID = """\ -Bad format: json file '/sonic/src/sonic-utilities/tests/config_reload_input/config_db_invalid.json' broken. -Expecting ',' delimiter: line 12 column 5 (char 321) -""" +RELOAD_CONFIG_DB_OUTPUT_INVALID_MSG = """\ +Bad format: json file""" + +RELOAD_CONFIG_DB_OUTPUT_INVALID_ERROR = """\ +Expecting ',' delimiter: line 12 column 5 (char 321)""" RELOAD_YANG_CFG_OUTPUT = """\ Stopping SONiC target ... @@ -534,8 +535,10 @@ def test_reload_config_invalid_config_file(self, get_cmd_module, setup_single_br print(result.output) traceback.print_tb(result.exc_info[2]) assert result.exit_code == 1 - assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \ - == RELOAD_CONFIG_DB_OUTPUT_INVALID + + output = "\n".join([l.rstrip() for l in result.output.split('\n')]) + assert RELOAD_CONFIG_DB_OUTPUT_INVALID_MSG in output + assert RELOAD_CONFIG_DB_OUTPUT_INVALID_ERROR in output def test_config_reload_disabled_service(self, get_cmd_module, setup_single_broadcom_asic): with mock.patch( @@ -599,8 +602,10 @@ def test_reload_config_masic_invalid(self, get_cmd_module, setup_multi_broadcom_ print(result.output) traceback.print_tb(result.exc_info[2]) assert result.exit_code == 1 - assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \ - == RELOAD_CONFIG_DB_OUTPUT_INVALID + + output = "\n".join([l.rstrip() for l in result.output.split('\n')]) + assert RELOAD_CONFIG_DB_OUTPUT_INVALID_MSG in output + assert RELOAD_CONFIG_DB_OUTPUT_INVALID_ERROR in output def test_reload_yang_config(self, get_cmd_module, setup_single_broadcom_asic): @@ -637,8 +642,10 @@ def test_reload_yang_config_invalid(self, get_cmd_module, print(result.output) traceback.print_tb(result.exc_info[2]) assert result.exit_code == 1 - assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \ - == RELOAD_CONFIG_DB_OUTPUT_INVALID + + output = "\n".join([l.rstrip() for l in result.output.split('\n')]) + assert RELOAD_CONFIG_DB_OUTPUT_INVALID_MSG in output + assert RELOAD_CONFIG_DB_OUTPUT_INVALID_ERROR in output @classmethod def teardown_class(cls): From 0f5c3bbbebed9f9ef3e5ca565556a63bf998ee2d Mon Sep 17 00:00:00 2001 From: cchoate Date: Wed, 30 Nov 2022 17:58:39 -0800 Subject: [PATCH 5/8] sonic-utilities: Update config reload() file verification step to skip the current namespace if a file does not exist and proceed to check the next one sonic-utilities: bugfix-9499 * Update config/main.py to not exit reload if a file does not exist * Update tests/config_test.py to include test cases for non existent input file Signed-off-by: cchoate54@gmail.com --- config/main.py | 7 ++++++- tests/config_test.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/config/main.py b/config/main.py index e00e7b11c1..fc5c708eee 100644 --- a/config/main.py +++ b/config/main.py @@ -1567,7 +1567,12 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form else: file = DEFAULT_CONFIG_YANG_FILE - # Check the file is properly formatted before proceeding. + # Check if the file exists before proceeding + # Instead of exiting, skip the current namespace and check the next one + if not os.path.exists(file): + continue + + # Check the file is properly formatted before proceeding. try: # Load golden config json read_json_file(file) diff --git a/tests/config_test.py b/tests/config_test.py index 7352d779bb..7a5a51e4bb 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -111,6 +111,15 @@ Reloading Monit configuration ... """ +RELOAD_MASIC_CONFIG_DB_OUTPUT_FILE_NOT_EXIST = """\ +Stopping SONiC target ... +Running command: /usr/local/bin/sonic-cfggen -j /tmp/config.json --write-to-db +The config file non_exist.json doesn't exist +Running command: /usr/local/bin/sonic-cfggen -j /tmp/config.json -n asic1 --write-to-db +Restarting SONiC target ... +Reloading Monit configuration ... +""" + reload_config_with_sys_info_command_output="""\ Running command: /usr/local/bin/sonic-cfggen -H -k Seastone-DX010-25-50 --write-to-db""" @@ -607,6 +616,29 @@ def test_reload_config_masic_invalid(self, get_cmd_module, setup_multi_broadcom_ assert RELOAD_CONFIG_DB_OUTPUT_INVALID_MSG in output assert RELOAD_CONFIG_DB_OUTPUT_INVALID_ERROR in output + def test_reload_config_masic_non_exist_file(self, get_cmd_module, setup_multi_broadcom_masic): + 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 + runner = CliRunner() + # 3 config files: 1 for host and 2 for asic + cfg_files = "{},{},{}".format( + self.dummy_cfg_file, + "non_exist.json", + self.dummy_cfg_file) + result = runner.invoke( + config.config.commands["reload"], + [cfg_files, '-y', '-f']) + + 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')]) \ + == RELOAD_MASIC_CONFIG_DB_OUTPUT_FILE_NOT_EXIST + def test_reload_yang_config(self, get_cmd_module, setup_single_broadcom_asic): with mock.patch( From e225899141b070238d74555a62749e6bbb0d92c3 Mon Sep 17 00:00:00 2001 From: cchoate Date: Wed, 30 Nov 2022 21:37:04 -0800 Subject: [PATCH 6/8] sonic-utilities: Reduce code dupication in config/main.py sonic-utilities: bugfix-9499 * Update config/main.py to reduce code duplication and add a step to validate INIT_CFG_FILE if it exists Signed-off-by: cchoate54@gmail.com --- config/main.py | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/config/main.py b/config/main.py index fc5c708eee..1245c7b430 100644 --- a/config/main.py +++ b/config/main.py @@ -1548,6 +1548,13 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form click.echo("Input {} config file(s) separated by comma for multiple files ".format(num_cfg_file)) return + # Create a dictionary to store each cfg_file, namespace, and a bool representing if a the file exists + cfg_file_dict = {} + + # In Single ASIC platforms we have single DB service. In multi-ASIC platforms we have a global DB + # service running in the host + DB services running in each ASIC namespace created per ASIC. + # In the below logic, we get all namespaces in this platform and add an empty namespace '' + # denoting the current namespace which we are in ( the linux host ) for inst in range(-1, num_cfg_file-1): # Get the namespace name, for linux host it is None if inst == -1: @@ -1570,12 +1577,17 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form # Check if the file exists before proceeding # Instead of exiting, skip the current namespace and check the next one if not os.path.exists(file): + cfg_file_dict[inst] = [file, namespace, False] continue + cfg_file_dict[inst] = [file, namespace, True] # Check the file is properly formatted before proceeding. try: # Load golden config json read_json_file(file) + # In the last iteration, verify INIT_CFG_FILE if it exits + if inst == num_cfg_file-2 and os.path.isfile(INIT_CFG_FILE): + read_json_file(INIT_CFG_FILE) except Exception as e: click.secho("Bad format: json file '{}' broken.\n{}".format(file, str(e)), fg='magenta') @@ -1590,28 +1602,8 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form # service running in the host + DB services running in each ASIC namespace created per ASIC. # In the below logic, we get all namespaces in this platform and add an empty namespace '' # denoting the current namespace which we are in ( the linux host ) - for inst in range(-1, num_cfg_file-1): - # Get the namespace name, for linux host it is None - if inst == -1: - namespace = None - else: - namespace = "{}{}".format(NAMESPACE_PREFIX, inst) - - # Get the file from user input, else take the default file /etc/sonic/config_db{NS_id}.json - if cfg_files: - file = cfg_files[inst+1] - else: - if file_format == 'config_db': - if namespace is None: - file = DEFAULT_CONFIG_DB_FILE - else: - file = "/etc/sonic/config_db{}.json".format(inst) - else: - file = DEFAULT_CONFIG_YANG_FILE - - - # Check the file exists before proceeding. - if not os.path.exists(file): + for file, namespace, file_exists in cfg_file_dict.values(): + if not file_exists: click.echo("The config file {} doesn't exist".format(file)) continue From 107cbca4b6e4815b63941da9bf2f6ba4f71a1bc9 Mon Sep 17 00:00:00 2001 From: cchoate Date: Thu, 1 Dec 2022 00:46:54 -0800 Subject: [PATCH 7/8] sonic-utilities: Create a function to validate config files in config/main.py sonic-utilities: bugfix-9499 * Update config/main.py Create a function to validate config files in config/main.py. Use it to add a step to validate INIT_CFG_FILE if it exists Signed-off-by: cchoate54@gmail.com --- config/main.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/config/main.py b/config/main.py index 1245c7b430..d07b67d7c5 100644 --- a/config/main.py +++ b/config/main.py @@ -1170,6 +1170,18 @@ def load_backend_acl(cfg_db, device_type): if os.path.isfile(BACKEND_ACL_FILE): clicommon.run_command("acl-loader update incremental {}".format(BACKEND_ACL_FILE), display_cmd=True) +def validate_config_file(file): + """ + A validator to check config files for syntax errors + """ + try: + # Load golden config json + read_json_file(file) + except Exception as e: + click.secho("Bad format: json file '{}' broken.\n{}".format(file, str(e)), + fg='magenta') + sys.exit(1) + # This is our main entrypoint - the main 'config' command @click.group(cls=clicommon.AbbreviationGroup, context_settings=CONTEXT_SETTINGS) @@ -1582,16 +1594,11 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form cfg_file_dict[inst] = [file, namespace, True] # Check the file is properly formatted before proceeding. - try: - # Load golden config json - read_json_file(file) - # In the last iteration, verify INIT_CFG_FILE if it exits - if inst == num_cfg_file-2 and os.path.isfile(INIT_CFG_FILE): - read_json_file(INIT_CFG_FILE) - except Exception as e: - click.secho("Bad format: json file '{}' broken.\n{}".format(file, str(e)), - fg='magenta') - sys.exit(1) + validate_config_file(file) + + #Validate INIT_CFG_FILE if it exits + if os.path.isfile(INIT_CFG_FILE): + validate_config_file(INIT_CFG_FILE) #Stop services before config push if not no_service_restart: From 36e41bb04be8729763fca887896f12be703174bb Mon Sep 17 00:00:00 2001 From: cchoate Date: Thu, 1 Dec 2022 14:44:26 -0800 Subject: [PATCH 8/8] sonic-utilities: Remove duplicate comment in config/main.py Signed-off-by: cchoate54@gmail.com --- config/main.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/config/main.py b/config/main.py index d07b67d7c5..760c28b60f 100644 --- a/config/main.py +++ b/config/main.py @@ -1182,7 +1182,6 @@ def validate_config_file(file): fg='magenta') sys.exit(1) - # This is our main entrypoint - the main 'config' command @click.group(cls=clicommon.AbbreviationGroup, context_settings=CONTEXT_SETTINGS) @click.pass_context @@ -1605,10 +1604,6 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form log.log_info("'reload' stopping services...") _stop_services() - # In Single ASIC platforms we have single DB service. In multi-ASIC platforms we have a global DB - # service running in the host + DB services running in each ASIC namespace created per ASIC. - # In the below logic, we get all namespaces in this platform and add an empty namespace '' - # denoting the current namespace which we are in ( the linux host ) for file, namespace, file_exists in cfg_file_dict.values(): if not file_exists: click.echo("The config file {} doesn't exist".format(file))