Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance the logic to wait for all buffer tables to be removed in _clear_qos #2720

Merged
merged 5 commits into from
Mar 14, 2023
Merged

Enhance the logic to wait for all buffer tables to be removed in _clear_qos #2720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
368bc63
Enhance the logic to wait for all buffer tables to be removed in _cle…
stephenxs Mar 7, 2023
9497ca2
Extend timeout to 120 seconds
stephenxs Mar 7, 2023
229d69a
Enhance more
stephenxs Mar 8, 2023
dd5bfa0
Add verbose output to avoid keep user waiting
stephenxs Mar 8, 2023
64b8619
Enhance the UT to satisfy the coverage requirement
stephenxs Mar 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions config/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,24 +743,28 @@ def storm_control_delete_entry(port_name, storm_type):
return True


def _wait_until_clear(table, interval=0.5, timeout=30):
def _wait_until_clear(tables, interval=0.5, timeout=30, verbose=False):
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, table)
if not current_profiles:
empty = True
else:
time.sleep(interval)
non_empty_table_count = 0
for table in tables:
keys = app_db.keys(app_db.APPL_DB, table)
if keys:
non_empty_table_count += 1
if verbose:
click.echo("Some entries matching {} still exist: {}".format(table, keys[0]))
time.sleep(interval)
empty = (non_empty_table_count == 0)
if not empty:
click.echo("Operation not completed successfully, please save and reload configuration.")
return empty


def _clear_qos(delay = False):
def _clear_qos(delay=False, verbose=False):
QOS_TABLE_NAMES = [
'PORT_QOS_MAP',
'QUEUE',
Expand Down Expand Up @@ -797,7 +801,10 @@ def _clear_qos(delay = False):
for qos_table in QOS_TABLE_NAMES:
config_db.delete_table(qos_table)
if delay:
_wait_until_clear("BUFFER_POOL_TABLE:*",interval=0.5, timeout=30)
device_metadata = config_db.get_entry('DEVICE_METADATA', 'localhost')
# Traditional buffer manager do not remove buffer tables in any case, no need to wait.
timeout = 120 if device_metadata and device_metadata.get('buffer_model') == 'dynamic' else 0
_wait_until_clear(["BUFFER_*_TABLE:*", "BUFFER_*_SET"], interval=0.5, timeout=timeout, verbose=verbose)

def _get_sonic_generated_services(num_asic):
if not os.path.isfile(SONIC_GENERATED_SERVICE_PATH):
Expand Down Expand Up @@ -2618,10 +2625,11 @@ def qos(ctx):
pass

@qos.command('clear')
def clear():
@click.option('--verbose', is_flag=True, help="Enable verbose output")
def clear(verbose):
"""Clear QoS configuration"""
log.log_info("'qos clear' executing...")
_clear_qos()
_clear_qos(verbose=verbose)

def _update_buffer_calculation_model(config_db, model):
"""Update the buffer calculation model into CONFIG_DB"""
Expand All @@ -2638,6 +2646,7 @@ def _update_buffer_calculation_model(config_db, model):
@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('--verbose', is_flag=True, help="Enable verbose output")
@click.option(
'--json-data', type=click.STRING,
help="json string with additional data, valid with --dry-run option"
Expand All @@ -2646,7 +2655,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, no_delay, dry_run, json_data, ports):
def reload(ctx, no_dynamic_buffer, no_delay, dry_run, json_data, ports, verbose):
"""Reload QoS configuration"""
if ports:
log.log_info("'qos reload --ports {}' executing...".format(ports))
Expand All @@ -2655,7 +2664,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 = not no_delay)
_clear_qos(delay = not no_delay, verbose=verbose)

_, hwsku_path = device_info.get_paths_to_platform_and_hwsku_dirs()
sonic_version_file = device_info.get_sonic_version_file()
Expand Down
10 changes: 8 additions & 2 deletions tests/config_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,17 +693,23 @@ def test_qos_wait_until_clear_empty(self):

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)
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)
empty = _wait_until_clear(["BUFFER_POOL_TABLE:*"], 0.5,2)
assert not empty

@mock.patch('config.main._wait_until_clear')
def test_qos_clear_no_wait(self, _wait_until_clear):
from config.main import _clear_qos
_clear_qos(True, False)
_wait_until_clear.assert_called_with(['BUFFER_*_TABLE:*', 'BUFFER_*_SET'], interval=0.5, timeout=0, verbose=False)

def test_qos_reload_single(
self, get_cmd_module, setup_qos_mock_apis,
setup_single_broadcom_asic
Expand Down