VxLAN Automation: Adding all remaining testcases, and the required modifications. by rraghav-cisco · Pull Request #5059 · sonic-net/sonic-mgmt

rraghav-cisco · 2022-02-02T04:54:32Z

This is part 2 of the Vxlan Test case Automation.
Adding the rest of test cases from https://github.com/Azure/SONiC/blob/8ca1ac93c8912fda7b09de9bfd51498e5038c292/doc/vxlan/Overlay%20ECMP%20with%20BFD.md#test-cases

I have combined 13th and 14th cases into a single case in this script.
The commit on mar 28th also added a new test, a variant of TC9.

prsunny · 2022-02-11T18:23:03Z

@dgsudharsan , @sumukhatv , could you folks please review?

dgsudharsan · 2022-02-11T18:24:46Z

@ihorchekh @roysr-nv Can you please review the tests?

sachinv-msft · 2022-02-22T23:35:58Z

@dgsudharsan @ihorchekh @roysr-nv any updates on this? please review this PR at your earliest conv

roy-sror

Hi,

Can you please provide the runtime of this test suite, how much time would it take to run the entire suite?

Can you please use allure steps, rather than comments, it would make the allure report much more readable.

ansible/roles/test/files/ptftests/vxlan_traffic.py

tests/vxlan/test_vxlan_ecmp.py

ihorchekh

Here are some suggestions from my side.

tests/vxlan/test_vxlan_ecmp.py

ansible/roles/test/files/ptftests/vxlan_traffic.py

lgtm-com · 2022-03-15T01:01:56Z

This pull request fixes 1 alert when merging 2c47f9d into 10debf5 - view on LGTM.com

fixed alerts:

1 for Unused import

lgtm-com · 2022-03-15T01:27:17Z

This pull request fixes 1 alert when merging 5528f35 into 10debf5 - view on LGTM.com

fixed alerts:

1 for Unused import

rraghav-cisco · 2022-03-15T03:14:07Z

@roysr-nv :
Thanks for your comments.

The latest test run took 6100 seconds, close to 2 hours. I have seen it varying from close to 1 hour to 2 hours.
I plan to convert the comment lines in all testcases to "Logger.info()" calls. Pls let me know that is what you are asking for.

prsunny · 2022-03-16T22:12:33Z

/azp run

azure-pipelines · 2022-03-16T22:12:42Z

Azure Pipelines successfully started running 1 pipeline(s).

sachinv-msft · 2022-03-23T16:13:36Z

@kevinwangsk @rraghav-cisco where are we with this PR?

kevinskwang · 2022-03-24T11:35:05Z

I tried on 8102, there are four test cases failed, other 52 are passed.
FAILED vxlan/test_vxlan_ecmp.py::Test_VxLAN_NHG_Modify::test_vxlan_remove_ecmp_route2[v4_in_v4] - Exception
FAILED vxlan/test_vxlan_ecmp.py::Test_VxLAN_NHG_Modify::test_vxlan_remove_ecmp_route2[v4_in_v6] - Exception
FAILED vxlan/test_vxlan_ecmp.py::Test_VxLAN_NHG_Modify::test_vxlan_remove_ecmp_route2[v6_in_v4] - Exception
FAILED vxlan/test_vxlan_ecmp.py::Test_VxLAN_NHG_Modify::test_vxlan_remove_ecmp_route2[v6_in_v6] - Exception

ihorchekh · 2022-03-25T14:58:53Z

@kevinskwang this PR by @dgsudharsan should fix this

ansible/roles/test/files/ptftests/vxlan_traffic.py

tests/vxlan/test_vxlan_ecmp.py

kevinskwang · 2022-03-28T09:10:17Z

Those cases looks good to me~ Just one question, why expect the overlay DMAC still to be the dut_mac after encap? should it be the dest VM's MAC in the vnet?

rraghav-cisco · 2022-03-29T01:07:54Z

Those cases looks good to me~ Just one question, why expect the overlay DMAC still to be the dut_mac after encap? should it be the dest VM's MAC in the vnet?
Hi @kevinskwang : I agree with you, it is actually ignored in the test. So it is ok.

…ic-mgmt into rraghav-vxlan2 Pulling changes from master to rraghav-vxlan2.

lgtm-com · 2022-04-23T01:06:44Z

This pull request fixes 1 alert when merging 9432578 into 35f25e1 - view on LGTM.com

fixed alerts:

1 for Unused import

Turned on the ECN testing, which was turned off till now. Needed a minor modification to the ECN calculation.

lgtm-com · 2022-04-23T08:02:44Z

This pull request fixes 1 alert when merging 9fb0a5c into 35f25e1 - view on LGTM.com

fixed alerts:

1 for Unused import

rraghav-cisco · 2022-04-23T17:00:59Z

@ihorchekh @dgsudharsan @prsunny : Can we merge this PR pls?

prsunny · 2022-04-26T16:48:06Z

/azp run

azure-pipelines · 2022-04-26T16:48:14Z

