Handle different response in storage data for redfish api

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,29 @@ 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.
+
+        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"]
+        # ILO version is 2.99 might have "Controllers" key instead of "StorageControllers"
+        elif "Controllers" in storage_data:
+            storage_controllers_list = []
+            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
+                    )
+
+        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 +319,21 @@ 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_controllers_list: List[Dict] = self._find_storage_controller_data(
+                    self.redfish_obj.get(curr_storage_uri).dict
+                )
                 storage_controller_count[system_id] += len(storage_controllers_list)
 
                 # 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", "")
                     storage_controller_data_in_curr_system.append(
                         {
                             "storage_id": storage_id,
-                            "controller_id": data["MemberId"],
+                            "controller_id": controller_id,
                             "state": data["Status"]["State"],
-                            "health": data["Status"]["Health"] or "NA",
+                            "health": data["Status"].get("Health", "NA"),
                         }
                     )
 

tests/unit/test_redfish.py

@@ -499,6 +499,181 @@ 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"]
+
+        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"
+                    }
+                }
+            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"}
+                    ]
+                }
+            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 GET request to /redfish/v1/Systems/<sys_id>/
+            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_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_non_standard_storage_uri_name(
+        self, mock_get_system_ids, mock_get_collection_ids, mock_redfish_client
+    ):
+        """Test non-standard name for "Storage" in URI for storage controller and drives.
+
+        Eg: /redfish/v1/Systems/S1/Storages
+        """
+        mock_redfish_obj = Mock()
+        mock_get_system_ids.return_value = ["s1"]
+        mock_get_collection_ids.return_value = ["STOR1"]
+        mock_redfish_client.return_value = mock_redfish_obj
+
+        def mock_get_response(uri):
+            response = Mock()
+            if "Systems/s1/Storages/STOR1/Drives/d11" in uri:
+                response.dict = {
+                    "Id": "d11",
+                    "Status": {"Health": "OK", "State": "Enabled"},
+                }
+            elif "Systems/s1/Storages/STOR1" in uri:
+                response.dict = {
+                    "StorageControllers": [
+                        {
+                            "MemberId": "sc0",
+                            "Status": {"Health": "OK", "State": "Enabled"},
+                        }
+                    ],
+                    "Drives": [
+                        {"@odata.id": "/redfish/v1/Systems/s1/Storages/STOR1/Drives/d11"},
+                    ],
+                }
+            # response for GET request to /redfish/v1/Systems/<sys_id>/
+            elif "Systems" in uri:
+                response.dict = {"Storage": {"@odata.id": "/redfish/v1/Systems/sX/Storages"}}
+            return response
+
+        mock_redfish_obj.get.side_effect = mock_get_response
+
+        with RedfishHelper(Mock()) as helper:
+            sc_count, sc_data = helper.get_storage_controller_data()
+            drive_count, drive_data = helper.get_storage_drive_data()
+
+        # storage controller
+        self.assertEqual(sc_count, {"s1": 1})
+        self.assertEqual(
+            sc_data,
+            {
+                "s1": [
+                    {
+                        "storage_id": "STOR1",
+                        "controller_id": "sc0",
+                        "health": "OK",
+                        "state": "Enabled",
+                    },
+                ],
+            },
+        )
+
+        # storage drives
+        self.assertEqual(drive_count, {"s1": 1})
+        self.assertEqual(
+            drive_data,
+            {
+                "s1": [
+                    {
+                        "storage_id": "STOR1",
+                        "drive_id": "d11",
+                        "health": "OK",
+                        "state": "Enabled",
+                    },
+                ],
+            },
+        )
+
     @patch(
         "prometheus_hardware_exporter.collectors.redfish.redfish_utilities.inventory.get_chassis_ids"  # noqa: E501
     )