Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix get_disk_inventory() not using Storage resource and add more prop… #52939

Merged
merged 4 commits into from
Mar 6, 2019
Merged

Fix get_disk_inventory() not using Storage resource and add more prop… #52939

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bfffb4e
Fix get_disk_inventory() not using Storage resource and add more prop…
Feb 25, 2019
1396837
don't include null entries
Feb 25, 2019
23d00ec
Add the SimpleStorage resources as well
Feb 28, 2019
ea7ac8a
Fix misspelling of disk_result
Mar 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
89 changes: 64 additions & 25 deletions lib/ansible/module_utils/redfish_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,47 +324,86 @@ def get_storage_controller_inventory(self):
return {'ret': False, 'msg': "Storage resource not found"}

def get_disk_inventory(self):
result = {}
result = {'entries': []}
controller_list = []
disk_results = []
# Get these entries, but does not fail if not found
properties = ['Name', 'Manufacturer', 'Model', 'Status',
'CapacityBytes']
properties = ['BlockSizeBytes', 'CapableSpeedGbs', 'CapacityBytes',
'EncryptionAbility', 'EncryptionStatus',
'FailurePredicted', 'HotspareType', 'Id', 'Identifiers',
'Manufacturer', 'MediaType', 'Model', 'Name',
'PartNumber', 'PhysicalLocation', 'Protocol', 'Revision',
'RotationSpeedRPM', 'SerialNumber', 'Status']

# Find Storage service
response = self.get_request(self.root_uri + self.systems_uri)
if response['ret'] is False:
return response
data = response['data']

if 'SimpleStorage' not in data:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is valid for a ComputerSystem to have a Storage resource or a SimpleStorage resource or both. And each of these can describe Drives in the system. So we should keep the original SimpleStorage logic along with your newly added Storage logic. If there are both SimpleStorage and Stroage resoures present, the returned result should include the drive info from both.

Something like this:

    if 'SimpleStorage' not in data and 'Storage' not in data:
        return {'ret': False, 'msg': "SimpleStorage or Storage resource not found"}
    if 'SimpleStorage' in data:
        # do the original SimpleStorage logic
    if 'Storage' in data:
        # do the new Storage logic
    ```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@billdodd, should this approach be applied to #52928 as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jose-delarosa - No, I asked Mike R. about this and he said that for Storage Controllers it should just look at Storage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, I can tackle this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be resolved now.

return {'ret': False, 'msg': "SimpleStorage resource not found"}
if 'SimpleStorage' not in data and 'Storage' not in data:
return {'ret': False, 'msg': "SimpleStorage and Storage resource \
not found"}

# Get a list of all storage controllers and build respective URIs
storage_uri = data["SimpleStorage"]["@odata.id"]
response = self.get_request(self.root_uri + storage_uri)
if response['ret'] is False:
return response
result['ret'] = True
data = response['data']

for controller in data[u'Members']:
controller_list.append(controller[u'@odata.id'])
if 'Storage' in data:
# Get a list of all storage controllers and build respective URIs
storage_uri = data[u'Storage'][u'@odata.id']
response = self.get_request(self.root_uri + storage_uri)
if response['ret'] is False:
return response
result['ret'] = True
data = response['data']

for c in controller_list:
uri = self.root_uri + c
response = self.get_request(uri)
if data[u'Members']:
for controller in data[u'Members']:
controller_list.append(controller[u'@odata.id'])
for c in controller_list:
uri = self.root_uri + c
response = self.get_request(uri)
if response['ret'] is False:
return response
data = response['data']
if 'Drives' in data:
for device in data[u'Drives']:
disk_uri = self.root_uri + device[u'@odata.id']
response = self.get_request(disk_uri)
data = response['data']

disk_result = {}
for property in properties:
if property in data:
if data[property] is not None:
disk_result[property] = data[property]
disk_results.append(disk_result)
result["entries"].append(disk_results)

if 'SimpleStorage' in data:
# Get a list of all storage controllers and build respective URIs
storage_uri = data["SimpleStorage"]["@odata.id"]
response = self.get_request(self.root_uri + storage_uri)
if response['ret'] is False:
return response
result['ret'] = True
data = response['data']

for device in data[u'Devices']:
disk = {}
for property in properties:
if property in device:
disk[property] = device[property]
disk_results.append(disk)
result["entries"] = disk_results
for controller in data[u'Members']:
controller_list.append(controller[u'@odata.id'])

for c in controller_list:
uri = self.root_uri + c
response = self.get_request(uri)
if response['ret'] is False:
return response
data = response['data']

for device in data[u'Devices']:
disk_result = {}
for property in properties:
if property in device:
disk_result[property] = device[property]
disk_results.append(disk_result)
result["entries"].append(disk_results)

return result

def restart_manager_gracefully(self):
Expand Down