From 65fb7f4caf4d858cd63750cc7dc8461870c7383b Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Thu, 25 Feb 2021 21:03:30 +0000 Subject: [PATCH 1/5] [platform] Update 'show platform psustatus' tests to handle new expanded output --- .../platform_tests/cli/test_show_platform.py | 41 ++++++++++++++++++- tests/platform_tests/test_platform_info.py | 31 ++++++++++---- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/tests/platform_tests/cli/test_show_platform.py b/tests/platform_tests/cli/test_show_platform.py index e9306a7c74e..3f12493b457 100644 --- a/tests/platform_tests/cli/test_show_platform.py +++ b/tests/platform_tests/cli/test_show_platform.py @@ -9,6 +9,7 @@ # TODO: Add tests for `show platform firmware updates` # TODO: Add tests for `show platform firmware version` +import json import logging import re @@ -174,10 +175,15 @@ def test_show_platform_psustatus(duthosts, enum_supervisor_dut_hostname): logging.info("Verifying output of '{}' on '{}' ...".format(cmd, duthost.hostname)) psu_status_output_lines = duthost.command(cmd)["stdout_lines"] - psu_line_pattern = re.compile(r"PSU\s+\d+\s+(OK|NOT OK|NOT PRESENT)") + + if "201811" in duthost.os_version or "201911" in duthost.os_version: + psu_line_pattern = re.compile(r"PSU\s+\d+\s+(OK|NOT OK|NOT PRESENT)") + else: + psu_line_pattern = re.compile(r"PSU\s+\d+\s+\w+\s+\w+\s+\w+\s+\w+\s+\w+\s+(OK|NOT OK|NOT PRESENT)\s+(green|amber|red|off)") # Check that all psus are showing valid status and also at-least one PSU is OK. num_psu_ok = 0 + for line in psu_status_output_lines[2:]: psu_match = psu_line_pattern.match(line) pytest_assert(psu_match, "Unexpected PSU status output: '{}' on '{}'".format(line, duthost.hostname)) @@ -187,6 +193,39 @@ def test_show_platform_psustatus(duthosts, enum_supervisor_dut_hostname): pytest_assert(num_psu_ok > 0, " No PSU's are displayed with OK status on '{}'".format(duthost.hostname)) +def test_show_platform_psustatus_json(duthosts, rand_one_dut_hostname): + """ + @summary: Verify output of `show platform psustatus --json` + """ + duthost = duthosts[rand_one_dut_hostname] + + if "201811" in duthost.os_version or "201911" in duthost.os_version: + pytest.skip("JSON output not available in this version") + + logging.info("Check pmon daemon status") + assert check_pmon_daemon_status(duthost), "Not all pmon daemons running." + + cmd = " ".join([CMD_SHOW_PLATFORM, "psustatus", "--json"]) + + logging.info("Verifying output of '{}' ...".format(cmd)) + psu_status_output = duthost.command(cmd)["stdout"] + psu_info_list = json.loads(psu_status_output) + + # TODO: Compare against expected platform-specific output + for psu_info in psu_info_list: + pytest_assert(("index" in psu_info and + "name" in psu_info and + "presence" in psu_info and + "status" in psu_info and psu_info["status"] in ["OK", "NOT OK", "NOT PRESENT"] and + "led_status" in psu_info and psu_info["led_status"] in ["green", "amber", "red", "off"] and + "model" in psu_info and + "serial" in psu_info and + "voltage" in psu_info and + "current" in psu_info and + "power" in psu_info), + "Unexpected PSU status JSON output: '{}'".format(psu_status_output)) + + def verify_show_platform_fan_output(duthost, raw_output_lines): """ @summary: Verify output of `show platform fan`. Expected output is diff --git a/tests/platform_tests/test_platform_info.py b/tests/platform_tests/test_platform_info.py index 0a2a1bfafb8..9552d2535fd 100644 --- a/tests/platform_tests/test_platform_info.py +++ b/tests/platform_tests/test_platform_info.py @@ -4,6 +4,7 @@ This script covers the test case 'Check platform information' in the SONiC platform test plan: https://github.com/Azure/SONiC/blob/master/doc/pmon/sonic_platform_test_plan.md """ +import json import logging import re import time @@ -20,6 +21,7 @@ ] CMD_PLATFORM_PSUSTATUS = "show platform psustatus" +CMD_PLATFORM_PSUSTATUS_JSON = "{} --json".format(CMD_PLATFORM_PSUSTATUS) CMD_PLATFORM_FANSTATUS = "show platform fan" CMD_PLATFORM_TEMPER = "show platform temperature" @@ -145,7 +147,11 @@ def check_vendor_specific_psustatus(dut, psu_status_line): if dut.facts["asic_type"] in ["mellanox"]: from .mellanox.check_sysfs import check_psu_sysfs - psu_line_pattern = re.compile(r"PSU\s+(\d)+\s+(OK|NOT OK|NOT PRESENT)") + if "201811" in dut.os_version or "201911" in dut.os_version: + psu_line_pattern = re.compile(r"PSU\s+(\d)+\s+(OK|NOT OK|NOT PRESENT)") + else: + psu_line_pattern = re.compile(r"PSU\s+(\d+)\s+\w+\s+\w+\s+\w+\s+\w+\s+\w+\s+(OK|NOT OK|NOT PRESENT)\s+(green|amber|red|off)") + psu_match = psu_line_pattern.match(psu_status_line) psu_id = psu_match.group(1) psu_status = psu_match.group(2) @@ -163,16 +169,25 @@ def turn_all_outlets_on(pdu_ctrl): def check_all_psu_on(dut, psu_test_results): - cli_psu_status = dut.command(CMD_PLATFORM_PSUSTATUS) power_off_psu_list = [] - for line in cli_psu_status["stdout_lines"][2:]: - fields = line.split() - psu_test_results[fields[1]] = False - if " ".join(fields[2:]) == "NOT OK": - power_off_psu_list.append(fields[1]) + + if "201811" in dut.os_version or "201911" in dut.os_version: + cli_psu_status = dut.command(CMD_PLATFORM_PSUSTATUS) + for line in cli_psu_status["stdout_lines"][2:]: + fields = line.split() + psu_test_results[fields[1]] = False + if " ".join(fields[2:]) == "NOT OK": + power_off_psu_list.append(fields[1]) + else: + # Use JSON output + cli_psu_status = dut.command(CMD_PLATFORM_PSUSTATUS_JSON) + psu_info_list = json.loads(cli_psu_status["stdout"]) + for psu_info in psu_info_list: + if psu_info["status"] == "NOT OK": + power_off_psu_list.append(psu_info["index"]) if power_off_psu_list: - logging.warn('Power off PSU list: {}'.format(power_off_psu_list)) + logging.warn('Powered off PSUs: {}'.format(power_off_psu_list)) return len(power_off_psu_list) == 0 From 6a5f86f2b26a2bbf9936b5e1a3c76509c0d650cf Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Thu, 25 Feb 2021 23:15:02 +0000 Subject: [PATCH 2/5] Prefer pytest_assert() --- tests/platform_tests/cli/test_show_platform.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/platform_tests/cli/test_show_platform.py b/tests/platform_tests/cli/test_show_platform.py index 3f12493b457..421f4faaf06 100644 --- a/tests/platform_tests/cli/test_show_platform.py +++ b/tests/platform_tests/cli/test_show_platform.py @@ -203,7 +203,7 @@ def test_show_platform_psustatus_json(duthosts, rand_one_dut_hostname): pytest.skip("JSON output not available in this version") logging.info("Check pmon daemon status") - assert check_pmon_daemon_status(duthost), "Not all pmon daemons running." + pytest_assert(check_pmon_daemon_status(duthost), "Not all pmon daemons running.") cmd = " ".join([CMD_SHOW_PLATFORM, "psustatus", "--json"]) From ffe60fb460e940e06ae8bc3d6ab2b4c8b25c58f9 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Thu, 25 Feb 2021 23:24:38 +0000 Subject: [PATCH 3/5] Refactor ensuring all keys in list --- tests/platform_tests/cli/test_show_platform.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/tests/platform_tests/cli/test_show_platform.py b/tests/platform_tests/cli/test_show_platform.py index 421f4faaf06..b591c830b92 100644 --- a/tests/platform_tests/cli/test_show_platform.py +++ b/tests/platform_tests/cli/test_show_platform.py @@ -213,17 +213,10 @@ def test_show_platform_psustatus_json(duthosts, rand_one_dut_hostname): # TODO: Compare against expected platform-specific output for psu_info in psu_info_list: - pytest_assert(("index" in psu_info and - "name" in psu_info and - "presence" in psu_info and - "status" in psu_info and psu_info["status"] in ["OK", "NOT OK", "NOT PRESENT"] and - "led_status" in psu_info and psu_info["led_status"] in ["green", "amber", "red", "off"] and - "model" in psu_info and - "serial" in psu_info and - "voltage" in psu_info and - "current" in psu_info and - "power" in psu_info), - "Unexpected PSU status JSON output: '{}'".format(psu_status_output)) + expected_keys = ["index", "name", "presence", "status", "led_status", "model", "serial", "voltage", "current", "power"] + pytest_assert(all(key in psu_info for key in expected_keys), "Unexpected PSU status JSON output: '{}'".format(psu_status_output)) + pytest_assert(psu_info["status"] in ["OK", "NOT OK", "NOT PRESENT"], "Unexpected PSU status value: '{}'".format(psu_info["status"])) + pytest_assert(psu_info["led_status"] in ["green", "amber", "red", "off"], "Unexpected PSU led_status value: '{}'".format(psu_info["led_status"])) def verify_show_platform_fan_output(duthost, raw_output_lines): From f6f62982e50ba8d5d8fd67fc640a55d92a1df1e5 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Fri, 26 Feb 2021 00:00:15 +0000 Subject: [PATCH 4/5] Rephrase error message --- tests/platform_tests/cli/test_show_platform.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/platform_tests/cli/test_show_platform.py b/tests/platform_tests/cli/test_show_platform.py index b591c830b92..5d300f2c450 100644 --- a/tests/platform_tests/cli/test_show_platform.py +++ b/tests/platform_tests/cli/test_show_platform.py @@ -214,7 +214,7 @@ def test_show_platform_psustatus_json(duthosts, rand_one_dut_hostname): # TODO: Compare against expected platform-specific output for psu_info in psu_info_list: expected_keys = ["index", "name", "presence", "status", "led_status", "model", "serial", "voltage", "current", "power"] - pytest_assert(all(key in psu_info for key in expected_keys), "Unexpected PSU status JSON output: '{}'".format(psu_status_output)) + pytest_assert(all(key in psu_info for key in expected_keys), "Expected key(s) missing from JSON output: '{}'".format(psu_status_output)) pytest_assert(psu_info["status"] in ["OK", "NOT OK", "NOT PRESENT"], "Unexpected PSU status value: '{}'".format(psu_info["status"])) pytest_assert(psu_info["led_status"] in ["green", "amber", "red", "off"], "Unexpected PSU led_status value: '{}'".format(psu_info["led_status"])) From 1b5e7b07adc0d40cd5993cac0e0ed7079f99ec58 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Fri, 26 Feb 2021 04:42:58 +0000 Subject: [PATCH 5/5] Tweaks after merge confict --- tests/platform_tests/cli/test_show_platform.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/platform_tests/cli/test_show_platform.py b/tests/platform_tests/cli/test_show_platform.py index 5d300f2c450..d00e92f3a20 100644 --- a/tests/platform_tests/cli/test_show_platform.py +++ b/tests/platform_tests/cli/test_show_platform.py @@ -181,7 +181,7 @@ def test_show_platform_psustatus(duthosts, enum_supervisor_dut_hostname): else: psu_line_pattern = re.compile(r"PSU\s+\d+\s+\w+\s+\w+\s+\w+\s+\w+\s+\w+\s+(OK|NOT OK|NOT PRESENT)\s+(green|amber|red|off)") - # Check that all psus are showing valid status and also at-least one PSU is OK. + # Check that all PSUs are showing valid status and also at least one PSU is OK num_psu_ok = 0 for line in psu_status_output_lines[2:]: @@ -190,7 +190,8 @@ def test_show_platform_psustatus(duthosts, enum_supervisor_dut_hostname): psu_status = psu_match.group(1) if psu_status == "OK": num_psu_ok += 1 - pytest_assert(num_psu_ok > 0, " No PSU's are displayed with OK status on '{}'".format(duthost.hostname)) + + pytest_assert(num_psu_ok > 0, "No PSUs are displayed with OK status on '{}'".format(duthost.hostname)) def test_show_platform_psustatus_json(duthosts, rand_one_dut_hostname):