Skip to content

Commit

Permalink
Adding ipv6_mgmt_only test case into PR testing and fix fixture errors (
Browse files Browse the repository at this point in the history
sonic-net#12393)

## Description of PR

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.

### Type of change

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

## Approach
#### What is the motivation for this PR?
##### 1. Include `ip/test_mgmt_ipv6_only.py` into PR pipeline testing for IPv6 hardening.
 
##### 2. Fix errors when running individual test cases. 
```
$ ./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
```

##### 3. Fix fixture teardown error when running whole ip/test_mgmt_ipv6_only.py.

```
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.

#### How did you do it?
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.

#### How did you verify/test it?
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) ===================================================================
```
  • Loading branch information
sdszhang authored and parmarkj committed Jul 17, 2024
1 parent 99a485f commit 565dac2
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 185 deletions.
1 change: 1 addition & 0 deletions .azure-pipelines/pr_test_scripts.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ t0:
- vlan/test_vlan.py
- vlan/test_vlan_ping.py
- vxlan/test_vnet_route_leak.py
- ip/test_mgmt_ipv6_only.py

t0-2vlans:
- dhcp_relay/test_dhcp_relay.py
Expand Down
185 changes: 0 additions & 185 deletions tests/ip/test_mgmt_ipv6_only.py

This file was deleted.

0 comments on commit 565dac2

Please sign in to comment.