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

Fixing bug in get_volume_inventory #6719

Merged
merged 8 commits into from
Jun 26, 2023
Merged

Fixing bug in get_volume_inventory #6719

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
65e61d4
Fixing bug in get_volume_inventory
TSKushalHPE Jun 17, 2023
c127690
Adding changelog fragment
TSKushalHPE Jun 17, 2023
8782f0c
sanity fix
TSKushalHPE Jun 17, 2023
1ea7182
Update changelogs/fragments/6719-redfish-utils-fix-for-get-volume-inv…
TSKushal Jun 19, 2023
3016c00
Updating changelog fragment
TSKushalHPE Jun 19, 2023
c495c6c
Update changelogs/fragments/6719-redfish-utils-fix-for-get-volume-inv…
TSKushal Jun 19, 2023
ecb4b9e
Updating changes as per PR comments
TSKushalHPE Jun 19, 2023
56c268f
PR comment changes
TSKushalHPE Jun 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/6719-redfish-utils-fix-for-get-volume-inventory.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- redfish_utils - fix for ``GetVolumeInventory`` (https://github.com/ansible-collections/community.general/pull/6719).
TSKushal marked this conversation as resolved.
Show resolved Hide resolved
15 changes: 6 additions & 9 deletions plugins/module_utils/redfish_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,16 +903,13 @@ def get_volume_inventory(self, systems_uri):
if response['ret'] is False:
return response
data = response['data']
controller_name = 'Controller 1'
if 'StorageControllers' in data:
sc = data['StorageControllers']
if sc:
if 'Name' in sc[0]:
controller_name = sc[0]['Name']
else:
sc_id = sc[0].get('Id', '1')
controller_name = 'Controller %s' % sc_id
if 'Name' 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.

This logic isn't correct; this pulls the name of the storage subsystem rather than of the storage controller.

Inside the Storage resource is a "StorageControllers" property; this property contains the set of storage controllers in the storage subsystem, which is what this controller_list is intended to contain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this is under a for loop which is iterating through each controller.

               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']
                    controller_name = 'Controller 1'
                    if 'StorageControllers' in data:
                        sc = data['StorageControllers']
                        if sc:
                            if 'Name' in sc[0]:
                                controller_name = sc[0]['Name']
                            else:
                                sc_id = sc[0].get('Id', '1')
                                controller_name = 'Controller %s' % sc_id
                    if 'Name' in data:

controller_name = data['Name']
else:
controller_id = data.get('Id', '1')
controller_name = 'Controller %s' % controller_id
volume_results = []
volume_list = []
if 'Volumes' in data:
# Get a list of all volumes and build respective URIs
volumes_uri = data[u'Volumes'][u'@odata.id']
Expand Down