Implementation of multi-DUT and multi-ASIC as per PR 2347 (#2417)

PR sonic-net/SONiC#644 introduced the HLD to support multi ASIC. In the future, multi DUT or Chassis will be supported by SONiC as well. The test infrastructure and some of the customized ansible modules need to be updated to support testing of the upcoming new architectures. This PR is implementation of PR 2347 which tried to propose how to improve the current test infrastructure to support multi-DUT and multi-ASIC systems. The target is to ensure that the existing test scripts are not broken and we can update the tests in incremental way.

This change is the implementation of PR 2347 - Add proposal for multi-DUT and multi-ASIC testing support
- Added the classes described in the PR:
  - SonicAsic - represents an asic, and implements the asic/namespace related operations to hide the complexity of handling the asic/namespace specific details.
      - For now, have added bgp_facts as an example to add 'instance_id' to the bgp_facts module call on a SonicHost.
  - MutliAsicSonicHost - a host with one or more SonicAsics.
  - DutHosts - represents all the DUT's in a testbed.
      - has 'nodes' list to represent each DUT in the testbed.

- Update duthosts fixture to return an instance of DutHosts instead of a list of SonicHosts
- Modify duthost fixture to return a MultiAsicSonicHost from duthosts.nodes

tests/common/devices.py

@@ -13,6 +13,7 @@
 import re
 import inspect
 import ipaddress
+import copy
 from multiprocessing.pool import ThreadPool
 from datetime import datetime
 
@@ -1302,6 +1303,248 @@ def execute (self, cmd) :
             eval(cmd)
 
 
+class SonicAsic(object):
+    """ This class represents an ASIC on a SONiC host. This class implements wrapper methods for ASIC/namespace related operations.
+    The purpose is to hide the complexity of handling ASIC/namespace specific details.
+    For example, passing asic_id, namespace, instance_id etc. to ansible module to deal with namespaces.
+    """
+    def __init__(self, sonichost, asic_index):
+        """ Initializing a ASIC on a SONiC host.
+
+        Args:
+            sonichost : SonicHost object to which this asic belongs
+            asic_index: ASIC / namespace id for this asic.
+        """
+        self.sonichost = sonichost
+        self.asic_index = asic_index
+
+
+    def bgp_facts(self, *module_args, **complex_args):
+        """ Wrapper method for bgp_facts ansible module.
+        If number of asics in SonicHost are more than 1, then add 'instance_id' param for this Asic
+
+        Args:
+            module_args: other ansible module args passed from the caller
+            complex_args: other ansible keyword args
+
+        Returns:
+            if SonicHost has only 1 asic, then return the bgp_facts for the global namespace, else bgp_facts for the bgp instance for my asic_index.
+        """
+        if self.sonichost.facts['num_asic'] != 1:
+            complex_args['instance_id'] = self.asic_index
+        return self.sonichost.bgp_facts(*module_args, **complex_args)
+
+
+class MultiAsicSonicHost(object):
+    """ This class represents a Multi-asic SonicHost It has two attributes:
+    sonic_host: a SonicHost instance. This object is for interacting with the SONiC host through pytest_ansible.
+    asics: a list of SonicAsic instances.
+
+    The 'duthost' fixture will return an instance of a MultiAsicSonicHost.
+    So, even a single asic pizza box is represented as a MultiAsicSonicHost with 1 SonicAsic.
+    """
+
+    def __init__(self, ansible_adhoc, hostname):
+        """ Initializing a MultiAsicSonicHost.
+
+        Args:
+            ansible_adhoc : The pytest-ansible fixture
+            hostname: Name of the host in the ansible inventory
+        """
+        self.sonichost = SonicHost(ansible_adhoc, hostname)
+        self.asics = [SonicAsic(self.sonichost, asic_index) for asic_index in range(self.sonichost.facts["num_asic"])]
+
+    def _run_on_asics(self, *module_args, **complex_args):
+        """ Run an asible module on asics based on 'asic_index' keyword in complex_args
+
+        Args:
+            module_args: other ansible module args passed from the caller
+            complex_args: other ansible keyword args
+
+        Raises:
+            ValueError:  if asic_index is specified and it is neither an int or string 'all'.
+            ValueError: if asic_index is specified and is an int, but greater than number of asics in the SonicHost
+
+        Returns:
+            if asic_index is not specified, then we return the output of the ansible module on global namespace (using SonicHost)
+            else
+                if asic_index is an int, the output of the ansible module on that asic namespace
+                    - for single asic SonicHost this would still be the same as the ansible module on the global namespace
+                else if asic_index is string 'all', then a list of ansible module output for all the asics on the SonicHost
+                    - for single asic, this would be a list of size 1.
+        """
+        if "asic_index" not in complex_args:
+            # Default ASIC/namespace
+            return getattr(self.sonichost, self.multi_asic_attr)(*module_args, **complex_args)
+        else:
+            asic_complex_args = copy.deepcopy(complex_args)
+            asic_index = asic_complex_args.pop("asic_index")
+            if type(asic_index) == int:
+                # Specific ASIC/namespace
+                if self.sonichost.facts['num_asic'] == 1:
+                    if asic_index != 0:
+                        raise ValueError("Trying to run module '{}' against asic_index '{}' on a single asic dut '{}'".format(self.multi_asic_attr, asic_index, self.sonichost.hostname))
+                return getattr(self.asics[asic_index], self.multi_asic_attr)(*module_args, **asic_complex_args)
+            elif type(asic_index) == str and asic_index.lower() == "all":
+                # All ASICs/namespace
+                return [getattr(asic, self.multi_asic_attr)(*module_args, **asic_complex_args) for asic in self.asics]
+            else:
+                raise ValueError("Argument 'asic_index' must be an int or string 'all'.")
+
+    def __getattr__(self, attr):
+        """ To support calling an ansible module on a MultiAsicSonicHost.
+
+        Args:
+            attr: attribute to get
+
+        Returns:
+            if attr doesn't start with '_' and is a method of SonicAsic, attr will be ansible module that has dependency on ASIC,
+                return the output of the ansible module on asics requested - using _run_on_asics method.
+            else
+                return the attribute from SonicHost.
+        """
+        sonic_asic_attr = getattr(SonicAsic, attr, None)
+        if not attr.startswith("_") and sonic_asic_attr and callable(sonic_asic_attr):
+            self.multi_asic_attr = attr
+            return self._run_on_asics
+        else:
+            return getattr(self.sonichost, attr)  # For backward compatibility
+
+
+class DutHosts(object):
+    """ Represents all the DUTs (nodes) in a testbed. class has 3 important attributes:
+    nodes: List of all the MultiAsicSonicHost instances for all the SONiC nodes (or cards for chassis) in a multi-dut testbed
+    frontend_nodes: subset of nodes and holds list of MultiAsicSonicHost instances for DUTs with front-panel ports (like linecards in chassis
+    supervisor_nodes: subset of nodes and holds list of MultiAsicSonicHost instances for supervisor cards.
+    """
+    class _Nodes(list):
+        """ Internal class representing a list of MultiAsicSonicHosts """
+        def _run_on_nodes(self, *module_args, **complex_args):
+            """ Delegate the call to each of the nodes, return the results in a dict."""
+            return {node.hostname: getattr(node, self.attr)(*module_args, **complex_args) for node in self}
+
+        def __getattr__(self, attr):
+            """ To support calling ansible modules on a list of MultiAsicSonicHost
+            Args:
+                attr: attribute to get
+
+            Returns:
+               a dictionary with key being the MultiAsicSonicHost's hostname, and value being the output of ansible module
+               on that MultiAsicSonicHost
+            """
+            self.attr = attr
+            return self._run_on_nodes
+
+        def __eq__(self, o):
+            """ To support eq operator on the DUTs (nodes) in the testbed """
+            return list.__eq__(o)
+
+        def __ne__(self, o):
+            """ To support ne operator on the DUTs (nodes) in the testbed """
+            return list.__ne__(o)
+
+        def __hash__(self):
+            """ To support hash operator on the DUTs (nodes) in the testbed """
+            return list.__hash__()
+
+    def __init__(self, ansible_adhoc, tbinfo):
+        """ Initialize a multi-dut testbed with all the DUT's defined in testbed info.
+
+        Args:
+            ansible_adhoc: The pytest-ansible fixture
+            tbinfo - Testbed info whose "duts" holds the hostnames for the DUT's in the multi-dut testbed.
+
+        """
+        # TODO: Initialize the nodes in parallel using multi-threads?
+        self.nodes = self._Nodes([MultiAsicSonicHost(ansible_adhoc, hostname) for hostname in tbinfo["duts"]])
+        self.supervisor_nodes = self._Nodes([node for node in self.nodes if self._is_supervisor_node(node)])
+        self.frontend_nodes = self._Nodes([node for node in self.nodes if self._is_frontend_node(node)])
+
+    def __getitem__(self, index):
+        """To support operations like duthosts[0] and duthost['sonic1_hostname']
+
+        Args:
+            index (int or string): Index or hostname of a duthost.
+
+        Raises:
+            KeyError: Raised when duthost with supplied hostname is not found.
+            IndexError: Raised when duthost with supplied index is not found.
+
+        Returns:
+            [MultiAsicSonicHost]: Returns the specified duthost in duthosts. It is an instance of MultiAsicSonicHost.
+        """
+        if type(index) == int:
+            return self.nodes[index]
+        elif type(index) == str:
+            for node in self.nodes:
+                if node.hostname == index:
+                    return node
+            raise KeyError("No node has hostname '{}'".format(index))
+        else:
+            raise IndexError("Bad index '{}'".format(index))
+
+    # Below method are to support treating an instance of DutHosts as a list
+    def __iter__(self):
+        """ To support iteration over all the DUTs (nodes) in the testbed"""
+        return iter(self.nodes)
+
+    def __len__(self):
+        """ To support length of the number of DUTs (nodes) in the testbed """
+        return len(self.nodes)
+
+    def __eq__(self, o):
+        """ To support eq operator on the DUTs (nodes) in the testbed """
+        return self.nodes.__eq__(o)
+
+    def __ne__(self, o):
+        """ To support ne operator on the DUTs (nodes) in the testbed """
+        return self.nodes.__ne__(o)
+
+    def __hash__(self):
+        """ To support hash operator on the DUTs (nodes) in the testbed """
+        return self.nodes.__hash__()
+
+    def __getattr__(self, attr):
+        """To support calling ansible modules directly on all the DUTs (nodes) in the testbed
+         Args:
+            attr: attribute to get
+
+        Returns:
+            a dictionary with key being the MultiAsicSonicHost's hostname, and value being the output of ansible module
+            on that MultiAsicSonicHost
+        """
+        return getattr(self.nodes, attr)
+
+    def _is_supervisor_node(self, node):
+        """ Is node a supervisor node
+
+        Args:
+            node: MultiAsicSonicHost object represent a DUT in the testbed.
+
+        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.
+        """
+        if 'type' in node.host.options["inventory_manager"].get_host(node.hostname).get_vars():
+            card_type = node.host.options["inventory_manager"].get_host(node.hostname).get_vars()["type"]
+            if card_type is not None and card_type == 'supervisor':
+                return True
+        return False
+
+    def _is_frontend_node(self, node):
+        """ Is not a frontend node
+        Args:
+            node: MultiAsicSonicHost object represent a DUT in the testbed.
+
+        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 node not in self.supervisor_nodes
+
+
 class FanoutHost(object):
     """
     @summary: Class for Fanout switch

tests/conftest.py

@@ -18,10 +18,11 @@
 from collections import defaultdict
 from datetime import datetime
 from tests.common.fixtures.conn_graph_facts import conn_graph_facts
-from tests.common.devices import SonicHost, Localhost
+from tests.common.devices import Localhost
 from tests.common.devices import PTFHost, EosHost, FanoutHost
 from tests.common.helpers.constants import ASIC_PARAM_TYPE_ALL, ASIC_PARAM_TYPE_FRONTEND, DEFAULT_ASIC_ID
 from tests.common.helpers.dut_ports import encode_dut_port_name
+from tests.common.devices import DutHosts
 
 logger = logging.getLogger(__name__)
 
@@ -214,7 +215,7 @@ def fixture_duthosts(ansible_adhoc, tbinfo):
         mandatory argument for the class constructors.
     @param tbinfo: fixture provides information about testbed.
     """
-    return [SonicHost(ansible_adhoc, dut) for dut in tbinfo["duts"]]
+    return DutHosts(ansible_adhoc, tbinfo)
 
 
 @pytest.fixture(scope="session")