Adding ipv6_mgmt_only test case into PR testing and fix fixture errors

[sdszhang]

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

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

Back port request

• 201911
• 202012
• 202205
• 202305
• 202311

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.

3. 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) ===================================================================

Any platform specific information?

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

Documentation

[sdszhang]

pending on #12377

[qiluo-msft]

Could you resolve the conflict and check the test failure?

[sdszhang]

Could you resolve the conflict and check the test failure?
@qiluo-msft I'm currently checking on the failures.

ERROR ip/test_mgmt_ipv6_only.py::test_telemetry_output_ipv6_only[vlab-01-True] - fixed in PR #12420
ERROR ip/test_mgmt_ipv6_only.py::test_ntp_ipv6_only -- missing "import setup_ntp". Easy fix.

For the rest 3, as they are all broken with error: "paramiko.ssh_exception.NoValidConnectionsError: [Errno None] Unable to connect to port 22 on 10.250.0.101". Seems to be feature related. still checking on that.

ERROR ip/test_mgmt_ipv6_only.py::test_snmp_ipv6_only[vlab-01] - paramiko.ssh_exception.NoValidConnectionsError: [Errno None] Unable to connect to port 22 on 10.250.0.101
ERROR ip/test_mgmt_ipv6_only.py::test_ro_user_ipv6_only[vlab-01] - paramiko.ssh_exception.NoValidConnectionsError: [Errno None] Unable to connect to port 22 on 10.250.0.101
ERROR ip/test_mgmt_ipv6_only.py::test_rw_user_ipv6_only[vlab-01] - paramiko.ssh_exception.NoValidConnectionsError: [Errno None] Unable to connect to port 22 on 10.250.0.101

[qiluo-msft]

May be fixed by Fix unreachable DUT mgmt_ip in IPv6 only test case by maipbui · Pull Request #12492 · sonic-net/sonic-mgmt (github.com)

[qiluo-msft]

/azp run Azure.sonic-mgmt

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[vivekverma-arista]

The RCA is correct and it's a race condition between fixture teardowns but merely changing the order of fixtures requests in test/fixture does not guarantee execution in a certain order.

https://docs.pytest.org/en/stable/reference/fixtures.html#fixture-instantiation-order

Quoting from the document Names of fixtures or tests, where they’re defined, the order they’re defined in, and the order fixtures are requested in have no bearing on execution order beyond coincidence. While pytest will try to make sure coincidences like these stay consistent from run to run, it’s not something that should be depended on.

So this issue is still hitting on our testbeds after this fix. These tests should be skipped until this issue is properly addressed.

[sdszhang]

Thanks for the comment, let's track it further in #12705

[mssonicbld]

@sdszhang PR conflicts with 202311 branch

[mssonicbld]

@sdszhang PR conflicts with 202305 branch