change fixture order to fix the setup sequence issue in ipv6_mgmt onl…

…y cases (sonic-net#12825)

Description of PR
This PR is to address the fixture setup sequence issue, and teardown out of sequence issue.

In convert_and_restore_config_db_to_ipv6_only, it will do a "config reload -y" during fixture setup or teardown.

For feature test cases where config is not saved into config_db.json, this reload needs to be done before feature fixture setup and after feature teardown, such as: tacacs_v6, setup_streaming_telemetry, or setup_ntp.

According to https://docs.pytest.org/en/latest/reference/fixtures.html#reference-fixtures, it only considers the following when deciding the fixture orders:

scope
dependencies
autouse
We shouldn't use autouse in this test module. So only two options to let convert_and_restore_config_db_to_ipv6_only runs before other fixtures:

define other fixtures in 'function' scope.
define the feature fixture to request convert_and_restore_config_db_to_ipv6_only explicit.
Using option sonic-net#1 in this PR as the new 'function' scope fixture can be reused by other cases. Option sonic-net#2 has good readability, but will limit the new fixture to be used by ipv6_only cases.

Summary:
Fixes sonic-net#12705

Approach
What is the motivation for this PR?
Multiple errors were observed in mgmt_ipv6 are related to fixture setup/teardown sequence.

How did you do it?
Added two 'function' scope fixture: check_tacacs_v6_func and setup_streaming_telemetry_func.
And modified 3 tests cases to use 'function' scope fixture.

test_ro_user_ipv6_only
test_rw_user_ipv6_only
test_telemetry_output_ipv6_only

co-authorized by: [email protected]

tests/ip/test_mgmt_ipv6_only.py

@@ -11,11 +11,11 @@
 from tests.tacacs.test_ro_user import ssh_remote_run
 from tests.ntp.test_ntp import run_ntp, setup_ntp  # noqa F401
 from tests.common.helpers.assertions import pytest_require
-from tests.tacacs.conftest import tacacs_creds, check_tacacs_v6 # noqa F401
+from tests.tacacs.conftest import tacacs_creds, check_tacacs_v6_func # noqa F401
 from tests.syslog.test_syslog import run_syslog, check_default_route # noqa F401
 from tests.common.fixtures.duthost_utils import convert_and_restore_config_db_to_ipv6_only  # noqa F401
 from tests.common.helpers.gnmi_utils import GNMIEnvironment
-from tests.telemetry.conftest import gnxi_path, setup_streaming_telemetry # noqa F401
+from tests.telemetry.conftest import gnxi_path, setup_streaming_telemetry_func # noqa F401
 
 pytestmark = [
     pytest.mark.disable_loganalyzer,
@@ -126,8 +126,10 @@ def test_snmp_ipv6_only(duthosts, enum_rand_one_per_hwsku_hostname, localhost, c
     assert "SONiC Software Version" in result[0], "Sysdescr not found in SNMP result from DUT IPv6 {}".format(hostipv6)
 
 
+# use function scope fixture so that convert_and_restore_config_db_to_ipv6_only will setup before check_tacacs_v6_func.
+# Otherwise, tacacs_v6 config may be lost after config reload in ipv6_only fixture.
 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
+                           tacacs_creds, convert_and_restore_config_db_to_ipv6_only, check_tacacs_v6_func): # noqa F811
     # Add a temporary debug log to see if DUTs are reachable via IPv6 mgmt-ip. Will remove later
     log_eth0_interface_info(duthosts)
     duthost = duthosts[enum_rand_one_per_hwsku_hostname]
@@ -138,8 +140,10 @@ def test_ro_user_ipv6_only(localhost, duthosts, enum_rand_one_per_hwsku_hostname
     check_output(res, 'test', 'remote_user')
 
 
+# use function scope fixture so that convert_and_restore_config_db_to_ipv6_only will setup before check_tacacs_v6_func.
+# Otherwise, tacacs_v6 config may be lost after config reload in ipv6_only fixture.
 def test_rw_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
+                           tacacs_creds, convert_and_restore_config_db_to_ipv6_only, check_tacacs_v6_func): # noqa F811
     # Add a temporary debug log to see if DUTs are reachable via IPv6 mgmt-ip. Will remove later
     log_eth0_interface_info(duthosts)
     duthost = duthosts[enum_rand_one_per_hwsku_hostname]
@@ -150,10 +154,12 @@ def test_rw_user_ipv6_only(localhost, duthosts, enum_rand_one_per_hwsku_hostname
     check_output(res, 'testadmin', 'remote_user_su')
 
 
-@pytest.mark.parametrize('setup_streaming_telemetry', [True], indirect=True)
+# use function scope fixture so that convert_and_restore_config_db_to_ipv6_only will setup before
+# setup_streaming_telemetry_func. Otherwise, telemetry config may be lost after config reload in ipv6_only fixture.
+@pytest.mark.parametrize('setup_streaming_telemetry_func', [True], indirect=True)
 def test_telemetry_output_ipv6_only(convert_and_restore_config_db_to_ipv6_only, # noqa F811
                                     duthosts, enum_rand_one_per_hwsku_hostname,
-                                    setup_streaming_telemetry): # noqa F811
+                                    setup_streaming_telemetry_func): # noqa F811
     # Add a temporary debug log to see if DUTs are reachable via IPv6 mgmt-ip. Will remove later
     log_eth0_interface_info(duthosts)
     duthost = duthosts[enum_rand_one_per_hwsku_hostname]
@@ -171,6 +177,7 @@ def test_telemetry_output_ipv6_only(convert_and_restore_config_db_to_ipv6_only,
                   "SAI_PORT_STAT_IF_IN_ERRORS not found in gnmi output")
 
 
+# use function scope fixture so that convert_and_restore_config_db_to_ipv6_only will run before setup_ntp_func
 def test_ntp_ipv6_only(duthosts, rand_one_dut_hostname,
                                   convert_and_restore_config_db_to_ipv6_only, setup_ntp): # noqa F811
     run_ntp(duthosts, rand_one_dut_hostname, setup_ntp)

tests/tacacs/conftest.py

@@ -3,6 +3,7 @@
 import yaml
 import pytest
 import copy
+from contextlib import contextmanager
 from .utils import setup_tacacs_client, setup_tacacs_server, cleanup_tacacs, restore_tacacs_servers
 
 logger = logging.getLogger(__name__)
@@ -39,7 +40,19 @@ def check_tacacs(ptfhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_cre
 
 
 @pytest.fixture(scope="module")
-def check_tacacs_v6(ptfhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_creds):
+def check_tacacs_v6(ptfhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_creds): # noqa F811
+    with _context_for_check_tacacs_v6(ptfhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_creds) as result:
+        yield result
+
+
+@pytest.fixture(scope="function")
+def check_tacacs_v6_func(ptfhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_creds): # noqa F811
+    with _context_for_check_tacacs_v6(ptfhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_creds) as result:
+        yield result
+
+
+@contextmanager
+def _context_for_check_tacacs_v6(ptfhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_creds): # noqa F811
     duthost = duthosts[enum_rand_one_per_hwsku_hostname]
     ptfhost_vars = ptfhost.host.options['inventory_manager'].get_host(ptfhost.hostname).vars
     if 'ansible_hostv6' not in ptfhost_vars:

tests/telemetry/conftest.py

@@ -8,6 +8,7 @@
 from tests.common.utilities import wait_until, wait_tcp_connection, get_mgmt_ipv6
 from tests.common.helpers.gnmi_utils import GNMIEnvironment
 from tests.telemetry.telemetry_utils import get_list_stdout, setup_telemetry_forpyclient, restore_telemetry_forpyclient
+from contextlib import contextmanager
 
 EVENTS_TESTS_PATH = "./telemetry/events"
 sys.path.append(EVENTS_TESTS_PATH)
@@ -87,6 +88,21 @@ def delete_gnmi_config(duthost):
 
 @pytest.fixture(scope="module")
 def setup_streaming_telemetry(request, duthosts, enum_rand_one_per_hwsku_hostname, localhost, ptfhost, gnxi_path):
+    with _context_for_setup_streaming_telemetry(request, duthosts, enum_rand_one_per_hwsku_hostname,
+                                                localhost, ptfhost, gnxi_path) as result:
+        yield result
+
+
+@pytest.fixture(scope="function")
+def setup_streaming_telemetry_func(request, duthosts, enum_rand_one_per_hwsku_hostname, localhost, ptfhost, gnxi_path):
+    with _context_for_setup_streaming_telemetry(request, duthosts, enum_rand_one_per_hwsku_hostname,
+                                                localhost, ptfhost, gnxi_path) as result:
+        yield result
+
+
+@contextmanager
+def _context_for_setup_streaming_telemetry(request, duthosts, enum_rand_one_per_hwsku_hostname,
+                                           localhost, ptfhost, gnxi_path):
     """
     @summary: Post setting up the streaming telemetry before running the test.
     """