[multi-dut] Added fixtures for selecting nodes from multi-duts based on card type and hwsku

tests/common/devices.py

@@ -24,6 +24,7 @@
 from errors import RunAnsibleModuleFail
 from errors import UnsupportedAnsibleModule
 from tests.common.helpers.constants import DEFAULT_ASIC_ID, DEFAULT_NAMESPACE, NAMESPACE_PREFIX
+from tests.common.helpers.dut_utils import is_supervisor_node
 
 # HACK: This is a hack for issue https://github.com/Azure/sonic-mgmt/issues/1941 and issue
 # https://github.com/ansible/pytest-ansible/issues/47
@@ -354,11 +355,9 @@ def is_supervisor_node(self):
             the inventory, and it is 'supervisor', then return True, else return False. In future, we can change this
             logic if possible to derive it from the DUT.
         """
-        if 'type' in self.host.options["inventory_manager"].get_host(self.hostname).get_vars():
-            node_type = self.host.options["inventory_manager"].get_host(self.hostname).get_vars()["type"]
-            if node_type is not None and node_type == 'supervisor':
-                return True
-        return False
+        im = self.host.options['inventory_manager']
+        inv_files = im._sources
+        return is_supervisor_node(inv_files, self.hostname)
 
     def is_frontend_node(self):
         """Check if the current node is a frontend node in case of multi-DUT.

tests/common/helpers/dut_utils.py

@@ -0,0 +1,32 @@
+from tests.common.utilities import get_host_visible_vars
+
+
+def is_supervisor_node(inv_files, hostname):
+    """Check if the current node is a supervisor node in case of multi-DUT.
+     @param inv_files: List of inventory file paths, In tests,
+            you can be get it from get_inventory_files in tests.common.utilities
+     @param hostname: hostname as defined in the inventory
+    Returns:
+          Currently, we are using 'type' in the inventory to make the decision. If 'type' for the node is defined in
+          the inventory, and it is 'supervisor', then return True, else return False. In future, we can change this
+          logic if possible to derive it from the DUT.
+    """
+    node_type = get_host_visible_vars(inv_files, hostname, variable='type')
+    if node_type and node_type == 'supervisor':
+        return True
+    return False
+
+
+def is_frontend_node(inv_files, hostname):
+    """Check if the current node is a frontend node in case of multi-DUT.
+     @param inv_files: List of inventory file paths, In tests,
+            you can be get it from get_inventory_files in tests.common.utilities
+     @param hostname: hostname as defined in the inventory
+     Returns:
+          True if it is not any other type of node. Currently, the only other type of node supported is 'supervisor'
+          node. If we add more types of nodes, then we need to exclude them from this method as well.
+    """
+    return not is_supervisor_node(inv_files, hostname)
+
+
+

tests/common/utilities.py

@@ -144,6 +144,20 @@ def get_variable_manager(inv_files):
     return VariableManager(loader=DataLoader(), inventory=get_inventory_manager(inv_files))
 
 
+def get_inventory_files(request):
+    """Use request.config.getoption('ansible_inventory') to the get list of inventory files.
+       The 'ansible_inventory' option could have already been converted to a list by #enchance_inventory fixture.
+       Args:
+            request: request paramater for pytest.
+    """
+    if isinstance(request.config.getoption("ansible_inventory"), list):
+        # enhance_inventory fixture changes ansible_inventory to a list.
+        inv_files = request.config.getoption("ansible_inventory")
+    else:
+        inv_files = [inv_file.strip() for inv_file in request.config.getoption("ansible_inventory").split(",")]
+    return inv_files
+
+
 def get_host_vars(inv_files, hostname, variable=None):
     """Use ansible's InventoryManager to get value of variables defined for the specified host in the specified
     inventory files.

tests/conftest.py

@@ -20,6 +20,9 @@
 from tests.common.helpers.dut_ports import encode_dut_port_name
 from tests.common.devices import DutHosts
 from tests.common.testbed import TestbedInfo
+from tests.common.utilities import get_inventory_files, get_host_visible_vars
+from tests.common.helpers.dut_utils import is_supervisor_node, is_frontend_node
+
 
 from tests.common.connections import ConsoleHost
 
@@ -139,18 +142,25 @@ def config_logging(request):
         dataplane_logger.setLevel(logging.ERROR)
 
 
-@pytest.fixture(scope="session")
-def tbinfo(request):
+def get_tbinfo(request):
     """
-    Create and return testbed information
+    Helper function to create and return testbed information
     """
     tbname = request.config.getoption("--testbed")
     tbfile = request.config.getoption("--testbed_file")
     if tbname is None or tbfile is None:
         raise ValueError("testbed and testbed_file are required!")
 
     testbedinfo = TestbedInfo(tbfile)
-    return testbedinfo.testbed_topo.get(tbname, {})
+    return tbname, testbedinfo.testbed_topo.get(tbname, {})
+
+@pytest.fixture(scope="session")
+def tbinfo(request):
+    """
+    Create and return testbed information
+    """
+    _, testbedinfo = get_tbinfo(request)
+    return testbedinfo
 
 
 @pytest.fixture(name="duthosts", scope="session")
@@ -187,11 +197,24 @@ def duthost(duthosts, request):
 def rand_one_dut_hostname(request):
     """
     """
-    dut_hostnames = generate_params_dut_hostname(request)
+    tbname, testbedinfo = get_tbinfo(request)
+    duts_in_testbed = testbedinfo["duts"]
+    dut_hostnames = generate_params_dut_hostname(duts_in_testbed, tbname)
     if len(dut_hostnames) > 1:
         dut_hostnames = random.sample(dut_hostnames, 1)
     return dut_hostnames[0]
 
+@pytest.fixture(scope="module")
+def rand_one_frontend_dut_hostname(request):
+    """
+    """
+    tbname, testbedinfo = get_tbinfo(request)
+    duts_in_testbed = testbedinfo["duts"]
+    frontend_dut_hostnames = generate_params_frontend_hostname(request, duts_in_testbed, tbname)
+    dut_hostnames = random.sample(frontend_dut_hostnames, 1)
+    return dut_hostnames[0]
+
+
 @pytest.fixture(scope="module", autouse=True)
 def reset_critical_services_list(duthosts):
     """
