Issue 20758: Changes to run telemetry/test_events.py on v6 topos by anders-nexthop · Pull Request #21592 · sonic-net/sonic-mgmt

anders-nexthop · 2025-12-06T01:02:56Z

Description of PR

Summary:

telemetry/test_events.py needs to be enhanced to run on ipv6 topos. is_mgmt_ipv6_only was added here, which can be used to check for ipv6-only topos. This can be used in this test to pass the correct ip address into the gnxi tool, which has already been enhanced to handle ipv6 addresses.

Changes were required to is_bgp_state_idle() in tests/common/devices/sonic.py to check for active neighbors in both address families.

The conditional skip has been removed.

Closes #20758

Type of change

Back port request

Approach

What is the motivation for this PR?

Increase IPv6-only coverage.

How did you do it?

extend bgp idle check to check ipv6 routes as well as ipv4
use ipv6 mgmt addr in test_events.py when running on ipv6 topo
remove conditional skip for test_events.py on ipv6 topo

How did you verify/test it?

Run the test on a virtual device with an ipv6-only topo. It fails before this change and passes after it.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

mssonicbld · 2025-12-06T01:03:04Z

/azp run

azure-pipelines · 2025-12-06T01:03:18Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-12-06T01:04:53Z

/azp run

azure-pipelines · 2025-12-06T01:05:07Z

Azure Pipelines successfully started running 1 pipeline(s).

bingwang-ms · 2025-12-10T16:12:37Z

-        return idle_count == (expected_idle_count - bgp_monitor_count)
+            if idle_count != (expected_idle_count - bgp_monitor_count):
+                return False
+        return True


Can you please confirm if this check is still working when we only have IPv4/IPv6 neighbors?
@yanmo96 Do we have IPv4 bgp neighbors in loopback IPv6-only testbed?

Hi @bingwang-ms in ipv6-only testbed, we do not have ipv4 bgp neighbors

wangxinbot · 2026-03-27T09:13:05Z

Thanks for working on this! A few things to address before this can be merged:

1. is_bgp_state_idle() change in sonic.py — already fixed on master, please drop

Master already handles both address families by concatenating the summaries:
python bgp_summary_v4 = self.command("show ip bgp summary")["stdout_lines"] bgp_summary_v6 = self.command("show ipv6 bgp summary")["stdout_lines"] bgp_summary = bgp_summary_v4 + bgp_summary_v6
This will conflict. Please rebase and drop this change.

2. generate_client_cli() — use duthost.get_mgmt_ip() instead of dut_basic_facts()

There's an existing helper duthost.get_mgmt_ip() in tests/common/devices/sonic.py that returns {"mgmt_ip": ..., "version": "v4"/"v6"}. It's already used in tests/common/fixtures/tacacs.py. Please use that instead of calling dut_basic_facts() directly:
python mgmt_info = duthost.get_mgmt_ip() mgmt_ip = mgmt_info["mgmt_ip"]

3. Also remove the xfail entry in tests_mark_conditions.yaml

Since this PR was created, an xfail entry was also added for this test on IPv6 topologies. After your fix, both the skip and xfail entries should be removed:
yaml telemetry/test_events.py::test_events: xfail: reason: "xfail for IPv6-only topologies, issue ..." conditions: - "... and '-v6-' in topo_name"

Please rebase onto latest master and update accordingly.

yxieca

AI agent on behalf of Ying. Reviewed; no issues found.

mssonicbld · 2026-04-27T22:36:29Z

/azp run

azure-pipelines · 2026-04-27T22:36:42Z

Azure Pipelines successfully started running 1 pipeline(s).

anders-nexthop · 2026-04-28T18:35:21Z

Thanks for the review! All three items addressed in the rebase. The branch now contains a single commit on top of latest master.

1. Dropped is_bgp_state_idle() change in sonic.py

Master already has the equivalent v4+v6 fix. Removed entirely.

2. generate_client_cli() now uses duthost.get_mgmt_ip()

Replaced the dut_basic_facts() + mgmt_ipv6 logic with the existing helper:

