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

Fix test_crm_fdb_entry #2701

Merged
merged 1 commit into from
Dec 29, 2020
Merged

Conversation

bingwang-ms
Copy link
Collaborator

Signed-off-by: bingwang [email protected]

Description of PR

Summary:
Fixes # (issue)
test_crm_fdb_entry was consistently failing on TD3. There are 2 reasons for this failure.

  1. FDB may get expired at any time
  2. Some time is needed for FDB to be cleared because fdbclear is an asynchronous interface.
    This PR fix these two issues.

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

What is the motivation for this PR?

This PR is to fix test_crm_fdb_entry.

How did you do it?

  1. Disable FDB aging
  2. Add a loop check after fdbclear

How did you verify/test it?

Verified on 7050cx3

py.test --inventory ../ansible/str2,../ansible/veos --host-pattern str2-7250cx3-acs-1 --module-path ../ansible --testbed vms20-t0-7050cx3-1 --testbed_file ../ansible/testbed.csv --junit-xml=tr.xml --log-cli-level warn --collect_techsupport=False --topology=t0,any,util crm/test_crm.py::test_crm_fdb_entry
========================================================================================= test session starts =========================================================================================
collected 1 item                                                                                                                                                                                      

crm/test_crm.py::test_crm_fdb_entry PASSED                                                                                                                                                      [100%]

------------------------------------------------------------------ generated xml file: /data/Networking-acs-sonic-mgmt/tests/tr.xml -------------------------------------------------------------------
===================================================================================== 1 passed in 236.99 seconds ======================================================================================

Any platform specific information?

No.

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

No.

Documentation

1. Disable FDB aging
2. Add a loop check after fdbclear

Signed-off-by: bingwang <[email protected]>
@bingwang-ms
Copy link
Collaborator Author

retest vsimage please

1 similar comment
@bingwang-ms
Copy link
Collaborator Author

retest vsimage please

@bingwang-ms bingwang-ms requested a review from a team December 29, 2020 01:06
Copy link
Contributor

@daall daall left a comment

Choose a reason for hiding this comment

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

LGTM, one minor suggestion

Comment on lines +894 to +900
while FDB_CLEAR_TIMEOUT > 0:
# Get new "crm_stats_fdb_entry" used and available counter value
new_crm_stats_fdb_entry_used, new_crm_stats_fdb_entry_available = get_crm_stats(get_fdb_stats, duthost)
if new_crm_stats_fdb_entry_used == 0:
break
FDB_CLEAR_TIMEOUT -= CRM_POLLING_INTERVAL
time.sleep(CRM_POLLING_INTERVAL)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do a wait_until here instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good suggestion. I didn't use wait_until because I think the current code is easier to read and understand. Or a new wraper function is needed to check the return value of get_crm_stats

@bingwang-ms bingwang-ms merged commit 931c969 into sonic-net:master Dec 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants