From c3a5c6dfd72bce6553597ac83223a7b91b890ce6 Mon Sep 17 00:00:00 2001 From: Danny Allen Date: Wed, 3 Jun 2020 14:11:33 -0700 Subject: [PATCH 1/2] [devices] Clean up device info - Deprecate get_asic_type and get_platform_info - Add facts and os_version properties to SonicHost - Clarify documentation Signed-off-by: Danny Allen --- tests/common/broadcom_data.py | 2 +- tests/common/devices.py | 136 ++++++++++++++++----------- tests/common/mellanox_data.py | 2 +- tests/common/system_utils/docker.py | 4 +- tests/copp/test_copp.py | 2 +- tests/platform_tests/test_sensors.py | 2 +- 6 files changed, 89 insertions(+), 59 deletions(-) diff --git a/tests/common/broadcom_data.py b/tests/common/broadcom_data.py index 5225d879869..8361b23eca0 100644 --- a/tests/common/broadcom_data.py +++ b/tests/common/broadcom_data.py @@ -1,2 +1,2 @@ def is_broadcom_device(dut): - return dut.get_asic_type() == "broadcom" + return dut.facts["asic_type"] == "broadcom" diff --git a/tests/common/devices.py b/tests/common/devices.py index f92697f4b3b..27e74220cdf 100644 --- a/tests/common/devices.py +++ b/tests/common/devices.py @@ -102,58 +102,103 @@ def __init__(self, ansible_adhoc, hostname): class SonicHost(AnsibleHostBase): """ - @summary: Class for SONiC switch + A remote host running SONiC. - For running ansible module on the SONiC switch + This type of host contains information about the SONiC device (device info, services, etc.), + and also provides the ability to run Ansible modules on the SONiC device. """ + + # TODO: Because people are editing this variable in a bunch of places it should probably be + # revised to "DEFAULT_CRITICAL_SERVICES", and we should make critical_services a property of + # SonicHost CRITICAL_SERVICES = ["swss", "syncd", "database", "teamd", "bgp", "pmon", "lldp", "snmp"] - def __init__(self, ansible_adhoc, hostname, gather_facts=False): + def __init__(self, ansible_adhoc, hostname): AnsibleHostBase.__init__(self, ansible_adhoc, hostname) - if gather_facts: - self.gather_facts() + self._facts = self._gather_facts() + self._os_version = self._get_os_version() + + if self.facts["num_npu"] > 1: + self._update_critical_services_for_multi_npu() - def _get_critical_services_for_multi_npu(self): + @property + def facts(self): """ - Update the critical_services with the service names for multi-npu platforms + Platform information for this SONiC device. + + Returns: + dict: A dictionary containing the device platform information. + + For example: + { + "platform": "x86_64-arista_7050_qx32s", + "hwsku": "Arista-7050-QX-32S", + "asic_type": "broadcom", + "num_npu": 1 + } """ - m_service = [] - for service in self.CRITICAL_SERVICES: - for npu in self.facts["num_npu"]: - npu_service = service+npu - m_service.insert(npu, npu_service) - self.CRITICAL_SERVICES = m_service - print self.CRITICAL_SERVICES - def _get_npu_info(self): + return self._facts + + @property + def os_version(self): + """ + The OS version running on this SONiC device. + + Returns: + str: The SONiC OS version (e.g. "20181130.31") + """ + + return self._os_version + + def _gather_facts(self): + """ + Gather facts about the platform for this SONiC device. + """ + + facts = dict() + facts.update(self._get_platform_info()) + facts["num_npu"] = self._get_npu_count(facts["platform"]) + + logging.debug("Gathered SonicHost facts: %s" % json.dumps(facts)) + return facts + + def _get_npu_count(self, platform): """ - Check if the DUT is multi-npu platfrom and store the number of npus in the facts + Gets the number of npus for this device. """ - asic_conf_file_path = os.path.join('/usr/share/sonic/device', self.facts["platform"], 'asic.conf') + + asic_conf_file_path = os.path.join("/usr/share/sonic/device", platform, "asic.conf") try: - output = self.shell('cat %s' % asic_conf_file_path)["stdout_lines"] - print output + output = self.shell("cat {}".format(asic_conf_file_path))["stdout_lines"] + logging.debug(output) + for line in output: - num_npu=line.split("=",1)[1].strip() - print "num_npu = {}".format(num_npu) - self.facts["num_npu"] = int(num_npu) + num_npu = line.split("=", 1)[1].strip() + + logging.debug("num_npu = %s" % num_npu) + return int(num_npu) except: - self.facts["num_npu"] =1 + return 1 - if self.facts["num_npu"] > 1: - self._get_critical_services_for_multi_npu + def _update_critical_services_for_multi_npu(self): + """ + Updates the critical services for this device with the services for multi-npu platforms. + """ + m_service = [] + for service in self.CRITICAL_SERVICES: + for npu in self.facts["num_npu"]: + npu_service = service + npu + m_service.insert(npu, npu_service) + self.CRITICAL_SERVICES = m_service + logging.debug(self.CRITICAL_SERVICES) - def get_platform_info(self): + def _get_platform_info(self): """ - @summary: Get the platform information of the SONiC switch. - @return: Returns a dictionary containing preperties of the platform information, for example: - { - "platform": "", - "hwsku": "", - "asic_type": "" - } + Gets platform information about this SONiC device. """ + platform_info = self.command("show platform summary")["stdout_lines"] result = {} for line in platform_info: @@ -165,15 +210,13 @@ def get_platform_info(self): result["asic_type"] = line.split(":")[1].strip() return result - def gather_facts(self): + def _get_os_version(self): """ - @summary: Gather facts of the SONiC switch and store the gathered facts in the dict type 'facts' attribute. + Gets the SONiC OS version that is running on this device. """ - self.facts = {} - platform_info = self.get_platform_info() - self.facts.update(platform_info) - self._get_npu_info() - logging.debug("SonicHost facts: %s" % json.dumps(self.facts)) + + output = self.command("sonic-cfggen -y /etc/sonic/sonic_version.yml -v build_version") + return output["stdout_lines"][0].strip() def get_service_props(self, service, props=["ActiveState", "SubState"]): """ @@ -424,9 +467,6 @@ def get_image_info(self): ret['installed_list'] = images return ret - def get_asic_type(self): - return self.facts["asic_type"] - def shutdown(self, ifname): """ Shutdown interface specified by ifname @@ -557,16 +597,6 @@ def check_bgp_session_nsf(self, neighbor_ip): return True return False - def get_version(self): - """ - Gets the SONiC version this device is running. - - Returns: - str: the firmware version number (e.g. 20181130.31) - """ - output = dut.command("sonic-cfggen -y /etc/sonic/sonic_version.yml -v build_version") - return output["stdout_lines"][0].strip() - class EosHost(AnsibleHostBase): """ @summary: Class for Eos switch diff --git a/tests/common/mellanox_data.py b/tests/common/mellanox_data.py index b2cf8b06036..adad8e8ef7e 100644 --- a/tests/common/mellanox_data.py +++ b/tests/common/mellanox_data.py @@ -552,4 +552,4 @@ } def is_mellanox_device(dut): - return dut.get_asic_type() == "mellanox" + return dut.facts["asic_type"] == "mellanox" diff --git a/tests/common/system_utils/docker.py b/tests/common/system_utils/docker.py index aecc2c3ba4d..a8869db3eb9 100644 --- a/tests/common/system_utils/docker.py +++ b/tests/common/system_utils/docker.py @@ -130,7 +130,7 @@ def swap_syncd(dut): elif is_mellanox_device(dut): vendor_id = "mlnx" else: - error_message = "\"{}\" is not currently supported".format(dut.get_asic_type()) + error_message = "\"{}\" is not currently supported".format(dut.facts["asic_type"]) _LOGGER.error(error_message) raise ValueError(error_message) @@ -176,7 +176,7 @@ def restore_default_syncd(dut): elif is_mellanox_device(dut): vendor_id = "mlnx" else: - error_message = "\"{}\" is not currently supported".format(dut.get_asic_type()) + error_message = "\"{}\" is not currently supported".format(dut.facts["asic_type"]) _LOGGER.error(error_message) raise ValueError(error_message) diff --git a/tests/copp/test_copp.py b/tests/copp/test_copp.py index 186201f690e..a714541a253 100644 --- a/tests/copp/test_copp.py +++ b/tests/copp/test_copp.py @@ -62,7 +62,7 @@ def test_policer(self, protocol, duthost, ptfhost, _copp_testbed): if protocol == "ARP" \ and is_broadcom_device(duthost) \ - and "201811" not in duthost.get_version(): + and "201811" not in duthost.os_version: pytest.xfail("ARP policy disabled on BRCM devices due to SAI bug") if protocol in ["IP2ME", "SNMP", "SSH"] and _copp_testbed.topo == "t1-lag": diff --git a/tests/platform_tests/test_sensors.py b/tests/platform_tests/test_sensors.py index 286af88d22b..768a7a8fd27 100644 --- a/tests/platform_tests/test_sensors.py +++ b/tests/platform_tests/test_sensors.py @@ -5,7 +5,7 @@ def test_sensors(duthost, creds): # Get platform name - platform = duthost.get_platform_info()['platform'] + platform = duthost.facts['platform'] # Prepare check list sensors_checks = creds['sensors_checks'] From 88a5d4112101cffdc1b67dbcc107a0c06a879b36 Mon Sep 17 00:00:00 2001 From: Danny Allen Date: Wed, 3 Jun 2020 14:31:30 -0700 Subject: [PATCH 2/2] Remove extra variable from duthost constructor --- tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 193014041aa..5a62899a1fe 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -199,7 +199,7 @@ def duthost(ansible_adhoc, testbed, request): dut_index = getattr(request.module, "dut_index", 0) assert dut_index < len(testbed["duts"]), "DUT index '{0}' is out of bound '{1}'".format(dut_index, len(testbed["duts"])) - duthost = SonicHost(ansible_adhoc, testbed["duts"][dut_index], gather_facts=True) + duthost = SonicHost(ansible_adhoc, testbed["duts"][dut_index]) if stop_ssh_timeout is not None: disable_ssh_timout(duthost)