sonic-utilities: Update config reload() to verify formatting of an input file

config/main.py

@@ -1548,6 +1548,39 @@ 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
 
+    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 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)
+        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:
         log.log_info("'reload' stopping services...")

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"
+        }
+    }
+}

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,12 @@
 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
@@ -104,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"""
 
@@ -195,6 +211,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 +223,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,14 +497,17 @@ 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)
-        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 +528,27 @@ 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",
@@ -549,6 +591,54 @@ 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(
@@ -568,6 +658,27 @@ 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"