Skip to content

Fix test_cont_link_flap #2634

Closed
bingwang-ms wants to merge 1 commit intosonic-net:masterfrom
bingwang-ms:fix_link_flap
Closed

Fix test_cont_link_flap #2634
bingwang-ms wants to merge 1 commit intosonic-net:masterfrom
bingwang-ms:fix_link_flap

Conversation

@bingwang-ms
Copy link
Copy Markdown
Collaborator

Signed-off-by: bingwang [email protected]

Description of PR

Summary:
Fixes # (issue)
Test case test_cont_link_flap consistently failed on master image. It was because recent update in show library made show ip route sum not availabe on lattest image. As a result, the command show ip route sum will not be able to retrieve routes count and an exception was thrown.

07/12/2020 17:51:39 INFO link_flap_utils.py:check_bgp_routes:195: IPv4 routes at end: 
07/12/2020 17:51:39 ERROR utilities.py:wait_until:43: Exception caught while checking check_bgp_routes: ValueError('could not convert string to float: ',)
07/12/2020 17:51:39 DEBUG utilities.py:wait_until:50: check_bgp_routes is False, wait 1 seconds and check again

This commit replaces 'show ip route sum' with vtysh command to fix the issue.

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_cont_link_flap on master image.

How did you do it?

This commit replaces show ip route sum with vtysh command to fix the issue.

How did you verify/test it?

Verified on DX010-4, running lattest image.

py.test --inventory ../ansible/str,../ansible/veos --host-pattern str-dx010-4 --module-path ../ansible --testbed vms7-t0-dx010-4 --testbed_file ../ansible/testbed.csv --junit-xml=tr.xml --log-cli-level warn --collect_techsupport=False --topology=t0,any,util  platform_tests/link_flap/test_cont_link_flap.py
========================================================================================= test session starts =========================================================================================
collected 1 item                                                                                                                                                                                      

platform_tests/link_flap/test_cont_link_flap.py::TestContLinkFlap::test_cont_link_flap ^@^@^@^@^@^@^@^@PASSED                                                                                                   [100%]

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

Any platform specific information?

No.

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

No.

Documentation

Recent update in 'show' library made 'show ip route sum' not availabe on
lattest image. This commit replaces 'show ip route sum' with vtysh command.

Signed-off-by: bingwang <[email protected]>
@bingwang-ms bingwang-ms requested a review from a team December 8, 2020 08:04
"""
if ipv4:
end_time_ip_route_counts = dut.shell("show ip route summary | grep Total | awk '{print $2}'")["stdout"]
end_time_ip_route_counts = dut.shell("vtysh -c 'show ip route summary' | grep Total | awk '{print $2}'")["stdout"]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you wrap this in a help function to call "show ip route summary" first, if not successful, call the vtysh command to return the result?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Why do we need that? The show ip route is actually calling vtysh -c inside.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should use cli command whenever possible. not having the command is an image issue and it will be addressed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agree. So I think we should drop this PR and let the issue to surface out since they have confirmed that this is an image issue. What do you think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We can keep this PR. But put the work-around at a helper function that can be updated in the future would help.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

If the workaround is added, how could we find such issue?

@bingwang-ms
Copy link
Copy Markdown
Collaborator Author

Closed. PR sonic-net/sonic-utilities#1302 addressed it.

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