From 37fcad76acc99bbcf41a1598b5a83f847e58bf68 Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Tue, 9 Aug 2022 22:09:24 +0000 Subject: [PATCH 1/3] static_info cached in device_info Signed-off-by: Vivek Reddy Karri --- .../sonic_py_common/device_info.py | 20 +++++-- src/sonic-py-common/tests/device_info_test.py | 60 +++++++++++++++++++ 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/src/sonic-py-common/sonic_py_common/device_info.py b/src/sonic-py-common/sonic_py_common/device_info.py index 73654b8819b8..58927abcfdc7 100644 --- a/src/sonic-py-common/sonic_py_common/device_info.py +++ b/src/sonic-py-common/sonic_py_common/device_info.py @@ -39,6 +39,10 @@ CHASSIS_INFO_MODEL_FIELD = 'model' CHASSIS_INFO_REV_FIELD = 'revision' +# Cacheable Objects +sonic_ver_info = None +is_chassis_type = None + def get_localhost_info(field, config_db=None): try: # TODO: enforce caller to provide config_db explicitly and remove its default value @@ -333,14 +337,17 @@ def get_sonic_version_info(): if not os.path.isfile(SONIC_VERSION_YAML_PATH): return None - data = {} + global sonic_ver_info + if sonic_ver_info: + return sonic_ver_info + with open(SONIC_VERSION_YAML_PATH) as stream: if yaml.__version__ >= "5.1": - data = yaml.full_load(stream) + sonic_ver_info = yaml.full_load(stream) else: - data = yaml.load(stream) + sonic_ver_info = yaml.load(stream) - return data + return sonic_ver_info def get_sonic_version_file(): if not os.path.isfile(SONIC_VERSION_YAML_PATH): @@ -437,7 +444,10 @@ def is_packet_chassis(): def is_chassis(): - return is_voq_chassis() or is_packet_chassis() + global is_chassis_type + if is_chassis_type is None: + is_chassis_type = is_voq_chassis() or is_packet_chassis() + return is_chassis_type def is_supervisor(): diff --git a/src/sonic-py-common/tests/device_info_test.py b/src/sonic-py-common/tests/device_info_test.py index 1256037ff560..d8a00aeae126 100644 --- a/src/sonic-py-common/tests/device_info_test.py +++ b/src/sonic-py-common/tests/device_info_test.py @@ -52,6 +52,32 @@ 'onie_kernel_version': '4.10.11' } +SONIC_VERISON_YML = """\ +--- +build_version: 'test_branch.1-a8fbac59d' +debian_version: '11.4' +kernel_version: '5.10.0-12-2-amd64' +asic_type: mellanox +asic_subtype: 'mellanox' +commit_id: 'a8fbac59d' +branch: 'test_branch' +release: 'master' +libswsscommon: 1.0.0 +sonic_utilities: 1.2""" + +SONIC_VERISON_YML_RESULT = { + 'build_version': 'test_branch.1-a8fbac59d', + 'debian_version': '11.4', + 'kernel_version': '5.10.0-12-2-amd64', + 'asic_type': 'mellanox', + 'asic_subtype': 'mellanox', + 'commit_id': 'a8fbac59d', + 'branch': 'test_branch', + 'release': 'master', + 'libswsscommon': '1.0.0', + 'sonic_utilities': 1.2 +} + class TestDeviceInfo(object): @pytest.fixture(scope="class", autouse=True) def sanitize_environment(self): @@ -83,6 +109,40 @@ def test_get_chassis_info(self): "revision": SonicV2Connector.TEST_REV} assert result == truth + def test_get_sonic_version(self): + with mock.patch("os.path.isfile") as mock_isfile: + mock_isfile.return_value = True + open_mocked = mock.mock_open(read_data=SONIC_VERISON_YML) + with mock.patch("{}.open".format(BUILTINS), open_mocked): + for _ in range(0,5): + assert device_info.get_sonic_version_info() == SONIC_VERISON_YML_RESULT + # Assert the file was read only once + open_mocked.assert_called_once_with(device_info.SONIC_VERSION_YAML_PATH) + + @mock.patch("sonic_py_common.device_info.get_platform_info") + def test_is_chassis(self, mock_platform_info): + mock_platform_info.return_value = {"switch_type": "npu"} + for _ in range(0,10): + assert device_info.is_chassis() == False + # assert that that get_platform_info is only hit twice + assert mock_platform_info.call_count == 2 + assert device_info.is_voq_chassis() == False + assert device_info.is_packet_chassis() == False + + # Clear Cache + device_info.is_chassis_type = None + mock_platform_info.return_value = {"switch_type": "voq"} + assert device_info.is_voq_chassis() == True + assert device_info.is_packet_chassis() == False + assert device_info.is_chassis() == True + + # Clear Cache + device_info.is_chassis_type = None + mock_platform_info.return_value = {"switch_type": "chassis-packet"} + assert device_info.is_voq_chassis() == False + assert device_info.is_packet_chassis() == True + assert device_info.is_chassis() == True + @classmethod def teardown_class(cls): print("TEARDOWN") From db765b2d95b6dc1b07c1ff7c3e2f50d5bda1c185 Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Tue, 9 Aug 2022 22:12:21 +0000 Subject: [PATCH 2/3] test updated Signed-off-by: Vivek Reddy Karri --- src/sonic-py-common/tests/device_info_test.py | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/sonic-py-common/tests/device_info_test.py b/src/sonic-py-common/tests/device_info_test.py index d8a00aeae126..c29fde808b41 100644 --- a/src/sonic-py-common/tests/device_info_test.py +++ b/src/sonic-py-common/tests/device_info_test.py @@ -109,15 +109,15 @@ def test_get_chassis_info(self): "revision": SonicV2Connector.TEST_REV} assert result == truth - def test_get_sonic_version(self): - with mock.patch("os.path.isfile") as mock_isfile: - mock_isfile.return_value = True - open_mocked = mock.mock_open(read_data=SONIC_VERISON_YML) - with mock.patch("{}.open".format(BUILTINS), open_mocked): - for _ in range(0,5): - assert device_info.get_sonic_version_info() == SONIC_VERISON_YML_RESULT - # Assert the file was read only once - open_mocked.assert_called_once_with(device_info.SONIC_VERSION_YAML_PATH) + @mock.patch("os.path.isfile") + def test_get_sonic_version(self, mock_isfile): + mock_isfile.return_value = True + open_mocked = mock.mock_open(read_data=SONIC_VERISON_YML) + with mock.patch("{}.open".format(BUILTINS), open_mocked): + for _ in range(0,5): + assert device_info.get_sonic_version_info() == SONIC_VERISON_YML_RESULT + # Assert the file was read only once + open_mocked.assert_called_once_with(device_info.SONIC_VERSION_YAML_PATH) @mock.patch("sonic_py_common.device_info.get_platform_info") def test_is_chassis(self, mock_platform_info): @@ -129,14 +129,12 @@ def test_is_chassis(self, mock_platform_info): assert device_info.is_voq_chassis() == False assert device_info.is_packet_chassis() == False - # Clear Cache device_info.is_chassis_type = None mock_platform_info.return_value = {"switch_type": "voq"} assert device_info.is_voq_chassis() == True assert device_info.is_packet_chassis() == False assert device_info.is_chassis() == True - # Clear Cache device_info.is_chassis_type = None mock_platform_info.return_value = {"switch_type": "chassis-packet"} assert device_info.is_voq_chassis() == False From a936640dd6762c6696b74faf29878b58de49418c Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Wed, 10 Aug 2022 23:32:32 +0000 Subject: [PATCH 3/3] chache platform_info instead of is_chassis Signed-off-by: Vivek Reddy Karri --- .../sonic_py_common/device_info.py | 16 ++++----- src/sonic-py-common/tests/device_info_test.py | 33 +++++++++++++++---- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/sonic-py-common/sonic_py_common/device_info.py b/src/sonic-py-common/sonic_py_common/device_info.py index 58927abcfdc7..9d66edbbb7c2 100644 --- a/src/sonic-py-common/sonic_py_common/device_info.py +++ b/src/sonic-py-common/sonic_py_common/device_info.py @@ -40,8 +40,8 @@ CHASSIS_INFO_REV_FIELD = 'revision' # Cacheable Objects -sonic_ver_info = None -is_chassis_type = None +sonic_ver_info = {} +hw_info_dict = {} def get_localhost_info(field, config_db=None): try: @@ -361,9 +361,12 @@ def get_platform_info(config_db=None): """ This function is used to get the HW info helper function """ - from .multi_asic import get_num_asics + global hw_info_dict + + if hw_info_dict: + return hw_info_dict - hw_info_dict = {} + from .multi_asic import get_num_asics version_info = get_sonic_version_info() @@ -444,10 +447,7 @@ def is_packet_chassis(): def is_chassis(): - global is_chassis_type - if is_chassis_type is None: - is_chassis_type = is_voq_chassis() or is_packet_chassis() - return is_chassis_type + return is_voq_chassis() or is_packet_chassis() def is_supervisor(): diff --git a/src/sonic-py-common/tests/device_info_test.py b/src/sonic-py-common/tests/device_info_test.py index c29fde808b41..44e0d3a5b162 100644 --- a/src/sonic-py-common/tests/device_info_test.py +++ b/src/sonic-py-common/tests/device_info_test.py @@ -122,25 +122,46 @@ def test_get_sonic_version(self, mock_isfile): @mock.patch("sonic_py_common.device_info.get_platform_info") def test_is_chassis(self, mock_platform_info): mock_platform_info.return_value = {"switch_type": "npu"} - for _ in range(0,10): - assert device_info.is_chassis() == False - # assert that that get_platform_info is only hit twice - assert mock_platform_info.call_count == 2 + assert device_info.is_chassis() == False assert device_info.is_voq_chassis() == False assert device_info.is_packet_chassis() == False - device_info.is_chassis_type = None mock_platform_info.return_value = {"switch_type": "voq"} assert device_info.is_voq_chassis() == True assert device_info.is_packet_chassis() == False assert device_info.is_chassis() == True - device_info.is_chassis_type = None mock_platform_info.return_value = {"switch_type": "chassis-packet"} assert device_info.is_voq_chassis() == False assert device_info.is_packet_chassis() == True assert device_info.is_chassis() == True + mock_platform_info.return_value = {} + assert device_info.is_voq_chassis() == False + assert device_info.is_packet_chassis() == False + assert device_info.is_chassis() == False + + @mock.patch("sonic_py_common.device_info.ConfigDBConnector", autospec=True) + @mock.patch("sonic_py_common.device_info.get_sonic_version_info") + @mock.patch("sonic_py_common.device_info.get_machine_info") + @mock.patch("sonic_py_common.device_info.get_hwsku") + def test_get_platform_info(self, mock_hwsku, mock_machine_info, mock_sonic_ver, mock_cfg_db): + mock_cfg_inst = mock_cfg_db.return_value + mock_cfg_inst.get_table.return_value = {"localhost": {"switch_type": "npu"}} + mock_sonic_ver.return_value = SONIC_VERISON_YML_RESULT + mock_machine_info.return_value = {"onie_platform" : "x86_64-mlnx_msn2700-r0"} + mock_hwsku.return_value = "Mellanox-SN2700" + for _ in range(0,5): + hw_info_dict = device_info.get_platform_info() + assert hw_info_dict["asic_type"] == "mellanox" + assert hw_info_dict["platform"] == "x86_64-mlnx_msn2700-r0" + assert hw_info_dict["hwsku"] == "Mellanox-SN2700" + assert hw_info_dict["switch_type"] == "npu" + assert mock_sonic_ver.called_once() + assert mock_machine_info.called_once() + assert mock_hwsku.called_once() + mock_cfg_inst.get_table.assert_called_once_with("DEVICE_METADATA") + @classmethod def teardown_class(cls): print("TEARDOWN")