[PR:12393] Adding ipv6_mgmt_only test case into PR testing and fix ntp fixture errors#12804
Merged
yejianquan merged 1 commit intosonic-net:202311from May 16, 2024
Merged
Conversation
sdszhang
changed the title
sdszhang
changed the title
sdszhang
requested review from
qiluo-msft and
yejianquan
and removed request for
qiluo-msft
Contributor
Author
|
/azp run Azure.sonic-mgmt |
|
Commenter does not have sufficient privileges for PR 12804 in repo sonic-net/sonic-mgmt |
sonic-net#12393) Summary: 1. Add `ip/test_mgmt_ipv6_only.py` into PR pipeline testing. 2. Rearrange fixture order for two test cases: `ip/test_mgmt_ipv6_only.py::test_rw_user_ipv6_only` and `ip/test_mgmt_ipv6_only.py::test_ro_user_ipv6_only`. 3. Workaround pytest fixture teardown bug affecting `setup_ntp` when run the `ip/test_mgmt_ipv6_only.py` tests. - [x] Bug fix - [ ] Testbed and Framework(new/improvement) - [x] Test case(new/improvement) ``` $ ./run_tests.sh -n vms-kvm-t0 -d vlab-01 -f vtestbed.yaml -i ../ansible/veos_vtb -u -c ip/test_mgmt_ipv6_only.py::test_ro_user_ipv6_only ...... ip/test_mgmt_ipv6_only.py::test_ro_user_ipv6_only[vlab-01] FAILED [100%] ...... ip/test_mgmt_ipv6_only.py:138: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ output = {'failed': True, 'changed': True, 'stdout': '', 'stderr': "Warning: Permanently added 'fec0::ffff:afa:1' (RSA) to the ...fec0::ffff:afa:1' (RSA) to the list of known hosts.", 'Permission denied, please try again.'], '_ansible_no_log': None} exp_val1 = 'test', exp_val2 = 'remote_user' def check_output(output, exp_val1, exp_val2): > pytest_assert(not output['failed'], output['stderr']) E Failed: Warning: Permanently added 'fec0::ffff:afa:1' (RSA) to the list of known hosts. E Permission denied, please try again. exp_val1 = 'test' exp_val2 = 'remote_user' output = {'failed': True, 'changed': True, 'stdout': '', 'stderr': "Warning: Permanently added 'fec0::ffff:afa:1' (RSA) to the ...fec0::ffff:afa:1' (RSA) to the list of known hosts.", 'Permission denied, please try again.'], '_ansible_no_log': None} tacacs/utils.py:25: Failed ``` The root case is: in current test case definition, the fixture setup sequence is: 1. `tacacs_v6` --> `sudo config tacacs add fec0::ffff:afa:2` 2. `convert_and_restore_config_db_to_ipv6_only` --> `config reload -y` after removing ipv4 mgmt address The `sudo config tacacs add fec0::ffff:afa:2` config is lost after the `config reload -y` in step 2. Therefore, causing tacacs authentication failure. If `convert_and_restore_config_db_to_ipv6_only` is called before `check_tacacs_v6`, there will be no issue. ``` Current definition: def test_ro_user_ipv6_only(localhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_creds, check_tacacs_v6, convert_and_restore_config_db_to_ipv6_only): # noqa F811 Correct definition: def test_ro_user_ipv6_only(localhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_creds, convert_and_restore_config_db_to_ipv6_only, check_tacacs_v6): # noqa F811 ``` ``` When running the full test cases, we are seeing the following fixture sequence and error. $./run_tests.sh -n vms-kvm-t0 -d vlab-01 -c ip/test_mgmt_ipv6_only.py -f vtestbed.yaml -i ../ansible/veos_vtb -u -e "--setup-show" SETUP M convert_and_restore_config_db_to_ipv6_only (fixtures used: duthosts) SETUP M setup_ntp (fixtures used: duthosts, ptf_use_ipv6, ptfhost, rand_one_dut_hostname) ...... TEARDOWN M convert_and_restore_config_db_to_ipv6_only ---> This is wrong. setup_ntp should be teardown first. TEARDOWN M setup_ntp ...... > raise RunAnsibleModuleFail("run module {} failed".format(self.module_name), res) E tests.common.errors.RunAnsibleModuleFail: run module command failed, Ansible Results => E {"changed": true, "cmd": ["config", "ntp", "del", "fec0::ffff:afa:2"], "delta": "0:00:00.277230", "end": "2024-05-02 11:32:22.404196", "failed": true, "msg": "non-zero return code", "rc": 2, "start": "2024-05-02 11:32:22.126966", "stderr": "Usage: config ntp del [OPTIONS] <ntp_ip_address>\nTry \"config ntp del -h\" for help.\n\nError: NTP server fec0::ffff:afa:2 is not configured.", "stderr_lines": ["Usage: config ntp del [OPTIONS] <ntp_ip_address>", "Try \"config ntp del -h\" for help.", "", "Error: NTP server fec0::ffff:afa:2 is not configured."], "stdout": "", "stdout_lines": []} ...... ``` The teardown should be the reverse of fixture setup. The expected setup/teardown order is: ``` SETUP M convert_and_restore_config_db_to_ipv6_only (fixtures used: duthosts) SETUP M setup_ntp (fixtures used: duthosts, ptf_use_ipv6, ptfhost, rand_one_dut_hostname) ...... TEARDOWN M setup_ntp TEARDOWN M convert_and_restore_config_db_to_ipv6_only ``` This error is linked to a known issue pytest-dev/pytest#12135 in pytest, and it has been fixed pytest 8.2.0 via pytest-dev/pytest#11833. Currently, SONiC is utilizing pytest version 7.4.0, which does not include the fix for this issue. To address this, a workaround will be necessary until sonic-mgmt is upgraded to pytest version 8.2.0. 1. Add it into the PR test case list. 2. changed the fixture request sequence, put `convert_and_restore_config_db_to_ipv6_only` to the left of `check_tacacs_v6.` so `convert_and_restore_config_db_to_ipv6_only` fixture will run before `tacacs_v6`. 4. As upgrading pytest version is not trial change, I duplicated the `setup_ntp` fixture at `function` scope. As ntp is only one case in `test_mgmt_ipv6_only.py`, it makes it more suitable to use a `function` scope fixture instead of `module` scope fixture. 1. pipeline check included test_mgmt_ipv6_only.py 2. Run individual test against test_rw_user_ipv6_only, test_ro_user_ipv6_only, test_ntp_ipv6_only. All passed: ``` $./run_tests.sh -n vms-kvm-t0 -d vlab-01 -f vtestbed.yaml -i ../ansible/veos_vtb -u -c ip/test_mgmt_ipv6_only.py::test_ro_user_ipv6_only .... ip/test_mgmt_ipv6_only.py::test_ro_user_ipv6_only[vlab-01] PASSED [100%] $ ./run_tests.sh -n vms-kvm-t0 -d vlab-01 -f vtestbed.yaml -i ../ansible/veos_vtb -u -c ip/test_mgmt_ipv6_only.py::test_rw_user_ipv6_only ...... ip/test_mgmt_ipv6_only.py::test_rw_user_ipv6_only[vlab-01] PASSED [100%] $./run_tests.sh -n vms-kvm-t0 -d vlab-01 -f vtestbed.yaml -i ../ansible/veos_vtb -u -c ip/test_mgmt_ipv6_only.py::test_ntp_ipv6_only ...... ip/test_mgmt_ipv6_only.py::test_ntp_ipv6_only[True-vlab-01] PASSED [100%] ``` 3. Full test passed: ``` $./run_tests.sh -n vms-kvm-t0 -d vlab-01 -f vtestbed.yaml -i ../ansible/veos_vtb -u -c ip/test_mgmt_ipv6_only.py ...... ip/test_mgmt_ipv6_only.py::test_bgp_facts_ipv6_only[vlab-01-None] PASSED [ 10%] ip/test_mgmt_ipv6_only.py::test_show_features_ipv6_only[vlab-01] PASSED [ 20%] ip/test_mgmt_ipv6_only.py::test_image_download_ipv6_only[vlab-01] SKIPPED (Cannot get image url) [ 30%] ip/test_mgmt_ipv6_only.py::test_syslog_ipv6_only[vlab-01-fd82:b34f:cc99::100-None] PASSED [ 40%] ip/test_mgmt_ipv6_only.py::test_syslog_ipv6_only[vlab-01-fd82:b34f:cc99::100-fd82:b34f:cc99::200] PASSED [ 50%] ip/test_mgmt_ipv6_only.py::test_ntp_ipv6_only[True-vlab-01] PASSED [ 60%] ip/test_mgmt_ipv6_only.py::test_snmp_ipv6_only[vlab-01] PASSED [ 70%] ip/test_mgmt_ipv6_only.py::test_ro_user_ipv6_only[vlab-01] PASSED [ 80%] ip/test_mgmt_ipv6_only.py::test_rw_user_ipv6_only[vlab-01] PASSED [ 90%] ip/test_mgmt_ipv6_only.py::test_telemetry_output_ipv6_only[vlab-01-True] PASSED [100%] ==================================================================================== warnings summary ==================================================================================== ../../../usr/local/lib/python3.8/dist-packages/paramiko/transport.py:236 /usr/local/lib/python3.8/dist-packages/paramiko/transport.py:236: CryptographyDeprecationWarning: Blowfish has been deprecated "class": algorithms.Blowfish, -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html ----------------------------------------------------------------- generated xml file: /data/sonic-mgmt/tests/logs/tr.xml ----------------------------------------------------------------- ================================================================================ short test summary info ================================================================================= SKIPPED [1] common/helpers/assertions.py:16: Cannot get image url ================================================================== 9 passed, 1 skipped, 1 warning in 745.28s (0:12:25) =================================================================== ```
sdszhang
force-pushed
the
cherry/202311/12393
branch
from
e6fc891 to
01efbd9
Compare
sdszhang
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
Summary:
Original PR: #12393
Summary:
ip/test_mgmt_ipv6_only.pyinto PR pipeline testing.ip/test_mgmt_ipv6_only.py::test_rw_user_ipv6_onlyandip/test_mgmt_ipv6_only.py::test_ro_user_ipv6_only.setup_ntpwhen run theip/test_mgmt_ipv6_only.pytests.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