Revert "sonic-utilities: Update config reload() to verify formatting of an input file (#2529)" (#2586)

qiluo-msft · web-flow · commit f63ef9a18289 · 2023-01-06T17:37:51.000-08:00
This reverts commit 42f51c2. Reverts sonic-net/sonic-utilities#2529 Reason: There are use cases like `config reload /dev/stdin`, for example [L2 Switch mode · sonic-net/SONiC Wiki (github.com)](https://github.com/sonic-net/SONiC/wiki/L2-Switch-mode). The original PR would read input file twice, so /dev/stdin does not work.
diff --git 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
diff --git 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,17 +479,14 @@ 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):
         os.environ['UTILITIES_UNIT_TESTING'] = "1"
         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"
