diff --git a/prometheus_hardware_exporter/collectors/redfish.py b/prometheus_hardware_exporter/collectors/redfish.py index 26da384..16da6f1 100644 --- a/prometheus_hardware_exporter/collectors/redfish.py +++ b/prometheus_hardware_exporter/collectors/redfish.py @@ -8,7 +8,6 @@ from redfish import redfish_client from redfish.rest.v1 import ( InvalidCredentialsError, - RestResponse, RetriesExhaustedError, SessionCreationError, ) @@ -18,7 +17,6 @@ logger = getLogger(__name__) -# pylint: disable=too-many-instance-attributes # pylint: disable=too-many-locals @@ -70,17 +68,12 @@ def _discover(host: str) -> bool: def __init__(self, config: Config) -> None: """Initialize values for class.""" - self.host = config.redfish_host - self.username = config.redfish_username - self.password = config.redfish_password - self.timeout = config.redfish_client_timeout - self.max_retry = config.redfish_client_max_retry self.redfish_obj = redfish_client( - base_url=self.host, - username=self.username, - password=self.password, - timeout=self.timeout, - max_retry=self.max_retry, + base_url=config.redfish_host, + username=config.redfish_username, + password=config.redfish_password, + timeout=config.redfish_client_timeout, + max_retry=config.redfish_client_max_retry, ) def __enter__(self) -> Self: @@ -142,11 +135,11 @@ def _verify_redfish_call(self, uri: str) -> Optional[Dict[str, Any]]: Returns None if URI isn't present. """ - resp: RestResponse = self.redfish_obj.get(uri) - if resp.status == 200: - return resp.dict - logger.debug("Not able to query from URI: %s.", uri) - return None + resp = self.redfish_obj.get(uri) + if resp.status != 200: + logger.debug("Not able to query from URI: %s.", uri) + return None + return resp.dict def get_processor_data(self) -> Tuple[Dict[str, int], Dict[str, List]]: """Return processor data and count. @@ -235,6 +228,34 @@ def _storage_root_uri(cls, system_id: str, storage_name: str) -> str: """ return cls.systems_root_uri + f"{system_id}/{storage_name}/" + def _find_storage_controller_data(self, storage_data: Dict) -> List[Dict]: + """Return storage controller list. + + Args: + storage_data: A dict obtained from a storage URI endpoint, eg: + /redfish/v1/Systems/1/Storage/XYZ123 + + Returns: + storage_controller_list: list of storage controllers for + requested storage id. + + """ + storage_controllers_list: List[Dict] = [] + if "StorageControllers" in storage_data: + storage_controllers_list = storage_data["StorageControllers"] + # There are cases we have "Controllers" key instead of "StorageControllers" + elif "Controllers" in storage_data: + for controller in storage_data["Controllers"].values(): + all_controller_data = self.redfish_obj.get(controller).dict + for controller in all_controller_data["Members"]: + storage_controllers_list.append( + self.redfish_obj.get(controller["@odata.id"]).dict + ) + else: + logger.warning("No storage controller information is found: %s", storage_data) + + return storage_controllers_list + def get_storage_controller_data(self) -> Tuple[Dict[str, int], Dict[str, List]]: """Return storage controller data and count. @@ -303,19 +324,27 @@ def get_storage_controller_data(self) -> Tuple[Dict[str, int], Dict[str, List]]: ) # list of storage controllers for that storage id - storage_controllers_list: List[Dict] = self.redfish_obj.get(curr_storage_uri).dict[ - "StorageControllers" - ] - storage_controller_count[system_id] += len(storage_controllers_list) - + storage_controllers_list: List[Dict] = self._find_storage_controller_data( + self.redfish_obj.get(curr_storage_uri).dict + ) # picking out the required data from each storage controller in the list for data in storage_controllers_list: + # "Id" is what we expect if the key being used is Controllers + controller_id = data.get("MemberId", "") or data.get("Id", "") + state = data.get("Status", {}).get("State", None) + health = data.get("Status", {}).get("Health", "NA") + if controller_id == "" or not state: # health is not required + logger.warning( + "No relavent data found in storage controller data: %s", data + ) + continue + storage_controller_count[system_id] += 1 storage_controller_data_in_curr_system.append( { "storage_id": storage_id, - "controller_id": data["MemberId"], - "state": data["Status"]["State"], - "health": data["Status"]["Health"] or "NA", + "controller_id": controller_id, + "state": state, + "health": health, } ) diff --git a/tests/unit/test_redfish.py b/tests/unit/test_redfish.py index ccea2a1..fc9292c 100644 --- a/tests/unit/test_redfish.py +++ b/tests/unit/test_redfish.py @@ -499,6 +499,107 @@ def mock_get_response(uri): }, ) + @patch("prometheus_hardware_exporter.collectors.redfish.redfish_client") + @patch( + "prometheus_hardware_exporter.collectors.redfish.redfish_utilities.collections.get_collection_ids" # noqa: E501 + ) + @patch( + "prometheus_hardware_exporter.collectors.redfish.redfish_utilities.systems.get_system_ids" + ) + def test_get_storage_controller_data_success_with_non_standard_api( + self, mock_get_system_ids, mock_get_collection_ids, mock_redfish_client + ): + mock_redfish_obj = Mock() + mock_system_ids = ["s1"] + mock_storage_ids = ["STOR1", "STOR2", "STOR3"] + + mock_get_system_ids.return_value = mock_system_ids + mock_redfish_client.return_value = mock_redfish_obj + mock_get_collection_ids.return_value = mock_storage_ids + + def mock_get_response(uri): + response = Mock() + if uri.endswith("Systems/s1/Storage/STOR1"): + response.dict = { + "Controllers": { + "@odata.id": "/redfish/v1/Systems/s1/Storage/STOR1/Controllers" + } + } + elif uri.endswith("Systems/s1/Storage/STOR2"): + response.dict = { + "Controllers": { + "@odata.id": "/redfish/v1/Systems/s1/Storage/STOR2/Controllers" + } + } + # response for non-standard api response + elif uri.endswith("Systems/s1/Storage/STOR3"): + response.dict = {"UnknownKey": {}} + elif uri.endswith("Systems/s1/Storage/STOR1/Controllers"): + response.dict = { + "Members": [ + {"@odata.id": "/redfish/v1/Systems/s1/Storage/STOR1/Controllers/sc0"} + ] + } + elif uri.endswith("Systems/s1/Storage/STOR2/Controllers"): + response.dict = { + "Members": [ + {"@odata.id": "/redfish/v1/Systems/s1/Storage/STOR2/Controllers/sc1"}, + {"@odata.id": "/redfish/v1/Systems/s1/Storage/STOR2/Controllers/sc2"}, + ] + } + elif uri.endswith("Systems/s1/Storage/STOR1/Controllers/sc0"): + response.dict = { + "Status": { + "State": "Enabled", + "Health": "OK", + }, + "Id": "sc0", + } + elif uri.endswith("Systems/s1/Storage/STOR2/Controllers/sc1"): + response.dict = { + "Status": { + "State": "Enabled", + "Health": "OK", + }, + "Id": "sc1", + } + # response for non-valid response + elif uri.endswith("Systems/s1/Storage/STOR2/Controllers/sc2"): + response.dict = {} + # response for GET request to /redfish/v1/Systems// + elif "Systems" in uri: + response.dict = {"Storage": {"@odata.id": "/redfish/v1/Systems/sX/Storage"}} + return response + + mock_redfish_obj.get.side_effect = mock_get_response + + with RedfishHelper(Mock()) as helper: + ( + storage_controller_count, + storage_controller_data, + ) = helper.get_storage_controller_data() + + self.assertEqual(storage_controller_count, {"s1": 2}) + self.assertEqual( + storage_controller_data, + { + "s1": [ + { + "storage_id": "STOR1", + "controller_id": "sc0", + "health": "OK", + "state": "Enabled", + }, + { + "storage_id": "STOR2", + "controller_id": "sc1", + "health": "OK", + "state": "Enabled", + }, + ] + }, + ) + @patch( "prometheus_hardware_exporter.collectors.redfish.redfish_utilities.inventory.get_chassis_ids" # noqa: E501 )