Improvising pre_checks for test_reload config #7953

ansrajpu-git · 2023-04-04T17:00:03Z

Description of PR

This PR is an add-on to the PR #7289.To resolve intermittent failures in test_reload_configuration_checks,

Summary:
Fixes # (issue)

Type of change

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

Back port request

201911
202012
202205

Approach

What is the motivation for this PR?

test_config_reload is failing intermittently at:

Line 97. Even though wait parameter to reboot is being passed as 5, but if plt_reboot_dict is defined in the inventory, this wait time would be overwritten to value in plt_reboot_dict - Line 220
Line 108. After successful config reload, we are not checking for database service has started before executing config reload again. If database service has not started, we see the following exception:

"Traceback (most recent call last):File \"/usr/local/bin/config\", line 8, in <module>
    sys.exit(config())\n  File \"/usr/local/lib/python3.9/dist-packages/click/core.py\", line 764, in __call__
   return self.main(*args, **kwargs)\n  File \"/usr/local/lib/python3.9/dist-packages/click/core.py\", line 717, in main\n    rv = self.invoke(ctx)  File \"/usr/local/lib/python3.9/dist-packages/click/core.py\", line 1134, in invoke   Command.invoke(self, ctx) File \"/usr/local/lib/python3.9/dist-packages/click/core.py\", line 956, in invoke    return ctx.invoke(self.callback, **ctx.params)  File \"/usr/local/lib/python3.9/dist-packages/click/core.py\", line 555, in invoke    return callback(*args, **kwargs)  File \"/usr/local/lib/python3.9/dist-packages/click/decorators.py\", line 17, in new_func    return f(get_current_context(), *args, **kwargs)  File \"/usr/local/lib/python3.9/dist-packages/config/main.py\", line 1104, in config    ctx.obj = Db()  File \"/usr/local/lib/python3.9/dist-packages/utilities_common/db.py\", line 12, in __init__    self.cfgdb.connect()  File \"/usr/lib/python3/dist-packages/swsscommon/swsscommon.py\", line 1862, in connect    return _swsscommon.ConfigDBConnector_Native_connect(self, wait_for_init, retry_on)\nRuntimeError: Unable to connect to redis: Cannot assign requested address"

How did you do it?

-the wait time should only be changed to value defined in plt_reboot_dict only when its value is 0. Else should pick the value sent in the function call.
-Check if interfaces-config.service is exited successfully & database containers has started before doing config reload for the second time

How did you verify/test it?

Ran the test against T2 chassis multiple times

Any platform specific information?

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

Documentation

mssonicbld · 2023-04-04T17:06:01Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

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/common/reboot.py:381:27: E127 continuation line over-indented for visual indent tests/common/reboot.py:390:30: E127 continuation line over-indented for visual indent tests/common/reboot.py:393:26: E127 continuation line over-indented for visual indent tests/platform_tests/test_reload_config.py:12:1: F401 'tests.common.fixtures.conn_graph_facts.conn_graph_facts' imported but unused tests/platform_tests/test_reload_config.py:25:1: E302 expected 2 blank lines, found 1 tests/platform_tests/test_reload_config.py:25:64: F811 redefinition of unused 'conn_graph_facts' from line 12 tests/platform_tests/test_reload_config.py:50:33: E231 missing whitespace after ':' tests/platform_tests/test_reload_config.py:78:1: E302 expected 2 blank lines, found 1 tests/platform_tests/test_reload_config.py:78:82: F811 redefinition of unused 'conn_graph_facts' from line 12

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

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.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

…al after reboot

mssonicbld · 2023-04-21T14:30:41Z

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/platform_tests/test_reload_config.py:118:1: W191 indentation contains tabs tests/platform_tests/test_reload_config.py:118:1: E101 indentation contains mixed spaces and tabs tests/platform_tests/test_reload_config.py:118:5: E128 continuation line under-indented for visual indent

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

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.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

wenyiz2021 · 2023-06-23T17:10:07Z

tests/common/reboot.py

-        if plt_reboot_ctrl:
-            wait = plt_reboot_ctrl['wait']
-            timeout = plt_reboot_ctrl['timeout']
+            if plt_reboot_ctrl:


why change indention here?

this is not a feasible fix, it'll only overwrite timeout if wait == 0, however, the intention for having plt_reboot_ctrl is to give privilege for some TBs(chassis) to predefine a new timeout/wait that has more tolerance.