mgmt_ip = duthost.get_mgmt_ip()["mgmt_ip"]
cmd = cmdFormat.format(mgmt_ip, env.gnmi_port, ...)

Same pattern that's already used in tests/common/fixtures/tacacs.py.

3. Removed both xfail and skip entries

The xfail block (added after this PR was opened) and the original skip block have both been removed from tests_mark_conditions.yaml for telemetry/test_events.py::test_events. Other "dut ipv6 mgmt ip not supported" entries are intentionally left alone in this PR; will follow up separately to verify and remove those as a batch.

Verification (run on an IPv6 VTB with the 202505 SONiC image):

test_events_cache_overflow[vlab-01] — PASSED
test_events[vlab-01]::host_events sub-test — PASSED, exercising listen_for_events → generate_client_cli with our get_mgmt_ip() change. The constructed gnmi command (py_gnmicli.py -g -t <mgmt_ip> -p 50052 ...) received responses from the DUT successfully, confirming the fix works end-to-end.
test_events[vlab-01]::dhcp-relay_events sub-test — failed with Unable to find vlan for test. This is a pre-existing issue with the v6 skip introduced by Fix telemetry/test_events.py for v6 topos #21649 and is not affected by this PR.

— anders-bot (AI-assisted, on behalf of @anders-nexthop)

anders-nexthop · 2026-04-28T18:46:42Z

Thanks for the review! All three items addressed in the rebase. The branch now contains a single commit on top of latest master.

1. Dropped is_bgp_state_idle() change in sonic.py

Master already has the equivalent v4+v6 fix. Removed entirely.

2. generate_client_cli() now uses duthost.get_mgmt_ip()

Replaced the dut_basic_facts() + mgmt_ipv6 logic with the existing helper:
mgmt_ip = duthost.get_mgmt_ip()["mgmt_ip"]
cmd = cmdFormat.format(mgmt_ip, env.gnmi_port, ...)
Same pattern that's already used in tests/common/fixtures/tacacs.py.

3. Removed both xfail and skip entries

The xfail block (added after this PR was opened) and the original skip block have both been removed from tests_mark_conditions.yaml for telemetry/test_events.py::test_events. Other "dut ipv6 mgmt ip not supported" entries are intentionally left alone in this PR; will follow up separately to verify and remove those as a batch.

Verification (run on an IPv6 VTB with the 202505 SONiC image):

test_events_cache_overflow[vlab-01] — PASSED

test_events[vlab-01]::host_events sub-test — PASSED, exercising listen_for_events → generate_client_cli with our get_mgmt_ip() change. The constructed gnmi command (py_gnmicli.py -g -t <mgmt_ip> -p 50052 ...) received responses from the DUT successfully, confirming the fix works end-to-end.

test_events[vlab-01]::dhcp-relay_events sub-test — failed with Unable to find vlan for test. This is a pre-existing issue with the v6 skip introduced by Fix telemetry/test_events.py for v6 topos #21649 and is not affected by this PR.

— anders-bot (AI-assisted, on behalf of @anders-nexthop)

@wangxinbot @yxieca please take a look at the updated PR, it needs to be re-approved after handling review comments.

* use duthost.get_mgmt_ip() in generate_client_cli() to pick the right mgmt ip (v4 or v6) for the gnxi tool * fix get_mgmt_ip() to read from the kernel via 'ip addr' rather than from SONiC's 'show ip[v6] interface', which filters out site-local v6 addresses (fec0::/10) and breaks the helper on v6-only mgmt setups; also skip link-local (fe80::/10) explicitly so the v6 regex never matches the '0/64' tail of 'eth0/64' * remove the IPv6-only conditional skip and xfail entries for test_events Signed-off-by: Anders Linn <anders@nexthop.ai>

mssonicbld · 2026-04-29T00:39:40Z

/azp run

azure-pipelines · 2026-04-29T00:39:54Z

Azure Pipelines successfully started running 1 pipeline(s).

anders-nexthop · 2026-04-29T06:57:05Z

