Skip to content
Merged
Changes from all commits
Commits
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
69 changes: 52 additions & 17 deletions tests/telemetry/test_telemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@

def load_new_cfg(duthost, data):
duthost.copy(content=json.dumps(data, indent=4), dest=CFG_DB_PATH)
config_reload(duthost, config_source='config_db', safe_reload=True, check_intf_up_ports=True, wait_for_bgp=True)
config_reload(duthost, config_source='config_db', safe_reload=True, check_intf_up_ports=True,
wait_for_bgp=True, yang_validate=False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the yang_validate to False intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, after modifying BUFFER_QUEUE and doing config reload, we will skip yang validation.

# config reload overrides testing telemetry config, ensure testing config exists
setup_telemetry_forpyclient(duthost)

Expand Down Expand Up @@ -185,26 +186,60 @@ def test_telemetry_queue_buffer_cnt(duthosts, enum_rand_one_per_hwsku_hostname,
pytest.skip("Skipping test as there are none interfaces in admin'up' state with buffer queues to check")

interface_buffer_queues = [bq for bq in buffer_queues if any(val in interface_to_check for val in bq.split('|'))]
if len(interface_buffer_queues) == 0:
pytest.skip("No valid entry for any interface:queue entry")

"""If all queues for that pool are in the same pool ex Ethernet0|0-9
We will modify to separate the first queue and the remaining such
that we get a separate entry for Ethernet0|0 and Ethernet0|1-9"""
Comment on lines +192 to +194
Copy link

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This triple-quoted string is a hanging literal used as a comment. Replace it with standard # comments to avoid leaving unused string literals in the function body.

Suggested change
"""If all queues for that pool are in the same pool ex Ethernet0|0-9
We will modify to separate the first queue and the remaining such
that we get a separate entry for Ethernet0|0 and Ethernet0|1-9"""
# If all queues for that pool are in the same pool, e.g., Ethernet0|0-9,
# we will modify to separate the first queue and the remaining such
# that we get a separate entry for Ethernet0|0 and Ethernet0|1-9.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol so useless

is_single_queue = False
bq_entry = interface_buffer_queues[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a check to ensure interface_buffer_queues is not empty before accessing interface_buffer_queues[0]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


if len(interface_buffer_queues) == 1:
iface, q_range = bq_entry.split('|')
if '-' in q_range: # Grouped queues
start, end = map(int, q_range.split('-'))
if start < end:
single_queue_entry = f"{iface}|{start}"
remaining_queue_entry = f"{iface}|{start + 1}-{end}"
profile = data['BUFFER_QUEUE'][bq_entry]['profile']
data['BUFFER_QUEUE'][remaining_queue_entry] = {"profile": profile}
data['BUFFER_QUEUE'][single_queue_entry] = {"profile": profile}
del data['BUFFER_QUEUE'][bq_entry]
bq_entry = single_queue_entry
else:
pytest.skip("Invalid buffer queue range")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the logic here is pretty useful for other use cases too. Wanna split the logic out into a helper func to make it more readable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can take this effort in some future PR.

else:
# Single queue entry for port such as Ethernet0|0
is_single_queue = True

# Add create_only_config_db_buffers entry to device metadata to enable
# counters optimization and get number of queue counters of Ethernet0 prior
# to removing buffer queues
data['DEVICE_METADATA']["localhost"]["create_only_config_db_buffers"] \
= "true"
load_new_cfg(duthost, data)
pytest_assert(wait_until(120, 20, 0, check_buffer_queues_cnt_cmd_output, ptfhost, gnxi_path,
dut_ip, interface_to_check, env.gnmi_port), "gnmi server not fully restarted")
pre_del_cnt = get_buffer_queues_cnt(ptfhost, gnxi_path, dut_ip, interface_to_check, env.gnmi_port)

# Remove buffer queue and reload and get new number of queue counters
del data['BUFFER_QUEUE'][interface_buffer_queues[0]]
load_new_cfg(duthost, data)
pytest_assert(wait_until(120, 20, 0, check_buffer_queues_cnt_cmd_output, ptfhost, gnxi_path,
dut_ip, interface_to_check, env.gnmi_port), "gnmi server not fully restarted")
post_del_cnt = get_buffer_queues_cnt(ptfhost, gnxi_path, dut_ip, interface_to_check, env.gnmi_port)

pytest_assert(pre_del_cnt > post_del_cnt,
"Number of queue counters count differs from expected")
try:
data['DEVICE_METADATA']["localhost"]["create_only_config_db_buffers"] \
= "true"
load_new_cfg(duthost, data)
pytest_assert(wait_until(120, 20, 0, check_buffer_queues_cnt_cmd_output, ptfhost, gnxi_path,
dut_ip, interface_to_check, env.gnmi_port),
"Unable to get count of buffer queues from COUNTERS_QUEUE_NAME_MAP")
pre_del_cnt = get_buffer_queues_cnt(ptfhost, gnxi_path, dut_ip, interface_to_check, env.gnmi_port)

# Remove buffer queue and reload and get new number of queue counters
del data['BUFFER_QUEUE'][bq_entry]
load_new_cfg(duthost, data)
if not is_single_queue:
pytest_assert(wait_until(120, 20, 0, check_buffer_queues_cnt_cmd_output, ptfhost, gnxi_path,
dut_ip, interface_to_check, env.gnmi_port),
"Unable to get count of buffer queues from COUNTERS_QUEUE_NAME_MAP")
post_del_cnt = get_buffer_queues_cnt(ptfhost, gnxi_path, dut_ip, interface_to_check, env.gnmi_port)

pytest_assert(pre_del_cnt > post_del_cnt,
"Number of queue counters count differs from expected")
finally:
data = json.loads(duthost.shell("cat {}".format(ORIG_CFG_DB),
verbose=False)['stdout'])
load_new_cfg(duthost, data)


@pytest.mark.parametrize('setup_streaming_telemetry', [False], indirect=True)
Expand Down
Loading