I just checked the wait is only used

sonic-mgmt/tests/common/reboot.py

Line 238 in 34ce7e3

time.sleep(wait)

, which is time for dut to be stabilize, and 5 sec is not enough for chassis. why do we want to keep it as 5?

**the wait has kept for small duration intentionally to verifythe test -for reload configuration checks. The test is simulated to perform config reload before the dut is stablize and expect a check to the proper message.
***The plt_reboot_ctrl should only apply, if the (wait) time is not explicitly provided for a known objective

thanks @ansrajpu-git, please check my latest change, I have added a flag to control this, instead of overwriting both timeout and wait only if wait is 0.
please see if it works for you too?

tests/platform_tests/test_reload_config.py

wenyiz2021 · 2023-06-23T19:59:04Z

hi @ansrajpu-git, thanks for your fix.
I have added some additional change:

added a flag plt_reboot_ctrl_overwrite in reboot function to decide whether we want to overwrite wait and timeout. by default is to overwrite, in this test test_reload_configuration_checks we don't want to wait for that long, we want to catch transient error, so the flag is False.
changed to run on every sku with enum_rand_one_per_hwsku_hostname

I have confirmed this worked on Arista chassis, please also confirm if it works for your case

mssonicbld · 2023-06-29T01:50:53Z

@ansrajpu-git PR conflicts with 202205 branch

rlhui · 2023-07-07T01:43:35Z

@ansrajpu-git please raise an explicit PR for 202205, thanks

…#8843) Description of PR This is cherry-pick of #7953 * Update multi_asic.py * Update sonic.py * added flag plt_reboot_ctrl_overwrite in reboot function to decide whether we want to overwrite wait and timeout. added a flag plt_reboot_ctrl_overwrite in reboot function to decide whether we want to overwrite wait and timeout. by default is to overwrite, in this test test_reload_configuration_checks we don't want to wait for that long, we want to catch transient error, so the flag is False. * changed to run on every sku with enum_rand_one_per_hwsku_hostname co-authorized by: [email protected]

This PR has a few fixes missed during cherry-pick of #7953 into 202205 earlier via PR 8843 Co-authored-by: mlok <[email protected]>

* Fix for test_config_reload.py_Retry later error within a short interval after reboot, by adding a flag, by default we overwrite with plt_reboot_ctrl in inventory, this test does not overwrite, keep small interval * Adding checks for system up before config_reload is initiated * change to run on every sku --------- Co-authored-by: wenyiz2021 <[email protected]>

1. Revert PR: sonic-net#7289, because the relevant issue has been fixed by PR:sonic-net/sonic-buildimage#15080 2. Remove wait_until(60, 1, 0, check_interfaces_config_service_status, duthost) in PR: sonic-net#7953 due to the above same reason Change-Id: Ifde0a83020bb06fb191a646e5ca91738b864ad7f

1. Revert PR: sonic-net#7289, because the relevant issue has been fixed by PR:sonic-net/sonic-buildimage#15080 2. Remove wait_until(60, 1, 0, check_interfaces_config_service_status, duthost) in PR: sonic-net#7953 due to the above same reason Change-Id: I022a948cb2e36ba7d5538258159ea047d13d80a7

1. Revert PR: #7289, because the relevant issue has been fixed by PR:sonic-net/sonic-buildimage#15080 2. Remove wait_until(60, 1, 0, check_interfaces_config_service_status, duthost) in PR: #7953 due to the above same reason

1. Revert PR: sonic-net#7289, because the relevant issue has been fixed by PR:sonic-net/sonic-buildimage#15080 2. Remove wait_until(60, 1, 0, check_interfaces_config_service_status, duthost) in PR: sonic-net#7953 due to the above same reason

Cherry-pick #11674 to 202205 branch 1. Revert PR: #7289, because the relevant issue has been fixed by PR:sonic-net/sonic-buildimage#15080 2. Remove wait_until(60, 1, 0, check_interfaces_config_service_status, duthost) in PR: #7953 due to the above same reason

1. Revert PR: #7289, because the relevant issue has been fixed by PR:sonic-net/sonic-buildimage#15080 2. Remove wait_until(60, 1, 0, check_interfaces_config_service_status, duthost) in PR: #7953 due to the above same reason

ansrajpu-git requested a review from prgeor as a code owner April 4, 2023 17:00

ansrajpu-git changed the title ~~Asrajput~~ Improvising system pre_checks for test_reload config Apr 4, 2023

