sonic-mgmt icon indicating copy to clipboard operation
sonic-mgmt copied to clipboard

[T2-Chassis] - New tests for Reliable-TSA feature

Open sanjair-git opened this issue 1 year ago • 3 comments

Description of PR

Summary: Fixes # (issue)

  • This PR has tests to verify the changes introduced as part of #https://github.com/sonic-net/sonic-buildimage/pull/19217. The original code PR for the feature is #https://github.com/sonic-net/sonic-buildimage/pull/18928
  • HLD link for the feature: https://github.com/skeesara-nokia/SONiC/blob/master/doc/voq/Reliable_TSA.md
  • Basically, this PR has 20 test cases to verify the feature functionality and also few modifications on the tests introduced as part of PR #12781

Type of change

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

Back port request

  • [ ] 202012
  • [ ] 202205
  • [ ] 202305
  • [ ] 202311
  • [x] 202405

Approach

What is the motivation for this PR?

  • To verify the feature functionality and the code changes introduced as part of the PR #https://github.com/sonic-net/sonic-buildimage/pull/18928

How did you do it?

  • Added 20 new test cases to verify feature in three sets.
  • Attaching test cases' description document here for further reference.
  • Reliable-TSA-Cases.xlsx
  1. First set: TSA/TSB config event on the supervisor or line card. (8 cases) test_sup_tsa_act_when_sup_duts_on_tsb_initially test_sup_tsa_act_when_sup_on_tsb_duts_on_tsa_initially test_sup_tsb_act_when_sup_on_tsa_duts_on_tsb_initially test_sup_tsb_act_when_sup_and_duts_on_tsa_initially test_dut_tsa_act_when_sup_duts_on_tsb_initially test_dut_tsa_act_when_sup_on_tsa_duts_on_tsb_initially test_dut_tsb_act_when_sup_on_tsb_duts_on_tsa_initially test_dut_tsb_act_when_sup_and_duts_on_tsa_initially

  2. Second set: Config event followed by certain action on the supervisor or line card. (8 cases) test_sup_tsa_act_with_sup_reboot test_sup_tsa_act_when_duts_on_tsa_with_sup_config_reload test_dut_tsa_act_with_reboot_when_sup_dut_on_tsb_init test_dut_tsa_with_conf_reload_when_sup_on_tsa_dut_on_tsb_init test_sup_tsb_followed_by_dut_bgp_restart_when_sup_on_tsa_duts_on_tsb test_sup_tsb_followed_by_dut_bgp_restart_when_sup_and_duts_on_tsa test_dut_tsb_followed_by_dut_bgp_restart_when_sup_on_tsb_duts_on_tsa test_dut_tsb_followed_by_dut_bgp_restart_when_sup_and_duts_on_tsa

  3. Miscellaneous tests: Use cases with 'startup-tsa-tsb' service etc. (4 cases) test_sup_tsa_when_startup_tsa_tsb_service_running test_sup_tsb_when_startup_tsa_tsb_service_running test_user_init_tsa_on_dut_followed_by_sup_tsa test_user_init_tsa_on_dut_followed_by_sup_tsb

How did you verify/test it?

  • Ran all the above-mentioned test cases on a T2 chassis and made sure tests passed with expected behavior.

Any platform specific information?

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

Documentation

Reliable TSA test result: image

Startup-tsa-tsb service test result: image

Note: PR #12781 is pending merge, and this PR is dependent on that.

sanjair-git avatar Jun 13 '24 19:06 sanjair-git

The pre-commit check detected issues in the files touched by this pull request. The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results: trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/bgp/test_startup_tsa_tsb_service.py:1164:1: F811 redefinition of unused 'test_user_init_tsa_while_service_run_on_dut' from line 743
tests/bgp/test_startup_tsa_tsb_service.py:1268:1: F811 redefinition of unused 'test_user_init_tsb_while_service_run_on_dut' from line 847
tests/bgp/test_startup_tsa_tsb_service.py:1360:1: F811 redefinition of unused 'test_user_init_tsb_on_sup_while_service_run_on_dut' from line 939
tests/bgp/test_startup_tsa_tsb_service.py:1484:1: F811 redefinition of unused 'test_tsa_tsb_timer_efficiency' from line 1059

flake8...............................................(no files to check)Skipped
check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

  1. Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt docker container.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld avatar Jul 10 '24 21:07 mssonicbld

This test PR is dependent on sonic-buildimage PR #(https://github.com/sonic-net/sonic-buildimage/pull/19539)

sanjair-git avatar Aug 02 '24 14:08 sanjair-git

@tjchadaga: Are we good to merge this ?

abdosi avatar Aug 15 '24 23:08 abdosi

Latest test results for reference:

image

sanjair-git avatar Sep 18 '24 19:09 sanjair-git

@bingwang-ms (202405 release manager for viz) Since this's a new test, reliable TSA /TSB that needed in chassis test and we want to cherry-pick to 202405 I have confirmed, most of them are with the test mark that runs only on T2, few of them are with the test mark T1,T2, but the change only influences the topology with T2.

So it wouldn't affect T1 tests and cause regression

yejianquan avatar Oct 10 '24 02:10 yejianquan

@bingwang-ms (202405 release manager for viz) Since this's a new test, reliable TSA /TSB that needed in chassis test and we want to cherry-pick to 202405 I have confirmed, most of them are with the test mark that runs only on T2, few of them are with the test mark T1,T2, but the change only influences the topology with T2.

So it wouldn't affect T1 tests and cause regression

And test_traffic_shift is covered by the PR on kvm t1-lag testbeds, it passed, test: https://elastictest.org/scheduler/testplan/66eda86ec4757bff3198c203?searchTestCase=traffic&testcase=bgp%2Ftest_traffic_shift.py&type=console

yejianquan avatar Oct 10 '24 02:10 yejianquan

General comments, generally LGTM 👍

Thanks @Javier-Tan for taking time to review.

sanjair-git avatar Oct 15 '24 17:10 sanjair-git

@sanjair-git, can please resolve conflicts

arlakshm avatar Oct 22 '24 23:10 arlakshm

The pre-commit check detected issues in the files touched by this pull request. The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results: trim trailing whitespace.................................................Passed
fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing tests/bgp/test_startup_tsa_tsb_service.py

check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/bgp/test_startup_tsa_tsb_service.py:91:21: E128 continuation line under-indented for visual indent

flake8...............................................(no files to check)Skipped
check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

  1. Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt docker container.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld avatar Oct 23 '24 15:10 mssonicbld

@sanjair-git, can please resolve conflicts

@arlakshm, I have resolved the conflicts and taken care of the new test added as part of #14855. Please help in reapproving and merge this PR. Thanks

sanjair-git avatar Oct 23 '24 15:10 sanjair-git

Hi @abdosi @yejianquan - Can you help getting it merged to 202405 - it has been 4 days since it has been merged to master ?

vperumal avatar Nov 03 '24 17:11 vperumal

Cherry-pick PR to 202405: https://github.com/sonic-net/sonic-mgmt/pull/15338

mssonicbld avatar Nov 04 '24 03:11 mssonicbld