Update — pushed one more change to this PR (force-pushed the same commit). While verifying on a true is_mgmt_ipv6_only==True setup, I found that duthost.get_mgmt_ip() (the helper @yxieca asked us to use) is itself broken on v6-only mgmt, so the previous version of this PR didn't actually fix the bug it claims to fix.

What I did

Modified tests/common/devices/sonic.py::get_mgmt_ip() to read the management interface IPs from the kernel via ip -[4|6] -o addr show eth0 instead of SONiC's show ip[v6] interface. The v6 lookup explicitly skips scope link so we never look at fe80:: addresses. This matches the source already used by ansible/library/dut_basic_facts.py to compute is_mgmt_ipv6_only, so the two stay consistent.

Why it's needed

Two pre-existing issues stack up on a v6-only mgmt DUT:

show ipv6 interface filters out site-local addresses (fec0::/10). That's where is_mgmt_ipv6_only==True test setups put the mgmt IP. The SONiC display layer returns only the link-local line:
```
eth0    fe80::5054:ff:feb1:6b01%eth0/64    up/up    N/A    N/A
```
The v6 regex ([a-fA-F0-9:]+)/\d+ is too loose. Against the link-local line above, the proper v6 address part (fe80::5054:ff:feb1:6b01) doesn't satisfy the regex because the next char is %, not /. The regex then scans forward and matches 0/64 from the literal eth0/64 at the end. match.group(1) returns "0".

End result: get_mgmt_ip() confidently returns {"mgmt_ip": "0", "version": "v6"} on a v6-only mgmt DUT, and the gnxi command becomes py_gnmicli.py -t 0 → ConnectionRefusedError.

The fix has two halves: switching from show ipv6 interface to ip -6 addr show (so we can actually see site-local addresses), and filtering out scope link (so we never return the link-local fe80:: as mgmt). Tightening the regex alone wouldn't have helped, because the SONiC display doesn't expose the address we need.

How I found it

After the rebase, I set up a true v6-only mgmt VTB:

t0-small-v6 topology with the 202505 SONiC image
sonic-mgmt-vm container restarted with --network=host so it could reach fec0::/64
Inventory ansible_host switched to the v6 address (fec0::ffff:afa:1)
v4 removed from DUT eth0 (sudo ip -4 addr del 10.250.0.101/24 dev eth0)

In that environment is_mgmt_ipv6_only was correctly evaluated as True, but the host_events sub-test of test_events failed with the gnxi command using -t 0 instead of the v6 address. Tracing through the helper turned up the two issues above.

Verification (with the fix)

On the same v6-only mgmt VTB:

py_gnmicli.py -g -t fec0::ffff:afa:1 -p 50052 -m subscribe ...

test_events_cache_overflow[vlab-01] — PASSED
test_events[vlab-01]::host_events — PASSED
test_events[vlab-01]::dhcp-relay_events — still fails with Unable to find vlan for test, the pre-existing Fix telemetry/test_events.py for v6 topos #21649 issue, unaffected by this PR

— anders-bot (AI-assisted, on behalf of @anders-nexthop)