@@ -561,29 +584,74 @@ def get_host_data(request, dut):
     This function parses multple inventory files and returns the dut information present in the inventory
     '''
     inv_data = None
-    inv_files = [inv_file.strip() for inv_file in request.config.getoption("ansible_inventory").split(",")]
+    inv_files = get_inventory_files(request)
     for inv_file in inv_files:
         inv_mgr = InventoryManager(loader=DataLoader(), sources=inv_file)
         if dut in inv_mgr.hosts:
             return inv_mgr.get_host(dut).get_vars()
 
     return inv_data
 
-def generate_param_asic_index(request, dut_indices, param_type):
+def generate_params_frontend_hostname(request, duts_in_testbed, tbname):
+    frontend_duts = []
+    inv_files = get_inventory_files(request)
+    for dut in duts_in_testbed:
+        if is_frontend_node(inv_files, dut):
+            frontend_duts.append(dut)
+    assert len(frontend_duts) > 0, \
+        "Test selected require at-least one frontend node, " \
+        "none of the DUTs '{}' in testbed '{}' are a supervisor node".format(duts_in_testbed, tbname)
+    return frontend_duts
+
+
+def generate_params_frontend_hostname_rand_per_hwsku(request, duts_in_testbed, tbname):
+    frontend_hosts = generate_params_frontend_hostname(request, duts_in_testbed, tbname)
+    inv_files = get_inventory_files(request)
+    # Create a list of hosts per hwsku
+    host_hwskus = {}
+    for a_host in frontend_hosts:
+        a_host_hwsku = get_host_visible_vars(inv_files, a_host, variable='hwsku')
+        if not a_host_hwsku:
+            # Lets try 'sonic_hwsku' as well
+            a_host_hwsku = get_host_visible_vars(inv_files, a_host, variable='sonic_hwsku')
+        if a_host_hwsku:
+            if a_host_hwsku not in host_hwskus:
+                host_hwskus[a_host_hwsku] = [a_host]
+            else:
+                host_hwskus[a_host_hwsku].append(a_host)
+        else:
+            pytest.fail("Test selected require a node per hwsku, but 'hwsku' for '{}' not defined in the inventory".format(a_host))
+
+    frontend_hosts_per_hwsku = []
+    for hosts in host_hwskus.values():
+        if len(hosts) == 1:
+            frontend_hosts_per_hwsku.append(hosts[0])
+        else:
+            frontend_hosts_per_hwsku.extend(random.sample(hosts, 1))
+
+    return frontend_hosts_per_hwsku
+
+
+def generate_params_supervisor_hostname(request, duts_in_testbed, tbname):
+    if len(duts_in_testbed) == 1:
+        # We have a single node - dealing with pizza box, return it
+        return [duts_in_testbed[0]]
+    inv_files = get_inventory_files(request)
+    for dut in duts_in_testbed:
+        # Expecting only a single supervisor node
+        if is_supervisor_node(inv_files, dut):
+            return [dut]
+    pytest.fail("Test selected require a supervisor node, " +
+                "none of the DUTs '{}' in testbed '{}' are a supervisor node".format(duts_in_testbed, tbname))
+
+
+def generate_param_asic_index(request, duts_in_testbed, dut_indices, param_type):
     logging.info("generating {} asic indicies for  DUT [{}] in ".format(param_type, dut_indices))
-
-    tbname = request.config.getoption("--testbed")
-    tbfile = request.config.getoption("--testbed_file")
-    if tbname is None or tbfile is None:
-        raise ValueError("testbed and testbed_file are required!")
-
-    tbinfo = TestbedInfo(tbfile)
-
     #if the params are not present treat the device as a single asic device
     asic_index_params = [DEFAULT_ASIC_ID]
 
     for dut_id in dut_indices:
-        dut = tbinfo.testbed_topo[tbname]["duts"][dut_id]
+        dut = duts_in_testbed[dut_id]
         inv_data = get_host_data(request, dut)
         if inv_data is not None:
             if param_type == ASIC_PARAM_TYPE_ALL and ASIC_PARAM_TYPE_ALL in inv_data:
@@ -594,26 +662,16 @@ def generate_param_asic_index(request, dut_indices, param_type):
                 dut_id, dut, asic_index_params))
     return asic_index_params
 
-def generate_params_dut_index(request):
-    tbname = request.config.getoption("--testbed")
-    tbfile = request.config.getoption("--testbed_file")
-    if tbname is None or tbfile is None:
-        raise ValueError("testbed and testbed_file are required!")
-    tbinfo = TestbedInfo(tbfile)
-    num_duts = len(tbinfo.testbed_topo[tbname]["duts"])
-    logging.info("Num of duts in testbed topology {}".format(num_duts))
+
+def generate_params_dut_index(duts_in_testbeds, tbname):
+    num_duts = len(duts_in_testbeds)
+    logging.info("Num of duts in testbed '{}' is {}".format(tbname, num_duts))
     return range(num_duts)
 
 
-def generate_params_dut_hostname(request):
-    tbname = request.config.getoption("--testbed")
-    tbfile = request.config.getoption("--testbed_file")
-    if tbname is None or tbfile is None:
-        raise ValueError("testbed and testbed_file are required!")
-    tbinfo = TestbedInfo(tbfile)
-    duts = tbinfo.testbed_topo[tbname]["duts"]
-    logging.info("DUTs in testbed topology: {}".format(str(duts)))
-    return duts
+def generate_params_dut_hostname(duts_in_testbed, tbname):
+    logging.info("DUTs in testbed '{}' are: {}".format(tbname, str(duts_in_testbed)))
+    return duts_in_testbed
 
 
 def generate_port_lists(request, port_scope):
@@ -722,19 +780,32 @@ def generate_priority_lists(request, prio_scope):
 def pytest_generate_tests(metafunc):
     # The topology always has atleast 1 dut
     dut_indices = [0]
-
+    tbname, testbedinfo = get_tbinfo(metafunc)
+    duts_in_testbed = testbedinfo["duts"]
     # Enumerators ("enum_dut_index", "enum_dut_hostname", "rand_one_dut_hostname") are mutually exclusive
     if "enum_dut_index" in metafunc.fixturenames:
-        dut_indices = generate_params_dut_index(metafunc)
-        metafunc.parametrize("enum_dut_index",dut_indices)
+        dut_indices = generate_params_dut_index(duts_in_testbed, tbname)
+        metafunc.parametrize("enum_dut_index", dut_indices, scope="module")
     elif "enum_dut_hostname" in metafunc.fixturenames:
-        dut_hostnames = generate_params_dut_hostname(metafunc)
-        metafunc.parametrize("enum_dut_hostname", dut_hostnames)
+        dut_hostnames = generate_params_dut_hostname(duts_in_testbed, tbname)
+        metafunc.parametrize("enum_dut_hostname", dut_hostnames, scope="module")
+    elif "enum_supervisor_dut_hostname" in metafunc.fixturenames:
+        supervisor_hosts = generate_params_supervisor_hostname(metafunc, duts_in_testbed, tbname)
+        metafunc.parametrize("enum_supervisor_dut_hostname", supervisor_hosts, scope="module")
+    elif "enum_frontend_dut_hostname" in metafunc.fixturenames:
+        frontend_hosts = generate_params_frontend_hostname(metafunc, duts_in_testbed, tbname)
+        metafunc.parametrize("enum_frontend_dut_hostname", frontend_hosts, scope="module")
+    elif "enum_rand_one_per_hwsku_frontend_hostname" in metafunc.fixturenames:
+        frontend_hosts_per_hwsku = generate_params_frontend_hostname_rand_per_hwsku(metafunc, duts_in_testbed, tbname)
+        metafunc.parametrize("enum_rand_one_per_hwsku_frontend_hostname", frontend_hosts_per_hwsku, scope="module")
+
 
     if "enum_asic_index" in metafunc.fixturenames:
-        metafunc.parametrize("enum_asic_index",generate_param_asic_index(metafunc, dut_indices, ASIC_PARAM_TYPE_ALL))
+        metafunc.parametrize("enum_asic_index",
+                             generate_param_asic_index(metafunc, duts_in_testbed, dut_indices, ASIC_PARAM_TYPE_ALL))
     if "enum_frontend_asic_index" in metafunc.fixturenames:
-        metafunc.parametrize("enum_frontend_asic_index",generate_param_asic_index(metafunc, dut_indices, ASIC_PARAM_TYPE_FRONTEND))
+        metafunc.parametrize("enum_frontend_asic_index",
+                             generate_param_asic_index(metafunc, duts_in_testbed, dut_indices, ASIC_PARAM_TYPE_FRONTEND))
 
     if "enum_dut_portname" in metafunc.fixturenames:
         metafunc.parametrize("enum_dut_portname", generate_port_lists(metafunc, "all_ports"))