change timeouts on changes involving bgp to allow for convergence#2058
change timeouts on changes involving bgp to allow for convergence#2058yxieca merged 9 commits intosonic-net:masterfrom slogan621:master
Conversation
|
@Blueve please review |
|
|
|
This pull request introduces 3 alerts and fixes 6 when merging 3bf58d6 into 384c948 - view on LGTM.com new alerts:
fixed alerts:
|
|
Retest vsimage please |
1 similar comment
|
Retest vsimage please |
|
@daall would you be able to review, merge? This replaces the sleep timeout with a polled wait for bgp convergence. I checked with bgp experts inside of Broadcom and I was told that 30 seconds may not be enough time, more like 120 (they suggested we try 180 but sticking with the 120 here). |
tests/pc/test_po_update.py
Outdated
| bgp_facts = duthost.bgp_facts()['ansible_facts'] | ||
| assert bgp_facts['bgp_statistics']['ipv4_idle'] == 1 | ||
| if not wait_until(120, 10, duthost.check_bgp_statistic, 'ipv4_idle', 1): | ||
| assert duthost.get_bgp_statistic('ipv4_idle') == 1 |
There was a problem hiding this comment.
Please use pytest_assert. assert sometimes triggers exception. (same for other asserts below)
There was a problem hiding this comment.
Done, thanks for the review.
|
This pull request introduces 1 alert when merging e1208f8 into 47fc3a2 - view on LGTM.com new alerts:
|
|
@wangxin thank you |
a6d35de Handling Invalid CRM configuration gracefully (sonic-net#2109) d6559e6 [Mellanox] '_8lane' not added to Mellanox 5xxx models with 800G (sonic-net#2090) 45551b2 [vnetorch] Advertise vnet tunnel routes (sonic-net#2058) ed58d2f Add initial value for weight in overlay nexthops (sonic-net#2096)
5 participants
[sonic-mgmt] replace 30 second sleeps around bgp changes to polling for up to 120 sec to allow time for convergence.
Signed-off-by: Syd Logan [email protected]