Verified on a true is_mgmt_ipv6_only==True VTB (t0-small-v6 with eth0 v4 removed) that these tests pass after the get_mgmt_ip() and generate_client_cli() fixes from sonic-net#21592 land. Drop their "dut ipv6 mgmt ip not supported" skip entries. Telemetry (10 tests): - test_telemetry.py::test_osbuild_version - test_telemetry.py::test_sysuptime - test_telemetry.py::test_telemetry_ouput - test_telemetry.py::test_virtualdb_table_streaming - test_telemetry_cert_rotation.py::test_telemetry_cert_rotate - test_telemetry_cert_rotation.py::test_telemetry_post_cert_add - test_telemetry_cert_rotation.py::test_telemetry_post_cert_del - test_telemetry_poll.py::test_poll_mode_delete - test_telemetry_poll.py::test_poll_mode_no_table_or_key - test_telemetry_poll.py::test_poll_mode_present_table_delayed_key gNMI (13 tests): - test_gnmi_appldb.py::test_gnmi_appldb_01 - test_gnmi_configdb.py::test_gnmi_configdb_full_replace_01 - test_gnmi_configdb.py::test_gnmi_configdb_get_authenticate - test_gnmi_configdb.py::test_gnmi_configdb_incremental_01 - test_gnmi_configdb.py::test_gnmi_configdb_incremental_02 - test_gnmi_configdb.py::test_gnmi_configdb_streaming_onchange_01 - test_gnmi_configdb.py::test_gnmi_configdb_streaming_onchange_02 - test_gnmi_configdb.py::test_gnmi_configdb_streaming_sample_01 - test_gnmi_configdb.py::test_gnmi_configdb_subscribe_authenticate - test_gnmi_countersdb.py::test_gnmi_counterdb_streaming_sample_01 - test_gnmi_countersdb.py::test_gnmi_counterdb_streaming_sample_02 - test_gnmi_countersdb.py::test_gnmi_output - test_gnmi_countersdb.py::test_gnmi_queue_buffer_cnt zmq (1 test): - test_gnmi_zmq.py::test_gnmi_zmq Skips intentionally retained: - gnmi/test_gnmi.py: file-level skip; multiple tests fail with hard-coded 10.250.0.101 in their gnmi_cli dialer logic and need their own refactor. - telemetry/test_telemetry_poll.py::test_poll_mode_default_route: fails on v6 due to a BGP default-route reconvergence issue, not mgmt IP. Signed-off-by: Anders Linn <anders@nexthop.ai>

yxieca · 2026-04-30T20:08:57Z

Review note (non-blocking):

get_mgmt_ip() now uses ip -6 -o addr show eth0 | grep -v scope link, which can return multiple v6 addresses. The regex picks the first match, which might be unexpected if both global/ULA exist. Consider filtering for scope global (or otherwise disambiguating).

Reviewed by AI agent on behalf of Ying.

anders-nexthop · 2026-05-01T17:50:19Z

Review note (non-blocking):

get_mgmt_ip() now uses ip -6 -o addr show eth0 | grep -v scope link, which can return multiple v6 addresses. The regex picks the first match, which might be unexpected if both global/ULA exist. Consider filtering for scope global (or otherwise disambiguating).

Reviewed by AI agent on behalf of Ying.

@yxieca @bingwang-ms I would like to address this issue in a follow-up commit. With these changes, the machinery is in place to remove the skips on several other telemetry tests. I'd rather get this one merged since it's been in the works for a long time, and then submit another which removes all of those skips (after I verify that they all still pass). Can we get this one approved and merged up? The skip-expiry just hit for this: #20758 (comment)

…ic-net#21592) What: Enabled telemetry/test_events.py to run on IPv6-only topologies. Why: The test was conditionally skipped on IPv6 topos (issue sonic-net#20758), reducing IPv6-only test coverage. How: Extended is_bgp_state_idle() to check both IPv4 and IPv6 address families. Used is_mgmt_ipv6_only to pass the correct IP address to gnxi tool. Removed the conditional skip for IPv6 topos. Testing: Verified on IPv6 topology. All CI checks passed. Signed-off-by: anders-nexthop <anders@nexthop.ai> Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>

…ic-net#21592) What: Enabled telemetry/test_events.py to run on IPv6-only topologies. Why: The test was conditionally skipped on IPv6 topos (issue sonic-net#20758), reducing IPv6-only test coverage. How: Extended is_bgp_state_idle() to check both IPv4 and IPv6 address families. Used is_mgmt_ipv6_only to pass the correct IP address to gnxi tool. Removed the conditional skip for IPv6 topos. Testing: Verified on IPv6 topology. All CI checks passed. Signed-off-by: anders-nexthop <anders@nexthop.ai> Signed-off-by: Raghavendran Ramanathan <rraghav@cisco.com>

…ic-net#21592) What: Enabled telemetry/test_events.py to run on IPv6-only topologies. Why: The test was conditionally skipped on IPv6 topos (issue sonic-net#20758), reducing IPv6-only test coverage. How: Extended is_bgp_state_idle() to check both IPv4 and IPv6 address families. Used is_mgmt_ipv6_only to pass the correct IP address to gnxi tool. Removed the conditional skip for IPv6 topos. Testing: Verified on IPv6 topology. All CI checks passed. Signed-off-by: anders-nexthop <anders@nexthop.ai> Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>