Azure Pipelines successfully started running 1 pipeline(s).

prsunny · 2022-04-27T00:08:18Z

@ihorchekh @dgsudharsan, please sign-off

prsunny · 2022-04-27T16:27:49Z

/azp run

azure-pipelines · 2022-04-27T16:27:57Z

Azure Pipelines successfully started running 1 pipeline(s).

ihorchekh · 2022-04-28T14:27:09Z

tests/vxlan/test_vxlan_ecmp.py

+                self.setup['duthost'].shell("sudo config interface shutdown {}".format(intf))
+


This test case sometimes fails here. I did debug and found that we need some delay after bringing interfaces down and sending packets. I believe 10-sec sleep will be enough.
I tried to use it here wait_until logic with passing all_intfs list, like wait_until(60, 30, 0, bgp_established, self.setup['duthost'], all_intfs) and I found another issue in bgp_established function, see below

tests/vxlan/test_vxlan_ecmp.py

ihorchekh · 2022-04-28T15:00:45Z

tests/vxlan/test_vxlan_ecmp.py

+            pytest_assert(wait_until(300, 30, 0, bgp_established, self.setup['duthost']), "BGP neighbors didn't come up after all interfaces have been brought up.")
+
+            logger.info("Verify the traffic is flowing through, again.")
+            self.dump_self_info_and_run_ptf("tc12", encap_type, True, packet_count=1000)


Suggested change

self.dump_self_info_and_run_ptf("tc12", encap_type, True, packet_count=1000)

self.dump_self_info_and_run_ptf("tc14", encap_type, True, packet_count=1000)

ihorchekh · 2022-04-28T15:01:34Z

tests/vxlan/test_vxlan_ecmp.py

+            pytest_assert(wait_until(300, 30, 0, bgp_established, self.setup['duthost']), "BGP neighbors didn't come up after all interfaces have been brought up.")
+            logger.info("Verify traffic flows after recovery.")
+            self.setup[encap_type]['t2_ports'] = all_t2_ports
+            self.dump_self_info_and_run_ptf("tc12", encap_type, True, packet_count=1000)


Suggested change

self.dump_self_info_and_run_ptf("tc12", encap_type, True, packet_count=1000)

self.dump_self_info_and_run_ptf("tc13", encap_type, True, packet_count=1000)

ihorchekh · 2022-04-28T15:05:34Z

tests/vxlan/test_vxlan_ecmp.py

+            logger.info("Bring down the T2 interfaces.")
+            for intf in all_intfs:
+                self.setup['duthost'].shell("sudo config interface shutdown {}".format(intf))
+


Suggested change

time.sleep(10)

or wait_until with passing all_intfs as ignore_list, but only if the issue mentioned above in bgp_established will be fixed.

Suggested change

pytest_assert(wait_until(60, 30, 0, bgp_established, self.setup['duthost'], all_intfs), "BGP neighbors didn't come down after all interfaces have been brought down.")

ihorchekh · 2022-04-28T15:12:12Z

@prsunny I believe I'll approve after my comments above will be resolved

lgtm-com · 2022-04-30T23:08:47Z

This pull request fixes 1 alert when merging f02fa5d into cb01d8b - view on LGTM.com

fixed alerts:

1 for Unused import

1. Added more stringent checks for bgp_established() function. Now it verifies the down neighbors as well as the established neighbors. 2. Fixed minor typos in TC names, comments and logs. 3. Called the bgp_established() after bringing down the interfaces in tc12. (as per review comments).

lgtm-com · 2022-05-01T00:02:20Z

This pull request fixes 1 alert when merging 411bd24 into cb01d8b - view on LGTM.com

fixed alerts:

1 for Unused import

prsunny · 2022-05-03T02:28:03Z

@prsunny I believe I'll approve after my comments above will be resolved

@ihorchekh , Please review the latest change

ihorchekh · 2022-05-03T13:51:42Z

@prsunny @rraghav-cisco I've run the updated test and I've got a 100% pass rate, 60/60.
So, I approve this PR.

prsunny

Thanks @rraghav-cisco , @ihorchekh, @roysr-nv for the great contributions on implementation and review

