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.

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

Add function to wrap verify_only_loopback_routes with assert and wait_until #16523

Merged

Conversation

Javier-Tan
Copy link
Contributor

@Javier-Tan Javier-Tan commented Jan 15, 2025

Description of PR

Summary:
Partially tackles #16577

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405
  • 202411

Approach

What is the motivation for this PR?

Tests are flaky, sometimes failing on verify_only_loopback_routes_are_announced_to_neighs: Failed to verify routes on nbr in TSA

How did you do it?

Add a function wrapping verify_only_loopback_routes_are_announced_to_neighs with wait_until and assert to allow time for neighbor to update routes

How did you verify/test it?

These logs were seen in passed tests, showing that wait_until helps avoid false negatives

17/01/2025 05:42:18 utilities.wait_until                     L0153 DEBUG  | verify_only_loopback_routes_are_announced_to_neighs is False, wait 10 seconds and check again

05:42:12 route_checker.verify_loopback_route_with L0014 INFO   | Verifying only loopback routes are announced to bgp neighbors
05:42:18 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71002> terminated with exit code None
05:42:18 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71003> terminated with exit code None
05:42:18 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71001> terminated with exit code None
05:42:18 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71000> terminated with exit code None
05:42:18 parallel.parallel_run                    L0221 INFO   | Completed running processes for target "parse_routes_process" in 0:00:03.470030 seconds
05:42:18 route_checker.verify_loopback_route_with L0035 INFO   | Verifying only loopback routes(ipv4) are announced to ARISTA04T3
05:42:18 route_checker.verify_loopback_route_with L0047 WARNING| missing loopback address or some other routes present on neighbor
05:42:28 route_checker.verify_loopback_route_with L0014 INFO   | Verifying only loopback routes are announced to bgp neighbors
05:42:34 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71002> terminated with exit code None
05:42:34 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71000> terminated with exit code None
05:42:34 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71003> terminated with exit code None
05:42:34 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71001> terminated with exit code None
05:42:34 parallel.parallel_run                    L0221 INFO   | Completed running processes for target "parse_routes_process" in 0:00:03.332572 seconds
05:42:34 route_checker.verify_loopback_route_with L0035 INFO   | Verifying only loopback routes(ipv4) are announced to ARISTA04T3
05:42:34 route_checker.verify_loopback_route_with L0035 INFO   | Verifying only loopback routes(ipv4) are announced to ARISTA01T3
05:42:34 route_checker.verify_loopback_route_with L0035 INFO   | Verifying only loopback routes(ipv4) are announced to ARISTA06T3
05:42:34 route_checker.verify_loopback_route_with L0035 INFO   | Verifying only loopback routes(ipv4) are announced to ARISTA03T3
05:42:34 route_checker.verify_loopback_route_with L0014 INFO   | Verifying only loopback routes are announced to bgp neighbors
05:42:39 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71000> terminated with exit code None
05:42:39 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71002> terminated with exit code None
05:42:39 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71001> terminated with exit code None
05:42:39 parallel.on_terminate                    L0085 INFO   | process parse_routes_process--<EosHost VM71003> terminated with exit code None
05:42:39 parallel.parallel_run                    L0221 INFO   | Completed running processes for target "parse_routes_process" in 0:00:02.681303 seconds

All affected test suites were run

Any platform specific information?

N/A

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

N/A

Documentation

N/A

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Javier-Tan Javier-Tan marked this pull request as ready for review January 17, 2025 05:46
@Javier-Tan
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@Javier-Tan Javier-Tan requested a review from arlakshm January 17, 2025 05:47
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

arlakshm
arlakshm previously approved these changes Jan 17, 2025
@arlakshm arlakshm requested a review from tjchadaga January 17, 2025 06:37
@Javier-Tan
Copy link
Contributor Author

One part of #16577

@Javier-Tan Javier-Tan changed the title Add wait_until for verify_only_loopback_routes in test_reliable_tsa.py Add function to wrap verify_only_loopback_routes with assert and wait_until Jan 21, 2025
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

arlakshm
arlakshm previously approved these changes Jan 28, 2025
@arlakshm
Copy link
Contributor

Hi @Javier-Tan, can you resolve the conflicts

… update

* Add wait_until for verify_only_loopback_routes

Signed-off-by: Javier Tan javiertan@microsoft.com
* Remove trailing whitespace

Signed-off-by: Javier Tan javiertan@microsoft.com
* Add function which wraps verify_only_loopback_routes_... with assert and wait_until

Signed-off-by: Javier Tan javiertan@microsoft.com
@yejianquan
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

* Remove trailing whitespace

Signed-off-by: Javier Tan javiertan@microsoft.com
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Javier-Tan
Copy link
Contributor Author

@arlakshm @yejianquan conflicts has been resolved, could you help re-review/merge? Thanks so much!

Copy link
Collaborator

@yejianquan yejianquan left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants