[config] Fix 'config reload -l' command to get filename by default (#…

…1611)

Fix #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 <samaity@linkedin.com>

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:

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

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

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):