…ic-net#21592) What: Enabled telemetry/test_events.py to run on IPv6-only topologies. Why: The test was conditionally skipped on IPv6 topos (issue sonic-net#20758), reducing IPv6-only test coverage. How: Extended is_bgp_state_idle() to check both IPv4 and IPv6 address families. Used is_mgmt_ipv6_only to pass the correct IP address to gnxi tool. Removed the conditional skip for IPv6 topos. Testing: Verified on IPv6 topology. All CI checks passed. Signed-off-by: anders-nexthop <anders@nexthop.ai> Signed-off-by: ssithaia-ebay <ssithaian@ebay.com>

…ic-net#21592) What: Enabled telemetry/test_events.py to run on IPv6-only topologies. Why: The test was conditionally skipped on IPv6 topos (issue sonic-net#20758), reducing IPv6-only test coverage. How: Extended is_bgp_state_idle() to check both IPv4 and IPv6 address families. Used is_mgmt_ipv6_only to pass the correct IP address to gnxi tool. Removed the conditional skip for IPv6 topos. Testing: Verified on IPv6 topology. All CI checks passed. Signed-off-by: anders-nexthop <anders@nexthop.ai> Signed-off-by: Yael Tzur <ytzur@nvidia.com>

github-actions Bot requested review from BYGX-wcr, guangyao6, wangxin, wsycqyz, xwjiang-ms, yejianquan, yutongzhang-microsoft and zbud-msft December 6, 2025 01:03

anders-nexthop force-pushed the anders.20758.test_events-for-ipv6 branch from 642d80f to 32b9583 Compare December 6, 2025 01:04

anders-nexthop mentioned this pull request Dec 6, 2025

Bug:[IPv6 only topo] [test_events.py] test_events.py::test_events is not updated to IPv6 only topology #20758

Closed

yxieca reviewed Dec 10, 2025

View reviewed changes

Comment thread tests/telemetry/telemetry_utils.py Outdated

yxieca requested a review from bingwang-ms December 10, 2025 16:11

bingwang-ms reviewed Dec 10, 2025

View reviewed changes

yxieca previously approved these changes Apr 21, 2026

View reviewed changes

anders-nexthop dismissed yxieca’s stale review via dd4e07f April 27, 2026 22:36

anders-nexthop force-pushed the anders.20758.test_events-for-ipv6 branch from 32b9583 to dd4e07f Compare April 27, 2026 22:36

anders-nexthop force-pushed the anders.20758.test_events-for-ipv6 branch from dd4e07f to ac7b286 Compare April 29, 2026 00:39

yxieca approved these changes May 1, 2026

View reviewed changes

yxieca merged commit f35d9e2 into sonic-net:master May 1, 2026
23 checks passed

Uh oh!

Conversation

anders-nexthop commented Dec 6, 2025

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

mssonicbld commented Dec 6, 2025

Uh oh!

azure-pipelines Bot commented Dec 6, 2025

Uh oh!

mssonicbld commented Dec 6, 2025

Uh oh!

azure-pipelines Bot commented Dec 6, 2025

Uh oh!

Uh oh!

bingwang-ms Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

yanmo96 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

wangxinbot commented Mar 27, 2026

Uh oh!

yxieca left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Apr 27, 2026

Uh oh!

azure-pipelines Bot commented Apr 27, 2026

Uh oh!

anders-nexthop commented Apr 28, 2026

Uh oh!

anders-nexthop commented Apr 28, 2026

Uh oh!

mssonicbld commented Apr 29, 2026

Uh oh!

azure-pipelines Bot commented Apr 29, 2026

Uh oh!

anders-nexthop commented Apr 29, 2026

Uh oh!

yxieca commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anders-nexthop commented May 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

yxieca commented Apr 30, 2026 •

edited

Loading