ansrajpu-git changed the title ~~Improvising system pre_checks for test_reload config~~ Improvising pre_checks for test_reload config Apr 4, 2023

ansrajpu-git added 2 commits April 21, 2023 10:27

Fix for test_config_reload.py_Retry later error within a short interv…

05c2a8c

…al after reboot

Adding checks for system up before config_reload is initiated

cd85d66

ansrajpu-git force-pushed the asrajput branch from 8adf4a3 to cd85d66 Compare April 21, 2023 14:29

Corrected Flake8 error

d22685b

rlhui requested a review from wenyiz2021 June 23, 2023 08:34

wenyiz2021 reviewed Jun 23, 2023

View reviewed changes

tests/platform_tests/test_reload_config.py Outdated Show resolved Hide resolved

wenyiz2021 added 3 commits June 23, 2023 12:05

Update reboot.py

da0cf57

Update test_reload_config.py

7857437

change to run on every sku

8f1294f

wenyiz2021 approved these changes Jun 23, 2023

View reviewed changes

wenyiz2021 merged commit ce8c71f into sonic-net:master Jun 26, 2023

wenyiz2021 added the Request for 202205 branch label Jun 26, 2023

wangxin added the Approved for 202205 branch label Jun 29, 2023

mssonicbld added the Cherry Pick Conflict_202205 label Jun 29, 2023

wenyiz2021 mentioned this pull request Jul 7, 2023

[cherry pick master PR] Improvising pre_checks for test_reload config #8843

Merged

6 tasks

wangxin added the Included in 202205 branch label Jul 10, 2023

ansrajpu-git mentioned this pull request Jul 10, 2023

[cherry-pick PR8843 (master PR7953)] missing change for 202205 branch #8894

Merged

6 tasks

wangxin pushed a commit that referenced this pull request Jul 19, 2023

[cherry-pick PR8843 (master PR7953)] missing change for 202205 branch

bdd9d14

This PR has a few fixes missed during cherry-pick of #7953 into 202205 earlier via PR 8843 Co-authored-by: mlok <[email protected]>

wenyiz2021 mentioned this pull request Aug 24, 2023

[all linecards] platform_tests/test_reload_config.py::test_reload_configuration_checks failure aristanetworks/sonic#91

Closed

JibinBao mentioned this pull request Feb 8, 2024

Fix test_reload_config issue #11674

Merged

8 tasks

JibinBao mentioned this pull request Feb 8, 2024

Fix test_reload_config issue #11675

Merged

8 tasks

mssonicbld mentioned this pull request Mar 4, 2024

[action] [PR:11674] Fix test_reload_config issue #11850

Merged

8 tasks

mssonicbld mentioned this pull request Mar 4, 2024

[action] [PR:11674] Fix test_reload_config issue #11851

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvising pre_checks for test_reload config #7953

Improvising pre_checks for test_reload config #7953

ansrajpu-git commented Apr 4, 2023 •

edited

Loading

mssonicbld commented Apr 4, 2023

mssonicbld commented Apr 21, 2023

wenyiz2021 Jun 23, 2023

wenyiz2021 Jun 23, 2023 •

edited

Loading

ansrajpu-git Jun 24, 2023

wenyiz2021 Jun 26, 2023

wenyiz2021 commented Jun 23, 2023 •

edited

Loading

mssonicbld commented Jun 29, 2023

rlhui commented Jul 7, 2023

Improvising pre_checks for test_reload config #7953

Improvising pre_checks for test_reload config #7953

Conversation

ansrajpu-git commented Apr 4, 2023 • edited Loading

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

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

Documentation

mssonicbld commented Apr 4, 2023

mssonicbld commented Apr 21, 2023

wenyiz2021 Jun 23, 2023

Choose a reason for hiding this comment

wenyiz2021 Jun 23, 2023 • edited Loading

Choose a reason for hiding this comment

ansrajpu-git Jun 24, 2023

Choose a reason for hiding this comment

wenyiz2021 Jun 26, 2023

Choose a reason for hiding this comment

wenyiz2021 commented Jun 23, 2023 • edited Loading

mssonicbld commented Jun 29, 2023

rlhui commented Jul 7, 2023

ansrajpu-git commented Apr 4, 2023 •

edited

Loading

wenyiz2021 Jun 23, 2023 •

edited

Loading

wenyiz2021 commented Jun 23, 2023 •

edited

Loading