From 687ff1daa142c6916b1d9e02e3b77b3189113b8d Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Fri, 6 Jan 2023 16:51:57 -0800 Subject: [PATCH] Revert "sonic-utilities: Update config reload() to verify formatting of an input file (#2529)" This reverts commit 42f51c26d1d0017f3211904ca19c023b5d784463. --- config/main.py | 39 +----- .../config_db_invalid.json | 62 ---------- tests/config_test.py | 115 +----------------- 3 files changed, 8 insertions(+), 208 deletions(-) delete mode 100644 tests/config_reload_input/config_db_invalid.json diff --git a/config/main.py b/config/main.py index 5ec91158a1..5fdc177e2e 100644 --- a/config/main.py +++ b/config/main.py @@ -1189,17 +1189,6 @@ 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) @@ -1578,8 +1567,10 @@ 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 = {} + #Stop services before config push + if not no_service_restart: + 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. @@ -1604,27 +1595,9 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form else: file = DEFAULT_CONFIG_YANG_FILE - # 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. - 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: - log.log_info("'reload' stopping services...") - _stop_services() - for file, namespace, file_exists in cfg_file_dict.values(): - if not file_exists: + # Check the file exists before proceeding. + if not os.path.exists(file): click.echo("The config file {} doesn't exist".format(file)) continue diff --git a/tests/config_reload_input/config_db_invalid.json b/tests/config_reload_input/config_db_invalid.json deleted file mode 100644 index cb394023d8..0000000000 --- a/tests/config_reload_input/config_db_invalid.json +++ /dev/null @@ -1,62 +0,0 @@ -{ - "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 98bdd1169d..5fa50abd00 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -4,7 +4,6 @@ import traceback import json import jsonpatch -import shutil import sys import unittest import ipaddress @@ -89,12 +88,6 @@ Reloading Monit configuration ... """ -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 ... Running command: /usr/local/bin/sonic-cfggen -Y /tmp/config.json --write-to-db @@ -111,15 +104,6 @@ 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""" @@ -211,7 +195,6 @@ 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): @@ -223,8 +206,7 @@ def setup_class(cls): import config.main importlib.reload(config.main) - shutil.copyfile(cls.dummy_cfg_file_contents, cls.dummy_cfg_file) - open(cls.dummy_cfg_file, 'r').close() + open(cls.dummy_cfg_file, 'w').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: @@ -497,8 +479,6 @@ 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): @@ -506,8 +486,7 @@ def setup_class(cls): print("SETUP") import config.main importlib.reload(config.main) - shutil.copyfile(cls.dummy_cfg_file_contents, cls.dummy_cfg_file) - open(cls.dummy_cfg_file, 'r').close() + open(cls.dummy_cfg_file, 'w').close() def test_reload_config(self, get_cmd_module, setup_single_broadcom_asic): with mock.patch( @@ -528,27 +507,6 @@ 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']) - - print(result.exit_code) - print(result.output) - traceback.print_tb(result.exc_info[2]) - assert result.exit_code == 1 - - 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( "utilities_common.cli.run_command", @@ -591,54 +549,6 @@ 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']) - - print(result.exit_code) - print(result.output) - traceback.print_tb(result.exc_info[2]) - assert result.exit_code == 1 - - 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_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( @@ -658,27 +568,6 @@ 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 - - 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): os.environ['UTILITIES_UNIT_TESTING'] = "0"