feat: add fixture for disabling route check by cyw233 · Pull Request #16876 · sonic-net/sonic-mgmt

cyw233 · 2025-02-10T10:08:16Z

Description of PR

Add a module-level fixture for temporarily disabling route check for a test module

Summary:
Fixes # (issue) Microsoft ADO 31326413

Type of change

Back port request

Approach

What is the motivation for this PR?

In our recent Cisco T2 Nightly run, we observed that we would get the following error syslog during some test modules:

E               Failed: Processes "['analyze_logs--<MultiAsicSonicHost dut-lc1-1>']" failed with exit code "1"
E               Exception:
E               match: 1
E               expected_match: 0
E               expected_missing_match: 0
E               
E               Match Messages:
E               2025 Feb  3 03:03:29.550827 svcstr2-8800-lc1-1 ERR monit[914]: 'routeCheck' status failed (255) -- Failure results: {{#012    "asic1": {#012        "Unaccounted_ROUTE_ENTRY_TABLE_entries": [#012            "100.1.0.22/32",#012

After discussion, we decided to add a fixture so users can disable route check for a test module if they think that test tends to have such error syslog.

How did you do it?

How did you verify/test it?

I ran the updated code and can confirm it's working well.

Any platform specific information?

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

Documentation

mssonicbld · 2025-02-10T10:08:20Z

/azp run

azure-pipelines · 2025-02-10T10:08:34Z

Azure Pipelines successfully started running 1 pipeline(s).

tests/conftest.py

yejianquan · 2025-02-10T23:33:40Z

tests/conftest.py

+    yield
+
+    if check_flag:
+        logging.info("!!!!route check teardown!!!!!!")


Is it a debug log?

😸 yep, let me delete it

tests/conftest.py

yejianquan · 2025-02-10T23:39:46Z

tests/conftest.py

+
+    if check_flag:
+        logging.info("!!!!route check teardown!!!!!!")
+        with SafeThreadPoolExecutor(max_workers=8) as executor:


Should we do another route check after the test to make sure route is healthy after test module?

Good idea, let me add it. If so, we need to probably use try...except...finally to always sudo monit start routeCheck in the fixture teardown stage

mssonicbld · 2025-02-11T04:52:38Z

/azp run

azure-pipelines · 2025-02-11T04:52:50Z

Azure Pipelines successfully started running 1 pipeline(s).

abdosi · 2025-02-11T19:33:33Z

don't see below change from my original PR. This will be good to do before stop/start of route check monit

 rc = duthost.shell("sudo route_check.py", module_ignore_errors=True)
        if rc['rc'] != 0:
            pytest.fail("route_check fail in test pre-setup stage")

cyw233 · 2025-02-11T22:19:26Z

don't see below change from my original PR. This will be good to do before stop/start of route check monit
 rc = duthost.shell("sudo route_check.py", module_ignore_errors=True)
        if rc['rc'] != 0:
            pytest.fail("route_check fail in test pre-setup stage")

Hey @abdosi, the above code is already included in the run_route_check() function, and we use python multhreading to run it on all frontend nodes before stop and start of route check monit. Thanks

with SafeThreadPoolExecutor(max_workers=8) as executor:
    for duthost in duthosts.frontend_nodes:
        executor.submit(run_route_check, duthost)

mssonicbld · 2025-02-11T22:32:32Z

/azp run

azure-pipelines · 2025-02-11T22:32:43Z

Azure Pipelines successfully started running 1 pipeline(s).

yejianquan

LGTM

Description of PR Add a module-level fixture for temporarily disabling route check for a test module Summary: Fixes # (issue) Microsoft ADO 31326413 Approach What is the motivation for this PR? In our recent Cisco T2 Nightly run, we observed that we would get the following error syslog during some test modules: E Failed: Processes "['analyze_logs--<MultiAsicSonicHost dut-lc1-1>']" failed with exit code "1" E Exception: E match: 1 E expected_match: 0 E expected_missing_match: 0 E E Match Messages: E 2025 Feb 3 03:03:29.550827 svcstr2-8800-lc1-1 ERR monit[914]: 'routeCheck' status failed (255) -- Failure results: {{sonic-net#12 "asic1": {sonic-net#12 "Unaccounted_ROUTE_ENTRY_TABLE_entries": [sonic-net#12 "100.1.0.22/32",sonic-net#12 After discussion, we decided to add a fixture so users can disable route check for a test module if they think that test tends to have such error syslog. How did you do it? How did you verify/test it? I ran the updated code and can confirm it's working well. co-authorized by: [email protected]

mssonicbld · 2025-02-12T06:12:04Z

Cherry-pick PR to 202411: #16924

mssonicbld · 2025-02-12T06:12:06Z

Cherry-pick PR to msft-202405: Azure/sonic-mgmt.msft#75

Description of PR Add a module-level fixture for temporarily disabling route check for a test module Summary: Fixes # (issue) Microsoft ADO 31326413 Approach What is the motivation for this PR? In our recent Cisco T2 Nightly run, we observed that we would get the following error syslog during some test modules: E Failed: Processes "['analyze_logs--<MultiAsicSonicHost dut-lc1-1>']" failed with exit code "1" E Exception: E match: 1 E expected_match: 0 E expected_missing_match: 0 E E Match Messages: E 2025 Feb 3 03:03:29.550827 svcstr2-8800-lc1-1 ERR monit[914]: 'routeCheck' status failed (255) -- Failure results: {{#12 "asic1": {#12 "Unaccounted_ROUTE_ENTRY_TABLE_entries": [#12 "100.1.0.22/32",#12 After discussion, we decided to add a fixture so users can disable route check for a test module if they think that test tends to have such error syslog. How did you do it? How did you verify/test it? I ran the updated code and can confirm it's working well. co-authorized by: [email protected]

Description of PR Add a module-level fixture for temporarily disabling route check for a test module Summary: Fixes # (issue) Microsoft ADO 31326413 Approach What is the motivation for this PR? In our recent Cisco T2 Nightly run, we observed that we would get the following error syslog during some test modules: E Failed: Processes "['analyze_logs--<MultiAsicSonicHost dut-lc1-1>']" failed with exit code "1" E Exception: E match: 1 E expected_match: 0 E expected_missing_match: 0 E E Match Messages: E 2025 Feb 3 03:03:29.550827 svcstr2-8800-lc1-1 ERR monit[914]: 'routeCheck' status failed (255) -- Failure results: {{sonic-net#12 "asic1": {sonic-net#12 "Unaccounted_ROUTE_ENTRY_TABLE_entries": [sonic-net#12 "100.1.0.22/32",sonic-net#12 After discussion, we decided to add a fixture so users can disable route check for a test module if they think that test tends to have such error syslog. How did you do it? How did you verify/test it? I ran the updated code and can confirm it's working well. co-authorized by: [email protected]

ZhaohuiS · 2025-12-05T03:16:43Z

@cyw233 this route check failures only happen on T2 chassis, right? But the test module is for all platforms, as I notice, route check didn't fail on T0/T1 platforms, is it probably to hide potential risk for other platforms?
Can we just enable this fixture only for impacted T2 platform?
Same concern for #21320.

…1627) What is the motivation for this PR? The current disable-and-enable routeCheck monitor logic is causing test flakiness on some non-T2 platforms (see #16876 (comment)). Certain platforms require additional time to restart the routeCheck monitor, which can leave it inactive when the next test begins and result in false failures. We would like to address this issue urgently in this PR. In a follow-up PR, I will properly enhance the temporarily_disable_route_check fixture so that: Users can choose which topologies apply the disable-and-enable routeCheck behavior The fixture uses a wait_until() timeout to verify the routeCheck status is as expected before proceeding to the next step How did you do it? How did you verify/test it? I ran the updated login on a non-T2 platform (Mx) and can confirm it's working well: https://elastictest.org/scheduler/testplan/693272f7392767e9bf67e930 image I also verified the logic on T2 platform and can confirm it's still having this logic: https://elastictest.org/scheduler/testplan/6932767fbcc3fac23371a83c image Any platform specific information?

Change the temporarily_disable_route_check fixture logic to only apply to T2 topology for now. What is the motivation for this PR? The current disable-and-enable routeCheck monitor logic is causing test flakiness on some non-T2 platforms (see #16876 (comment)). Certain platforms require additional time to restart the routeCheck monitor, which can leave it inactive when the next test begins and result in false failures. We would like to address this issue urgently in this PR. In a follow-up PR, I will properly enhance the temporarily_disable_route_check fixture so that: Users can choose which topologies apply the disable-and-enable routeCheck behavior The fixture uses a wait_until() timeout to verify the routeCheck status is as expected before proceeding to the next step How did you do it? How did you verify/test it? I ran the updated login on a non-T2 platform (Mx) and can confirm it's working well: https://elastictest.org/scheduler/testplan/693272f7392767e9bf67e930 image I also verified the logic on T2 platform and can confirm it's still having this logic: https://elastictest.org/scheduler/testplan/6932767fbcc3fac23371a83c image Signed-off-by: Chenyang Wang <[email protected]> Co-authored-by: Chenyang Wang <[email protected]>

### Description of PR  Change the `temporarily_disable_route_check` fixture logic to only apply to T2 topology for now. Summary: Fixes # (issue) Microsoft ADO 36101536 ### Type of change  - [x] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] New Test case - [ ] Skipped for non-supported platforms - [ ] Test case improvement ### Back port request - [ ] 202205 - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [x] 202505 - [x] 202511 ### Approach #### What is the motivation for this PR? The current disable-and-enable routeCheck monitor logic is causing test flakiness on some non-T2 platforms (see sonic-net/sonic-mgmt#16876 (comment)). Certain platforms require additional time to restart the routeCheck monitor, which can leave it inactive when the next test begins and result in false failures. We would like to address this issue urgently in this PR. In a follow-up PR, I will properly enhance the `temporarily_disable_route_check` fixture so that: - Users can choose which topologies apply the disable-and-enable routeCheck behavior - The fixture uses a `wait_until()` timeout to verify the routeCheck status is as expected before proceeding to the next step #### How did you do it? #### How did you verify/test it? I ran the updated login on a non-T2 platform (Mx) and can confirm it's working well: https://elastictest.org/scheduler/testplan/693272f7392767e9bf67e930 <img width="1609" height="202" alt="image" src="https://github.com/user-attachments/assets/e631a351-1372-412d-bca7-6b4ef5d8112a" /> I also verified the logic on T2 platform and can confirm it's still having this logic: https://elastictest.org/scheduler/testplan/6932767fbcc3fac23371a83c <img width="1963" height="546" alt="image" src="https://github.com/user-attachments/assets/261dd79b-c847-4a82-9409-d87e48a3cfa8" /> #### Any platform specific information? #### Supported testbed topology if it's a new test case? ### Documentation

…1613) Change the temporarily_disable_route_check fixture logic to only apply to T2 topology for now. What is the motivation for this PR? The current disable-and-enable routeCheck monitor logic is causing test flakiness on some non-T2 platforms (see sonic-net#16876 (comment)). Certain platforms require additional time to restart the routeCheck monitor, which can leave it inactive when the next test begins and result in false failures. We would like to address this issue urgently in this PR. In a follow-up PR, I will properly enhance the temporarily_disable_route_check fixture so that: Users can choose which topologies apply the disable-and-enable routeCheck behavior The fixture uses a wait_until() timeout to verify the routeCheck status is as expected before proceeding to the next step How did you do it? How did you verify/test it? I ran the updated login on a non-T2 platform (Mx) and can confirm it's working well: https://elastictest.org/scheduler/testplan/693272f7392767e9bf67e930 image I also verified the logic on T2 platform and can confirm it's still having this logic: https://elastictest.org/scheduler/testplan/6932767fbcc3fac23371a83c image Signed-off-by: Chenyang Wang <[email protected]> Co-authored-by: Chenyang Wang <[email protected]> Signed-off-by: Lakshmi Yarramaneni <[email protected]>

feat: add fixture for disabling route check

d96b0b7

cyw233 requested a review from prgeor as a code owner February 10, 2025 10:08

cyw233 commented Feb 10, 2025

View reviewed changes

tests/conftest.py Show resolved Hide resolved

cyw233 requested review from abdosi and yejianquan and removed request for prgeor February 10, 2025 22:51

yejianquan reviewed Feb 10, 2025

View reviewed changes

fix: resolve comments

9407cfd

fix: resolve comments

1f49817

yejianquan approved these changes Feb 12, 2025

View reviewed changes

yejianquan merged commit 5427836 into sonic-net:master Feb 12, 2025
13 checks passed

yejianquan added Request for 202411 branch Approved for 202411 branch Request for msft-202405 Branch Approved for msft-202405 Branch labels Feb 12, 2025

mssonicbld mentioned this pull request Feb 12, 2025

[action] [PR:16876] feat: add fixture for disabling route check Azure/sonic-mgmt.msft#75

Merged

11 tasks

mssonicbld added the Created PR to 202411 branch label Feb 12, 2025

mssonicbld added the Created PR to msft-202405 Branch label Feb 12, 2025

mssonicbld mentioned this pull request Feb 12, 2025

[action] [PR:16876] feat: add fixture for disabling route check #16924

Merged

11 tasks

mssonicbld added Included in msft-202405 Branch and removed Created PR to msft-202405 Branch labels Feb 12, 2025

mssonicbld added Included in 202411 branch and removed Created PR to 202411 branch labels Feb 13, 2025

yejianquan mentioned this pull request Feb 14, 2025

fix the route check monit error seen when test_route_consistency.py #16586

Closed

cyw233 mentioned this pull request Dec 6, 2025

fix: only disable route check for T2 #21582

Merged

12 tasks

mssonicbld mentioned this pull request Dec 9, 2025

[action] [PR:21582] fix: only disable route check for T2 #21613

Merged

12 tasks

cyw233 mentioned this pull request Dec 11, 2025

[manual] [PR:21582] fix: only disable route check for T2 #21627

Merged

12 tasks

mssonicbld mentioned this pull request Dec 12, 2025

[action] [PR:21582] fix: only disable route check for T2 Azure/sonic-mgmt.msft#930

Merged

12 tasks

Conversation

cyw233 commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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 Feb 10, 2025

Uh oh!

azure-pipelines bot commented Feb 10, 2025

Uh oh!

Uh oh!

yejianquan Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

cyw233 Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yejianquan Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

cyw233 Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Feb 11, 2025

Uh oh!

azure-pipelines bot commented Feb 11, 2025

Uh oh!

abdosi commented Feb 11, 2025

Uh oh!

cyw233 commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Feb 11, 2025

Uh oh!

azure-pipelines bot commented Feb 11, 2025

Uh oh!

yejianquan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mssonicbld commented Feb 12, 2025

Uh oh!

mssonicbld commented Feb 12, 2025

Uh oh!

ZhaohuiS commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cyw233 commented Feb 10, 2025 •

edited

Loading

cyw233 commented Feb 11, 2025 •

edited

Loading