From 1fa4b949257bf87037b7f8d73d8f1070627f8b1a Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Thu, 25 Feb 2021 12:09:57 -0800 Subject: [PATCH] [psushow] Add more output columns; Add option to output in JSON format (#1416) - Enhance `psushow -s` output, adding columns to display PSU Model, Serial, Voltage, Current and Power - Add `-j/--json` option to display output in JSON format - Update mock State DB PSU_INFO table to provide values which we can ensure floats are padded to two decimal places in tabular display mode - Add `--json` option to `show platform psustatus` which in turn adds the `-j` flag to the underlying `psushow` call - Add unit tests for `psushow` in psushow_test.py - Add more unit tests for `show platform psustatus` and move to show_platform_test.py --- scripts/psushow | 147 +++++++++++++++------- show/platform.py | 8 +- tests/mock_tables/state_db.json | 20 +++ tests/psu_test.py | 51 -------- tests/psushow_test.py | 213 ++++++++++++++++++++++++++++++++ tests/show_platform_test.py | 78 +++++++----- 6 files changed, 393 insertions(+), 124 deletions(-) delete mode 100644 tests/psu_test.py create mode 100644 tests/psushow_test.py diff --git a/scripts/psushow b/scripts/psushow index e46bad3697e8..b0e4dfc32ef2 100755 --- a/scripts/psushow +++ b/scripts/psushow @@ -1,24 +1,19 @@ #!/usr/bin/env python3 import argparse +import json import sys -import os + +from swsscommon.swsscommon import SonicV2Connector from tabulate import tabulate -# mock the redis for unit test purposes # -try: - if os.environ["UTILITIES_UNIT_TESTING"] == "1": - modules_path = os.path.join(os.path.dirname(__file__), "..") - test_path = os.path.join(modules_path, "tests") - sys.path.insert(0, modules_path) - sys.path.insert(0, test_path) - import mock_tables.dbconnector -except KeyError: - pass +VERSION = '1.0' -from swsscommon.swsscommon import SonicV2Connector -def psu_status_show(index): + +def get_psu_status_list(): + psu_status_list = [] + db = SonicV2Connector(host="127.0.0.1") db.connect(db.STATE_DB) @@ -27,59 +22,125 @@ def psu_status_show(index): chassis_name = "chassis {}".format(chassis_num) num_psus = db.get(db.STATE_DB, 'CHASSIS_INFO|{}'.format(chassis_name), 'psu_num') if not num_psus: - print("Error! Failed to get the number of PSUs!") - return -1 + print('Error: Failed to get the number of PSUs') + return None - supported_psu = range(1, int(num_psus) + 1) - if (index < 0): - psu_ids = supported_psu - else: - psu_ids = [index] + for psu_idx in range(1, int(num_psus) + 1): + psu_status = {} - header = ['PSU', 'Status', 'LED'] - status_table = [] + psu_status['index'] = str(psu_idx) + + psu_name = 'PSU {}'.format(psu_idx) + psu_status['name'] = psu_name - for psu in psu_ids: - msg = "" - psu_name = "PSU {}".format(psu) - if psu not in supported_psu: - print("Error! The {} is not available on the platform.\n" - "Number of supported PSU - {}.".format(psu_name, num_psus)) - continue presence = db.get(db.STATE_DB, 'PSU_INFO|{}'.format(psu_name), 'presence') + psu_status['presence'] = presence + if presence == 'true': oper_status = db.get(db.STATE_DB, 'PSU_INFO|{}'.format(psu_name), 'status') - msg = 'OK' if oper_status == 'true' else "NOT OK" + status = 'OK' if oper_status == 'true' else "NOT OK" else: - msg = 'NOT PRESENT' - led_status = db.get(db.STATE_DB, 'PSU_INFO|{}'.format(psu_name), 'led_status') - status_table.append([psu_name, msg, led_status]) + status = 'NOT PRESENT' + psu_status['status'] = status + + psu_status['led_status'] = db.get(db.STATE_DB, 'PSU_INFO|{}'.format(psu_name), 'led_status') + + psu_status['model'] = db.get(db.STATE_DB, 'PSU_INFO|{}'.format(psu_name), 'model') if presence else 'N/A' + psu_status['serial'] = db.get(db.STATE_DB, 'PSU_INFO|{}'.format(psu_name), 'serial') if presence else 'N/A' + psu_status['voltage'] = db.get(db.STATE_DB, 'PSU_INFO|{}'.format(psu_name), 'voltage') if presence else 'N/A' + psu_status['current'] = db.get(db.STATE_DB, 'PSU_INFO|{}'.format(psu_name), 'current') if presence else 'N/A' + psu_status['power'] = db.get(db.STATE_DB, 'PSU_INFO|{}'.format(psu_name), 'power') if presence else 'N/A' + + psu_status_list.append(psu_status) + + return psu_status_list + + +def psu_status_show_table(index): + psu_status_list = get_psu_status_list() + + if not psu_status_list: + print('Error: Failed to get PSU status') + return None + + header = ['PSU', 'Model', 'Serial', 'Voltage (V)', 'Current (A)', 'Power (W)', 'Status', 'LED'] + status_table = [] + + if index > 0: + if index > len(psu_status_list): + print("Error: PSU {} is not available. Number of supported PSUs: {}".format(index, len(psu_status_list))) + return -1 + + # Trim the list down to contain only the requested PSU + psu_status_list = [psu_status_list[index-1]] + + for psu_status in psu_status_list: + status_table.append([psu_status['name'], + psu_status['model'], + psu_status['serial'], + psu_status['voltage'], + psu_status['current'], + psu_status['power'], + psu_status['status'], + psu_status['led_status']]) if status_table: - print(tabulate(status_table, header, tablefmt="simple")) + print(tabulate(status_table, header, tablefmt="simple", floatfmt='.2f')) + + return 0 + + +def psu_status_show_json(index): + psu_status_list = get_psu_status_list() + + if not psu_status_list: + print('Error: Failed to get PSU status') + return None + + if index > 0: + if index > len(psu_status_list): + print("Error: PSU {} is not available. Number of supported PSUs: {}".format(index, len(psu_status_list))) + return -1 + + # Trim the list down to contain only the requested PSU + psu_status_list = [psu_status_list[index-1]] + + print(json.dumps(psu_status_list, indent=4)) return 0 + def main(): parser = argparse.ArgumentParser(description='Display the psu status information', formatter_class=argparse.RawTextHelpFormatter, epilog=""" Examples: + psushow -s + psushow -s -j psushow -s -i 1 """) - parser.add_argument('-s', '--status', action='store_true', help='show the status information') - parser.add_argument('-i', '--index', type=int, default=-1, help='the index of psu') - parser.add_argument('-v', '--version', action='version', version='%(prog)s 1.0') + parser.add_argument('-s', '--status', action='store_true', help='Show PSU status information') + parser.add_argument('-i', '--index', type=int, default=-1, help='The index of PSU to display') + parser.add_argument('-j', '--json', action='store_true', help='Display output in JSON format') + parser.add_argument('-v', '--version', action='version', version='%(prog)s {}'.format(VERSION)) args = parser.parse_args() status_show = args.status psu_index = args.index + output_json = args.json + if status_show: - err = psu_status_show(psu_index) - if err: - print("Error: fail to get psu status from state DB") - sys.exit(1) + if output_json: + ret = psu_status_show_json(psu_index) + else: + ret = psu_status_show_table(psu_index) -if __name__ == "__main__": - main() + if ret != 0: + print("Error: failed to get PSU status from state DB") + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/show/platform.py b/show/platform.py index b6f21df97928..029e28f485fa 100644 --- a/show/platform.py +++ b/show/platform.py @@ -41,7 +41,7 @@ def platform(): # 'summary' subcommand ("show platform summary") @platform.command() -@click.option('--json', is_flag=True, help="JSON output") +@click.option('--json', is_flag=True, help="Output in JSON format") def summary(json): """Show hardware platform information""" @@ -69,14 +69,18 @@ def syseeprom(verbose): # 'psustatus' subcommand ("show platform psustatus") @platform.command() @click.option('-i', '--index', default=-1, type=int, help="the index of PSU") +@click.option('--json', is_flag=True, help="Output in JSON format") @click.option('--verbose', is_flag=True, help="Enable verbose output") -def psustatus(index, verbose): +def psustatus(index, json, verbose): """Show PSU status information""" cmd = "psushow -s" if index >= 0: cmd += " -i {}".format(index) + if json: + cmd += " -j" + clicommon.run_command(cmd, display_cmd=verbose) diff --git a/tests/mock_tables/state_db.json b/tests/mock_tables/state_db.json index a1855596534c..64d685752e69 100644 --- a/tests/mock_tables/state_db.json +++ b/tests/mock_tables/state_db.json @@ -121,11 +121,31 @@ "PSU_INFO|PSU 1": { "presence": "true", "status": "true", + "model": "0J6J4K", + "serial": "CN-0J6J4K-17972-5AF-0086-A00", + "temp": "None", + "temp_threshold": "None", + "voltage": "12.19", + "voltage_min_threshold": "None", + "voltage_max_threshold": "None", + "current": "8.37", + "power": "102.7", + "is_replaceable": "False", "led_status": "green" }, "PSU_INFO|PSU 2": { "presence": "true", "status": "true", + "model": "0J6J4K", + "serial": "CN-0J6J4K-17972-5AF-008M-A00", + "temp": "None", + "temp_threshold": "None", + "voltage": "12.18", + "voltage_min_threshold": "None", + "voltage_max_threshold": "None", + "current": "10.07", + "power": "122.0", + "is_replaceable": "False", "led_status": "green" }, "EEPROM_INFO|TlvHeader": { diff --git a/tests/psu_test.py b/tests/psu_test.py deleted file mode 100644 index 1109ef279287..000000000000 --- a/tests/psu_test.py +++ /dev/null @@ -1,51 +0,0 @@ -import sys -import os -from click.testing import CliRunner - -test_path = os.path.dirname(os.path.abspath(__file__)) -modules_path = os.path.dirname(test_path) -scripts_path = os.path.join(modules_path, "scripts") -sys.path.insert(0, modules_path) - -import show.main as show - -class TestPsu(object): - @classmethod - def setup_class(cls): - print("SETUP") - os.environ["PATH"] += os.pathsep + scripts_path - os.environ["UTILITIES_UNIT_TESTING"] = "1" - - def test_no_param(self): - runner = CliRunner() - result = runner.invoke(show.cli.commands["platform"].commands["psustatus"], []) - print(result.output) - result_lines = result.output.strip('\n').split('\n') - psus = ["PSU 1", "PSU 2"] - for i, psu in enumerate(psus): - assert psu in result_lines[i+2] - header_lines = 2 - assert len(result_lines) == header_lines + len(psus) - - def test_verbose(self): - runner = CliRunner() - result = runner.invoke(show.cli.commands["platform"].commands["psustatus"], ["--verbose"]) - print(result.output) - assert result.output.split('\n')[0] == "Running command: psushow -s" - - def test_single_psu(self): - runner = CliRunner() - result = runner.invoke(show.cli.commands["platform"].commands["psustatus"], ["--index=1"]) - expected = """\ -PSU Status LED ------ -------- ----- -PSU 1 OK green -""" - assert result.output == expected - - @classmethod - def teardown_class(cls): - print("TEARDOWN") - os.environ["PATH"] = os.pathsep.join(os.environ["PATH"].split(os.pathsep)[:-1]) - os.environ["UTILITIES_UNIT_TESTING"] = "0" - diff --git a/tests/psushow_test.py b/tests/psushow_test.py new file mode 100644 index 000000000000..c5038ba6c386 --- /dev/null +++ b/tests/psushow_test.py @@ -0,0 +1,213 @@ +import importlib +import os +import sys +from unittest import mock + +import pytest +from click.testing import CliRunner + +from .mock_tables import dbconnector + +test_path = os.path.dirname(os.path.abspath(__file__)) +modules_path = os.path.dirname(test_path) +scripts_path = os.path.join(modules_path, 'scripts') +sys.path.insert(0, modules_path) + +sys.modules['sonic_platform'] = mock.MagicMock() + +# Load the file under test +psushow_path = os.path.join(scripts_path, 'psushow') +loader = importlib.machinery.SourceFileLoader('psushow', psushow_path) +spec = importlib.util.spec_from_loader(loader.name, loader) +psushow = importlib.util.module_from_spec(spec) +loader.exec_module(psushow) + +# Replace swsscommon objects with mocked objects +psushow.SonicV2Connector = dbconnector.SonicV2Connector + + +class TestPsushow(object): + def test_get_psu_status_list(self): + expected_psu_status_list = [ + { + 'index': '1', + 'name': 'PSU 1', + 'presence': 'true', + 'status': 'OK', + 'led_status': 'green', + 'model': '0J6J4K', + 'serial': 'CN-0J6J4K-17972-5AF-0086-A00', + 'voltage': '12.19', + 'current': '8.37', + 'power': '102.7' + }, + { + 'index': '2', + 'name': 'PSU 2', + 'presence': 'true', + 'status': 'OK', + 'led_status': 'green', + 'model': '0J6J4K', + 'serial': 'CN-0J6J4K-17972-5AF-008M-A00', + 'voltage': '12.18', + 'current': '10.07', + 'power': '122.0' + } + ] + + psu_status_list = psushow.get_psu_status_list() + assert psu_status_list == expected_psu_status_list + + def test_status_table(self, capsys): + expected_output = '''\ +PSU Model Serial Voltage (V) Current (A) Power (W) Status LED +----- ------- ---------------------------- ------------- ------------- ----------- -------- ----- +PSU 1 0J6J4K CN-0J6J4K-17972-5AF-0086-A00 12.19 8.37 102.70 OK green +PSU 2 0J6J4K CN-0J6J4K-17972-5AF-008M-A00 12.18 10.07 122.00 OK green +''' + for arg in ['-s', '--status']: + with mock.patch('sys.argv', ['psushow', arg]): + ret = psushow.main() + assert ret == 0 + captured = capsys.readouterr() + assert captured.out == expected_output + + expected_output = '''\ +PSU Model Serial Voltage (V) Current (A) Power (W) Status LED +----- ------- ---------------------------- ------------- ------------- ----------- -------- ----- +PSU 1 0J6J4K CN-0J6J4K-17972-5AF-0086-A00 12.19 8.37 102.70 OK green +''' + for arg in ['-s', '--status']: + with mock.patch('sys.argv', ['psushow', arg, '-i', '1']): + ret = psushow.main() + assert ret == 0 + captured = capsys.readouterr() + assert captured.out == expected_output + + expected_output = '''\ +PSU Model Serial Voltage (V) Current (A) Power (W) Status LED +----- ------- ---------------------------- ------------- ------------- ----------- -------- ----- +PSU 2 0J6J4K CN-0J6J4K-17972-5AF-008M-A00 12.18 10.07 122.00 OK green +''' + for arg in ['-s', '--status']: + with mock.patch('sys.argv', ['psushow', arg, '-i', '2']): + ret = psushow.main() + assert ret == 0 + captured = capsys.readouterr() + assert captured.out == expected_output + + # Test trying to display a non-existent PSU + expected_output = '''\ +Error: PSU 3 is not available. Number of supported PSUs: 2 +Error: failed to get PSU status from state DB +''' + for arg in ['-s', '--status']: + with mock.patch('sys.argv', ['psushow', arg, '-i', '3']): + ret = psushow.main() + assert ret == 1 + captured = capsys.readouterr() + assert captured.out == expected_output + + def test_status_json(self, capsys): + expected_output = '''\ +[ + { + "index": "1", + "name": "PSU 1", + "presence": "true", + "status": "OK", + "led_status": "green", + "model": "0J6J4K", + "serial": "CN-0J6J4K-17972-5AF-0086-A00", + "voltage": "12.19", + "current": "8.37", + "power": "102.7" + }, + { + "index": "2", + "name": "PSU 2", + "presence": "true", + "status": "OK", + "led_status": "green", + "model": "0J6J4K", + "serial": "CN-0J6J4K-17972-5AF-008M-A00", + "voltage": "12.18", + "current": "10.07", + "power": "122.0" + } +] +''' + for arg in ['-j', '--json']: + with mock.patch('sys.argv', ['psushow', '-s', arg]): + ret = psushow.main() + assert ret == 0 + captured = capsys.readouterr() + assert captured.out == expected_output + + expected_output = '''\ +[ + { + "index": "1", + "name": "PSU 1", + "presence": "true", + "status": "OK", + "led_status": "green", + "model": "0J6J4K", + "serial": "CN-0J6J4K-17972-5AF-0086-A00", + "voltage": "12.19", + "current": "8.37", + "power": "102.7" + } +] +''' + for arg in ['-j', '--json']: + with mock.patch('sys.argv', ['psushow', '-s', '-i', '1', arg]): + ret = psushow.main() + assert ret == 0 + captured = capsys.readouterr() + assert captured.out == expected_output + + expected_output = '''\ +[ + { + "index": "2", + "name": "PSU 2", + "presence": "true", + "status": "OK", + "led_status": "green", + "model": "0J6J4K", + "serial": "CN-0J6J4K-17972-5AF-008M-A00", + "voltage": "12.18", + "current": "10.07", + "power": "122.0" + } +] +''' + for arg in ['-j', '--json']: + with mock.patch('sys.argv', ['psushow', '-s', '-i', '2', arg]): + ret = psushow.main() + assert ret == 0 + captured = capsys.readouterr() + assert captured.out == expected_output + + # Test trying to display a non-existent PSU + expected_output = '''\ +Error: PSU 3 is not available. Number of supported PSUs: 2 +Error: failed to get PSU status from state DB +''' + for arg in ['-j', '--json']: + with mock.patch('sys.argv', ['psushow', '-s', '-i', '3', arg]): + ret = psushow.main() + assert ret == 1 + captured = capsys.readouterr() + assert captured.out == expected_output + + def test_version(self, capsys): + for arg in ['-v', '--version']: + with pytest.raises(SystemExit) as pytest_wrapped_e: + with mock.patch('sys.argv', ['psushow', arg]): + psushow.main() + assert pytest_wrapped_e.type == SystemExit + assert pytest_wrapped_e.value.code == 0 + captured = capsys.readouterr() + assert captured.out == 'psushow {}\n'.format(psushow.VERSION) diff --git a/tests/show_platform_test.py b/tests/show_platform_test.py index dfa6ba648ae8..937059dcde19 100644 --- a/tests/show_platform_test.py +++ b/tests/show_platform_test.py @@ -3,39 +3,29 @@ import textwrap from unittest import mock +import pytest from click.testing import CliRunner test_path = os.path.dirname(os.path.abspath(__file__)) modules_path = os.path.dirname(test_path) sys.path.insert(0, modules_path) - import show.main as show -TEST_PLATFORM = "x86_64-mlnx_msn2700-r0" -TEST_HWSKU = "Mellanox-SN2700" -TEST_ASIC_TYPE = "mellanox" +@pytest.fixture(scope='class') +def config_env(): + os.environ["UTILITIES_UNIT_TESTING"] = "1" + yield -""" - Note: The following 'show platform' commands simply call other SONiC - CLI utilities, so the unit tests for the other utilities are expected - to cover testing their functionality: + os.environ["UTILITIES_UNIT_TESTING"] = "0" - show platform fan - show platform firmware - show platform mlnx - show platform psustatus - show platform ssdhealth - show platform syseeprom - show platform temperature -""" +@pytest.mark.usefixtures('config_env') class TestShowPlatform(object): - @classmethod - def setup_class(cls): - print("SETUP") - os.environ["UTILITIES_UNIT_TESTING"] = "1" + TEST_PLATFORM = "x86_64-mlnx_msn2700-r0" + TEST_HWSKU = "Mellanox-SN2700" + TEST_ASIC_TYPE = "mellanox" # Test 'show platform summary' def test_summary(self): @@ -43,15 +33,47 @@ def test_summary(self): Platform: {} HwSKU: {} ASIC: {} - """.format(TEST_PLATFORM, TEST_HWSKU, TEST_ASIC_TYPE) + """.format(self.TEST_PLATFORM, self.TEST_HWSKU, self.TEST_ASIC_TYPE) with mock.patch("show.platform.get_hw_info_dict", - return_value={"platform": TEST_PLATFORM, "hwsku": TEST_HWSKU, "asic_type": TEST_ASIC_TYPE}): - runner = CliRunner() - result = runner.invoke(show.cli.commands["platform"].commands["summary"], []) + return_value={"platform": self.TEST_PLATFORM, "hwsku": self.TEST_HWSKU, "asic_type": self.TEST_ASIC_TYPE}): + result = CliRunner().invoke(show.cli.commands["platform"].commands["summary"], []) assert result.output == textwrap.dedent(expected_output) - @classmethod - def teardown_class(cls): - print("TEARDOWN") - os.environ["UTILITIES_UNIT_TESTING"] = "0" + +class TestShowPlatformPsu(object): + """ + Note: `show platform psustatus` simply calls the `psushow` utility and + passes a variety of options. Here we test that the utility is called + with the appropriate option(s). The functionality of the underlying + `psushow` utility is expected to be tested by a separate suite of unit tests + """ + def test_all_psus(self): + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + CliRunner().invoke(show.cli.commands['platform'].commands['psustatus'], []) + assert mock_run_command.call_count == 1 + mock_run_command.assert_called_with('psushow -s', display_cmd=False) + + def test_all_psus_json(self): + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + CliRunner().invoke(show.cli.commands['platform'].commands['psustatus'], ['--json']) + assert mock_run_command.call_count == 1 + mock_run_command.assert_called_with('psushow -s -j', display_cmd=False) + + def test_single_psu(self): + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + CliRunner().invoke(show.cli.commands['platform'].commands['psustatus'], ['--index=1']) + assert mock_run_command.call_count == 1 + mock_run_command.assert_called_with('psushow -s -i 1', display_cmd=False) + + def test_single_psu_json(self): + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + CliRunner().invoke(show.cli.commands['platform'].commands['psustatus'], ['--index=1', '--json']) + assert mock_run_command.call_count == 1 + mock_run_command.assert_called_with('psushow -s -i 1 -j', display_cmd=False) + + def test_verbose(self): + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + CliRunner().invoke(show.cli.commands['platform'].commands['psustatus'], ['--verbose']) + assert mock_run_command.call_count == 1 + mock_run_command.assert_called_with('psushow -s', display_cmd=True)