[warm-reboot] Add preboot LAG sad path automation#945
[warm-reboot] Add preboot LAG sad path automation#945neethajohn merged 5 commits intosonic-net:masterfrom
Conversation
There was a problem hiding this comment.
you don't need the template here.
There was a problem hiding this comment.
Didn't get you. Are you referring to passing the state string directly? If yes, this was a feedback I received from Qi in my earlier PR to pass in a bool to avoid misspelling name.
There was a problem hiding this comment.
You can write
self.do_cmd(state[is_up])
instead of
self.do_cmd('%s' % state[is_up])
There was a problem hiding this comment.
lag [](start = 37, length = 3)
lag [](start = 37, length = 3)
You are concat cmd string, always check input parameter to prevent attack. #Closed
There was a problem hiding this comment.
lag [](start = 59, length = 3)
lag [](start = 59, length = 3)
the same #Closed
There was a problem hiding this comment.
pre_check [](start = 21, length = 9)
The variable is effective conditionally, suggest name it as ''sad_lag_pre_check" #Closed
There was a problem hiding this comment.
This var is not specific to the LAG test. I will be using it for other preboot types as well. I am using this var to tag the failed msg with Preboot/Postboot since the failed msgs are all consolidated and printed at the end of the test. Hard to figure out when the failure occurred since the same method is used for both preboot and postboot checks.
There was a problem hiding this comment.
state [](start = 8, length = 5)
state [](start = 8, length = 5)
better not to reuse var, and name it as 'states' #Closed
There was a problem hiding this comment.
sudo config interface %s %s [](start = 100, length = 27)
sudo config interface %s %s [](start = 100, length = 27)
input injection risk #Closed
There was a problem hiding this comment.
added validation
There was a problem hiding this comment.
'sudo config interface %s %s' % (state[is_up], self.vm_dut_map[self.neigh_name]['dut_portchannel']) [](start = 99, length = 99)
extract to another statement, don't make any one too much complex.
There was a problem hiding this comment.
fails [](start = 8, length = 5)
fails [](start = 8, length = 5)
We prefer return bool positively, like 'successful' #Closed
There was a problem hiding this comment.
renamed var to 'success'
qiluo-msft
left a comment
qiluo-msft
left a comment
There was a problem hiding this comment.
Looks good to me, please check with other reviewers.
…boot/postboot check
* preboot LAG sad path automation for neigh_lag_down and dut_lag_down scenarios
) <!-- Please make sure you've read and understood our contributing guidelines; https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md Please provide following information to help code review process a bit easier: --> ### Description of PR On Scale testbed there are 2 main differences: 1. There are much more PTF ports 2. There are much more OVS and interface configurations Those lead to: 1. NIC would take more time to handle the receiving packets 2. PTF could take more time to process As a result: 1. More time to spend in ptf function testutils.verify_packet Summary: Fixes # (issue) ### Type of change <!-- - Fill x for your type of change. - e.g. - [x] Bug fix --> - [ ] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] New Test case - [ ] Skipped for non-supported platforms - [ ] Test case improvement ### Back port request - [ ] 202205 - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 - [x] 202511 ### Approach #### What is the motivation for this PR? To increase the PTF traffic handling stability in the scale testbed #### How did you do it? Add more wait time in ptf function testutils.verify_packet #### How did you verify/test it? Run it in the local scale testbed #### Any platform specific information? #### Supported testbed topology if it's a new test case? ### Documentation <!-- (If it's a new feature, new test case) Did you update documentation/Wiki relevant to your implementation? Link to the wiki page? -->
Submodule src/sonic-platform-common 42119e1..5d7954e: > [ChassisBase] Make reboot cause constant strings human-readable (sonic-net#35) > Add .gitignore file (sonic-net#28) > [sonic_platform_base] Add sonic_sfp and sonic_eeprom to sonic_platform_base (sonic-net#27) > Enhance new platform API (sonic-net#19) > fix typo in platform API base class (sonic-net#25) Submodule src/sonic-swss 9cf7b01..1e99c93: > Set timer only when interval changes. Not in each firing of the timer. (sonic-net#945) Submodule src/sonic-utilities ec1e93f..24958f1: > [fast reboot] stop removing opennsl module before reboot (sonic-net#560) Submodule src/sonic-swss-common b472f6e..d6140fa: > timerfd:read failure - Record in logs as error. (sonic-net#295) > do not abort when read timerfd return 0 and errno = 0 (sonic-net#291) > Add an assert to logger, which will log a message and abort. (sonic-net#286) Signed-off-by: Ying Xie <[email protected]>
sonic-swss: * 67f7191 2019-06-20 | Set timer only when interval changes. Not in each firing of the timer. (sonic-net#945) (HEAD -> 201803, origin/201803) [Renuka Manavalan] * 2cae61b 2019-07-07 | Fix: crash while destructing crmorch (sonic-net#731) (sonic-net#949) [Renuka Manavalan] sonic-swss-common: * 58b1930 2019-07-07 | Add an assert to logger, which will log a message and abort. (sonic-net#286) (sonic-net#287) (HEAD -> 201803, origin/201803) [Renuka Manavalan]
Add watchdogutil to control the hw watchdog (sonic-net#945) [db_migrator] Support migrating database regarding buffer configuration for all Mellanox switches (sonic-net#1053) Signed-off-by: Abhishek Dosi <[email protected]>
4 participants
Description of PR
Extend the existing warm-reboot test to include the preboot lag sad path for both neigh end and DUT end
Type of change
Approach
How did you verify/test it?
Ran the warm-reboot-sad.yml . All cases except 'neigh_lag_down' case passed. Neigh_lag_down failure is a known issue and will be fixed soon
Verified on T0-64 topology