…difications. (#5059) * Implement part 2 of the Vxlan Overlay ECMP Test cases.

antonptashnik · 2022-07-22T10:31:18Z

ansible/roles/test/files/ptftests/vxlan_traffic.py

+            # Every next-hop should have received within 1% of the packets that we sent per nexthop(which is self.packet_count).
+            # This check is valid only if there are large enough number of packets(300). Any lower number will need higher tolerance(more than 2%).
+            if self.packet_count > MINIMUM_PACKETS_FOR_ECMP_VALIDATION:
+                tolerance = 0.01


@rraghav-cisco please clarify what this 1% value is based on. Unfortunately I was not able to find it in feature requirement docs

Adding all testcases, and the required modifications.

caad974

rraghav-cisco requested a review from a team as a code owner February 2, 2022 04:54

rraghav-cisco marked this pull request as draft February 2, 2022 04:54

Removed the skips.

962c002

rraghav-cisco marked this pull request as ready for review February 2, 2022 05:13

sachinv-msft requested a review from prsunny February 16, 2022 04:32

prsunny previously approved these changes Feb 28, 2022

View reviewed changes

roy-sror reviewed Mar 1, 2022

View reviewed changes

ihorchekh suggested changes Mar 7, 2022

View reviewed changes

Completed review comments.

2c47f9d

Merge branch 'Azure:master' into rraghav-vxlan2

5528f35

lizhijianrd dismissed prsunny’s stale review via 5528f35 March 15, 2022 06:00

prsunny requested a review from kevinskwang March 17, 2022 01:42

prsunny mentioned this pull request Mar 23, 2022

Overlay ECMP with BFD support sonic-net/SONiC#861

Merged

ihorchekh suggested changes Mar 25, 2022

View reviewed changes

rraghav-cisco added 2 commits March 29, 2022 01:10

Final review comments, from Ihor Chekh, and roysr-nv

a7c803c

Merge branch 'rraghav-vxlan2' of https://github.com/rraghav-cisco/son…

85e3ffe

…ic-mgmt into rraghav-vxlan2 Pulling changes from master to rraghav-vxlan2.

rraghav-cisco dismissed roy-sror’s stale review via 9432578 April 23, 2022 00:40

Updating the traffic/ptf script to turn on ECN

9fb0a5c

Turned on the ECN testing, which was turned off till now. Needed a minor modification to the ECN calculation.

sonic-net deleted a comment from azure-pipelines bot Apr 26, 2022

sonic-net deleted a comment from rraghav-cisco Apr 26, 2022

sonic-net deleted a comment from azure-pipelines bot Apr 26, 2022

ihorchekh suggested changes Apr 28, 2022

View reviewed changes

Merge branch 'Azure:master' into rraghav-vxlan2

f02fa5d

ihorchekh approved these changes May 3, 2022

View reviewed changes

prsunny approved these changes May 3, 2022

View reviewed changes

prsunny merged commit e279fda into sonic-net:master May 3, 2022

prsunny added the Request for 202012 branch label May 4, 2022

wangxin pushed a commit that referenced this pull request May 5, 2022

VxLAN Automation: Adding all remaining testcases, and the required mo…

af9b707

…difications. (#5059) * Implement part 2 of the Vxlan Overlay ECMP Test cases.

wangxin added the Included in 202012 branch label May 5, 2022

rraghav-cisco deleted the rraghav-vxlan2 branch May 9, 2022 22:50

antonptashnik reviewed Jul 22, 2022

View reviewed changes

		self.setup['duthost'].shell("sudo config interface shutdown {}".format(intf))

	self.dump_self_info_and_run_ptf("tc12", encap_type, True, packet_count=1000)
	self.dump_self_info_and_run_ptf("tc14", encap_type, True, packet_count=1000)

	self.dump_self_info_and_run_ptf("tc12", encap_type, True, packet_count=1000)
	self.dump_self_info_and_run_ptf("tc13", encap_type, True, packet_count=1000)


	pytest_assert(wait_until(60, 30, 0, bgp_established, self.setup['duthost'], all_intfs), "BGP neighbors didn't come down after all interfaces have been brought down.")

Conversation

rraghav-cisco commented Feb 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prsunny commented Feb 11, 2022

Uh oh!

dgsudharsan commented Feb 11, 2022

Uh oh!

sachinv-msft commented Feb 22, 2022

Uh oh!

roy-sror left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ihorchekh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgtm-com bot commented Mar 15, 2022

Uh oh!

lgtm-com bot commented Mar 15, 2022

Uh oh!

rraghav-cisco commented Mar 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prsunny commented Mar 16, 2022

Uh oh!

azure-pipelines bot commented Mar 16, 2022

Uh oh!

sachinv-msft commented Mar 23, 2022

Uh oh!

kevinskwang commented Mar 24, 2022

Uh oh!

ihorchekh commented Mar 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevinskwang commented Mar 28, 2022

Uh oh!

rraghav-cisco commented Mar 29, 2022

Uh oh!

lgtm-com bot commented Apr 23, 2022

Uh oh!

lgtm-com bot commented Apr 23, 2022

Uh oh!

rraghav-cisco commented Apr 23, 2022

Uh oh!

prsunny commented Apr 26, 2022

Uh oh!

azure-pipelines bot commented Apr 26, 2022

Uh oh!

prsunny commented Apr 27, 2022

Uh oh!

prsunny commented Apr 27, 2022

Uh oh!

azure-pipelines bot commented Apr 27, 2022

Uh oh!

rraghav-cisco commented Feb 2, 2022 •

edited

Loading

rraghav-cisco commented Mar 15, 2022 •

edited

Loading

ihorchekh commented Mar 25, 2022 •

edited

Loading

ihorchekh Apr 28, 2022 •

edited

Loading