From 72d1faa7aedf986f45d363838483b7398e2a4967 Mon Sep 17 00:00:00 2001 From: HP Date: Mon, 7 Oct 2024 13:34:06 -0700 Subject: [PATCH 01/22] Fix key error when checking for UTILITIES_UNIT_TESTING env var (#3563) --- pfc/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfc/main.py b/pfc/main.py index 071b4a304e..f894a5d7c5 100644 --- a/pfc/main.py +++ b/pfc/main.py @@ -28,7 +28,7 @@ def dump_config_to_json(self, table_name, namespace): This function dumps the current config in a JSON file for unit testing. """ # Only dump files in unit testing mode - if os.environ["UTILITIES_UNIT_TESTING"] != "2": + if os.getenv("UTILITIES_UNIT_TESTING") != "2": return if namespace not in self.updated_port_tables.keys(): From 910252c996a59279101a7116f9bf89e1f936c596 Mon Sep 17 00:00:00 2001 From: DavidZagury <32644413+DavidZagury@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:57:57 +0300 Subject: [PATCH 02/22] [Mellanox] Rename SKU to Mellanox-SN5600-C256X1 (#3546) --- generic_config_updater/gcu_field_operation_validators.conf.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generic_config_updater/gcu_field_operation_validators.conf.json b/generic_config_updater/gcu_field_operation_validators.conf.json index c1921470d4..75a2d03a00 100644 --- a/generic_config_updater/gcu_field_operation_validators.conf.json +++ b/generic_config_updater/gcu_field_operation_validators.conf.json @@ -22,7 +22,7 @@ "spc2": [ "ACS-MSN3800", "Mellanox-SN3800-D112C8", "ACS-MSN3420", "ACS-MSN3700C", "ACS-MSN3700", "Mellanox-SN3800-C64", "Mellanox-SN3800-D100C12S2", "Mellanox-SN3800-D24C52", "Mellanox-SN3800-D28C49S1", "Mellanox-SN3800-D28C50" ], "spc3": [ "ACS-MSN4700", "ACS-MSN4600", "ACS-MSN4600C", "ACS-MSN4410", "ACS-SN4280", "Mellanox-SN4600C-D112C8", "Mellanox-SN4600C-C64", "Mellanox-SN4700-O8C48", "Mellanox-SN4600C-D100C12S2", "Mellanox-SN4600C-D48C40","Mellanox-SN4700-O32","Mellanox-SN4700-V64", "Mellanox-SN4700-A96C8V8", "Mellanox-SN4700-C128", "Mellanox-SN4700-O28", "Mellanox-SN4700-O8V48", "Mellanox-SN4700-V48C32", "Mellanox-SN4280-O28"], - "spc4": [ "ACS-SN5600", "Mellanox-SN5600-O128", "Mellanox-SN5600-V256", "Mellanox-SN5600-C256", "ACS-SN5400" ], + "spc4": [ "ACS-SN5600", "Mellanox-SN5600-O128", "Mellanox-SN5600-V256", "Mellanox-SN5600-C256X1", "ACS-SN5400" ], "spc5": ["ACS-SN5640"] }, "broadcom_asics": { From 88ef85cbfee066c34cce27a58e7053371eb4025f Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com> Date: Sat, 12 Oct 2024 12:15:43 +0800 Subject: [PATCH 03/22] Add a command to update log level and refresh configuration (#3428) * Add a command to update log level and refresh configuration * Multi ASIC support * Fix pre-commit issue * Fix comment Fix review comment --- config/syslog.py | 54 +++++++++++++++++++++++++ doc/Command-Reference.md | 29 ++++++++++++++ tests/syslog_multi_asic_test.py | 16 ++++++++ tests/syslog_test.py | 70 +++++++++++++++++++++++++++++++++ 4 files changed, 169 insertions(+) diff --git a/config/syslog.py b/config/syslog.py index a5d520d9cf..7228e365c8 100644 --- a/config/syslog.py +++ b/config/syslog.py @@ -642,3 +642,57 @@ def disable_rate_limit_feature(db, service_name, namespace): if not failed: click.echo(f'Disabled syslog rate limit feature for {feature_name}') + + +@syslog.command('level') +@click.option("-i", "--identifier", + required=True, + help="Log identifier in DB for which loglevel is applied (provided with -l)") +@click.option("-l", "--level", + required=True, + help="Loglevel value", + type=click.Choice(['DEBUG', 'INFO', 'NOTICE', 'WARN', 'ERROR'])) +@click.option("--container", + help="Container name to which the SIGHUP is sent (provided with --pid or --program)") +@click.option("--program", + help="Program name to which the SIGHUP is sent (provided with --container)") +@click.option("--pid", + help="Process ID to which the SIGHUP is sent (provided with --container if PID is from container)") +@click.option('--namespace', '-n', 'namespace', default=None, + type=click.Choice(multi_asic_util.multi_asic_ns_choices()), + show_default=True, help='Namespace name') +@clicommon.pass_db +def level(db, identifier, level, container, program, pid, namespace): + """ Configure log level """ + if program and not container: + raise click.UsageError('--program must be specified with --container') + + if container and not program and not pid: + raise click.UsageError('--container must be specified with --pid or --program') + + if not namespace: + cfg_db = db.cfgdb + else: + asic_id = multi_asic.get_asic_id_from_name(namespace) + container = f'{container}{asic_id}' + cfg_db = db.cfgdb_clients[namespace] + + cfg_db.mod_entry('LOGGER', identifier, {'LOGLEVEL': level}) + if not container and not program and not pid: + return + + log_config = cfg_db.get_entry('LOGGER', identifier) + require_manual_refresh = log_config.get('require_manual_refresh') + if not require_manual_refresh: + return + + if container: + if program: + command = ['docker', 'exec', '-i', container, 'supervisorctl', 'signal', 'HUP', program] + else: + command = ['docker', 'exec', '-i', container, 'kill', '-s', 'SIGHUP', pid] + else: + command = ['kill', '-s', 'SIGHUP', pid] + output, ret = clicommon.run_command(command, return_cmd=True) + if ret != 0: + raise click.ClickException(f'Failed: {output}') diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index e9009ef67d..1aa9c6523f 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -10858,6 +10858,35 @@ This command is used to disable syslog rate limit feature. config syslog rate-limit-feature disable database -n asci0 ``` +**config syslog level** + +This command is used to configure log level for a given log identifier. + +- Usage: + ``` + config syslog level -i -l --container [] --program [] + + config syslog level -i -l --container [] --pid [] + + config syslog level -i -l ---pid [] + ``` + +- Example: + + ``` + # Update the log level without refresh the configuration + config syslog level -i xcvrd -l DEBUG + + # Update the log level and send SIGHUP to xcvrd running in PMON + config syslog level -i xcvrd -l DEBUG --container pmon --program xcvrd + + # Update the log level and send SIGHUP to PID 20 running in PMON + config syslog level -i xcvrd -l DEBUG --container pmon --pid 20 + + # Update the log level and send SIGHUP to PID 20 running in host + config syslog level -i xcvrd -l DEBUG --pid 20 + ``` + Go Back To [Beginning of the document](#) or [Beginning of this section](#syslog) ## System State diff --git a/tests/syslog_multi_asic_test.py b/tests/syslog_multi_asic_test.py index 7933edcd66..c1a136582c 100644 --- a/tests/syslog_multi_asic_test.py +++ b/tests/syslog_multi_asic_test.py @@ -279,3 +279,19 @@ def test_disable_syslog_rate_limit_feature(self, setup_cmd_module): ['database', '-n', 'asic0'] ) assert result.exit_code == 0 + + @mock.patch('config.syslog.clicommon.run_command') + def test_config_log_level(self, mock_run, setup_cmd_module): + _, config = setup_cmd_module + db = Db() + runner = CliRunner() + + mock_run.return_value = ('something', 0) + result = runner.invoke( + config.config.commands["syslog"].commands["level"], + ['-i', 'component', '-l', 'DEBUG', '-n', 'asic0'], obj=db + ) + assert result.exit_code == 0 + cfg_db = db.cfgdb_clients['asic0'] + data = cfg_db.get_entry('LOGGER', 'component') + assert data.get('LOGLEVEL') == 'DEBUG' diff --git a/tests/syslog_test.py b/tests/syslog_test.py index c1cbee1127..e77f6d0e6c 100644 --- a/tests/syslog_test.py +++ b/tests/syslog_test.py @@ -484,3 +484,73 @@ def side_effect(*args, **kwargs): config.config.commands["syslog"].commands["rate-limit-feature"].commands["disable"], obj=db ) assert result.exit_code == SUCCESS + + @mock.patch('config.syslog.clicommon.run_command') + def test_config_log_level(self, mock_run): + db = Db() + db.cfgdb.set_entry('LOGGER', 'log1', {'require_manual_refresh': 'true'}) + + runner = CliRunner() + + mock_run.return_value = ('something', 0) + result = runner.invoke( + config.config.commands["syslog"].commands["level"], + ['-i', 'component', '-l', 'DEBUG'], obj=db + ) + assert result.exit_code == SUCCESS + data = db.cfgdb.get_entry('LOGGER', 'component') + assert data.get('LOGLEVEL') == 'DEBUG' + + result = runner.invoke( + config.config.commands["syslog"].commands["level"], + ['-i', 'component', '-l', 'DEBUG', '--pid', '123'], obj=db + ) + assert result.exit_code == SUCCESS + + result = runner.invoke( + config.config.commands["syslog"].commands["level"], + ['-i', 'component', '-l', 'DEBUG', '--container', 'pmon', '--pid', '123'], obj=db + ) + assert result.exit_code == SUCCESS + + result = runner.invoke( + config.config.commands["syslog"].commands["level"], + ['-i', 'component', '-l', 'DEBUG', '--container', 'pmon', '--program', 'xcvrd'], obj=db + ) + assert result.exit_code == SUCCESS + + @mock.patch('config.syslog.clicommon.run_command') + def test_config_log_level_negative(self, mock_run): + db = Db() + + runner = CliRunner() + + mock_run.return_value = ('something', 0) + result = runner.invoke( + config.config.commands["syslog"].commands["level"], + ['-i', 'log1', '-l', 'DEBUG', '--container', 'pmon'], obj=db + ) + assert result.exit_code != SUCCESS + + result = runner.invoke( + config.config.commands["syslog"].commands["level"], + ['-i', 'log1', '-l', 'DEBUG', '--program', 'xcvrd'], obj=db + ) + assert result.exit_code != SUCCESS + + mock_run.reset_mock() + result = runner.invoke( + config.config.commands["syslog"].commands["level"], + ['-i', 'log1', '-l', 'DEBUG', '--container', 'swss', '--program', 'orchagent'], obj=db + ) + assert result.exit_code == SUCCESS + # Verify it does not send signal to orchagent if require_manual_refresh is not true + assert mock_run.call_count == 0 + + mock_run.return_value = ('something', -1) + db.cfgdb.set_entry('LOGGER', 'log1', {'require_manual_refresh': 'true'}) + result = runner.invoke( + config.config.commands["syslog"].commands["level"], + ['-i', 'log1', '-l', 'DEBUG', '--container', 'pmon', '--program', 'xcvrd'], obj=db + ) + assert result.exit_code != SUCCESS From 319f58d5d5dbe86e2f0aeece67b8274dcd3f3ce8 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak <38952541+stepanblyschak@users.noreply.github.com> Date: Mon, 14 Oct 2024 19:03:46 +0300 Subject: [PATCH 04/22] [fast/warm-reboot] add cpufreq.default_governor=performance to BOOT_OPTIONS (#3435) * [fast/warm-reboot] add cpufreq.default_governor=performance to BOOT_OPTIONS Append this option to BOOT_OPTIONS variable. How to verify it Run fast-reboot or warm-reboot. Check: admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor performance After boot is finalized check that it is reset back to default: admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor schedutil --------- Signed-off-by: Stepan Blyschak --- scripts/fast-reboot | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/fast-reboot b/scripts/fast-reboot index aef71d6cd6..935188c393 100755 --- a/scripts/fast-reboot +++ b/scripts/fast-reboot @@ -422,6 +422,12 @@ function setup_reboot_variables() local fstype=$(blkid -o value -s TYPE ${sonic_dev}) BOOT_OPTIONS="${BOOT_OPTIONS} ssd-upgrader-part=${sonic_dev},${fstype}" fi + + if [[ "$sonic_asic_type" == "mellanox" ]]; then + # Set governor to performance to speed up boot process. + # The governor is reset back to kernel default in warmboot-finalizer script. + BOOT_OPTIONS="${BOOT_OPTIONS} cpufreq.default_governor=performance" + fi } function check_docker_exec() From 244a18853f56d40ad8460455fc20f9352402d022 Mon Sep 17 00:00:00 2001 From: Saikrishna Arcot Date: Tue, 15 Oct 2024 23:36:02 -0700 Subject: [PATCH 05/22] Update the .NET core version to 8.0 (#3280) For the HTML code coverage report, .NET 6 or newer is needed. Install .NET 8 as it is the latest version currently. Signed-off-by: Saikrishna Arcot --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 8cb6586a9b..5781be9436 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -121,7 +121,7 @@ stages: curl -sSL https://packages.microsoft.com/keys/microsoft.asc | sudo apt-key add - sudo apt-add-repository https://packages.microsoft.com/debian/11/prod sudo apt-get update - sudo apt-get install -y dotnet-sdk-5.0 + sudo apt-get install -y dotnet-sdk-8.0 displayName: "Install .NET CORE" - script: | From 4a6d12180763af3a62c8ae773f140737052d0011 Mon Sep 17 00:00:00 2001 From: vdahiya12 <67608553+vdahiya12@users.noreply.github.com> Date: Fri, 18 Oct 2024 11:06:48 -0700 Subject: [PATCH 06/22] [doc] correct the fec histogram output for show int counters fec-histogram (#3579) Signed-off-by: Vaibhav Dahiya --- doc/Command-Reference.md | 42 ++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 1aa9c6523f..92d35aa3b8 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -5016,29 +5016,25 @@ In a FEC histogram, "bins" represent ranges of errors or specific categories of - Example: ``` admin@str-s6000-acs-11:/usr/bin$ show interface counters fec-histogram -i - -Symbol Errors Per Codeword Codewords --------------------------- --------- -BIN0: 1000000 -BIN1: 900000 -BIN2: 800000 -BIN3: 700000 -BIN4: 600000 -BIN5: 500000 -BIN6: 400000 -BIN7: 300000 -BIN8: 0 -BIN9: 0 -BIN10: 0 -BIN11: 0 -BIN12: 0 -BIN13: 0 -BIN14: 0 -BIN15: 0 - - ``` - - + Symbol Errors Per Codeword Codewords + -------------------------- --------- + BIN0: 1000000 + BIN1: 900000 + BIN2: 800000 + BIN3: 700000 + BIN4: 600000 + BIN5: 500000 + BIN6: 400000 + BIN7: 300000 + BIN8: 0 + BIN9: 0 + BIN10: 0 + BIN11: 0 + BIN12: 0 + BIN13: 0 + BIN14: 0 + BIN15: 0 + ``` **show interfaces description** From 89bb87ad7c319662f35c03b9054ee2a55218868a Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Mon, 21 Oct 2024 14:48:29 +0800 Subject: [PATCH 07/22] Add YANG validation for config reload if file is given (#3576) What I did Add constraint for YANG when config reload, which is already enabled in load_minigraph How I did it Check YANG vaidation for the config How to verify it Unit test --- config/main.py | 30 +++++++++++++++++++++--------- tests/config_test.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/config/main.py b/config/main.py index bfa6dccadc..0a35b102c4 100644 --- a/config/main.py +++ b/config/main.py @@ -1372,6 +1372,19 @@ def multiasic_write_to_db(filename, load_sysinfo): migrate_db_to_lastest(ns) +def config_file_yang_validation(filename): + config_to_check = read_json_file(filename) + sy = sonic_yang.SonicYang(YANG_DIR) + sy.loadYangModel() + try: + sy.loadData(configdbJson=config_to_check) + sy.validate_data_tree() + except sonic_yang.SonicYangException as e: + click.secho("{} fails YANG validation! Error: {}".format(filename, str(e)), + fg='magenta') + raise click.Abort() + + # This is our main entrypoint - the main 'config' command @click.group(cls=clicommon.AbbreviationGroup, context_settings=CONTEXT_SETTINGS) @click.pass_context @@ -1810,6 +1823,13 @@ 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 + if filename is not None: + if multi_asic.is_multi_asic(): + # Multiasic has not 100% fully validated. Thus pass here. + pass + else: + config_file_yang_validation(filename) + #Stop services before config push if not no_service_restart: log.log_notice("'reload' stopping services...") @@ -2000,15 +2020,7 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, override_config, # Multiasic has not 100% fully validated. Thus pass here. pass else: - sy = sonic_yang.SonicYang(YANG_DIR) - sy.loadYangModel() - try: - sy.loadData(configdbJson=config_to_check) - sy.validate_data_tree() - except sonic_yang.SonicYangException as e: - click.secho("{} fails YANG validation! Error: {}".format(golden_config_path, str(e)), - fg='magenta') - raise click.Abort() + config_file_yang_validation(golden_config_path) # Dependency check golden config json if multi_asic.is_multi_asic(): diff --git a/tests/config_test.py b/tests/config_test.py index 21eb095789..3e87c3e9eb 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -1384,6 +1384,34 @@ 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.format(config.SYSTEM_RELOAD_LOCK) + def test_reload_config_fails_yang_validation(self, get_cmd_module, setup_single_broadcom_asic): + with open(self.dummy_cfg_file, 'w') as f: + device_metadata = { + "DEVICE_METADATA": { + "localhost": { + "invalid_hwsku": "some_hwsku" + } + } + } + f.write(json.dumps(device_metadata)) + + with mock.patch( + "utilities_common.cli.run_command", + mock.MagicMock(side_effect=mock_run_command_side_effect) + ): + (config, _) = get_cmd_module + runner = CliRunner() + + result = runner.invoke( + config.config.commands["reload"], + [self.dummy_cfg_file, '-y', '-f']) + + print(result.exit_code) + print(result.output) + traceback.print_tb(result.exc_info[2]) + assert result.exit_code != 0 + assert "fails YANG validation! Error" in result.output + @classmethod def teardown_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "0" From 6c8007faaf15f7b6a4d28467ad0d365842cef21e Mon Sep 17 00:00:00 2001 From: Yuanzhe <150663541+yuazhe@users.noreply.github.com> Date: Tue, 22 Oct 2024 21:07:58 +0800 Subject: [PATCH 08/22] [Mellanox]Remove deprecated sdk sniffer cli and collect sdk dump in show techsupport (#3491) - What I did SDK had upgraded the way of api sniffer and this feature will be enabled by default sonic-net/sonic-buildimage#19925, so make relevant changes: 1. Remove deprecated sdk sniffer cli 2. Collect sdk dump in show techsupport 3. Update the command reference doc. Signed-off-by: Yuanzhe, Liu --- config/plugins/mlnx.py | 34 ------------------- doc/Command-Reference.md | 69 ++------------------------------------- scripts/generate_dump | 12 +++++++ show/plugins/mlnx.py | 14 -------- tests/config_mlnx_test.py | 47 -------------------------- 5 files changed, 15 insertions(+), 161 deletions(-) delete mode 100644 tests/config_mlnx_test.py diff --git a/config/plugins/mlnx.py b/config/plugins/mlnx.py index f61335d4f4..115b310f69 100644 --- a/config/plugins/mlnx.py +++ b/config/plugins/mlnx.py @@ -164,40 +164,6 @@ def mlnx(): """ Mellanox platform configuration tasks """ pass - -# 'sniffer' group -@mlnx.group() -def sniffer(): - """ Utility for managing Mellanox SDK/PRM sniffer """ - pass - - -# 'sdk' subgroup -@sniffer.group() -def sdk(): - """SDK Sniffer - Command Line to enable/disable SDK sniffer""" - pass - - -@sdk.command() -@click.option('-y', '--yes', is_flag=True, callback=_abort_if_false, expose_value=False, - prompt='Swss service will be restarted, continue?') -def enable(): - """Enable SDK Sniffer""" - click.echo("Enabling SDK sniffer") - sdk_sniffer_enable() - click.echo("Note: the sniffer file may exhaust the space on /var/log, please disable it when you are done with this sniffering.") - - -@sdk.command() -@click.option('-y', '--yes', is_flag=True, callback=_abort_if_false, expose_value=False, - prompt='Swss service will be restarted, continue?') -def disable(): - """Disable SDK Sniffer""" - click.echo("Disabling SDK sniffer") - sdk_sniffer_disable() - - def sdk_sniffer_enable(): """Enable SDK Sniffer""" sdk_sniffer_filename = sniffer_filename_generate(SDK_SNIFFER_TARGET_PATH, diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 92d35aa3b8..1b505ced4a 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -8600,74 +8600,11 @@ Go Back To [Beginning of the document](#) or [Beginning of this section](#platfo ### Mellanox Platform Specific Commands -There are few commands that are platform specific. Mellanox has used this feature and implemented Mellanox specific commands as follows. +config platform mlnx -**show platform mlnx sniffer** +This command is valid only on mellanox devices. The sub-commands for "config platform" gets populated only on mellanox platforms. There are no other subcommands on non-Mellanox devices and hence this command appears empty and useless in other platforms. -This command shows the SDK sniffer status - -- Usage: - ``` - show platform mlnx sniffer - ``` - -- Example: - ``` - admin@sonic:~$ show platform mlnx sniffer - sdk sniffer is disabled - ``` - -**show platform mlnx sniffer** - -Another show command available on ‘show platform mlnx’ which is the issu status. -This means if ISSU is enabled on this SKU or not. A warm boot command can be executed only when ISSU is enabled on the SKU. - -- Usage: - ``` - show platform mlnx issu - ``` - -- Example: - ``` - admin@sonic:~$ show platform mlnx issu - ISSU is enabled - ``` - -In the case ISSU is disabled and warm-boot is called, the user will get a notification message explaining that the command cannot be invoked. - -- Example: - ``` - admin@sonic:~$ sudo warm-reboot - ISSU is not enabled on this HWSKU - Warm reboot is not supported - ``` - -**config platform mlnx** - -This command is valid only on mellanox devices. The sub-commands for "config platform" gets populated only on mellanox platforms. -There are no other subcommands on non-Mellanox devices and hence this command appears empty and useless in other platforms. -The platform mellanox command currently includes a single sub command which is the SDK sniffer. -The SDK sniffer is a troubleshooting tool which records the RPC calls from the Mellanox SDK user API library to the sx_sdk task into a .pcap file. -This .pcap file can be replayed afterward to get the exact same configuration state on SDK and FW to reproduce and investigate issues. - -A new folder will be created to store the sniffer files: "/var/log/mellanox/sniffer/". The result file will be stored in a .pcap file, which includes a time stamp of the starting time in the file name, for example, "sx_sdk_sniffer_20180224081306.pcap" -In order to have a complete .pcap file with all the RPC calls, the user should disable the SDK sniffer. Swss service will be restarted and no capturing is taken place from that moment. -It is recommended to review the .pcap file while sniffing is disabled. -Once SDK sniffer is enabled/disabled, the user is requested to approve that swss service will be restarted. -For example: To change SDK sniffer status, swss service will be restarted, continue? [y/N]: -In order to avoid that confirmation the -y / --yes option should be used. - -- Usage: - ``` - config platform mlnx sniffer sdk [-y|--yes] - ``` - -- Example: - ``` - admin@sonic:~$ config platform mlnx sniffer sdk - To change SDK sniffer status, swss service will be restarted, continue? [y/N]: y - NOTE: In order to avoid that confirmation the -y / --yes option should be used. - ``` +The platform mellanox command currently includes no sub command. ### Barefoot Platform Specific Commands diff --git a/scripts/generate_dump b/scripts/generate_dump index 38774c4a37..5858242ce6 100755 --- a/scripts/generate_dump +++ b/scripts/generate_dump @@ -1241,6 +1241,18 @@ collect_mellanox() { fi fi + # collect the sdk dump + local sdk_dbg_folder="/var/log/sdk_dbg" + for file in $(find $sdk_dbg_folder -name "sx_sdk_*") + do + if [[ $file != *.gz ]] + then + save_file $file sai_sdk_dump true + else + save_file $file sai_sdk_dump false + fi + done + # run 'hw-management-generate-dump.sh' script and save the result file HW_DUMP_FILE=/usr/bin/hw-management-generate-dump.sh if [ -f "$HW_DUMP_FILE" ]; then diff --git a/show/plugins/mlnx.py b/show/plugins/mlnx.py index 04d6a78b0a..09eacbc70a 100644 --- a/show/plugins/mlnx.py +++ b/show/plugins/mlnx.py @@ -132,20 +132,6 @@ def is_issu_status_enabled(): return issu_enabled - -@mlnx.command('sniffer') -def sniffer_status(): - """ Show sniffer status """ - components = ['sdk'] - env_variable_strings = [ENV_VARIABLE_SX_SNIFFER] - for index in range(len(components)): - enabled = sniffer_status_get(env_variable_strings[index]) - if enabled is True: - click.echo(components[index] + " sniffer is enabled") - else: - click.echo(components[index] + " sniffer is disabled") - - @mlnx.command('issu') def issu_status(): """ Show ISSU status """ diff --git a/tests/config_mlnx_test.py b/tests/config_mlnx_test.py deleted file mode 100644 index 0cf2e117b4..0000000000 --- a/tests/config_mlnx_test.py +++ /dev/null @@ -1,47 +0,0 @@ -import sys -import click -import pytest -import config.plugins.mlnx as config -from unittest.mock import patch, Mock -from click.testing import CliRunner -from utilities_common.db import Db - - -@patch('config.plugins.mlnx.sniffer_env_variable_set', Mock(return_value=False)) -@patch('config.plugins.mlnx.sniffer_filename_generate', Mock(return_value="sdk_file_name")) -class TestConfigMlnx(object): - def setup(self): - print('SETUP') - - - @patch('config.plugins.mlnx.restart_swss', Mock(return_value=0)) - def test_config_sniffer_enable(self): - db = Db() - runner = CliRunner() - result = runner.invoke(config.mlnx.commands["sniffer"].commands["sdk"].commands["enable"],["-y"]) - assert "SDK sniffer is Enabled, recording file is sdk_file_name." in result.output - - @patch('config.plugins.mlnx.restart_swss', Mock(return_value=0)) - def test_config_sniffer_disble(self): - db = Db() - runner = CliRunner() - result = runner.invoke(config.mlnx.commands["sniffer"].commands["sdk"].commands["disable"],["-y"]) - assert "SDK sniffer is Disabled." in result.output - - @patch('config.plugins.mlnx.restart_swss', Mock(return_value=1)) - def test_config_sniffer_enable_fail(self): - db = Db() - runner = CliRunner() - result = runner.invoke(config.mlnx.commands["sniffer"].commands["sdk"].commands["enable"],["-y"]) - assert "SDK sniffer is Enabled, recording file is sdk_file_name." not in result.output - - @patch('config.plugins.mlnx.restart_swss', Mock(return_value=1)) - def test_config_sniffer_disble_fail(self): - db = Db() - runner = CliRunner() - result = runner.invoke(config.mlnx.commands["sniffer"].commands["sdk"].commands["disable"],["-y"]) - assert "SDK sniffer is Disabled." not in result.output - - def teardown(self): - print('TEARDOWN') - From dd34d7c47a5dda304a90b6121560e2f95fa54ad7 Mon Sep 17 00:00:00 2001 From: Xincun Li <147451452+xincunli-sonic@users.noreply.github.com> Date: Tue, 22 Oct 2024 12:08:29 -0700 Subject: [PATCH 09/22] Revert "Skip default lanes dup check (#3489)" (#3572) Reverts sonic-net/sonic-utilities#3489 since the PR: https://github.com/sonic-net/sonic-buildimage/pull/19968 has merged. --- generic_config_updater/gu_common.py | 3 +-- tests/generic_config_updater/gu_common_test.py | 7 ------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 452bad1ee7..938aa1d034 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -239,8 +239,7 @@ def validate_lanes(self, config_db): for port in port_to_lanes_map: lanes = port_to_lanes_map[port] for lane in lanes: - # default lane would be 0, it does not need validate duplication. - if lane in existing and lane != '0': + if lane in existing: return False, f"'{lane}' lane is used multiple times in PORT: {set([port, existing[lane]])}" existing[lane] = port return True, None diff --git a/tests/generic_config_updater/gu_common_test.py b/tests/generic_config_updater/gu_common_test.py index 21f50e0b7b..4a16a5ca4f 100644 --- a/tests/generic_config_updater/gu_common_test.py +++ b/tests/generic_config_updater/gu_common_test.py @@ -361,13 +361,6 @@ def test_validate_lanes__same_valid_lanes_multi_ports_no_spaces__failure(self): }} self.validate_lanes(config, '67') - def test_validate_lanes_default_value_duplicate_check(self): - config = {"PORT": { - "Ethernet0": {"lanes": "0", "speed": "10000"}, - "Ethernet1": {"lanes": "0", "speed": "10000"}, - }} - self.validate_lanes(config) - def validate_lanes(self, config_db, expected_error=None): # Arrange config_wrapper = gu_common.ConfigWrapper() From aeda86a1e15735ababcb10faae1854c5fd09db0f Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Wed, 23 Oct 2024 13:08:55 +0800 Subject: [PATCH 10/22] Record and warn tables which not covered by YANG (#3583) What I did Sent a warn syslog if there are tables not covered by yang. How I did it Sent a warn syslog if there are tables not covered by yang. How to verify it Manual test --- config/main.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config/main.py b/config/main.py index 0a35b102c4..3baf5114d3 100644 --- a/config/main.py +++ b/config/main.py @@ -2323,6 +2323,9 @@ def validate_config_by_cm_alerting(cm, config_json, jname): except Exception as ex: log.log_warning("Failed to validate {}. Alerting: {}".format(jname, ex)) + if len(cm.tablesWithOutYang()): + log.log_warning("YANG failed to cover tables: {}.".format(str(cm.tablesWithOutYang))) + def override_config_db(config_db, config_input): # Deserialized golden config to DB recognized format From d0aa94a1a12285bedcfc245802c457a84fbe976f Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Wed, 23 Oct 2024 15:47:38 +0800 Subject: [PATCH 11/22] Revert "Record and warn tables which not covered by YANG (#3583)" (#3588) This reverts commit aeda86a1e15735ababcb10faae1854c5fd09db0f. --- config/main.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/config/main.py b/config/main.py index 3baf5114d3..0a35b102c4 100644 --- a/config/main.py +++ b/config/main.py @@ -2323,9 +2323,6 @@ def validate_config_by_cm_alerting(cm, config_json, jname): except Exception as ex: log.log_warning("Failed to validate {}. Alerting: {}".format(jname, ex)) - if len(cm.tablesWithOutYang()): - log.log_warning("YANG failed to cover tables: {}.".format(str(cm.tablesWithOutYang))) - def override_config_db(config_db, config_input): # Deserialized golden config to DB recognized format From b2b97340f5d7f63f8cdf875821f9a7591acb9500 Mon Sep 17 00:00:00 2001 From: Sviatoslav Boichuk Date: Sat, 26 Oct 2024 04:12:27 +0300 Subject: [PATCH 12/22] [Banner] Added CLI commands to configure Banner and display current configuration (#3021) What I did Added CLI commands for Banner feature according to HLD: sonic-net/SONiC#1361 How I did it Added CLI commands to: Enable/disable Banner feature Configure Banner messages: login/motd/logout Related show command How to verify it Manual testing --- config/main.py | 53 ++++++++++++++++++++++++ doc/Command-Reference.md | 89 ++++++++++++++++++++++++++++++++++++++++ show/main.py | 20 +++++++++ tests/config_test.py | 60 +++++++++++++++++++++++++++ tests/show_test.py | 6 +++ 5 files changed, 228 insertions(+) diff --git a/config/main.py b/config/main.py index 0a35b102c4..e797ed0238 100644 --- a/config/main.py +++ b/config/main.py @@ -8065,5 +8065,58 @@ def max_sessions(max_sessions): {'max_sessions': max_sessions}) +# +# 'banner' group ('config banner ...') +# +@config.group() +def banner(): + """Configuring system banner messages""" + pass + + +@banner.command() +@click.argument('state', metavar='', required=True, type=click.Choice(['enabled', 'disabled'])) +def state(state): + """Set banner feature state""" + + config_db = ConfigDBConnector() + config_db.connect() + config_db.mod_entry(swsscommon.CFG_BANNER_MESSAGE_TABLE_NAME, 'global', + {'state': state}) + + +@banner.command() +@click.argument('message', metavar='', required=True) +def login(message): + """Set login message""" + + config_db = ConfigDBConnector() + config_db.connect() + config_db.mod_entry(swsscommon.CFG_BANNER_MESSAGE_TABLE_NAME, 'global', + {'login': message}) + + +@banner.command() +@click.argument('message', metavar='', required=True) +def logout(message): + """Set logout message""" + + config_db = ConfigDBConnector() + config_db.connect() + config_db.mod_entry(swsscommon.CFG_BANNER_MESSAGE_TABLE_NAME, 'global', + {'logout': message}) + + +@banner.command() +@click.argument('message', metavar='', required=True) +def motd(message): + """Set message of the day""" + + config_db = ConfigDBConnector() + config_db.connect() + config_db.mod_entry(swsscommon.CFG_BANNER_MESSAGE_TABLE_NAME, 'global', + {'motd': message}) + + if __name__ == '__main__': config() diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 1b505ced4a..c6c6f2003a 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -225,6 +225,9 @@ * [Static DNS show command](#static-dns-show-command) * [Wake-on-LAN Commands](#wake-on-lan-commands) * [Send Wake-on-LAN Magic Packet command](#send-wake-on-lan-magic-packet-command) +* [Banner Commands](#banner-commands) + * [Banner config commands](#banner-config-commands) + * [Banner show command](#banner-show-command) ## Document History @@ -13819,3 +13822,89 @@ Sending 3 magic packet to 11:33:55:77:99:bb via interface Vlan1000 ``` For the 4th example, it specifise 2 target MAC addresses and `count` is 3. So it'll send 6 magic packets in total. + +# Banner Commands + +This sub-section explains the list of the configuration options available for Banner feature. + +## Banner config commands + +- Set banner feature state + +``` +admin@sonic:~$ config banner state +Usage: config config banner state + + Set banner feature state + +Options: + -?, -h, --help Show this message and exit. +``` + +- Set login message + +``` +admin@sonic:~$ config banner login +Usage: config banner login + + Set login message + +Options: + -?, -h, --help Show this message and exit. +``` + +- Set logout message + +``` +admin@sonic:~$ config banner logout +Usage: config banner logout + + Set logout message + +Options: + -?, -h, --help Show this message and exit. +``` + +- Set message of the day + +``` +admin@sonic:~$ config banner motd +Usage: config banner motd + + Set message of the day + +Options: + -?, -h, --help Show this message and exit. +``` + +## Banner show command + +- how banner messages + +``` +admin@sonic:~$ show banner +Usage: show banner + + Show banner messages + +Options: + -h, -?, --help Show this message and exit. +``` +``` +admin@sonic:~$ show banner +state login motd logout +------- ------- ------------------------------------------------ -------- +enabled Login You are on + Message ____ ___ _ _ _ ____ + / ___| / _ \| \ | (_)/ ___| + \___ \| | | | \| | | | + ___) | |_| | |\ | | |___ + |____/ \___/|_| \_|_|\____| + + -- Software for Open Networking in the Cloud -- + + Unauthorized access and/or use are prohibited. + All access and/or use are subject to monitoring. + + Help: https://sonic-net.github.io/SONiC/ +``` \ No newline at end of file diff --git a/show/main.py b/show/main.py index b7e75b24cf..3151e4d61b 100755 --- a/show/main.py +++ b/show/main.py @@ -2616,6 +2616,26 @@ def ssh(db): click.echo(tabulate(configuration, headers=hdrs, tablefmt='simple', missingval='')) +# +# 'banner' command group ("show banner ...") +# +@cli.group('banner', invoke_without_command=True) +@clicommon.pass_db +def banner(db): + """Show banner messages""" + + banner_table = db.cfgdb.get_entry('BANNER_MESSAGE', 'global') + + hdrs = ['state', 'login', 'motd', 'logout'] + data = [] + + for key in hdrs: + data.append(banner_table.get(key, '').replace('\\n', '\n')) + + messages = [data] + click.echo(tabulate(messages, headers=hdrs, tablefmt='simple', missingval='')) + + # Load plugins and register them helper = util_base.UtilHelper() helper.load_and_register_plugins(plugins, cli) diff --git a/tests/config_test.py b/tests/config_test.py index 3e87c3e9eb..fc3b206646 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -3905,3 +3905,63 @@ def teardown_class(cls): from .mock_tables import mock_single_asic importlib.reload(mock_single_asic) dbconnector.load_database_config() + + +class TestConfigBanner(object): + @classmethod + def setup_class(cls): + print('SETUP') + import config.main + importlib.reload(config.main) + + @patch('utilities_common.cli.run_command', + mock.MagicMock(side_effect=mock_run_command_side_effect)) + def test_banner_state(self): + runner = CliRunner() + obj = {'db': Db().cfgdb} + + result = runner.invoke( + config.config.commands['banner'].commands['state'], + ['enabled'], obj=obj) + + assert result.exit_code == 0 + + @patch('utilities_common.cli.run_command', + mock.MagicMock(side_effect=mock_run_command_side_effect)) + def test_banner_login(self): + runner = CliRunner() + obj = {'db': Db().cfgdb} + + result = runner.invoke( + config.config.commands['banner'].commands['login'], + ['Login message'], obj=obj) + + assert result.exit_code == 0 + + @patch('utilities_common.cli.run_command', + mock.MagicMock(side_effect=mock_run_command_side_effect)) + def test_banner_logout(self): + runner = CliRunner() + obj = {'db': Db().cfgdb} + + result = runner.invoke( + config.config.commands['banner'].commands['logout'], + ['Logout message'], obj=obj) + + assert result.exit_code == 0 + + @patch('utilities_common.cli.run_command', + mock.MagicMock(side_effect=mock_run_command_side_effect)) + def test_banner_motd(self): + runner = CliRunner() + obj = {'db': Db().cfgdb} + + result = runner.invoke( + config.config.commands['banner'].commands['motd'], + ['Motd message'], obj=obj) + + assert result.exit_code == 0 + + @classmethod + def teardown_class(cls): + print('TEARDOWN') diff --git a/tests/show_test.py b/tests/show_test.py index d81192367a..819f197343 100644 --- a/tests/show_test.py +++ b/tests/show_test.py @@ -1040,6 +1040,12 @@ def test_show_ztp(self, mock_run_command): assert result.exit_code == 0 mock_run_command.assert_called_with(['ztp', 'status', '--verbose'], display_cmd=True) + @patch('show.main.run_command') + def test_show_banner(self, mock_run_command): + runner = CliRunner() + result = runner.invoke(show.cli.commands['banner']) + assert result.exit_code == 0 + def teardown(self): print('TEAR DOWN') From d64a90a02ffe07ef0bec060a4df03a7b2f1ada5a Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan <93744978+assrinivasan@users.noreply.github.com> Date: Sun, 27 Oct 2024 19:19:38 -0700 Subject: [PATCH 13/22] Adds logic to get default disk and check disk type (#3399) * Added function to check if disk is SATA * Fixed eval bug unconvered by UT * Changed logger to syslogger; fixed UT failures * Added checks for partitions and filtered_disks; fixed static analysis issues * Fix static analysis errors E303, E711 * Changed information delivery semantics per review comment * Reverted syslogger to logger to maintain backward compatibility The syslogger-to-logger change is unrelated to the sonic-utilities change and breaks backwards compatibility. It should be a separate commit once SysLogger is in all the older versions. * Changed Disk 'type' --> 'Type' for uniformity * Made the fields look uniform * Disk Type Support for eMMC devices * Removed old sonic_ssd import --- ssdutil/main.py | 63 ++++++++++++++++++++---- tests/mocked_libs/__init__.py | 0 tests/mocked_libs/blkinfo.py | 90 +++++++++++++++++++++++++++++++++++ tests/mocked_libs/psutil.py | 6 +++ tests/ssdutil_test.py | 38 +++++++++++++++ 5 files changed, 188 insertions(+), 9 deletions(-) create mode 100644 tests/mocked_libs/__init__.py create mode 100644 tests/mocked_libs/blkinfo.py create mode 100644 tests/mocked_libs/psutil.py diff --git a/ssdutil/main.py b/ssdutil/main.py index 7b6f2c1ca1..460c7f769a 100755 --- a/ssdutil/main.py +++ b/ssdutil/main.py @@ -6,21 +6,61 @@ # try: - import argparse import os import sys + import argparse + import psutil + from blkinfo import BlkDiskInfo from sonic_py_common import device_info, logger except ImportError as e: raise ImportError("%s - required module not found" % str(e)) -DEFAULT_DEVICE="/dev/sda" +DEFAULT_DEVICE = "/dev/sda" SYSLOG_IDENTIFIER = "ssdutil" +DISK_TYPE_SSD = "sata" # Global logger instance log = logger.Logger(SYSLOG_IDENTIFIER) +def get_default_disk(): + """Check default disk""" + default_device = DEFAULT_DEVICE + host_mnt = '/host' + host_partition = None + partitions = psutil.disk_partitions() + + if partitions is None: + return (default_device, None) + + for parts in partitions: + if parts.mountpoint == host_mnt: + host_partition = parts + break + + disk_major = os.major(os.stat(host_partition.device).st_rdev) + filters = { + 'maj:min': '{}:0'.format(disk_major) + } + + myblkd = BlkDiskInfo() + my_filtered_disks = myblkd.get_disks(filters) + + if my_filtered_disks is None: + return (default_device, None) + + json_output = my_filtered_disks[0] + blkdev = json_output['name'] + disk_type = json_output['tran'] + default_device = os.path.join("/dev/", blkdev) + + # Disk Type Support for eMMC devices + disk_type = 'eMMC' if len(disk_type) == 0 and 'mmcblk' in host_partition.device else disk_type # noqa: E501 + + return default_device, disk_type + + def import_ssd_api(diskdev): """ Loads platform specific or generic ssd_util module from source @@ -37,15 +77,16 @@ def import_ssd_api(diskdev): sys.path.append(os.path.abspath(platform_plugins_path)) from ssd_util import SsdUtil except ImportError as e: - log.log_warning("Platform specific SsdUtil module not found. Falling down to the generic implementation") + log.log_warning("Platform specific SsdUtil module not found. Falling down to the generic implementation") # noqa: E501 try: from sonic_platform_base.sonic_storage.ssd import SsdUtil except ImportError as e: - log.log_error("Failed to import default SsdUtil. Error: {}".format(str(e)), True) + log.log_error("Failed to import default SsdUtil. Error: {}".format(str(e)), True) # noqa: E501 raise e return SsdUtil(diskdev) + def is_number(s): try: float(s) @@ -53,6 +94,7 @@ def is_number(s): except ValueError: return False + # ==================== Entry point ==================== def ssdutil(): if os.geteuid() != 0: @@ -60,21 +102,24 @@ def ssdutil(): sys.exit(1) parser = argparse.ArgumentParser() - parser.add_argument("-d", "--device", help="Device name to show health info", default=DEFAULT_DEVICE) - parser.add_argument("-v", "--verbose", action="store_true", default=False, help="Show verbose output (some additional parameters)") - parser.add_argument("-e", "--vendor", action="store_true", default=False, help="Show vendor output (extended output if provided by platform vendor)") + (default_device, disk_type) = get_default_disk() + parser.add_argument("-d", "--device", help="Device name to show health info", default=default_device) # noqa: E501 + parser.add_argument("-v", "--verbose", action="store_true", default=False, help="Show verbose output (some additional parameters)") # noqa: E501 + parser.add_argument("-e", "--vendor", action="store_true", default=False, help="Show vendor output (extended output if provided by platform vendor)") # noqa: E501 args = parser.parse_args() + print("Disk Type : {0}".format(disk_type.upper())) ssd = import_ssd_api(args.device) print("Device Model : {}".format(ssd.get_model())) if args.verbose: print("Firmware : {}".format(ssd.get_firmware())) print("Serial : {}".format(ssd.get_serial())) - print("Health : {}{}".format(ssd.get_health(), "%" if is_number(ssd.get_health()) else "")) - print("Temperature : {}{}".format(ssd.get_temperature(), "C" if is_number(ssd.get_temperature()) else "")) + print("Health : {}{}".format(ssd.get_health(), "%" if is_number(ssd.get_health()) else "")) # noqa: E501 + print("Temperature : {}{}".format(ssd.get_temperature(), "C" if is_number(ssd.get_temperature()) else "")) # noqa: E501 if args.vendor: print(ssd.get_vendor_output()) + if __name__ == '__main__': ssdutil() diff --git a/tests/mocked_libs/__init__.py b/tests/mocked_libs/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/mocked_libs/blkinfo.py b/tests/mocked_libs/blkinfo.py new file mode 100644 index 0000000000..6d5d809837 --- /dev/null +++ b/tests/mocked_libs/blkinfo.py @@ -0,0 +1,90 @@ +mock_json_op = \ + [ + { + "name": "sdx", + "kname": "sdx", + "fstype": "", + "label": "", + "mountpoint": "", + "size": "3965714432", + "maj:min": "8:0", + "rm": "0", + "model": "SMART EUSB", + "vendor": "SMART EUSB", + "serial": "SPG200807J1", + "hctl": "2:0:0:0", + "tran": "usb", + "rota": "1", + "type": "disk", + "ro": "0", + "owner": "", + "group": "", + "mode": "brw-rw----", + "children": [ + { + "name": "sdx1", + "kname": "sdx1", + "fstype": "ext4", + "label": "", + "mountpoint": "/host", + "size": "3964665856", + "maj:min": "8:1", + "rm": "0", + "model": " ", + "vendor": " ", + "serial": "", + "hctl": "", + "tran": "", + "rota": "1", + "type": "part", + "ro": "0", + "owner": "", + "group": "", + "mode": "brw-rw----", + "children": [], + "parents": ["sdx"], + "statistics": { + "major": "8", + "minor": "1", + "kname": "sdx1", + "reads_completed": "22104", + "reads_merged": "5299", + "sectors_read": "1091502", + "time_spent_reading_ms": "51711", + "writes_completed": "11283", + "writes_merged": "13401", + "sectors_written": "443784", + "time_spent_ writing": "133398", + "ios_in_progress": "0", + "time_spent_doing_ios_ms": "112040", + "weighted_time_ios_ms": "112040", + }, + } + ], + "parents": [], + "statistics": { + "major": "8", + "minor": "0", + "kname": "sdx", + "reads_completed": "22151", + "reads_merged": "5299", + "sectors_read": "1093606", + "time_spent_reading_ms": "52005", + "writes_completed": "11283", + "writes_merged": "13401", + "sectors_written": "443784", + "time_spent_ writing": "133398", + "ios_in_progress": "0", + "time_spent_doing_ios_ms": "112220", + "weighted_time_ios_ms": "112220", + }, + } + ] + + +class BlkDiskInfo: + def __init__(self): + return + + def get_disks(self, filters): + return mock_json_op diff --git a/tests/mocked_libs/psutil.py b/tests/mocked_libs/psutil.py new file mode 100644 index 0000000000..f43f024d1c --- /dev/null +++ b/tests/mocked_libs/psutil.py @@ -0,0 +1,6 @@ +from collections import namedtuple + + +def disk_partitions(): + sdiskpart = namedtuple('sdiskpart', ['mountpoint', 'device']) + return [sdiskpart(mountpoint="/host", device="/dev/sdx1")] diff --git a/tests/ssdutil_test.py b/tests/ssdutil_test.py index bd57b0cbe7..dc27526ea7 100644 --- a/tests/ssdutil_test.py +++ b/tests/ssdutil_test.py @@ -1,8 +1,21 @@ +import os import sys import argparse from unittest.mock import patch, MagicMock import sonic_platform_base # noqa: F401 +tests_path = os.path.dirname(os.path.abspath(__file__)) + +# Add mocked_libs path so that the file under test +# can load mocked modules from there +mocked_libs_path = os.path.join(tests_path, "mocked_libs") # noqa: E402,F401 +sys.path.insert(0, mocked_libs_path) + +from .mocked_libs import psutil # noqa: E402,F401 +from .mocked_libs.blkinfo import BlkDiskInfo # noqa: E402,F401 + +sys.modules['os.stat'] = MagicMock() +sys.modules['os.major'] = MagicMock(return_value=8) sys.modules['sonic_platform'] = MagicMock() sys.modules['sonic_platform_base.sonic_ssd.ssd_generic'] = MagicMock() @@ -32,8 +45,33 @@ def get_vendor_output(self): class TestSsdutil: + @patch('os.geteuid', MagicMock(return_value=0)) + @patch('os.stat', MagicMock(st_rdev=2049)) + @patch('os.major', MagicMock(return_value=8)) + def test_get_default_disk(self): + (default_device, disk_type) = ssdutil.get_default_disk() + + assert default_device == "/dev/sdx" + assert disk_type == 'usb' + + @patch('os.geteuid', MagicMock(return_value=0)) + @patch('os.stat', MagicMock(st_rdev=2049)) + @patch('os.major', MagicMock(return_value=8)) + @patch('psutil.disk_partitions', MagicMock(return_value=None)) + def test_get_default_disk_none_partitions(self): + (default_device, disk_type) = ssdutil.get_default_disk() + + assert default_device == "/dev/sda" + assert disk_type is None + + def test_is_number_valueerror(self): + outcome = ssdutil.is_number("nope") + assert outcome is False + @patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', MagicMock(return_value=("test_path", ""))) # noqa: E501 @patch('os.geteuid', MagicMock(return_value=0)) + @patch('os.stat', MagicMock(st_rdev=2049)) + @patch('os.major', MagicMock(return_value=8)) def test_sonic_storage_path(self): with patch('argparse.ArgumentParser.parse_args', MagicMock()) as mock_args: # noqa: E501 From 5b37ee6cd705198e34e5e2c6ca9a5c965cd4d86b Mon Sep 17 00:00:00 2001 From: siqbal1986 Date: Tue, 29 Oct 2024 13:01:04 -0700 Subject: [PATCH 14/22] Vnet_route_check TCP socket for DB connection. (#3578) Currently the Vnet_route_check fails if a user calls it witout sudo with the following error. ``` Traceback (most recent call last): File "/usr/local/bin/vnet_route_check.py", line 401, in sys.exit(main()) File "/usr/local/bin/vnet_route_check.py", line 364, in main if not check_vnet_cfg(): File "/usr/local/bin/vnet_route_check.py", line 77, in check_vnet_cfg db = swsscommon.DBConnector('APPL_DB', 0) File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1656, in __init__ _swsscommon.DBConnector_swiginit(self, _swsscommon.new_DBConnector(*args)) RuntimeError: Unable to connect to redis (unix-socket): Cannot assign requested address ``` #### What I did The **route_check** script accesses the same DB tables but is able to run without the sudo rights. To solve this problem I have changed the **Vnet_route_check** to use a TCP socket to connect to the DB as done in **route_check**. As a result the script doesn't fail with a run time error. #### How I did it #### How to verify it create a new user on a T1 device which has no docker or sudoers privilage. run vnet_route check. it should fail. #### Previous command output (if the output of a command-line utility has changed) ``` Traceback (most recent call last): File "/usr/local/bin/vnet_route_check.py", line 401, in sys.exit(main()) File "/usr/local/bin/vnet_route_check.py", line 364, in main if not check_vnet_cfg(): File "/usr/local/bin/vnet_route_check.py", line 77, in check_vnet_cfg db = swsscommon.DBConnector('APPL_DB', 0) File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1656, in __init__ _swsscommon.DBConnector_swiginit(self, _swsscommon.new_DBConnector(*args)) RuntimeError: Unable to connect to redis (unix-socket): Cannot assign requested address ``` --- scripts/vnet_route_check.py | 16 ++++++++-------- tests/vnet_route_check_test.py | 4 +++- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/scripts/vnet_route_check.py b/scripts/vnet_route_check.py index d925427d40..c747bf7efb 100755 --- a/scripts/vnet_route_check.py +++ b/scripts/vnet_route_check.py @@ -74,7 +74,7 @@ def print_message(lvl, *args): def check_vnet_cfg(): ''' Returns True if VNET is configured in APP_DB or False if no VNET configuration. ''' - db = swsscommon.DBConnector('APPL_DB', 0) + db = swsscommon.DBConnector('APPL_DB', 0, True) vnet_db_keys = swsscommon.Table(db, 'VNET_TABLE').getKeys() @@ -85,7 +85,7 @@ def get_vnet_intfs(): ''' Returns dictionary of VNETs and related VNET interfaces. Format: { : [ ] } ''' - db = swsscommon.DBConnector('APPL_DB', 0) + db = swsscommon.DBConnector('APPL_DB', 0, True) intfs_table = swsscommon.Table(db, 'INTF_TABLE') intfs_keys = swsscommon.Table(db, 'INTF_TABLE').getKeys() @@ -109,7 +109,7 @@ def get_all_rifs_oids(): ''' Returns dictionary of all router interfaces and their OIDs. Format: { : } ''' - db = swsscommon.DBConnector('COUNTERS_DB', 0) + db = swsscommon.DBConnector('COUNTERS_DB', 0, True) rif_table = swsscommon.Table(db, 'COUNTERS_RIF_NAME_MAP') rif_name_oid_map = dict(rif_table.get('')[1]) @@ -140,7 +140,7 @@ def get_vrf_entries(): ''' Returns dictionary of VNET interfaces and corresponding VRF OIDs. Format: { : } ''' - db = swsscommon.DBConnector('ASIC_DB', 0) + db = swsscommon.DBConnector('ASIC_DB', 0, True) rif_table = swsscommon.Table(db, 'ASIC_STATE') vnet_rifs_oids = get_vnet_rifs_oids() @@ -162,7 +162,7 @@ def filter_out_vnet_ip2me_routes(vnet_routes): ''' Filters out IP2ME routes from the provided dictionary with VNET routes Format: { : { 'routes': [ ], 'vrf_oid': } } ''' - db = swsscommon.DBConnector('APPL_DB', 0) + db = swsscommon.DBConnector('APPL_DB', 0, True) all_rifs_db_keys = swsscommon.Table(db, 'INTF_TABLE').getKeys() vnet_intfs = get_vnet_intfs() @@ -198,7 +198,7 @@ def get_vnet_routes_from_app_db(): ''' Returns dictionary of VNET routes configured per each VNET in APP_DB. Format: { : { 'routes': [ ], 'vrf_oid': } } ''' - db = swsscommon.DBConnector('APPL_DB', 0) + db = swsscommon.DBConnector('APPL_DB', 0, True) vnet_intfs = get_vnet_intfs() vnet_vrfs = get_vrf_entries() @@ -245,7 +245,7 @@ def get_vnet_routes_from_asic_db(): ''' Returns dictionary of VNET routes configured per each VNET in ASIC_DB. Format: { : { 'routes': [ ], 'vrf_oid': } } ''' - db = swsscommon.DBConnector('ASIC_DB', 0) + db = swsscommon.DBConnector('ASIC_DB', 0, True) tbl = swsscommon.Table(db, 'ASIC_STATE') @@ -363,7 +363,7 @@ def main(): # Don't run VNET routes consistancy logic if there is no VNET configuration if not check_vnet_cfg(): return rc - asic_db = swsscommon.DBConnector('ASIC_DB', 0) + asic_db = swsscommon.DBConnector('ASIC_DB', 0, True) virtual_router = swsscommon.Table(asic_db, 'ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER') if virtual_router.getKeys() != []: global default_vrf_oid diff --git a/tests/vnet_route_check_test.py b/tests/vnet_route_check_test.py index 092a89e2f9..10d97f21a3 100644 --- a/tests/vnet_route_check_test.py +++ b/tests/vnet_route_check_test.py @@ -341,7 +341,9 @@ def get(self, key): db_conns = {"APPL_DB": APPL_DB, "ASIC_DB": ASIC_DB, "COUNTERS_DB": CNTR_DB} -def conn_side_effect(arg, _): + + +def conn_side_effect(arg, _, __): return db_conns[arg] From 0ae2ec1e2cfd4c4a6e6bf4f7967650288c5985c1 Mon Sep 17 00:00:00 2001 From: Feng-msft Date: Wed, 30 Oct 2024 10:09:25 +0800 Subject: [PATCH 15/22] Add CLI for bmp configdb entity Enable/Disable (#3286) * Add CLI for bmp configdb entity Enable/Disable. * Add CLI for bmp configdb entity Enable/Disable. * Use pytest.mark.parametrize to reduce duplicated code * Use pytest.mark.parametrize to reduce duplicated code --- config/main.py | 99 ++++++++++++++++++++++++++++++++ tests/bmp_input/bmp.json | 9 +++ tests/bmp_input/bmp_invalid.json | 6 ++ tests/config_test.py | 48 ++++++++++++++++ 4 files changed, 162 insertions(+) create mode 100644 tests/bmp_input/bmp.json create mode 100644 tests/bmp_input/bmp_invalid.json diff --git a/config/main.py b/config/main.py index e797ed0238..a042ad8234 100644 --- a/config/main.py +++ b/config/main.py @@ -4248,6 +4248,105 @@ def del_user(db, user): click.echo("Restart service snmp failed with error {}".format(e)) raise click.Abort() + +# +# 'bmp' group ('config bmp ...') +# +@config.group() +@clicommon.pass_db +def bmp(db): + """BMP-related configuration""" + pass + + +# +# common function to update bmp config table +# +@clicommon.pass_db +def update_bmp_table(db, table_name, value): + log.log_info(f"'bmp {value} {table_name}' executing...") + bmp_table = db.cfgdb.get_table('BMP') + if not bmp_table: + bmp_table = {'table': {table_name: value}} + else: + bmp_table['table'][table_name] = value + db.cfgdb.mod_entry('BMP', 'table', bmp_table['table']) + + +# +# 'enable' subgroup ('config bmp enable ...') +# +@bmp.group() +@clicommon.pass_db +def enable(db): + """Enable BMP table dump """ + pass + + +# +# 'bgp-neighbor-table' command ('config bmp enable bgp-neighbor-table') +# +@enable.command('bgp-neighbor-table') +@clicommon.pass_db +def enable_bgp_neighbor_table(db): + update_bmp_table('bgp_neighbor_table', 'true') + + +# +# 'bgp-rib-out-table' command ('config bmp enable bgp-rib-out-table') +# +@enable.command('bgp-rib-out-table') +@clicommon.pass_db +def enable_bgp_rib_out_table(db): + update_bmp_table('bgp_rib_out_table', 'true') + + +# +# 'bgp-rib-in-table' command ('config bmp enable bgp-rib-in-table') +# +@enable.command('bgp-rib-in-table') +@clicommon.pass_db +def enable_bgp_rib_in_table(db): + update_bmp_table('bgp_rib_in_table', 'true') + + +# +# 'disable' subgroup ('config bmp disable ...') +# +@bmp.group() +@clicommon.pass_db +def disable(db): + """Disable BMP table dump """ + pass + + +# +# 'bgp-neighbor-table' command ('config bmp disable bgp-neighbor-table') +# +@disable.command('bgp-neighbor-table') +@clicommon.pass_db +def disable_bgp_neighbor_table(db): + update_bmp_table('bgp_neighbor_table', 'false') + + +# +# 'bgp-rib-out-table' command ('config bmp disable bgp-rib-out-table') +# +@disable.command('bgp-rib-out-table') +@clicommon.pass_db +def diable_bgp_rib_out_table(db): + update_bmp_table('bgp_rib_out_table', 'false') + + +# +# 'bgp-rib-in-table' command ('config bmp disable bgp-rib-in-table') +# +@disable.command('bgp-rib-in-table') +@clicommon.pass_db +def disable_bgp_rib_in_table(db): + update_bmp_table('bgp_rib_in_table', 'false') + + # # 'bgp' group ('config bgp ...') # diff --git a/tests/bmp_input/bmp.json b/tests/bmp_input/bmp.json new file mode 100644 index 0000000000..6f3583f549 --- /dev/null +++ b/tests/bmp_input/bmp.json @@ -0,0 +1,9 @@ +{ + "BMP": { + "table": { + "bgp_neighbor_table": "false", + "bgp_rib_in_table": "false", + "bgp_rib_out_table": "false" + } + } +} diff --git a/tests/bmp_input/bmp_invalid.json b/tests/bmp_input/bmp_invalid.json new file mode 100644 index 0000000000..87a4f937da --- /dev/null +++ b/tests/bmp_input/bmp_invalid.json @@ -0,0 +1,6 @@ +{ + "BMP": { + "table": { + } + } +} diff --git a/tests/config_test.py b/tests/config_test.py index fc3b206646..1809b5545d 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -40,6 +40,9 @@ # Config Reload input Path mock_db_path = os.path.join(test_path, "config_reload_input") +mock_bmp_db_path = os.path.join(test_path, "bmp_input") + + # Load minigraph input Path load_minigraph_input_path = os.path.join(test_path, "load_minigraph_input") load_minigraph_platform_path = os.path.join(load_minigraph_input_path, "platform") @@ -702,6 +705,51 @@ def teardown_class(cls): dbconnector.load_namespace_config() +class TestBMPConfig(object): + @classmethod + def setup_class(cls): + print("SETUP") + os.environ['UTILITIES_UNIT_TESTING'] = "1" + yield + print("TEARDOWN") + os.environ["UTILITIES_UNIT_TESTING"] = "0" + + @pytest.mark.parametrize("table_name", [ + "bgp-neighbor-table", + "bgp-rib-in-table", + "bgp-rib-out-table" + ]) + @pytest.mark.parametrize("enabled", ["true", "false"]) + @pytest.mark.parametrize("filename", ["bmp_invalid.json", "bmp.json"]) + def test_enable_disable_table( + self, + get_cmd_module, + setup_single_broadcom_asic, + table_name, + enabled, + filename): + (config, show) = get_cmd_module + jsonfile_config = os.path.join(mock_bmp_db_path, filename) + config.DEFAULT_CONFIG_DB_FILE = jsonfile_config + runner = CliRunner() + db = Db() + + # Enable table + result = runner.invoke(config.config.commands["bmp"].commands["enable"], + [table_name], obj=db) + assert result.exit_code == 0 + + # Disable table + result = runner.invoke(config.config.commands["bmp"].commands["disable"], + [table_name], obj=db) + assert result.exit_code == 0 + + # Enable table again + result = runner.invoke(config.config.commands["bmp"].commands["enable"], + [table_name], obj=db) + assert result.exit_code == 0 + + class TestConfigReloadMasic(object): @classmethod def setup_class(cls): From 329fc223a6fd5613f86ca1d6c9726a5747c4f03e Mon Sep 17 00:00:00 2001 From: BHUKYA SIDDHU <47605508+Siddhu27@users.noreply.github.com> Date: Wed, 30 Oct 2024 23:41:21 +0530 Subject: [PATCH 16/22] Add support of the pensando-dpu platform to generate-dump utility. (#3557) #### What I did Add support of the pensando-dpu platform to generate-dump utility to collect platform-specific dumps on Pensando DPU. #### How I did it Extend platform-specific section of generate-dump utility #### How to verify it Run "show techsupport" command to generate dump file. Verify that `{dpu_container}_techsupport` directory exists in the created dump file. #### show techsupport command output logs:- ``` { cat }; docker exec syncd saidump | dummy_cleanup_method &> '/var/dump/sonic_dump_sonic_20240910_045531/dump/saidump'" mkdir: created directory '/root/dpu_dump' timeout --foreground 5m sudo docker exec polaris touch /data/techsupport//DSC_TechSupport_b08d.57cd.360f_2024-09-10_04-57-04_1725944224.tar.gz timeout --foreground 5m sudo docker cp polaris:/data/techsupport//DSC_TechSupport_b08d.57cd.360f_2024-09-10_04-57-04_1725944224.tar.gz /root/dpu_dump mkdir: created directory '/var/dump/sonic_dump_sonic_20240910_045531/polaris_techsupport' removed '/root/dpu_dump/DSC_TechSupport_b08d.57cd.360f_2024-09-10_04-57-04_1725944224.tar.gz' removed directory '/root/dpu_dump' timeout --foreground 5m bash -c "dummy_cleanup_method () { cat }; echo 10/09/2024 04:57:43:857701 | dummy_cleanup_method &> '/var/dump/sonic_dump_sonic_20240910_045531/dump/date.counter_2'" ``` ``` sonic_dump_sonic_20240910_045531/proc/iomem sonic_dump_sonic_20240910_045531/polaris_techsupport/ sonic_dump_sonic_20240910_045531/polaris_techsupport/DSC_TechSupport_b08d.57cd.360f_2024-09-10_04-57-04_1725944224.tar.gz sonic_dump_sonic_20240910_045531/core/ ``` ``` root@sonic:/home/admin# cd /var/dump/ root@sonic:/var/dump# ls sonic_dump_sonic_20240910_045531.tar.gz root@sonic:/var/dump# tar -xzf sonic_dump_sonic_20240910_045531.tar.gz root@sonic:/var/dump# cd sonic_dump_sonic_20240910_045531/ root@sonic:/var/dump/sonic_dump_sonic_20240910_045531# cd polaris_techsupport/ root@sonic:/var/dump/sonic_dump_sonic_20240910_045531/polaris_techsupport# ls DSC_TechSupport_b08d.57cd.360f_2024-09-10_04-57-04_1725944224.tar.gz root@sonic:/var/dump/sonic_dump_sonic_20240910_045531/polaris_techsupport# ``` #### /usr/local/bin/generate_dump -n output ``` { cat }; docker exec syncd saidump | dummy_cleanup_method &> '/var/dump/sonic_dump_sonic_20240910_050411/dump/saidump'" mkdir -p /root/dpu_dump docker exec polaris /nic/tools/collect_techsupport.sh rm -rf /root/dpu_dump mkdir -p /var/dump/sonic_dump_sonic_20240910_050411/dump timeout --foreground 5m bash -c "dummy_cleanup_method () { cat }; echo 10/09/2024 05:04:40:130209 | dummy_cleanup_method &> '/var/dump/sonic_dump_sonic_20240910_050411/dump/date.counter_2'" ``` --- scripts/generate_dump | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/scripts/generate_dump b/scripts/generate_dump index 5858242ce6..215cd489d5 100755 --- a/scripts/generate_dump +++ b/scripts/generate_dump @@ -1688,6 +1688,45 @@ collect_nvidia_bluefield() { fi } +############################################################################### +# Collect Pensando specific information +# Globals: +# MKDIR +# V +# NOOP +# RM +# Arguments: +# None +# Returns: +# None +############################################################################### +collect_pensando() { + trap 'handle_error $? $LINENO' ERR + platform=$(grep 'onie_platform=' /host/machine.conf | cut -d '=' -f 2) + pipeline=`cat /usr/share/sonic/device/${platform}/default_pipeline` + if [ ${pipeline} = "polaris" ]; then + dpu_container_name="polaris" + else + dpu_container_name="dpu" + fi + local dpu_dump_folder="/root/dpu_dump" + $MKDIR $V -p $dpu_dump_folder + if $NOOP; then + echo "docker exec ${dpu_container_name} /nic/tools/collect_techsupport.sh" + else + output=$(docker exec ${dpu_container_name} /nic/tools/collect_techsupport.sh 2>&1) + if echo "${output}" | grep -q "Techsupport collected at"; then + file_path=$(echo "${output}" | grep -oP '(?<=Techsupport collected at ).*') + file_name=$(basename "${file_path}") + copy_from_docker ${dpu_container_name} ${file_path} ${dpu_dump_folder} + save_file ${dpu_dump_folder}/${file_name} ${dpu_container_name}_techsupport false + else + echo "Failed to collect ${dpu_container_name} container techsupport..." + fi + fi + $RM $V -rf $dpu_dump_folder +} + ############################################################################### # Save log file # Globals: @@ -2122,6 +2161,10 @@ main() { collect_marvell fi + if [ "$asic" = "pensando" ]; then + collect_pensando + fi + # 2nd counter snapshot late. Need 2 snapshots to make sense of counters trend. save_counter_snapshot $asic 2 From 7cbcfda5e517e5c8a8cfe7140468774b107d6280 Mon Sep 17 00:00:00 2001 From: Deepak Singhal <115033986+deepak-singhal0408@users.noreply.github.com> Date: Sat, 2 Nov 2024 19:11:33 -0700 Subject: [PATCH 17/22] Speed up route_check script (#3544) This PR fixes sonic-net/sonic-buildimage#18773 How I did it Parallely execute route_check on each Asic. Parallelly fetch ipv4 routes and ipv6 routes. How to verify it execute "time route_check.py" on T2 chassis having 32k v4+32k v6 routes. Results: Before: Checking routes for namespaces: ['asic0', 'asic1'] real 3m16.387s user 1m26.084s sys 0m7.275s After: time route_check.py real 1m30.675s user 1m33.777s sys 0m8.209s --- scripts/route_check.py | 190 ++++++++++++++++++++++++----------------- 1 file changed, 110 insertions(+), 80 deletions(-) diff --git a/scripts/route_check.py b/scripts/route_check.py index a1abd3c352..56c845424c 100755 --- a/scripts/route_check.py +++ b/scripts/route_check.py @@ -46,6 +46,7 @@ import signal import traceback import subprocess +import concurrent.futures from ipaddress import ip_network from swsscommon import swsscommon @@ -338,10 +339,18 @@ def is_suppress_fib_pending_enabled(namespace): return state == 'enabled' -def get_frr_routes(namespace): +def fetch_routes(cmd): """ - Read routes from zebra through CLI command - :return frr routes dictionary + Fetch routes using the given command. + """ + output = subprocess.check_output(cmd, text=True) + return json.loads(output) + + +def get_frr_routes_parallel(namespace): + """ + Read routes from zebra through CLI command for IPv4 and IPv6 in parallel + :return combined IPv4 and IPv6 routes dictionary. """ if namespace == multi_asic.DEFAULT_NAMESPACE: v4_route_cmd = ['show', 'ip', 'route', 'json'] @@ -350,12 +359,18 @@ def get_frr_routes(namespace): v4_route_cmd = ['show', 'ip', 'route', '-n', namespace, 'json'] v6_route_cmd = ['show', 'ipv6', 'route', '-n', namespace, 'json'] - output = subprocess.check_output(v4_route_cmd, text=True) - routes = json.loads(output) - output = subprocess.check_output(v6_route_cmd, text=True) - routes.update(json.loads(output)) - print_message(syslog.LOG_DEBUG, "FRR Routes: namespace={}, routes={}".format(namespace, routes)) - return routes + with concurrent.futures.ThreadPoolExecutor() as executor: + future_v4 = executor.submit(fetch_routes, v4_route_cmd) + future_v6 = executor.submit(fetch_routes, v6_route_cmd) + + # Wait for both results to complete + v4_routes = future_v4.result() + v6_routes = future_v6.result() + + # Combine both IPv4 and IPv6 routes + v4_routes.update(v6_routes) + print_message(syslog.LOG_DEBUG, "FRR Routes: namespace={}, routes={}".format(namespace, v4_routes)) + return v4_routes def get_interfaces(namespace): @@ -556,7 +571,7 @@ def check_frr_pending_routes(namespace): retries = FRR_CHECK_RETRIES for i in range(retries): missed_rt = [] - frr_routes = get_frr_routes(namespace) + frr_routes = get_frr_routes_parallel(namespace) for _, entries in frr_routes.items(): for entry in entries: @@ -689,8 +704,9 @@ def _filter_out_neigh_route(routes, neighs): return rt_appl_miss, rt_asic_miss -def check_routes(namespace): +def check_routes_for_namespace(namespace): """ + Process a Single Namespace: The heart of this script which runs the checks. Read APPL-DB & ASIC-DB, the relevant tables for route checking. Checkout routes in ASIC-DB to match APPL-DB, discounting local & @@ -708,98 +724,113 @@ def check_routes(namespace): :return (0, None) on sucess, else (-1, results) where results holds the unjustifiable entries. """ - namespace_list = [] - if namespace is not multi_asic.DEFAULT_NAMESPACE and namespace in multi_asic.get_namespace_list(): - namespace_list.append(namespace) - else: - namespace_list = multi_asic.get_namespace_list() - print_message(syslog.LOG_INFO, "Checking routes for namespaces: ", namespace_list) results = {} - adds = {} - deletes = {} - for namespace in namespace_list: - intf_appl_miss = [] - rt_appl_miss = [] - rt_asic_miss = [] - rt_frr_miss = [] - adds[namespace] = [] - deletes[namespace] = [] + adds = [] + deletes = [] + intf_appl_miss = [] + rt_appl_miss = [] + rt_asic_miss = [] + rt_frr_miss = [] - selector, subs, rt_asic = get_asicdb_routes(namespace) + selector, subs, rt_asic = get_asicdb_routes(namespace) - rt_appl = get_appdb_routes(namespace) - intf_appl = get_interfaces(namespace) + rt_appl = get_appdb_routes(namespace) + intf_appl = get_interfaces(namespace) - # Diff APPL-DB routes & ASIC-DB routes - rt_appl_miss, rt_asic_miss = diff_sorted_lists(rt_appl, rt_asic) + # Diff APPL-DB routes & ASIC-DB routes + rt_appl_miss, rt_asic_miss = diff_sorted_lists(rt_appl, rt_asic) - # Check missed ASIC routes against APPL-DB INTF_TABLE - _, rt_asic_miss = diff_sorted_lists(intf_appl, rt_asic_miss) - rt_asic_miss = filter_out_default_routes(rt_asic_miss) - rt_asic_miss = filter_out_vnet_routes(namespace, rt_asic_miss) - rt_asic_miss = filter_out_standalone_tunnel_routes(namespace, rt_asic_miss) - rt_asic_miss = filter_out_soc_ip_routes(namespace, rt_asic_miss) + # Check missed ASIC routes against APPL-DB INTF_TABLE + _, rt_asic_miss = diff_sorted_lists(intf_appl, rt_asic_miss) + rt_asic_miss = filter_out_default_routes(rt_asic_miss) + rt_asic_miss = filter_out_vnet_routes(namespace, rt_asic_miss) + rt_asic_miss = filter_out_standalone_tunnel_routes(namespace, rt_asic_miss) + rt_asic_miss = filter_out_soc_ip_routes(namespace, rt_asic_miss) + # Check APPL-DB INTF_TABLE with ASIC table route entries + intf_appl_miss, _ = diff_sorted_lists(intf_appl, rt_asic) - # Check APPL-DB INTF_TABLE with ASIC table route entries - intf_appl_miss, _ = diff_sorted_lists(intf_appl, rt_asic) + if rt_appl_miss: + rt_appl_miss = filter_out_local_interfaces(namespace, rt_appl_miss) - if rt_appl_miss: - rt_appl_miss = filter_out_local_interfaces(namespace, rt_appl_miss) + if rt_appl_miss: + rt_appl_miss = filter_out_voq_neigh_routes(namespace, rt_appl_miss) - if rt_appl_miss: - rt_appl_miss = filter_out_voq_neigh_routes(namespace, rt_appl_miss) + # NOTE: On dualtor environment, ignore any route miss for the + # neighbors learned from the vlan subnet. + if rt_appl_miss or rt_asic_miss: + rt_appl_miss, rt_asic_miss = filter_out_vlan_neigh_route_miss(namespace, rt_appl_miss, rt_asic_miss) - # NOTE: On dualtor environment, ignore any route miss for the - # neighbors learned from the vlan subnet. - if rt_appl_miss or rt_asic_miss: - rt_appl_miss, rt_asic_miss = filter_out_vlan_neigh_route_miss(namespace, rt_appl_miss, rt_asic_miss) + if rt_appl_miss or rt_asic_miss: + # Look for subscribe updates for a second + adds, deletes = get_subscribe_updates(selector, subs) - if rt_appl_miss or rt_asic_miss: - # Look for subscribe updates for a second - adds[namespace], deletes[namespace] = get_subscribe_updates(selector, subs) + # Drop all those for which SET received + rt_appl_miss, _ = diff_sorted_lists(rt_appl_miss, adds) - # Drop all those for which SET received - rt_appl_miss, _ = diff_sorted_lists(rt_appl_miss, adds[namespace]) + # Drop all those for which DEL received + rt_asic_miss, _ = diff_sorted_lists(rt_asic_miss, deletes) - # Drop all those for which DEL received - rt_asic_miss, _ = diff_sorted_lists(rt_asic_miss, deletes[namespace]) + if rt_appl_miss: + results["missed_ROUTE_TABLE_routes"] = rt_appl_miss - if rt_appl_miss: - if namespace not in results: - results[namespace] = {} - results[namespace]["missed_ROUTE_TABLE_routes"] = rt_appl_miss + if intf_appl_miss: + results["missed_INTF_TABLE_entries"] = intf_appl_miss - if intf_appl_miss: - if namespace not in results: - results[namespace] = {} - results[namespace]["missed_INTF_TABLE_entries"] = intf_appl_miss + if rt_asic_miss: + results["Unaccounted_ROUTE_ENTRY_TABLE_entries"] = rt_asic_miss - if rt_asic_miss: - if namespace not in results: - results[namespace] = {} - results[namespace]["Unaccounted_ROUTE_ENTRY_TABLE_entries"] = rt_asic_miss + rt_frr_miss = check_frr_pending_routes(namespace) - rt_frr_miss = check_frr_pending_routes(namespace) + if rt_frr_miss: + results["missed_FRR_routes"] = rt_frr_miss - if rt_frr_miss: - if namespace not in results: - results[namespace] = {} - results[namespace]["missed_FRR_routes"] = rt_frr_miss + if results: + if rt_frr_miss and not rt_appl_miss and not rt_asic_miss: + print_message(syslog.LOG_ERR, "Some routes are not set offloaded in FRR{} \ + but all routes in APPL_DB and ASIC_DB are in sync".format(namespace)) + if is_suppress_fib_pending_enabled(namespace): + mitigate_installed_not_offloaded_frr_routes(namespace, rt_frr_miss, rt_appl) + + return results, adds, deletes - if results: - if rt_frr_miss and not rt_appl_miss and not rt_asic_miss: - print_message(syslog.LOG_ERR, "Some routes are not set offloaded in FRR{} \ - but all routes in APPL_DB and ASIC_DB are in sync".format(namespace)) - if is_suppress_fib_pending_enabled(namespace): - mitigate_installed_not_offloaded_frr_routes(namespace, rt_frr_miss, rt_appl) + +def check_routes(namespace): + """ + Main function to parallelize route checks across all namespaces. + """ + namespace_list = [] + if namespace is not multi_asic.DEFAULT_NAMESPACE and namespace in multi_asic.get_namespace_list(): + namespace_list.append(namespace) + else: + namespace_list = multi_asic.get_namespace_list() + print_message(syslog.LOG_INFO, "Checking routes for namespaces: ", namespace_list) + + results = {} + all_adds = {} + all_deletes = {} + + # Use ThreadPoolExecutor to parallelize the check for each namespace + with concurrent.futures.ThreadPoolExecutor() as executor: + futures = {executor.submit(check_routes_for_namespace, ns): ns for ns in namespace_list} + + for future in concurrent.futures.as_completed(futures): + ns = futures[future] + try: + result, adds, deletes = future.result() + if result: + results[ns] = result + all_adds[ns] = adds + all_deletes[ns] = deletes + except Exception as e: + print_message(syslog.LOG_ERR, "Error processing namespace {}: {}".format(ns, e)) if results: print_message(syslog.LOG_WARNING, "Failure results: {", json.dumps(results, indent=4), "}") print_message(syslog.LOG_WARNING, "Failed. Look at reported mismatches above") - print_message(syslog.LOG_WARNING, "add: ", json.dumps(adds, indent=4)) - print_message(syslog.LOG_WARNING, "del: ", json.dumps(deletes, indent=4)) + print_message(syslog.LOG_WARNING, "add: ", json.dumps(all_adds, indent=4)) + print_message(syslog.LOG_WARNING, "del: ", json.dumps(all_deletes, indent=4)) return -1, results else: print_message(syslog.LOG_INFO, "All good!") @@ -862,6 +893,5 @@ def main(): return ret, res - if __name__ == "__main__": sys.exit(main()[0]) From 3354d08a2bf47d93f260e94acdf43f03f6c20b6d Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Wed, 6 Nov 2024 16:19:54 +0800 Subject: [PATCH 18/22] [config] Bypass standard input for reload (#3597) What I did Bypass stdin input for config reload to avoid double json read which will cause issue How I did it Add a bypass check How to verify it UT and manual test --- config/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/main.py b/config/main.py index a042ad8234..9a71f1078b 100644 --- a/config/main.py +++ b/config/main.py @@ -1823,7 +1823,7 @@ 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 - if filename is not None: + if filename is not None and filename != "/dev/stdin": if multi_asic.is_multi_asic(): # Multiasic has not 100% fully validated. Thus pass here. pass From 964b4895df1e6d8cb710360b548209754c04a369 Mon Sep 17 00:00:00 2001 From: harjotsinghpawra Date: Thu, 7 Nov 2024 00:59:02 -0800 Subject: [PATCH 19/22] Fix for integer overflow of counter value if its too large (#3596) What I did Changed the data type casting to float if count coming is too large or in scientific notation . How I did it made it float then type cast to int for display purpose How to verify it Manually passed big value and see the count output if it breaks or works --- utilities_common/portstat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utilities_common/portstat.py b/utilities_common/portstat.py index 6942fa5f2a..8fcbc6b0d9 100644 --- a/utilities_common/portstat.py +++ b/utilities_common/portstat.py @@ -229,7 +229,7 @@ def get_counters(port): if counter_name not in fvs: fields[pos] = STATUS_NA elif fields[pos] != STATUS_NA: - fields[pos] = str(int(fields[pos]) + int(fvs[counter_name])) + fields[pos] = str(int(fields[pos]) + int(float(fvs[counter_name]))) cntr = NStats._make(fields)._asdict() return cntr From 0af4386ea23e63924f9ac46e210c52d91d57cf5c Mon Sep 17 00:00:00 2001 From: Xincun Li <147451452+xincunli-sonic@users.noreply.github.com> Date: Thu, 7 Nov 2024 10:03:24 -0800 Subject: [PATCH 20/22] Consolidate the get running config way. (#3585) #### What I did Addressing the [issue 20508](https://github.com/sonic-net/sonic-buildimage/issues/20508). ADO: 29979987 #### How I did it Remove temp file as intermediate steps. #### How to verify it ``` admin@str2-7250-lc1-2:~$ cat test.json [ { "op": "add", "path": "/asic0/BGP_DEVICE_GLOBAL/STATE/idf_isolation_state", "value": "unisolated" } ] admin@str2-7250-lc1-2:~$ sudo config apply-patch test.json sonic_yang(6):Note: Below table(s) have no YANG models: DHCP_SERVER sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER Patch Applier: asic0: Patch application starting. Patch Applier: asic0: Patch: [{"op": "add", "path": "/BGP_DEVICE_GLOBAL/STATE/idf_isolation_state", "value": "unisolated"}] Patch Applier: asic0 getting current config db. Patch Applier: asic0: simulating the target full config after applying the patch. Patch Applier: asic0: validating all JsonPatch operations are permitted on the specified fields Patch Applier: asic0: validating target config does not have empty tables, since they do not show up in ConfigDb. Patch Applier: asic0: sorting patch updates. Patch Applier: The asic0 patch was converted into 1 change: Patch Applier: asic0: applying 1 change in order: Patch Applier: * [{"op": "replace", "path": "/BGP_DEVICE_GLOBAL/STATE/idf_isolation_state", "value": "unisolated"}] Patch Applier: asic0: verifying patch updates are reflected on ConfigDB. Patch Applier: asic0 patch application completed. Patch applied successfully. admin@str2-7250-lc1-2:~$ show ver SONiC Software Version: SONiC.20220532.72 SONiC OS Version: 11 Distribution: Debian 11.9 Kernel: 5.10.0-23-2-amd64 Build commit: 7766169087 Build date: Fri Oct 4 00:15:40 UTC 2024 Built by: azureuser@98b2318ac000000 Platform: x86_64-nokia_ixr7250e_36x400g-r0 HwSKU: Nokia-IXR7250E-36x100G ASIC: broadcom ASIC Count: 2 Serial Number: NS220304200 Model Number: 3HE12578AARA01 Hardware Revision: 56 Uptime: 05:08:45 up 2 days, 10:16, 1 user, load average: 1.64, 1.82, 1.74 Date: Fri 25 Oct 2024 05:08:45 Docker images: REPOSITORY TAG IMAGE ID SIZE docker-mux 20220532.72 a27de04f0900 375MB docker-mux latest a27de04f0900 375MB docker-macsec latest 9cad4ac054db 372MB docker-sonic-restapi 20220532.72 2dc9b6c42cdb 345MB docker-sonic-restapi latest 2dc9b6c42cdb 345MB docker-orchagent 20220532.72 560867c70e69 360MB docker-orchagent latest 560867c70e69 360MB docker-fpm-frr 20220532.72 525aad3b1670 367MB docker-fpm-frr latest 525aad3b1670 367MB docker-teamd 20220532.72 9bc2875ba21c 343MB docker-teamd latest 9bc2875ba21c 343MB docker-syncd-brcm-dnx 20220532.72 58ee35f9df5b 674MB docker-syncd-brcm-dnx latest 58ee35f9df5b 674MB docker-gbsyncd-credo 20220532.72 5084ec39b3fc 346MB docker-gbsyncd-credo latest 5084ec39b3fc 346MB docker-gbsyncd-broncos 20220532.72 f1011e5ed75c 374MB docker-gbsyncd-broncos latest f1011e5ed75c 374MB docker-dhcp-relay latest 137faf5f4038 337MB docker-platform-monitor 20220532.72 41d6954ab85a 452MB docker-platform-monitor latest 41d6954ab85a 452MB docker-snmp 20220532.72 916f66a40a77 376MB docker-snmp latest 916f66a40a77 376MB docker-sonic-telemetry 20220532.72 e8037e0fd00c 407MB docker-sonic-telemetry latest e8037e0fd00c 407MB docker-router-advertiser 20220532.72 a5afbccec5da 327MB docker-router-advertiser latest a5afbccec5da 327MB docker-lldp 20220532.72 01386dd544cf 369MB docker-lldp latest 01386dd544cf 369MB docker-database 20220532.72 2da62f2abd04 327MB docker-database latest 2da62f2abd04 327MB docker-acms 20220532.72 264a51a7a259 374MB docker-acms latest 264a51a7a259 374MB k8s.gcr.io/pause 3.5 ed210e3e4a5b 683kB ``` --- generic_config_updater/change_applier.py | 33 +------ generic_config_updater/gu_common.py | 40 ++++---- .../change_applier_test.py | 25 +++-- .../gcu_feature_patch_application_test.py | 15 ++- .../multiasic_change_applier_test.py | 93 ++++++++----------- 5 files changed, 88 insertions(+), 118 deletions(-) diff --git a/generic_config_updater/change_applier.py b/generic_config_updater/change_applier.py index 8d8d23f87a..b5712d024f 100644 --- a/generic_config_updater/change_applier.py +++ b/generic_config_updater/change_applier.py @@ -9,6 +9,7 @@ from swsscommon.swsscommon import ConfigDBConnector from sonic_py_common import multi_asic from .gu_common import GenericConfigUpdaterError, genericUpdaterLogging +from .gu_common import get_config_db_as_json SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) UPDATER_CONF_FILE = f"{SCRIPT_DIR}/gcu_services_validator.conf.json" @@ -137,7 +138,7 @@ def _report_mismatch(self, run_data, upd_data): str(jsondiff.diff(run_data, upd_data))[0:40])) def apply(self, change): - run_data = self._get_running_config() + run_data = get_config_db_as_json(self.scope) upd_data = prune_empty_table(change.apply(copy.deepcopy(run_data))) upd_keys = defaultdict(dict) @@ -146,7 +147,7 @@ def apply(self, change): ret = self._services_validate(run_data, upd_data, upd_keys) if not ret: - run_data = self._get_running_config() + run_data = get_config_db_as_json(self.scope) self.remove_backend_tables_from_config(upd_data) self.remove_backend_tables_from_config(run_data) if upd_data != run_data: @@ -159,31 +160,3 @@ def apply(self, change): def remove_backend_tables_from_config(self, data): for key in self.backend_tables: data.pop(key, None) - - def _get_running_config(self): - _, fname = tempfile.mkstemp(suffix="_changeApplier") - - if self.scope: - cmd = ['sonic-cfggen', '-d', '--print-data', '-n', self.scope] - else: - cmd = ['sonic-cfggen', '-d', '--print-data'] - - with open(fname, "w") as file: - result = subprocess.Popen(cmd, stdout=file, stderr=subprocess.PIPE, text=True) - _, err = result.communicate() - - return_code = result.returncode - if return_code: - os.remove(fname) - raise GenericConfigUpdaterError( - f"Failed to get running config for scope: {self.scope}," + - f"Return code: {return_code}, Error: {err}") - - run_data = {} - try: - with open(fname, "r") as file: - run_data = json.load(file) - finally: - if os.path.isfile(fname): - os.remove(fname) - return run_data diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 938aa1d034..7821557e71 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -53,6 +53,28 @@ def __eq__(self, other): return self.patch == other.patch return False + +def get_config_db_as_json(scope=None): + text = get_config_db_as_text(scope=scope) + config_db_json = json.loads(text) + config_db_json.pop("bgpraw", None) + return config_db_json + + +def get_config_db_as_text(scope=None): + if scope is not None and scope != multi_asic.DEFAULT_NAMESPACE: + cmd = ['sonic-cfggen', '-d', '--print-data', '-n', scope] + else: + cmd = ['sonic-cfggen', '-d', '--print-data'] + result = subprocess.Popen(cmd, shell=False, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + text, err = result.communicate() + return_code = result.returncode + if return_code: + raise GenericConfigUpdaterError(f"Failed to get running config for namespace: {scope}," + f" Return code: {return_code}, Error: {err}") + return text + + class ConfigWrapper: def __init__(self, yang_dir=YANG_DIR, scope=multi_asic.DEFAULT_NAMESPACE): self.scope = scope @@ -60,24 +82,10 @@ def __init__(self, yang_dir=YANG_DIR, scope=multi_asic.DEFAULT_NAMESPACE): self.sonic_yang_with_loaded_models = None def get_config_db_as_json(self): - text = self._get_config_db_as_text() - config_db_json = json.loads(text) - config_db_json.pop("bgpraw", None) - return config_db_json + return get_config_db_as_json(self.scope) def _get_config_db_as_text(self): - if self.scope is not None and self.scope != multi_asic.DEFAULT_NAMESPACE: - cmd = ['sonic-cfggen', '-d', '--print-data', '-n', self.scope] - else: - cmd = ['sonic-cfggen', '-d', '--print-data'] - - result = subprocess.Popen(cmd, shell=False, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - text, err = result.communicate() - return_code = result.returncode - if return_code: # non-zero means failure - raise GenericConfigUpdaterError(f"Failed to get running config for namespace: {self.scope}," - f" Return code: {return_code}, Error: {err}") - return text + return get_config_db_as_text(self.scope) def get_sonic_yang_as_json(self): config_db_json = self.get_config_db_as_json() diff --git a/tests/generic_config_updater/change_applier_test.py b/tests/generic_config_updater/change_applier_test.py index 7aad111f18..6a8926f013 100644 --- a/tests/generic_config_updater/change_applier_test.py +++ b/tests/generic_config_updater/change_applier_test.py @@ -72,28 +72,25 @@ def debug_print(msg): print(msg) - -# Mimics os.system call for sonic-cfggen -d --print-data > filename +# Mimics os.system call for `sonic-cfggen -d --print-data` output def subprocess_Popen_cfggen(cmd, *args, **kwargs): global running_config - # Extract file name from kwargs if 'stdout' is a file object - stdout = kwargs.get('stdout') - if hasattr(stdout, 'name'): - fname = stdout.name + stdout = kwargs.get('stdout', None) + + if stdout is None: + output = json.dumps(running_config, indent=4) + elif isinstance(stdout, int) and stdout == -1: + output = json.dumps(running_config, indent=4) else: - raise ValueError("stdout is not a file") + raise ValueError("stdout must be set to subprocess.PIPE or omitted for capturing output") - # Write the running configuration to the file specified in stdout - with open(fname, "w") as s: - json.dump(running_config, s, indent=4) - class MockPopen: def __init__(self): - self.returncode = 0 # Simulate successful command execution + self.returncode = 0 def communicate(self): - return "", "" # Simulate empty stdout and stderr + return output.encode(), "".encode() return MockPopen() @@ -225,7 +222,7 @@ def vlan_validate(old_cfg, new_cfg, keys): class TestChangeApplier(unittest.TestCase): - @patch("generic_config_updater.change_applier.subprocess.Popen") + @patch("generic_config_updater.gu_common.subprocess.Popen") @patch("generic_config_updater.change_applier.get_config_db") @patch("generic_config_updater.change_applier.set_config") def test_change_apply(self, mock_set, mock_db, mock_subprocess_Popen): diff --git a/tests/generic_config_updater/gcu_feature_patch_application_test.py b/tests/generic_config_updater/gcu_feature_patch_application_test.py index db625e8cd1..27d9ebf216 100644 --- a/tests/generic_config_updater/gcu_feature_patch_application_test.py +++ b/tests/generic_config_updater/gcu_feature_patch_application_test.py @@ -6,13 +6,15 @@ from mock import patch import generic_config_updater.change_applier +import generic_config_updater.gu_common import generic_config_updater.patch_sorter as ps import generic_config_updater.generic_updater as gu from .gutest_helpers import Files from generic_config_updater.gu_common import ConfigWrapper, PatchWrapper running_config = {} - + + def set_entry(config_db, tbl, key, data): global running_config if data != None: @@ -26,9 +28,11 @@ def set_entry(config_db, tbl, key, data): if not running_config[tbl]: running_config.pop(tbl) -def get_running_config(): + +def get_running_config(scope="localhost"): return running_config + class TestFeaturePatchApplication(unittest.TestCase): def setUp(self): self.config_wrapper = ConfigWrapper() @@ -87,13 +91,13 @@ def create_patch_applier(self, config): config_wrapper = self.config_wrapper config_wrapper.get_config_db_as_json = MagicMock(side_effect=get_running_config) change_applier = generic_config_updater.change_applier.ChangeApplier() - change_applier._get_running_config = MagicMock(side_effect=get_running_config) patch_wrapper = PatchWrapper(config_wrapper) return gu.PatchApplier(config_wrapper=config_wrapper, patch_wrapper=patch_wrapper, changeapplier=change_applier) + @patch('generic_config_updater.change_applier.get_config_db_as_json', side_effect=get_running_config) @patch("generic_config_updater.change_applier.get_config_db") @patch("generic_config_updater.change_applier.set_config") - def run_single_success_case_applier(self, data, mock_set, mock_db): + def run_single_success_case_applier(self, data, mock_set, mock_db, mock_get_config_db_as_json): current_config = data["current_config"] expected_config = data["expected_config"] patch = jsonpatch.JsonPatch(data["patch"]) @@ -121,7 +125,8 @@ def run_single_success_case_applier(self, data, mock_set, mock_db): self.assertEqual(simulated_config, expected_config) @patch("generic_config_updater.change_applier.get_config_db") - def run_single_failure_case_applier(self, data, mock_db): + @patch('generic_config_updater.change_applier.get_config_db_as_json', side_effect=get_running_config) + def run_single_failure_case_applier(self, data, mock_db, mock_get_config_db_as_json): current_config = data["current_config"] patch = jsonpatch.JsonPatch(data["patch"]) expected_error_substrings = data["expected_error_substrings"] diff --git a/tests/generic_config_updater/multiasic_change_applier_test.py b/tests/generic_config_updater/multiasic_change_applier_test.py index 0102cfff00..afa1b449f3 100644 --- a/tests/generic_config_updater/multiasic_change_applier_test.py +++ b/tests/generic_config_updater/multiasic_change_applier_test.py @@ -7,6 +7,30 @@ import generic_config_updater.gu_common +def mock_get_running_config_side_effect(scope): + print(f"mocked_value_for_{scope}") + return { + "tables": { + "ACL_TABLE": { + "services_to_validate": ["aclservice"], + "validate_commands": ["acl_loader show table"] + }, + "PORT": { + "services_to_validate": ["portservice"], + "validate_commands": ["show interfaces status"] + } + }, + "services": { + "aclservice": { + "validate_commands": ["acl_loader show table"] + }, + "portservice": { + "validate_commands": ["show interfaces status"] + } + } + } + + class TestMultiAsicChangeApplier(unittest.TestCase): @patch('sonic_py_common.multi_asic.is_multi_asic') @@ -137,7 +161,7 @@ def test_extract_scope_singleasic(self, mock_is_multi_asic): except Exception: assert(not result) - @patch('generic_config_updater.change_applier.ChangeApplier._get_running_config', autospec=True) + @patch('generic_config_updater.change_applier.get_config_db_as_json', autospec=True) @patch('generic_config_updater.change_applier.ConfigDBConnector', autospec=True) def test_apply_change_default_scope(self, mock_ConfigDBConnector, mock_get_running_config): # Setup mock for ConfigDBConnector @@ -145,26 +169,7 @@ def test_apply_change_default_scope(self, mock_ConfigDBConnector, mock_get_runni mock_ConfigDBConnector.return_value = mock_db # Setup mock for json.load to return some running configuration - mock_get_running_config.return_value = { - "tables": { - "ACL_TABLE": { - "services_to_validate": ["aclservice"], - "validate_commands": ["acl_loader show table"] - }, - "PORT": { - "services_to_validate": ["portservice"], - "validate_commands": ["show interfaces status"] - } - }, - "services": { - "aclservice": { - "validate_commands": ["acl_loader show table"] - }, - "portservice": { - "validate_commands": ["show interfaces status"] - } - } - } + mock_get_running_config.side_effect = mock_get_running_config_side_effect # Instantiate ChangeApplier with the default scope applier = generic_config_updater.change_applier.ChangeApplier() @@ -178,34 +183,13 @@ def test_apply_change_default_scope(self, mock_ConfigDBConnector, mock_get_runni # Assert ConfigDBConnector called with the correct namespace mock_ConfigDBConnector.assert_called_once_with(use_unix_socket_path=True, namespace="") - @patch('generic_config_updater.change_applier.ChangeApplier._get_running_config', autospec=True) + @patch('generic_config_updater.change_applier.get_config_db_as_json', autospec=True) @patch('generic_config_updater.change_applier.ConfigDBConnector', autospec=True) def test_apply_change_given_scope(self, mock_ConfigDBConnector, mock_get_running_config): # Setup mock for ConfigDBConnector mock_db = MagicMock() mock_ConfigDBConnector.return_value = mock_db - - # Setup mock for json.load to return some running configuration - mock_get_running_config.return_value = { - "tables": { - "ACL_TABLE": { - "services_to_validate": ["aclservice"], - "validate_commands": ["acl_loader show table"] - }, - "PORT": { - "services_to_validate": ["portservice"], - "validate_commands": ["show interfaces status"] - } - }, - "services": { - "aclservice": { - "validate_commands": ["acl_loader show table"] - }, - "portservice": { - "validate_commands": ["show interfaces status"] - } - } - } + mock_get_running_config.side_effect = mock_get_running_config_side_effect # Instantiate ChangeApplier with the default scope applier = generic_config_updater.change_applier.ChangeApplier(scope="asic0") @@ -219,7 +203,7 @@ def test_apply_change_given_scope(self, mock_ConfigDBConnector, mock_get_running # Assert ConfigDBConnector called with the correct scope mock_ConfigDBConnector.assert_called_once_with(use_unix_socket_path=True, namespace="asic0") - @patch('generic_config_updater.change_applier.ChangeApplier._get_running_config', autospec=True) + @patch('generic_config_updater.change_applier.get_config_db_as_json', autospec=True) @patch('generic_config_updater.change_applier.ConfigDBConnector', autospec=True) def test_apply_change_failure(self, mock_ConfigDBConnector, mock_get_running_config): # Setup mock for ConfigDBConnector @@ -241,7 +225,7 @@ def test_apply_change_failure(self, mock_ConfigDBConnector, mock_get_running_con self.assertTrue('Failed to get running config' in str(context.exception)) - @patch('generic_config_updater.change_applier.ChangeApplier._get_running_config', autospec=True) + @patch('generic_config_updater.change_applier.get_config_db_as_json', autospec=True) @patch('generic_config_updater.change_applier.ConfigDBConnector', autospec=True) def test_apply_patch_with_empty_tables_failure(self, mock_ConfigDBConnector, mock_get_running_config): # Setup mock for ConfigDBConnector @@ -249,14 +233,17 @@ def test_apply_patch_with_empty_tables_failure(self, mock_ConfigDBConnector, moc mock_ConfigDBConnector.return_value = mock_db # Setup mock for json.load to simulate configuration where crucial tables are unexpectedly empty - mock_get_running_config.return_value = { - "tables": { - # Simulate empty tables or missing crucial configuration - }, - "services": { - # Normally, services would be listed here + def mock_get_empty_running_config_side_effect(): + return { + "tables": { + # Simulate empty tables or missing crucial configuration + }, + "services": { + # Normally, services would be listed here + } } - } + + mock_get_running_config.side_effect = mock_get_empty_running_config_side_effect # Instantiate ChangeApplier with a specific scope to simulate applying changes in a multi-asic environment applier = generic_config_updater.change_applier.ChangeApplier(scope="asic0") From 7d013dff07d6b8a32f0f9d635822956d25d1a821 Mon Sep 17 00:00:00 2001 From: Xincun Li <147451452+xincunli-sonic@users.noreply.github.com> Date: Thu, 7 Nov 2024 16:07:47 -0800 Subject: [PATCH 21/22] Fix slash in path. (#3573) #### What I did Addressing issue [#20377](https://github.com/sonic-net/sonic-buildimage/issues/20377). The issue caused by unescape in JsonPointer implementation which followed [RFC 6901](https://www.rfc-editor.org/rfc/rfc6901) ```python pointer = jsonpointer.JsonPointer(path) ... class JsonPointer(object): """A JSON Pointer that can reference parts of a JSON document""" # Array indices must not contain: # leading zeros, signs, spaces, decimals, etc _RE_ARRAY_INDEX = re.compile('0|[1-9][0-9]*$') _RE_INVALID_ESCAPE = re.compile('(~[^01]|~$)') def __init__(self, pointer): # validate escapes invalid_escape = self._RE_INVALID_ESCAPE.search(pointer) if invalid_escape: raise JsonPointerException('Found invalid escape {}'.format( invalid_escape.group())) parts = pointer.split('/') if parts.pop(0) != '': raise JsonPointerException('Location must start with /') parts = [unescape(part) for part in parts] self.parts = parts ``` #### How I did it Re-escape `/` to `~1` to match the real key in json, and [JsonPatch](https://www.rfc-editor.org/rfc/rfc6902#appendix-A.14) will handle it correctly. #### How to verify it ```shell admin@str2-7250-lc1-2:~$ cat link.json [ { "op": "add", "path": "/asic1/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131", "value": {} } ] admin@str2-7250-lc1-2:~$ sudo config apply-patch link.json sonic_yang(6):Note: Below table(s) have no YANG models: DHCP_SERVER sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER Patch Applier: asic1: Patch application starting. Patch Applier: asic1: Patch: [{"op": "add", "path": "/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131", "value": {}}] Patch Applier: asic1 getting current config db. Patch Applier: asic1: simulating the target full config after applying the patch. Patch Applier: asic1: validating all JsonPatch operations are permitted on the specified fields Patch Applier: asic1: validating target config does not have empty tables, since they do not show up in ConfigDb. Patch Applier: asic1: sorting patch updates. Patch Applier: The asic1 patch was converted into 0 changes. Patch Applier: asic1: applying 0 changes in order. Patch Applier: asic1: verifying patch updates are reflected on ConfigDB. Patch Applier: asic1 patch application completed. Patch applied successfully. ``` --- generic_config_updater/generic_updater.py | 6 ++++-- .../multiasic_change_applier_test.py | 20 +++++++++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/generic_config_updater/generic_updater.py b/generic_config_updater/generic_updater.py index 8ce27455bb..e8bb021808 100644 --- a/generic_config_updater/generic_updater.py +++ b/generic_config_updater/generic_updater.py @@ -17,9 +17,11 @@ def extract_scope(path): if not path: - raise Exception("Wrong patch with empty path.") + raise GenericConfigUpdaterError("Wrong patch with empty path.") pointer = jsonpointer.JsonPointer(path) - parts = pointer.parts + + # Re-escapes + parts = [jsonpointer.escape(part) for part in pointer.parts] if not parts: raise GenericConfigUpdaterError("Wrong patch with empty path.") if parts[0].startswith("asic"): diff --git a/tests/generic_config_updater/multiasic_change_applier_test.py b/tests/generic_config_updater/multiasic_change_applier_test.py index afa1b449f3..743969737d 100644 --- a/tests/generic_config_updater/multiasic_change_applier_test.py +++ b/tests/generic_config_updater/multiasic_change_applier_test.py @@ -1,7 +1,9 @@ +import jsonpointer import unittest from importlib import reload from unittest.mock import patch, MagicMock from generic_config_updater.generic_updater import extract_scope +from generic_config_updater.generic_updater import GenericConfigUpdaterError import generic_config_updater.change_applier import generic_config_updater.services_validator import generic_config_updater.gu_common @@ -49,6 +51,12 @@ def test_extract_scope_multiasic(self, mock_is_multi_asic): "/asic0123456789/PORTCHANNEL/PortChannel102/admin_status": ( True, "asic0123456789", "/PORTCHANNEL/PortChannel102/admin_status" ), + "/asic1/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6/31": ( + True, "asic1", "/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6/31" + ), + "/asic1/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131": ( + True, "asic1", "/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131" + ), "/localhost/BGP_DEVICE_GLOBAL/STATE/tsa_enabled": ( True, "localhost", "/BGP_DEVICE_GLOBAL/STATE/tsa_enabled" ), @@ -95,7 +103,11 @@ def test_extract_scope_multiasic(self, mock_is_multi_asic): scope, remainder = extract_scope(test_path) assert(scope == expectedscope) assert(remainder == expectedremainder) - except Exception: + except AssertionError: + assert(not result) + except GenericConfigUpdaterError: + assert(not result) + except jsonpointer.JsonPointerException: assert(not result) @patch('sonic_py_common.multi_asic.is_multi_asic') @@ -158,7 +170,11 @@ def test_extract_scope_singleasic(self, mock_is_multi_asic): scope, remainder = extract_scope(test_path) assert(scope == expectedscope) assert(remainder == expectedremainder) - except Exception: + except AssertionError: + assert(not result) + except GenericConfigUpdaterError: + assert(not result) + except jsonpointer.JsonPointerException: assert(not result) @patch('generic_config_updater.change_applier.get_config_db_as_json', autospec=True) From 093ed4aaa555f25d8909c6a63b4b10bb23c06724 Mon Sep 17 00:00:00 2001 From: Vadym Hlushko <62022266+vadymhlushko-mlnx@users.noreply.github.com> Date: Mon, 11 Nov 2024 18:35:45 +0100 Subject: [PATCH 22/22] [SPM] Add logic to disable the feature before stopping it and enabling it before starting (#3344) What I did Add logic to disable the feature before stopping and enabling it before starting in order to properly clean the systemd symlinks to avoid issues with delayed attribute explained in the How to verify it section. How I did it Add the systemctl disable ... after the systemctl stop... and the systemctl enable ... before the systemctl start .. for some feature. How to verify it Add repository for some featureX sonic-package-manager repository Install featureX version 1.0.0 where the delayed flag is equal to false (delayed flag means - the feature will be started right after the system boots or after the PortInitDone event) sonic-package-manager install featureX==1.0.0 -y Enable the feature in SONiC config feature state featureX enabled Install featureX version 1.0.1 where the delayed flag is equal to true sonic-package-manager install featureX==1.0.1 -y Check the manifest file to verify the delayed field value sonic-package-manager show package manifest featureX config save -y reboot Check that the featureX is delayed on the system start --- sonic_package_manager/manager.py | 2 ++ tests/sonic_package_manager/test_manager.py | 13 +++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/sonic_package_manager/manager.py b/sonic_package_manager/manager.py index a052479607..b6a3be50c3 100644 --- a/sonic_package_manager/manager.py +++ b/sonic_package_manager/manager.py @@ -1017,8 +1017,10 @@ def _get_installed_packages_except(self, package: Package) -> Dict[str, Package] def _stop_feature(self, package: Package): self._systemctl_action(package, 'stop') + self._systemctl_action(package, 'disable') def _start_feature(self, package: Package): + self._systemctl_action(package, 'enable') self._systemctl_action(package, 'start') def _systemctl_action(self, package: Package, action: str): diff --git a/tests/sonic_package_manager/test_manager.py b/tests/sonic_package_manager/test_manager.py index a3a311ebb2..26e838ce6d 100644 --- a/tests/sonic_package_manager/test_manager.py +++ b/tests/sonic_package_manager/test_manager.py @@ -324,7 +324,7 @@ def test_manager_installation_version_range(package_manager): package_manager.install(f'test-package>=1.6.0') -def test_manager_upgrade(package_manager, sonic_fs): +def test_manager_upgrade(package_manager, sonic_fs, mock_run_command): package_manager.install('test-package-6=1.5.0') package = package_manager.get_installed_package('test-package-6') @@ -333,6 +333,15 @@ def test_manager_upgrade(package_manager, sonic_fs): assert upgraded_package.entry.version == Version.parse('2.0.0') assert upgraded_package.entry.default_reference == package.entry.default_reference + mock_run_command.assert_has_calls( + [ + call(['systemctl', 'stop', 'test-package-6']), + call(['systemctl', 'disable', 'test-package-6']), + call(['systemctl', 'enable', 'test-package-6']), + call(['systemctl', 'start', 'test-package-6']), + ] + ) + def test_manager_package_reset(package_manager, sonic_fs): package_manager.install('test-package-6=1.5.0') @@ -370,7 +379,7 @@ def __init__(self, dockerd_sock): class Image: def __init__(self, image_id): self.image_id = image_id - + def save(self, named): return ["named: {}".format(named).encode()]