[warm-reboot] Add Preboot n BGP member down and n Lag down tests by neethajohn · Pull Request #1004 · sonic-net/sonic-mgmt

neethajohn · 2019-07-11T21:53:03Z

Signed-off-by: Neetha John nejo@microsoft.com

Description of PR

Extend the warm-reboot sad path BGP and LAG tests for bringing n BGP members or n lags down instead of a single one

Type of change

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

Approach

How did you do it?

warm-reboot-sad.yml
- Added ':' separated parameter for each preboot operation which indicates the number of bgp members/lag down. If no ':' separated parameter exists, the count is set to 1
validate_preboot_list.yml
- Validate if the cnt for each preboot operation is within allowed limits. For bgp and lag down case, the max value is set to one less than the number of Vms. For lag member case, max allowed value is the total number of members in a lag
sad_path.py
- VM selection method is extended to select count number of VMs after picking a start VM.
- extend all cases to make changes for count number of VMs instead of a single VM

How did you verify/test it?

Ran the test on T0 topology and all cases passed
Regression run for fast-reboot and warm-reboot tests also passed

yxieca · 2019-07-12T00:42:36Z

ansible/roles/test/tasks/warm-reboot-sad.yml

Why was single failure test got removed?

I think it might make more sense to pass preboot_list in from test case instead of define here. The benefit is:

The test case can define how many failures to inject freely.

Each test will pass/fail by itself. e.g. we might be able to pass single lag down, but having trouble with 2 lags down. With 2 different tests. It is easier to see which one failed.

retained the original yml and moved the n member case to a separate one

yxieca · 2019-07-12T00:43:59Z

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

Maybe make these variable names plural?

qiluo-msft · 2019-07-12T01:41:36Z

ansible/roles/test/files/ptftests/advanced-reboot.py

(oper_type, cnt) = self.preboot_oper.split(':') if self.preboot_oper and ':' in self.preboot_oper else (self.preboot_oper, 1) [](start = 11, length = 126)

(oper_type, cnt) = self.preboot_oper.split(':') if self.preboot_oper and ':' in self.preboot_oper else (self.preboot_oper, 1) [](start = 11, length = 126)

Use if-else block instead of one liner. #Closed

qiluo-msft · 2019-07-12T01:43:30Z

ansible/roles/test/tasks/advanced-reboot.yml

[](start = 21, length = 1)

[](start = 21, length = 1)

one extra space #Closed

qiluo-msft · 2019-07-12T01:43:41Z

ansible/roles/test/tasks/advanced-reboot.yml

item.find(':') != -1 [](start = 41, length = 20)

item.find(':') != -1 [](start = 41, length = 20)

':' in item #Closed

qiluo-msft · 2019-07-13T00:45:47Z

    self.check_param('preboot_oper', None, required = False)

These new params are difficult to understand by a new user. Could you provide usage comments? #Closed

Refers to: ansible/roles/test/files/ptftests/advanced-reboot.py:146 in 600e6b7. [](commit_id = 600e6b7, deletion_comment = False)

qiluo-msft · 2019-07-13T00:46:56Z

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

(self.oper_type, self.cnt) = oper_type.split(':') if ':' in oper_type else (oper_type, 1) [](start = 8, length = 89)

What is your design of the constructor parameter? Why not assume it in one format? #Closed

oper_type will either be 'value' or 'value:cnt' if cnt is greater than 1. Kept it this way since I didn't want to make changes in parameter format for the existing single member case.

I will have to pass the cnt as an additional parameter if I do the split outside the class.

I mean change everything into 'value:cnt' will make code easier.

In reply to: 303570408 [](ancestors = 303570408)

I will be adding one more parameter for lag member down case. so it will be value:cnt1:cnt2 . cnt1 will be the same as the number of VMS. cnt2 will be number of lag members.
I prefer to handle it in the code for cases that don't need it rather than doing it in the yml. for eg,
single bgp member down should be changed to 'bgp_down:1:0' in the yml or I can keep it as 'bgp_down' in the yml and handle it in the script.

qiluo-msft · 2019-07-13T00:48:25Z

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

vm_len [](start = 49, length = 6)

vm_len [](start = 49, length = 6)

How to prevent div by 0? #Closed

will do the mod operation only if vm_list is non empty otherwise I will set it to 0

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

qiluo-msft · 2019-07-13T00:55:20Z

ansible/roles/test/tasks/advanced-reboot.yml

item != 'None' and item.find(':') != -1 [](start = 22, length = 39)

item != 'None' and item.find(':') != -1 [](start = 22, length = 39)

You can remove

item != 'None' and

You can use ':' in item #Closed

Signed-off-by: Neetha John <nejo@microsoft.com>

* Add Preboot n BGP member down and n Lag down tests Signed-off-by: Neetha John <nejo@microsoft.com>

neethajohn requested review from pavel-shirshov, qiluo-msft and yxieca July 11, 2019 21:53

neethajohn added Enhancement Request for 201811 branch labels Jul 11, 2019

yxieca reviewed Jul 12, 2019

View reviewed changes

qiluo-msft reviewed Jul 12, 2019

View reviewed changes

qiluo-msft reviewed Jul 13, 2019

View reviewed changes

ansible/roles/test/files/ptftests/sad_path.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Jul 13, 2019

View reviewed changes

ansible/roles/test/files/ptftests/sad_path.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Jul 13, 2019

View reviewed changes

neethajohn force-pushed the warmboot-nbgp-nlag branch from 31a1fcf to f86054c Compare July 15, 2019 19:41

neethajohn added 5 commits July 17, 2019 09:38

Add Preboot n BGP member down and n Lag down tests

7f3fdfc

Signed-off-by: Neetha John <nejo@microsoft.com>

Separate multiple member and single member sad path testcases

c32902a

Make var names plural

6088513

Address review comments

774c88d

Fix indentation in .py and count in .yml

600e6b7

neethajohn force-pushed the warmboot-nbgp-nlag branch from f86054c to 600e6b7 Compare July 17, 2019 16:39

yxieca approved these changes Jul 18, 2019

View reviewed changes

qiluo-msft approved these changes Jul 23, 2019

View reviewed changes

neethajohn merged commit d5498f0 into sonic-net:master Jul 24, 2019

neethajohn deleted the warmboot-nbgp-nlag branch July 24, 2019 17:54

yxieca pushed a commit that referenced this pull request Jul 31, 2019

[warm-reboot] Add Preboot n BGP member down and n Lag down tests (#1004)

998b959

* Add Preboot n BGP member down and n Lag down tests Signed-off-by: Neetha John <nejo@microsoft.com>

yxieca added the Included in 201811 branch label Jul 31, 2019

Conversation

neethajohn commented Jul 11, 2019

Description of PR

Type of change

Approach

How did you do it?

How did you verify/test it?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Jul 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Jul 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Jul 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft commented Jul 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qiluo-msft Jul 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Jul 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

qiluo-msft Jul 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

qiluo-msft Jul 12, 2019 •

edited

Loading

qiluo-msft Jul 12, 2019 •

edited

Loading

qiluo-msft Jul 12, 2019 •

edited

Loading

qiluo-msft commented Jul 13, 2019 •

edited

Loading

qiluo-msft Jul 13, 2019 •

edited

Loading

qiluo-msft Jul 13, 2019 •

edited

Loading

qiluo-msft Jul 13, 2019 •

edited

Loading