From 53b334168d7743bbea52b3d720bbd6e9ad05893c Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Fri, 31 May 2024 07:29:02 +0000 Subject: [PATCH 1/6] [cnfig]Support single file reload for multiasic --- config/main.py | 285 ++++++++++++++++++++++++++++++------------------- 1 file changed, 178 insertions(+), 107 deletions(-) diff --git a/config/main.py b/config/main.py index b750b49820..9da8b3fe80 100644 --- a/config/main.py +++ b/config/main.py @@ -31,7 +31,7 @@ from sonic_yang_cfg_generator import SonicYangCfgDbGenerator from utilities_common import util_base from swsscommon import swsscommon -from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector +from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector, SonicDBConfig, ConfigDBPipeConnector from utilities_common.db import Db from utilities_common.intf_filter import parse_interface_in_filter from utilities_common import bgp_util @@ -1159,7 +1159,7 @@ def validate_gre_type(ctx, _, value): raise click.UsageError("{} is not a valid GRE type".format(value)) -def multi_asic_save_config(db, filename): +def multiasic_save_to_singlefile(db, filename): """A function to save all asic's config to single file """ all_current_config = {} @@ -1226,6 +1226,91 @@ def validate_patch(patch): except Exception as e: raise GenericConfigUpdaterError(f"Validate json patch: {patch} failed due to:{e}") + +def multiasic_validate_single_file(filename): + ns_list = multi_asic.get_namespace_list() + file_input = read_json_file(filename) + file_ns_list = [DEFAULT_NAMESPACE if key == HOST_NAMESPACE else key for key in file_input] + if set(ns_list) != set(file_ns_list): + click.echo("Input file {} must contain all asics config".format(filename)) + raise click.Abort() + + +def load_sysinfo_if_missing(asic_config): + platform = asic_config.get("DEVICE_METADATA", {}).\ + get(HOST_NAMESPACE, {}).get("platform") + mac = asic_config.get("DEVICE_METADATA", {}).\ + get(HOST_NAMESPACE, {}).get("mac") + if not platform or not mac: + log.log_warning("Input file does't have platform or mac. platform: {}, mac: {}" + .format(None if platform is None else platform, None if mac is None else mac)) + return True + return False + + +def flush_configdb(namespace=DEFAULT_NAMESPACE): + if namespace is DEFAULT_NAMESPACE: + config_db = ConfigDBConnector() + else: + config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace) + + config_db.connect() + client = config_db.get_redis_client(config_db.CONFIG_DB) + client.flushdb() + return client, config_db + + +def migrate_db_to_lastest(namespace=DEFAULT_NAMESPACE): + # Migrate DB contents to latest version + db_migrator='/usr/local/bin/db_migrator.py' + if os.path.isfile(db_migrator) and os.access(db_migrator, os.X_OK): + if not namespace: + command = [db_migrator, '-o', 'migrate'] + else: + command = [db_migrator, '-o', 'migrate', '-n', str(namespace)] + clicommon.run_command(command, display_cmd=True) + + +def multiasic_write_to_db(filename, load_sysinfo): + file_input = read_json_file(filename) + for ns in multi_asic.get_namespace_list(): + asic_name = HOST_NAMESPACE if ns == DEFAULT_NAMESPACE else ns + asic_config = file_input[asic_name] + + if not load_sysinfo: + load_sysinfo = load_sysinfo_if_missing(asic_config) + + if load_sysinfo: + cfg_hwsku = asic_config.get("DEVICE_METADATA", {}).\ + get(HOST_NAMESPACE, {}).get("hwsku") + if not cfg_hwsku: + click.secho("Could not get the HWSKU from config file, Exiting!!!", fg='magenta') + sys.exit(1) + + + client, _ = flush_configdb(ns) + + if load_sysinfo: + if ns is DEFAULT_NAMESPACE: + command = [str(SONIC_CFGGEN_PATH), '-H', '-k', str(cfg_hwsku), '--write-to-db'] + else: + command = [str(SONIC_CFGGEN_PATH), '-H', '-k', str(cfg_hwsku), '-n', str(ns), '--write-to-db'] + clicommon.run_command(command, display_cmd=True) + + if ns is DEFAULT_NAMESPACE: + config_db = ConfigDBPipeConnector(use_unix_socket_path=True) + else: + config_db = ConfigDBPipeConnector(use_unix_socket_path=True, namespace=ns) + + config_db.connect(False) + sonic_cfggen.FormatConverter.to_deserialized(asic_config) + data = sonic_cfggen.FormatConverter.output_to_db(asic_config) + config_db.mod_config(sonic_cfggen.FormatConverter.output_to_db(data)) + client.set(config_db.INIT_INDICATOR, 1) + + migrate_db_to_lastest(ns) + + # This is our main entrypoint - the main 'config' command @click.group(cls=clicommon.AbbreviationGroup, context_settings=CONTEXT_SETTINGS) @click.pass_context @@ -1313,7 +1398,7 @@ def save(db, filename): # save all ASIC configurations to that single file. if len(cfg_files) == 1 and multi_asic.is_multi_asic(): filename = cfg_files[0] - multi_asic_save_config(db, filename) + multiasic_save_to_singlefile(db, filename) return elif len(cfg_files) != num_cfg_file: click.echo("Input {} config file(s) separated by comma for multiple files ".format(num_cfg_file)) @@ -1631,11 +1716,15 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form if multi_asic.is_multi_asic() and file_format == 'config_db': num_cfg_file += num_asic + multiasic_single_file_mode = False # If the user give the filename[s], extract the file names. if filename is not None: cfg_files = filename.split(',') - if len(cfg_files) != num_cfg_file: + if len(cfg_files) == 1 and multi_asic.is_multi_asic(): + multiasic_validate_single_file(cfg_files[0]) + multiasic_single_file_mode = True + elif len(cfg_files) != num_cfg_file: click.echo("Input {} config file(s) separated by comma for multiple files ".format(num_cfg_file)) return @@ -1644,127 +1733,109 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form log.log_notice("'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 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] - # Save to tmpfile in case of stdin input which can only be read once - if file == "/dev/stdin": - file_input = read_json_file(file) - (_, tmpfname) = tempfile.mkstemp(dir="/tmp", suffix="_configReloadStdin") - write_json_file(file_input, tmpfname) - file = tmpfname - else: - if file_format == 'config_db': - if namespace is None: - file = DEFAULT_CONFIG_DB_FILE - else: - file = "/etc/sonic/config_db{}.json".format(inst) + if multiasic_single_file_mode: + multiasic_write_to_db(cfg_files[0], load_sysinfo) + else: + # 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 DEFAULT_NAMESPACE + if inst == -1: + namespace = DEFAULT_NAMESPACE else: - file = DEFAULT_CONFIG_YANG_FILE + 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] + # Save to tmpfile in case of stdin input which can only be read once + if file == "/dev/stdin": + file_input = read_json_file(file) + (_, tmpfname) = tempfile.mkstemp(dir="/tmp", suffix="_configReloadStdin") + write_json_file(file_input, tmpfname) + file = tmpfname + else: + if file_format == 'config_db': + if namespace is DEFAULT_NAMESPACE: + 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): - click.echo("The config file {} doesn't exist".format(file)) - continue + # Check the file exists before proceeding. + if not os.path.exists(file): + click.echo("The config file {} doesn't exist".format(file)) + continue - if file_format == 'config_db': - file_input = read_json_file(file) + if file_format == 'config_db': + file_input = read_json_file(file) + if not load_sysinfo: + load_sysinfo = load_sysinfo_if_missing(file_input) + + if load_sysinfo: + try: + command = [SONIC_CFGGEN_PATH, "-j", file, '-v', "DEVICE_METADATA.localhost.hwsku"] + proc = subprocess.Popen(command, 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) - platform = file_input.get("DEVICE_METADATA", {}).\ - get(HOST_NAMESPACE, {}).get("platform") - mac = file_input.get("DEVICE_METADATA", {}).\ - get(HOST_NAMESPACE, {}).get("mac") + cfg_hwsku = output.strip() - if not platform or not mac: - log.log_warning("Input file does't have platform or mac. platform: {}, mac: {}" - .format(None if platform is None else platform, None if mac is None else mac)) - load_sysinfo = True + client, config_db = flush_configdb(namespace) - if load_sysinfo: - try: - command = [SONIC_CFGGEN_PATH, "-j", file, '-v', "DEVICE_METADATA.localhost.hwsku"] - proc = subprocess.Popen(command, text=True, stdout=subprocess.PIPE) - output, err = proc.communicate() + if load_sysinfo: + if namespace is DEFAULT_NAMESPACE: + command = [str(SONIC_CFGGEN_PATH), '-H', '-k', str(cfg_hwsku), '--write-to-db'] + else: + command = [str(SONIC_CFGGEN_PATH), '-H', '-k', str(cfg_hwsku), '-n', str(namespace), '--write-to-db'] + clicommon.run_command(command, display_cmd=True) - 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() + # For the database service running in linux host we use the file user gives as input + # or by default DEFAULT_CONFIG_DB_FILE. In the case of database service running in namespace, + # the default config_db.json format is used. - if not output: - click.secho("Could not get the HWSKU from config file, Exiting!!!", fg='magenta') - sys.exit(1) - cfg_hwsku = output.strip() + config_gen_opts = [] - if namespace is None: - config_db = ConfigDBConnector() - else: - config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace) - - config_db.connect() - client = config_db.get_redis_client(config_db.CONFIG_DB) - client.flushdb() + if os.path.isfile(INIT_CFG_FILE): + config_gen_opts += ['-j', str(INIT_CFG_FILE)] - if load_sysinfo: - if namespace is None: - command = [str(SONIC_CFGGEN_PATH), '-H', '-k', str(cfg_hwsku), '--write-to-db'] + if file_format == 'config_db': + config_gen_opts += ['-j', str(file)] else: - command = [str(SONIC_CFGGEN_PATH), '-H', '-k', str(cfg_hwsku), '-n', str(namespace), '--write-to-db'] - clicommon.run_command(command, display_cmd=True) - - # For the database service running in linux host we use the file user gives as input - # or by default DEFAULT_CONFIG_DB_FILE. In the case of database service running in namespace, - # the default config_db.json format is used. + config_gen_opts += ['-Y', str(file)] + if namespace is not DEFAULT_NAMESPACE: + config_gen_opts += ['-n', str(namespace)] - config_gen_opts = [] + command = [SONIC_CFGGEN_PATH] + config_gen_opts + ['--write-to-db'] - if os.path.isfile(INIT_CFG_FILE): - config_gen_opts += ['-j', str(INIT_CFG_FILE)] - - if file_format == 'config_db': - config_gen_opts += ['-j', str(file)] - else: - config_gen_opts += ['-Y', str(file)] - - if namespace is not None: - config_gen_opts += ['-n', str(namespace)] - - command = [SONIC_CFGGEN_PATH] + config_gen_opts + ['--write-to-db'] - - clicommon.run_command(command, display_cmd=True) - client.set(config_db.INIT_INDICATOR, 1) + clicommon.run_command(command, display_cmd=True) + client.set(config_db.INIT_INDICATOR, 1) - if os.path.exists(file) and file.endswith("_configReloadStdin"): - # Remove tmpfile - try: - os.remove(file) - except OSError as e: - click.echo("An error occurred while removing the temporary file: {}".format(str(e)), err=True) + if os.path.exists(file) and file.endswith("_configReloadStdin"): + # Remove tmpfile + try: + os.remove(file) + except OSError as e: + click.echo("An error occurred while removing the temporary file: {}".format(str(e)), err=True) - # Migrate DB contents to latest version - db_migrator='/usr/local/bin/db_migrator.py' - if os.path.isfile(db_migrator) and os.access(db_migrator, os.X_OK): - if namespace is None: - command = [db_migrator, '-o', 'migrate'] - else: - command = [db_migrator, '-o', 'migrate', '-n', str(namespace)] - clicommon.run_command(command, display_cmd=True) + # Migrate DB contents to latest version + migrate_db_to_lastest(namespace) # Re-generate the environment variable in case config_db.json was edited update_sonic_environment() From a9533ea5894d94012c336e8ada20c19631257c97 Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Mon, 3 Jun 2024 02:35:45 +0000 Subject: [PATCH 2/6] style --- config/main.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/config/main.py b/config/main.py index 9da8b3fe80..5ee8247c28 100644 --- a/config/main.py +++ b/config/main.py @@ -31,7 +31,7 @@ from sonic_yang_cfg_generator import SonicYangCfgDbGenerator from utilities_common import util_base from swsscommon import swsscommon -from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector, SonicDBConfig, ConfigDBPipeConnector +from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector, ConfigDBPipeConnector from utilities_common.db import Db from utilities_common.intf_filter import parse_interface_in_filter from utilities_common import bgp_util @@ -1243,7 +1243,7 @@ def load_sysinfo_if_missing(asic_config): get(HOST_NAMESPACE, {}).get("mac") if not platform or not mac: log.log_warning("Input file does't have platform or mac. platform: {}, mac: {}" - .format(None if platform is None else platform, None if mac is None else mac)) + .format(None if platform is None else platform, None if mac is None else mac)) return True return False @@ -1262,9 +1262,9 @@ def flush_configdb(namespace=DEFAULT_NAMESPACE): def migrate_db_to_lastest(namespace=DEFAULT_NAMESPACE): # Migrate DB contents to latest version - db_migrator='/usr/local/bin/db_migrator.py' + db_migrator = '/usr/local/bin/db_migrator.py' if os.path.isfile(db_migrator) and os.access(db_migrator, os.X_OK): - if not namespace: + if namespace is DEFAULT_NAMESPACE: command = [db_migrator, '-o', 'migrate'] else: command = [db_migrator, '-o', 'migrate', '-n', str(namespace)] @@ -1287,7 +1287,6 @@ def multiasic_write_to_db(filename, load_sysinfo): click.secho("Could not get the HWSKU from config file, Exiting!!!", fg='magenta') sys.exit(1) - client, _ = flush_configdb(ns) if load_sysinfo: @@ -1765,7 +1764,6 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form else: file = DEFAULT_CONFIG_YANG_FILE - # Check the file exists before proceeding. if not os.path.exists(file): click.echo("The config file {} doesn't exist".format(file)) @@ -1799,16 +1797,17 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form if load_sysinfo: if namespace is DEFAULT_NAMESPACE: - command = [str(SONIC_CFGGEN_PATH), '-H', '-k', str(cfg_hwsku), '--write-to-db'] + command = [ + str(SONIC_CFGGEN_PATH), '-H', '-k', str(cfg_hwsku), '--write-to-db'] else: - command = [str(SONIC_CFGGEN_PATH), '-H', '-k', str(cfg_hwsku), '-n', str(namespace), '--write-to-db'] + command = [ + str(SONIC_CFGGEN_PATH), '-H', '-k', str(cfg_hwsku), '-n', str(namespace), '--write-to-db'] clicommon.run_command(command, display_cmd=True) # For the database service running in linux host we use the file user gives as input # or by default DEFAULT_CONFIG_DB_FILE. In the case of database service running in namespace, # the default config_db.json format is used. - config_gen_opts = [] if os.path.isfile(INIT_CFG_FILE): From 0c1f86ba9be68d992fb9fc699c6090cf15670997 Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Thu, 6 Jun 2024 06:25:02 +0000 Subject: [PATCH 3/6] unit --- config/main.py | 14 +-- tests/config_test.py | 220 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 228 insertions(+), 6 deletions(-) diff --git a/config/main.py b/config/main.py index 5ee8247c28..3a6ef56a15 100644 --- a/config/main.py +++ b/config/main.py @@ -1228,7 +1228,7 @@ def validate_patch(patch): def multiasic_validate_single_file(filename): - ns_list = multi_asic.get_namespace_list() + ns_list = [DEFAULT_NAMESPACE, *multi_asic.get_namespace_list()] file_input = read_json_file(filename) file_ns_list = [DEFAULT_NAMESPACE if key == HOST_NAMESPACE else key for key in file_input] if set(ns_list) != set(file_ns_list): @@ -1273,14 +1273,15 @@ def migrate_db_to_lastest(namespace=DEFAULT_NAMESPACE): def multiasic_write_to_db(filename, load_sysinfo): file_input = read_json_file(filename) - for ns in multi_asic.get_namespace_list(): + for ns in [DEFAULT_NAMESPACE, *multi_asic.get_namespace_list()]: asic_name = HOST_NAMESPACE if ns == DEFAULT_NAMESPACE else ns asic_config = file_input[asic_name] - if not load_sysinfo: - load_sysinfo = load_sysinfo_if_missing(asic_config) + asic_load_sysinfo = True if load_sysinfo else False + if not asic_load_sysinfo: + asic_load_sysinfo = load_sysinfo_if_missing(asic_config) - if load_sysinfo: + if asic_load_sysinfo: cfg_hwsku = asic_config.get("DEVICE_METADATA", {}).\ get(HOST_NAMESPACE, {}).get("hwsku") if not cfg_hwsku: @@ -1289,7 +1290,7 @@ def multiasic_write_to_db(filename, load_sysinfo): client, _ = flush_configdb(ns) - if load_sysinfo: + if asic_load_sysinfo: if ns is DEFAULT_NAMESPACE: command = [str(SONIC_CFGGEN_PATH), '-H', '-k', str(cfg_hwsku), '--write-to-db'] else: @@ -4117,6 +4118,7 @@ def bgp(): """BGP-related configuration tasks""" pass + # # 'shutdown' subgroup ('config bgp shutdown ...') # diff --git a/tests/config_test.py b/tests/config_test.py index f69f799561..71bab5e67a 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -168,6 +168,21 @@ Reloading Monit configuration ... """ +reload_config_masic_onefile_output="""\ +Stopping SONiC target ... +Restarting SONiC target ... +Reloading Monit configuration ... +""" + +reload_config_masic_onefile_gen_sysinfo_output="""\ +Stopping SONiC target ... +Running command: /usr/local/bin/sonic-cfggen -H -k Mellanox-SN3800-D112C8 --write-to-db +Running command: /usr/local/bin/sonic-cfggen -H -k multi_asic -n asic0 --write-to-db +Running command: /usr/local/bin/sonic-cfggen -H -k multi_asic -n asic1 --write-to-db +Restarting SONiC target ... +Reloading Monit configuration ... +""" + save_config_output = """\ Running command: /usr/local/bin/sonic-cfggen -d --print-data > /etc/sonic/config_db.json """ @@ -635,6 +650,211 @@ def teardown_class(cls): dbconnector.load_namespace_config() +class TestConfigReloadMasic(object): + @classmethod + def setup_class(cls): + print("SETUP") + os.environ['UTILITIES_UNIT_TESTING'] = "2" + os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "multi_asic" + import config.main + importlib.reload(config.main) + # change to multi asic config + from .mock_tables import dbconnector + from .mock_tables import mock_multi_asic + importlib.reload(mock_multi_asic) + dbconnector.load_namespace_config() + + def test_config_reload_onefile_masic(self): + def read_json_file_side_effect(filename): + return { + "localhost": { + "DEVICE_METADATA": { + "localhost": { + "default_bgp_status": "down", + "default_pfcwd_status": "enable", + "deployment_id": "1", + "docker_routing_config_mode": "separated", + "hostname": "sonic-switch", + "hwsku": "Mellanox-SN3800-D112C8", + "mac": "1d:34:db:16:a6:00", + "platform": "x86_64-mlnx_msn3800-r0", + "peer_switch": "sonic-switch", + "type": "ToRRouter", + "suppress-fib-pending": "enabled" + } + } + }, + "asic0": { + "DEVICE_METADATA": { + "localhost": { + "asic_id": "01.00.0", + "asic_name": "asic0", + "bgp_asn": "65100", + "cloudtype": "None", + "default_bgp_status": "down", + "default_pfcwd_status": "enable", + "deployment_id": "None", + "docker_routing_config_mode": "separated", + "hostname": "sonic", + "hwsku": "multi_asic", + "mac": "02:42:f0:7f:01:05", + "platform": "multi_asic", + "region": "None", + "sub_role": "FrontEnd", + "type": "LeafRouter" + } + } + }, + "asic1": { + "DEVICE_METADATA": { + "localhost": { + "asic_id": "08:00.0", + "asic_name": "asic1", + "bgp_asn": "65100", + "cloudtype": "None", + "default_bgp_status": "down", + "default_pfcwd_status": "enable", + "deployment_id": "None", + "docker_routing_config_mode": "separated", + "hostname": "sonic", + "hwsku": "multi_asic", + "mac": "02:42:f0:7f:01:06", + "platform": "multi_asic", + "region": "None", + "sub_role": "BackEnd", + "type": "LeafRouter" + } + } + } + } + + with mock.patch("utilities_common.cli.run_command", + mock.MagicMock(side_effect=mock_run_command_side_effect)),\ + mock.patch('config.main.read_json_file', + mock.MagicMock(side_effect=read_json_file_side_effect)): + + runner = CliRunner() + + result = runner.invoke(config.config.commands["reload"], ["-y", "-f", "all_config_db.json"]) + + print(result.exit_code) + print(result.output) + traceback.print_tb(result.exc_info[2]) + + assert result.exit_code == 0 + assert "\n".join([li.rstrip() for li in result.output.split('\n')]) == reload_config_masic_onefile_output + + def test_config_reload_onefile_gen_sysinfo_masic(self): + def read_json_file_side_effect(filename): + return { + "localhost": { + "DEVICE_METADATA": { + "localhost": { + "default_bgp_status": "down", + "default_pfcwd_status": "enable", + "deployment_id": "1", + "docker_routing_config_mode": "separated", + "hostname": "sonic-switch", + "hwsku": "Mellanox-SN3800-D112C8", + "peer_switch": "sonic-switch", + "type": "ToRRouter", + "suppress-fib-pending": "enabled" + } + } + }, + "asic0": { + "DEVICE_METADATA": { + "localhost": { + "asic_id": "01.00.0", + "asic_name": "asic0", + "bgp_asn": "65100", + "cloudtype": "None", + "default_bgp_status": "down", + "default_pfcwd_status": "enable", + "deployment_id": "None", + "docker_routing_config_mode": "separated", + "hostname": "sonic", + "hwsku": "multi_asic", + "region": "None", + "sub_role": "FrontEnd", + "type": "LeafRouter" + } + } + }, + "asic1": { + "DEVICE_METADATA": { + "localhost": { + "asic_id": "08:00.0", + "asic_name": "asic1", + "bgp_asn": "65100", + "cloudtype": "None", + "default_bgp_status": "down", + "default_pfcwd_status": "enable", + "deployment_id": "None", + "docker_routing_config_mode": "separated", + "hostname": "sonic", + "hwsku": "multi_asic", + "region": "None", + "sub_role": "BackEnd", + "type": "LeafRouter" + } + } + } + } + + with mock.patch("utilities_common.cli.run_command", + mock.MagicMock(side_effect=mock_run_command_side_effect)),\ + mock.patch('config.main.read_json_file', + mock.MagicMock(side_effect=read_json_file_side_effect)): + + runner = CliRunner() + + result = runner.invoke(config.config.commands["reload"], ["-y", "-f", "all_config_db.json"]) + + print(result.exit_code) + print(result.output) + traceback.print_tb(result.exc_info[2]) + + assert result.exit_code == 0 + assert "\n".join( + [li.rstrip() for li in result.output.split('\n')] + ) == reload_config_masic_onefile_gen_sysinfo_output + + def test_config_reload_onefile_bad_format_masic(self): + def read_json_file_side_effect(filename): + return { + "localhost": {}, + "asic0": {} + } + + with mock.patch("utilities_common.cli.run_command", + mock.MagicMock(side_effect=mock_run_command_side_effect)),\ + mock.patch('config.main.read_json_file', + mock.MagicMock(side_effect=read_json_file_side_effect)): + + runner = CliRunner() + + result = runner.invoke(config.config.commands["reload"], ["-y", "-f", "all_config_db.json"]) + + print(result.exit_code) + print(result.output) + traceback.print_tb(result.exc_info[2]) + + assert result.exit_code != 0 + assert "Input file all_config_db.json must contain all asics config" in result.output + + @classmethod + def teardown_class(cls): + print("TEARDOWN") + os.environ['UTILITIES_UNIT_TESTING'] = "0" + os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "" + # 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): From 48185bef9a08de0cb995a422fe2b8d5485d9ee5c Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Fri, 7 Jun 2024 01:42:46 +0000 Subject: [PATCH 4/6] comment --- config/main.py | 5 +++-- tests/config_test.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/config/main.py b/config/main.py index 3a6ef56a15..411119b376 100644 --- a/config/main.py +++ b/config/main.py @@ -1232,7 +1232,8 @@ def multiasic_validate_single_file(filename): file_input = read_json_file(filename) file_ns_list = [DEFAULT_NAMESPACE if key == HOST_NAMESPACE else key for key in file_input] if set(ns_list) != set(file_ns_list): - click.echo("Input file {} must contain all asics config".format(filename)) + diff = set(ns_list) - set(file_ns_list) + click.echo("Input file {} must contain all asics config. Missing {}".format(filename, diff)) raise click.Abort() @@ -1267,7 +1268,7 @@ def migrate_db_to_lastest(namespace=DEFAULT_NAMESPACE): if namespace is DEFAULT_NAMESPACE: command = [db_migrator, '-o', 'migrate'] else: - command = [db_migrator, '-o', 'migrate', '-n', str(namespace)] + command = [db_migrator, '-o', 'migrate', '-n', namespace] clicommon.run_command(command, display_cmd=True) diff --git a/tests/config_test.py b/tests/config_test.py index 71bab5e67a..41f3e4c1e0 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -168,13 +168,13 @@ Reloading Monit configuration ... """ -reload_config_masic_onefile_output="""\ +reload_config_masic_onefile_output = """\ Stopping SONiC target ... Restarting SONiC target ... Reloading Monit configuration ... """ -reload_config_masic_onefile_gen_sysinfo_output="""\ +reload_config_masic_onefile_gen_sysinfo_output = """\ Stopping SONiC target ... Running command: /usr/local/bin/sonic-cfggen -H -k Mellanox-SN3800-D112C8 --write-to-db Running command: /usr/local/bin/sonic-cfggen -H -k multi_asic -n asic0 --write-to-db From ecc2e4f83f6d20195662c3680449738cabf151b8 Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Tue, 11 Jun 2024 03:50:30 +0000 Subject: [PATCH 5/6] comment --- config/main.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/config/main.py b/config/main.py index a0c12aaf1c..e3e1bb207d 100644 --- a/config/main.py +++ b/config/main.py @@ -1233,19 +1233,22 @@ def multiasic_validate_single_file(filename): file_input = read_json_file(filename) file_ns_list = [DEFAULT_NAMESPACE if key == HOST_NAMESPACE else key for key in file_input] if set(ns_list) != set(file_ns_list): - diff = set(ns_list) - set(file_ns_list) - click.echo("Input file {} must contain all asics config. Missing {}".format(filename, diff)) + click.echo( + "Input file {} must contain all asics config. ns_list: {} file ns_list: {}".format( + filename, ns_list, file_ns_list) + ) raise click.Abort() def load_sysinfo_if_missing(asic_config): - platform = asic_config.get("DEVICE_METADATA", {}).\ - get(HOST_NAMESPACE, {}).get("platform") - mac = asic_config.get("DEVICE_METADATA", {}).\ - get(HOST_NAMESPACE, {}).get("mac") + device_metadata = asic_config.get('DEVICE_METADATA', {}) + platform = device_metadata.get("localhost", {}).get("platform") + mac = device_metadata.get("localhost", {}).get("mac") if not platform or not mac: - log.log_warning("Input file does't have platform or mac. platform: {}, mac: {}" - .format(None if platform is None else platform, None if mac is None else mac)) + if not platform: + log.log_warning("platfrom is missing from Input file") + if not mac: + log.log_warning("mac is missing from Input file") return True return False @@ -1285,7 +1288,7 @@ def multiasic_write_to_db(filename, load_sysinfo): if asic_load_sysinfo: cfg_hwsku = asic_config.get("DEVICE_METADATA", {}).\ - get(HOST_NAMESPACE, {}).get("hwsku") + get("localhost", {}).get("hwsku") if not cfg_hwsku: click.secho("Could not get the HWSKU from config file, Exiting!!!", fg='magenta') sys.exit(1) From d748e0f7cb9a787b04d1d07662684728bf5eba24 Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Wed, 12 Jun 2024 10:02:51 +0000 Subject: [PATCH 6/6] comment --- config/main.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/config/main.py b/config/main.py index e3e1bb207d..312411ba7c 100644 --- a/config/main.py +++ b/config/main.py @@ -1244,13 +1244,14 @@ def load_sysinfo_if_missing(asic_config): device_metadata = asic_config.get('DEVICE_METADATA', {}) platform = device_metadata.get("localhost", {}).get("platform") mac = device_metadata.get("localhost", {}).get("mac") - if not platform or not mac: - if not platform: - log.log_warning("platfrom is missing from Input file") - if not mac: - log.log_warning("mac is missing from Input file") + if not platform: + log.log_warning("platform is missing from Input file") return True - return False + elif not mac: + log.log_warning("mac is missing from Input file") + return True + else: + return False def flush_configdb(namespace=DEFAULT_NAMESPACE): @@ -1290,7 +1291,7 @@ def multiasic_write_to_db(filename, load_sysinfo): cfg_hwsku = asic_config.get("DEVICE_METADATA", {}).\ get("localhost", {}).get("hwsku") if not cfg_hwsku: - click.secho("Could not get the HWSKU from config file, Exiting!!!", fg='magenta') + click.secho("Could not get the HWSKU from config file, Exiting!", fg='magenta') sys.exit(1) client, _ = flush_configdb(ns)