From d4e83ad3b6cb3e34646be1a4f11a4dad582e39b4 Mon Sep 17 00:00:00 2001 From: Nisha Agarwal Date: Thu, 12 May 2016 07:55:04 +0000 Subject: [PATCH] Fix the logic for fetching the gpu device count This patch fixes the logic for fetching the gpu device count on Gen9 and above. The Active gpu devices are not listed by RIBCL while it can be achieved via RIS. Change-Id: Iabac9ab2473848683f0ae7661aac064370c26d9b --- proliantutils/ilo/client.py | 15 +- proliantutils/ilo/ris.py | 59 +++++++ proliantutils/tests/ilo/ris_sample_outputs.py | 148 ++++++++++++++++++ proliantutils/tests/ilo/test_client.py | 27 +--- proliantutils/tests/ilo/test_ris.py | 76 ++++++++- 5 files changed, 290 insertions(+), 35 deletions(-) diff --git a/proliantutils/ilo/client.py b/proliantutils/ilo/client.py index dac3be1..bc62f1d 100644 --- a/proliantutils/ilo/client.py +++ b/proliantutils/ilo/client.py @@ -26,11 +26,13 @@ SUPPORTED_RIS_METHODS = [ 'get_current_boot_mode', 'get_host_power_status', 'get_http_boot_url', + 'get_ilo_firmware_version_as_major_minor', 'get_one_time_boot', 'get_pending_boot_mode', 'get_persistent_boot_device', 'get_product_name', 'get_secure_boot_mode', + 'get_server_capabilities', 'get_vm_status', 'hold_pwr_btn', 'insert_virtual_media', @@ -355,16 +357,9 @@ class IloClient(operations.IloOperations): :raises: IloCommandNotSupportedError, if the command is not supported on the server. """ - capabilities = {} - if 'Gen9' in self.model: - capabilities = self.ris.get_server_capabilities() - data = self.ribcl.get_host_health_data() - gpu = self.ribcl._get_number_of_gpu_devices_connected(data) - capabilities.update(gpu) - major_minor = self.ris.get_ilo_firmware_version_as_major_minor() - else: - capabilities = self.ribcl.get_server_capabilities() - major_minor = self.ribcl.get_ilo_firmware_version_as_major_minor() + capabilities = self._call_method('get_server_capabilities') + major_minor = ( + self._call_method('get_ilo_firmware_version_as_major_minor')) # NOTE(vmud213): Even if it is None, pass it on to get_nic_capacity # as we still want to try getting nic capacity through ipmitool diff --git a/proliantutils/ilo/ris.py b/proliantutils/ilo/ris.py index 8c2ea15..a7539ce 100755 --- a/proliantutils/ilo/ris.py +++ b/proliantutils/ilo/ris.py @@ -48,6 +48,19 @@ POWER_STATE = { 'OFF': 'ForceOff', } +# The PCI standards mention following categories of PCI devices as +# GPU devices. +# Base Class Code 03 indicate VGA devices +# Sub Class Code +# 00h: VGA-compatible controller +# 01h: XGA controller +# 02h: 3D controller +# 80h: Other display controller +# RIS data reports the SubclassCode in integer rather than in hexadecimal form. + +CLASSCODE_FOR_GPU_DEVICES = [3] +SUBCLASSCODE_FOR_GPU_DEVICES = [0, 1, 2, 128] + LOG = log.get_logger(__name__) @@ -376,6 +389,45 @@ class RISOperations(operations.IloOperations): ' does not exist') raise exception.IloCommandNotSupportedError(msg) + def _get_pci_devices(self): + """Gets the PCI devices. + + :returns: PCI devices list if the pci resource exist. + :raises: IloCommandNotSupportedError if the PCI resource + doesn't exist. + :raises: IloError, on an error from iLO. + """ + + system = self._get_host_details() + if ('links' in system['Oem']['Hp'] and + 'PCIDevices' in system['Oem']['Hp']['links']): + # Get the PCI URI and Settings + pci_uri = system['Oem']['Hp']['links']['PCIDevices']['href'] + status, headers, pci_device_list = self._rest_get(pci_uri) + + if status >= 300: + msg = self._get_extended_error(pci_device_list) + raise exception.IloError(msg) + + return pci_device_list + + else: + msg = ('links/PCIDevices section in ComputerSystem/Oem/Hp' + ' does not exist') + raise exception.IloCommandNotSupportedError(msg) + + def _get_gpu_pci_devices(self): + """Returns the list of gpu devices.""" + pci_device_list = self._get_pci_devices() + + gpu_list = [] + items = pci_device_list['Items'] + for item in items: + if item['ClassCode'] in CLASSCODE_FOR_GPU_DEVICES: + if item['SubclassCode'] in SUBCLASSCODE_FOR_GPU_DEVICES: + gpu_list.append(item) + return gpu_list + def _get_bios_settings_resource(self, data): """Get the BIOS settings resource.""" try: @@ -1064,6 +1116,7 @@ class RISOperations(operations.IloOperations): system['Oem']['Hp']['Bios']['Current']['VersionString']) capabilities['rom_firmware_version'] = rom_firmware_version capabilities.update(self._get_ilo_firmware_version()) + capabilities.update(self._get_number_of_gpu_devices_connected()) try: self.get_secure_boot_mode() capabilities['secure_boot'] = 'true' @@ -1546,3 +1599,9 @@ class RISOperations(operations.IloOperations): LOG.debug(self._('Flashing firmware file ... in progress %d%%'), fw_update_progress_percent) return fw_update_state, fw_update_progress_percent + + def _get_number_of_gpu_devices_connected(self): + """get the number of GPU devices connected.""" + gpu_devices = self._get_gpu_pci_devices() + gpu_devices_count = len(gpu_devices) + return {'pci_gpu_devices': gpu_devices_count} diff --git a/proliantutils/tests/ilo/ris_sample_outputs.py b/proliantutils/tests/ilo/ris_sample_outputs.py index 6e696ca..232eff7 100755 --- a/proliantutils/tests/ilo/ris_sample_outputs.py +++ b/proliantutils/tests/ilo/ris_sample_outputs.py @@ -3727,3 +3727,151 @@ UEFI_BOOTSOURCES_MISSING = """ } } """ +PCI_DEVICE_DETAILS_NO_GPU = """ +{ + "@odata.context": "/redfish/v1/$metadata#Systems/Members/1/PCIDevices", + "@odata.id": "/redfish/v1/Systems/1/PCIDevices/", + "@odata.type": "#HpServerPciDeviceCollection.HpServerPciDeviceCollection", + "Description": " PciDevices view", + "Items": [ + { + "@odata.context": "/redfish/v1/$metadata#Systems/Members/\ +1/PCIDevices/Members/$entity", + "@odata.id": "/redfish/v1/Systems/1/PCIDevices/6/", + "@odata.type": "#HpServerPciDevice.1.0.0.HpServerPciDevice", + "BusNumber": 132, + "ClassCode": 6, + "DeviceID": 34631, + "DeviceInstance": 2, + "DeviceLocation": "PCI Slot", + "DeviceNumber": 0, + "DeviceSubInstance": 1, + "DeviceType": "Other PCI Device", + "FunctionNumber": 0, + "Id": "6", + "Name": "PCIe Controller", + "SegmentNumber": 0, + "StructuredName": "PCI.Slot.2.1", + "SubclassCode": 4, + "SubsystemDeviceID": 34631, + "SubsystemVendorID": 4277, + "Type": "HpServerPciDevice.1.0.0", + "UEFIDevicePath": "PciRoot(0x1)/Pci(0x3,0x0)/Pci(0x0,0x0)", + "VendorID": 4277, + "links": { + "self": { + "href": "/rest/v1/Systems/1/PCIDevices/6" + } + } + } + ] +} +""" + +PCI_GPU_LIST = """ +[ + { + "@odata.context": "/redfish/v1/$metadata#Systems/Members/1\ +/PCIDevices/Members/$entity", + "@odata.id": "/redfish/v1/Systems/1/PCIDevices/6/", + "@odata.type": "#HpServerPciDevice.1.0.0.HpServerPciDevice", + "BusNumber": 5, + "ClassCode": 3, + "DeviceID": 26528, + "DeviceInstance": 3, + "DeviceLocation": "PCI Slot", + "DeviceNumber": 0, + "DeviceSubInstance": 1, + "DeviceType": "Other PCI Device", + "FunctionNumber": 0, + "Id": "6", + "Name": "HAWAII XTGL", + "SegmentNumber": 0, + "StructuredName": "PCI.Slot.3.1", + "SubclassCode": 128, + "SubsystemDeviceID": 821, + "SubsystemVendorID": 4098, + "Type": "HpServerPciDevice.1.0.0", + "UEFIDevicePath": "PciRoot(0x0)/Pci(0x2,0x0)/Pci(0x0,0x0)/\ +Pci(0x8,0x0)/Pci(0x0,0x0)", + "VendorID": 4098, + "links": { + "self": { + "href": "/rest/v1/Systems/1/PCIDevices/6" + } + } + } +] +""" + +PCI_DEVICE_DETAILS = """ +{ + "@odata.context": "/redfish/v1/$metadata#Systems/Members/1/PCIDevices", + "@odata.id": "/redfish/v1/Systems/1/PCIDevices/", + "@odata.type": "#HpServerPciDeviceCollection.HpServerPciDeviceCollection", + "Description": " PciDevices view", + "Items": [ + { + "@odata.context": "/redfish/v1/$metadata#Systems/Members/\ +1/PCIDevices/Members/$entity", + "@odata.id": "/redfish/v1/Systems/1/PCIDevices/6/", + "@odata.type": "#HpServerPciDevice.1.0.0.HpServerPciDevice", + "BusNumber": 132, + "ClassCode": 6, + "DeviceID": 34631, + "DeviceInstance": 2, + "DeviceLocation": "PCI Slot", + "DeviceNumber": 0, + "DeviceSubInstance": 1, + "DeviceType": "Other PCI Device", + "FunctionNumber": 0, + "Id": "6", + "Name": "PCIe Controller", + "SegmentNumber": 0, + "StructuredName": "PCI.Slot.2.1", + "SubclassCode": 4, + "SubsystemDeviceID": 34631, + "SubsystemVendorID": 4277, + "Type": "HpServerPciDevice.1.0.0", + "UEFIDevicePath": "PciRoot(0x1)/Pci(0x3,0x0)/Pci(0x0,0x0)", + "VendorID": 4277, + "links": { + "self": { + "href": "/rest/v1/Systems/1/PCIDevices/6" + } + } + }, + { + "@odata.context": "/redfish/v1/$metadata#Systems/Members/1\ +/PCIDevices/Members/$entity", + "@odata.id": "/redfish/v1/Systems/1/PCIDevices/6/", + "@odata.type": "#HpServerPciDevice.1.0.0.HpServerPciDevice", + "BusNumber": 5, + "ClassCode": 3, + "DeviceID": 26528, + "DeviceInstance": 3, + "DeviceLocation": "PCI Slot", + "DeviceNumber": 0, + "DeviceSubInstance": 1, + "DeviceType": "Other PCI Device", + "FunctionNumber": 0, + "Id": "6", + "Name": "HAWAII XTGL", + "SegmentNumber": 0, + "StructuredName": "PCI.Slot.3.1", + "SubclassCode": 128, + "SubsystemDeviceID": 821, + "SubsystemVendorID": 4098, + "Type": "HpServerPciDevice.1.0.0", + "UEFIDevicePath": "PciRoot(0x0)/Pci(0x2,0x0)/Pci(0x0,0x0)/\ +Pci(0x8,0x0)/Pci(0x0,0x0)", + "VendorID": 4098, + "links": { + "self": { + "href": "/rest/v1/Systems/1/PCIDevices/6" + } + } + } + ] +} +""" diff --git a/proliantutils/tests/ilo/test_client.py b/proliantutils/tests/ilo/test_client.py index 3391616..9a3a303 100644 --- a/proliantutils/tests/ilo/test_client.py +++ b/proliantutils/tests/ilo/test_client.py @@ -14,8 +14,6 @@ # under the License. """Test class for Client Module.""" -import json - import mock import testtools @@ -23,7 +21,6 @@ from proliantutils.ilo import client from proliantutils.ilo import ipmi from proliantutils.ilo import ribcl from proliantutils.ilo import ris -from proliantutils.tests.ilo import ribcl_sample_outputs as constants class IloClientInitTestCase(testtools.TestCase): @@ -330,24 +327,17 @@ class IloClientTestCase(testtools.TestCase): @mock.patch.object(ris.RISOperations, 'get_ilo_firmware_version_as_major_minor') - @mock.patch.object(ribcl.RIBCLOperations, 'get_host_health_data') - @mock.patch.object(ribcl.RIBCLOperations, - '_get_number_of_gpu_devices_connected') @mock.patch.object(ipmi, 'get_nic_capacity') @mock.patch.object(ris.RISOperations, 'get_server_capabilities') def test_get_server_capabilities_no_nic_Gen9(self, cap_mock, nic_mock, - gpu_mock, host_mock, mm_mock): - info = {'address': "1.2.3.4", 'username': "admin", 'password': "Admin"} - data = constants.GET_EMBEDDED_HEALTH_OUTPUT - json_data = json.loads(data) - host_mock.return_value = json_data + mm_mock): str_val = mm_mock.return_value = '2.10' self.client.model = 'Gen9' nic_mock.return_value = None - gpu_mock.return_value = {'pci_gpu_devices': 2} cap_mock.return_value = {'ilo_firmware_version': '2.10', 'rom_firmware_version': 'x', 'server_model': 'Gen9', + 'pci_gpu_devices': 2, 'secure_boot': 'true'} capabilities = self.client.get_server_capabilities() cap_mock.assert_called_once_with() @@ -358,28 +348,20 @@ class IloClientTestCase(testtools.TestCase): 'pci_gpu_devices': 2, 'secure_boot': 'true'} self.assertEqual(expected_capabilities, capabilities) - self.assertEqual(info, self.client.info) @mock.patch.object(ris.RISOperations, 'get_ilo_firmware_version_as_major_minor') - @mock.patch.object(ribcl.RIBCLOperations, 'get_host_health_data') - @mock.patch.object(ribcl.RIBCLOperations, - '_get_number_of_gpu_devices_connected') @mock.patch.object(ipmi, 'get_nic_capacity') @mock.patch.object(ris.RISOperations, 'get_server_capabilities') def test_get_server_capabilities_Gen9(self, cap_mock, nic_mock, - gpu_mock, host_mock, mm_mock): - info = {'address': "1.2.3.4", 'username': "admin", 'password': "Admin"} - data = constants.GET_EMBEDDED_HEALTH_OUTPUT - json_data = json.loads(data) - host_mock.return_value = json_data + mm_mock): str_val = mm_mock.return_value = '2.10' self.client.model = 'Gen9' - gpu_mock.return_value = {'pci_gpu_devices': 2} nic_mock.return_value = '10Gb' cap_mock.return_value = {'ilo_firmware_version': '2.10', 'rom_firmware_version': 'x', 'server_model': 'Gen9', + 'pci_gpu_devices': 2, 'secure_boot': 'true'} capabilities = self.client.get_server_capabilities() cap_mock.assert_called_once_with() @@ -391,7 +373,6 @@ class IloClientTestCase(testtools.TestCase): 'secure_boot': 'true', 'nic_capacity': '10Gb'} self.assertEqual(expected_capabilities, capabilities) - self.assertEqual(info, self.client.info) @mock.patch.object(client.IloClient, '_call_method') def test_activate_license(self, call_mock): diff --git a/proliantutils/tests/ilo/test_ris.py b/proliantutils/tests/ilo/test_ris.py index dd395ce..38179b3 100755 --- a/proliantutils/tests/ilo/test_ris.py +++ b/proliantutils/tests/ilo/test_ris.py @@ -386,19 +386,23 @@ class IloRisTestCase(testtools.TestCase): validate_mock.assert_called_once_with(ris_outputs.GET_HEADERS, settings_uri) + @mock.patch.object(ris.RISOperations, + '_get_number_of_gpu_devices_connected') @mock.patch.object(ris.RISOperations, 'get_secure_boot_mode') @mock.patch.object(ris.RISOperations, '_get_ilo_firmware_version') @mock.patch.object(ris.RISOperations, '_get_host_details') def test_get_server_capabilities(self, get_details_mock, ilo_firm_mock, - secure_mock): + secure_mock, gpu_mock): host_details = json.loads(ris_outputs.RESPONSE_BODY_FOR_REST_OP) get_details_mock.return_value = host_details ilo_firm_mock.return_value = {'ilo_firmware_version': 'iLO 4 v2.20'} + gpu_mock.return_value = {'pci_gpu_devices': 2} secure_mock.return_value = False expected_caps = {'secure_boot': 'true', 'ilo_firmware_version': 'iLO 4 v2.20', 'rom_firmware_version': u'I36 v1.40 (01/28/2015)', - 'server_model': u'ProLiant BL460c Gen9'} + 'server_model': u'ProLiant BL460c Gen9', + 'pci_gpu_devices': 2} capabilities = self.client.get_server_capabilities() self.assertEqual(expected_caps, capabilities) @@ -1680,6 +1684,74 @@ class TestRISOperationsPrivateMethods(testtools.TestCase): check_bios_mock.assert_called_once_with() boot_mock.assert_called_once_with(bios_settings) + @mock.patch.object(ris.RISOperations, '_rest_get') + @mock.patch.object(ris.RISOperations, '_get_host_details') + def test__get_pci_devices(self, get_host_details_mock, get_mock): + system_data = json.loads(ris_outputs.RESPONSE_BODY_FOR_REST_OP) + get_host_details_mock.return_value = system_data + pci_uri = '/rest/v1/Systems/1/PCIDevices' + pci_device_list = json.loads(ris_outputs.PCI_DEVICE_DETAILS) + + get_mock.return_value = (200, ris_outputs.GET_HEADERS, + pci_device_list) + self.client._get_pci_devices() + get_mock.assert_called_once_with(pci_uri) + + @mock.patch.object(ris.RISOperations, '_rest_get') + @mock.patch.object(ris.RISOperations, '_get_host_details') + def test__get_pci_devices_fail(self, get_host_details_mock, + get_mock): + system_data = json.loads(ris_outputs.RESPONSE_BODY_FOR_REST_OP) + get_host_details_mock.return_value = system_data + pci_uri = '/rest/v1/Systems/1/PCIDevices' + pci_device_list = json.loads(ris_outputs.PCI_DEVICE_DETAILS) + get_mock.return_value = (301, ris_outputs.GET_HEADERS, + pci_device_list) + self.assertRaises(exception.IloError, + self.client._get_pci_devices) + get_mock.assert_called_once_with(pci_uri) + + @mock.patch.object(ris.RISOperations, '_get_host_details') + def test__get_pci_devices_not_supported(self, get_details_mock): + host_response = json.loads(ris_outputs.RESPONSE_BODY_FOR_REST_OP) + del host_response['Oem']['Hp']['links']['PCIDevices'] + get_details_mock.return_value = host_response + self.assertRaises(exception.IloCommandNotSupportedError, + self.client._get_pci_devices) + get_details_mock.assert_called_once_with() + + @mock.patch.object(ris.RISOperations, '_get_pci_devices') + def test__get_gpu_pci_devices(self, pci_mock): + pci_mock.return_value = json.loads(ris_outputs.PCI_DEVICE_DETAILS) + pci_gpu_list = self.client._get_gpu_pci_devices() + self.assertEqual(pci_gpu_list, json.loads(ris_outputs.PCI_GPU_LIST)) + self.assertTrue(pci_mock.called) + + @mock.patch.object(ris.RISOperations, '_get_pci_devices') + def test__get_gpu_pci_devices_returns_empty(self, pci_mock): + pci_response = json.loads(ris_outputs.PCI_DEVICE_DETAILS_NO_GPU) + pci_mock.return_value = pci_response + pci_gpu_list = self.client._get_gpu_pci_devices() + self.assertEqual(len(pci_gpu_list), 0) + self.assertTrue(pci_mock.called) + + @mock.patch.object(ris.RISOperations, '_get_pci_devices') + def test__get_gpu_pci_devices_fail_not_supported_error(self, pci_mock): + msg = ('links/PCIDevices section in ComputerSystem/Oem/Hp' + ' does not exist') + pci_mock.side_effect = exception.IloCommandNotSupportedError(msg) + self.assertRaises(exception.IloCommandNotSupportedError, + self.client._get_gpu_pci_devices) + self.assertTrue(pci_mock.called) + + @mock.patch.object(ris.RISOperations, '_get_gpu_pci_devices') + def test__get_number_of_gpu_devices_connected(self, gpu_list_mock): + gpu_list_mock.return_value = json.loads(ris_outputs.PCI_GPU_LIST) + expected_gpu_count = {'pci_gpu_devices': 1} + gpu_count_returned = self.client._get_number_of_gpu_devices_connected() + self.assertEqual(gpu_count_returned, expected_gpu_count) + self.assertTrue(gpu_list_mock.called) + @mock.patch.object(ris.RISOperations, '_get_ilo_details', autospec=True) def test__get_firmware_update_service_resource_traverses_manager_as( self, _get_ilo_details_mock):