Skip to content

Commit

Permalink
Handle different response in storage data for redfish api (#71)
Browse files Browse the repository at this point in the history 
* Remove unused attributes.

* Reorder logic in `_verify_redfish_call`.

* Handle different storage controller responses #57

* Add unit tests for both type of storage controller responses

---------

Co-authored-by: Serhat Rıfat Demircan <[email protected]>
  • Loading branch information
@chanchiwai-ray @sdemircan
chanchiwai-ray and sdemircan authored Apr 25, 2024
1 parent db40f9d commit 5f33417
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 25 deletions.
79 changes: 54 additions & 25 deletions prometheus_hardware_exporter/collectors/redfish.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from redfish import redfish_client
from redfish.rest.v1 import (
InvalidCredentialsError,
RestResponse,
RetriesExhaustedError,
SessionCreationError,
)
Expand All @@ -18,7 +17,6 @@

logger = getLogger(__name__)

# pylint: disable=too-many-instance-attributes
# pylint: disable=too-many-locals


Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
}
)

Expand Down
101 changes: 101 additions & 0 deletions tests/unit/test_redfish.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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/<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_utilities.inventory.get_chassis_ids" # noqa: E501
)
Expand Down

0 comments on commit 5f33417

Please sign in to comment.