From 0985385f88e64717ee864d5b8de2f7e5d79017fe Mon Sep 17 00:00:00 2001 From: "david.zagury" Date: Thu, 17 Nov 2022 12:17:46 +0200 Subject: [PATCH 1/3] Introduce delay to the qos reload flow Fix issue with qos reload with dry run clears qos configurations --- config/main.py | 28 ++++++++++++++++++++++++---- tests/config_test.py | 2 +- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/config/main.py b/config/main.py index 9bb403284d..558e7e5490 100644 --- a/config/main.py +++ b/config/main.py @@ -728,7 +728,23 @@ def storm_control_delete_entry(port_name, storm_type): return True -def _clear_qos(): +def wait_until_clear(interval=0.5, timeout=30): + start = time.time() + empty = False + app_db = SonicV2Connector(host='127.0.0.1') + app_db.connect(app_db.APPL_DB) + + while not empty and time.time() - start < timeout: + current_profiles = app_db.keys(app_db.APPL_DB, "BUFFER_POOL_TABLE:*") + if not current_profiles: + empty = True + else: + time.sleep(interval) + if not empty: + click.echo("Operation not completed successfully, please save and reload configuration.") + + +def _clear_qos(delay = False): QOS_TABLE_NAMES = [ 'PORT_QOS_MAP', 'QUEUE', @@ -764,6 +780,8 @@ def _clear_qos(): config_db.connect() for qos_table in QOS_TABLE_NAMES: config_db.delete_table(qos_table) + if delay: + wait_until_clear(interval=0.5, timeout=30) def _get_sonic_generated_services(num_asic): if not os.path.isfile(SONIC_GENERATED_SERVICE_PATH): @@ -1775,7 +1793,7 @@ def load_minigraph(db, no_service_restart, traffic_shift_away): click.secho("Failed to load port_config.json, Error: {}".format(str(e)), fg='magenta') # generate QoS and Buffer configs - clicommon.run_command("config qos reload --no-dynamic-buffer", display_cmd=True) + clicommon.run_command("config qos reload --no-dynamic-buffer --no-delay", display_cmd=True) if device_type != 'MgmtToRRouter' and device_type != 'MgmtTsToR' and device_type != 'BmcMgmtToRRouter' and device_type != 'EPMS': clicommon.run_command("pfcwd start_default", display_cmd=True) @@ -2583,6 +2601,7 @@ def _update_buffer_calculation_model(config_db, model): @click.pass_context @click.option('--ports', is_flag=False, required=False, help="List of ports that needs to be updated") @click.option('--no-dynamic-buffer', is_flag=True, help="Disable dynamic buffer calculation") +@click.option('--no-delay', is_flag=True, hidden=True) @click.option( '--json-data', type=click.STRING, help="json string with additional data, valid with --dry-run option" @@ -2591,7 +2610,7 @@ def _update_buffer_calculation_model(config_db, model): '--dry_run', type=click.STRING, help="Dry run, writes config to the given file" ) -def reload(ctx, no_dynamic_buffer, dry_run, json_data, ports): +def reload(ctx, no_dynamic_buffer, no_delay, dry_run, json_data, ports): """Reload QoS configuration""" if ports: log.log_info("'qos reload --ports {}' executing...".format(ports)) @@ -2599,7 +2618,8 @@ def reload(ctx, no_dynamic_buffer, dry_run, json_data, ports): return log.log_info("'qos reload' executing...") - _clear_qos() + if not dry_run: + _clear_qos(delay = no_delay) _, hwsku_path = device_info.get_paths_to_platform_and_hwsku_dirs() sonic_version_file = device_info.get_sonic_version_file() diff --git a/tests/config_test.py b/tests/config_test.py index a9f4982548..cb1456d1c8 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -36,7 +36,7 @@ load_minigraph_command_output="""\ Stopping SONiC target ... Running command: /usr/local/bin/sonic-cfggen -H -m --write-to-db -Running command: config qos reload --no-dynamic-buffer +Running command: config qos reload --no-dynamic-buffer --no-delay Running command: pfcwd start_default Restarting SONiC target ... Reloading Monit configuration ... From 6f3a879ebfbb7ecc1bda80d83158206a0a799f89 Mon Sep 17 00:00:00 2001 From: "david.zagury" Date: Tue, 22 Nov 2022 14:44:00 +0200 Subject: [PATCH 2/3] Add unit test for _wait_until_clear --- config/main.py | 7 ++++--- tests/config_test.py | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/config/main.py b/config/main.py index 558e7e5490..c278409632 100644 --- a/config/main.py +++ b/config/main.py @@ -728,20 +728,21 @@ def storm_control_delete_entry(port_name, storm_type): return True -def wait_until_clear(interval=0.5, timeout=30): +def _wait_until_clear(table, interval=0.5, timeout=30): start = time.time() empty = False app_db = SonicV2Connector(host='127.0.0.1') app_db.connect(app_db.APPL_DB) while not empty and time.time() - start < timeout: - current_profiles = app_db.keys(app_db.APPL_DB, "BUFFER_POOL_TABLE:*") + current_profiles = app_db.keys(app_db.APPL_DB, table) if not current_profiles: empty = True else: time.sleep(interval) if not empty: click.echo("Operation not completed successfully, please save and reload configuration.") + return empty def _clear_qos(delay = False): @@ -781,7 +782,7 @@ def _clear_qos(delay = False): for qos_table in QOS_TABLE_NAMES: config_db.delete_table(qos_table) if delay: - wait_until_clear(interval=0.5, timeout=30) + _wait_until_clear("BUFFER_POOL_TABLE:*",interval=0.5, timeout=30) def _get_sonic_generated_services(num_asic): if not os.path.isfile(SONIC_GENERATED_SERVICE_PATH): diff --git a/tests/config_test.py b/tests/config_test.py index cb1456d1c8..9054dcd529 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -668,6 +668,28 @@ def setup_class(cls): import config.main importlib.reload(config.main) + def _keys(args, kwargs): + if not TestConfigQos._keys_counter: + return [] + TestConfigQos._keys_counter-=1 + return ["BUFFER_POOL_TABLE:egress_lossy_pool"] + + def test_qos_wait_until_clear_empty(self): + from config.main import _wait_until_clear + + with mock.patch('swsscommon.swsscommon.SonicV2Connector.keys', side_effect=TestConfigQos._keys): + TestConfigQos._keys_counter = 1 + empty = _wait_until_clear("BUFFER_POOL_TABLE:*", 0.5,2) + assert empty + + def test_qos_wait_until_clear_not_empty(self): + from config.main import _wait_until_clear + + with mock.patch('swsscommon.swsscommon.SonicV2Connector.keys', side_effect=TestConfigQos._keys): + TestConfigQos._keys_counter = 10 + empty = _wait_until_clear("BUFFER_POOL_TABLE:*", 0.5,2) + assert not empty + def test_qos_reload_single( self, get_cmd_module, setup_qos_mock_apis, setup_single_broadcom_asic From daa72d5206a4e2b977458604365def7a75157c9d Mon Sep 17 00:00:00 2001 From: "david.zagury" Date: Wed, 30 Nov 2022 10:16:55 +0200 Subject: [PATCH 3/3] Fix delay --- config/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/main.py b/config/main.py index c278409632..d90922b105 100644 --- a/config/main.py +++ b/config/main.py @@ -2620,7 +2620,7 @@ def reload(ctx, no_dynamic_buffer, no_delay, dry_run, json_data, ports): log.log_info("'qos reload' executing...") if not dry_run: - _clear_qos(delay = no_delay) + _clear_qos(delay = not no_delay) _, hwsku_path = device_info.get_paths_to_platform_and_hwsku_dirs() sonic_version_file = device_info.get_sonic_version_file()