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.

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

Disk description now contains a StorageId and a RedfishURI #8937

Merged

Conversation

pyfontan
Copy link
Contributor

@pyfontan pyfontan commented Sep 26, 2024

SUMMARY

Add a StorageID to the drives description and a RedfishURI to each disk in the redfish_facts returned by the GetDiskInventory command.

Fixes #8935

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

redfish_utils

ADDITIONAL INFORMATION

entries description before

{
    "Controller": "HPE Smart Array P816i-a SR Gen10",
    "Drives": [
        {
            "BlockSizeBytes": 512,
            "CapableSpeedGbs": 12,
            "CapacityBytes": 16000900661248,
            "EncryptionAbility": "None",
            "FailurePredicted": false,
            "HotspareType": "None",
            "Id": "16",
            "Identifiers": [
                {
                    "DurableName": "5000039D28430101",
                    "DurableNameFormat": "NAA"
                }
            ],
            "MediaType": "HDD",
            "Model": "MB016000JZYVQ",
            "Name": "16TB 12G SAS HDD",
            "PhysicalLocation": {
                "PartLocation": {
                    "LocationOrdinalValue": 1,
                    "LocationType": "Bay",
                    "ServiceLabel": "Slot=12:Port=1I:Box=1:Bay=1"
                }
            },
            "Protocol": "SAS",
            "Revision": "HPD4",
            "RotationSpeedRPM": 7200,
            "SerialNumber": "34K0A0LCFU8H",
            "Status": {
                "Health": "OK",
                "State": "Enabled"
            },
            "Volumes": [
                "/redfish/v1/Systems/1/Storage/DE07A000/Volumes/129"
            ]
        }
    ]
}

And after

{
    "Controller": "HPE Smart Array P816i-a SR Gen10",
    "Drives": [
        {
            "BlockSizeBytes": 512,
            "CapableSpeedGbs": 12,
            "CapacityBytes": 16000900661248,
            "EncryptionAbility": "None",
            "FailurePredicted": false,
            "HotspareType": "None",
            "Id": "16",
            "Identifiers": [
                {
                    "DurableName": "5000039D28430101",
                    "DurableNameFormat": "NAA"
                }
            ],
            "MediaType": "HDD",
            "Model": "MB016000JZYVQ",
            "Name": "16TB 12G SAS HDD",
            "PhysicalLocation": {
                "PartLocation": {
                    "LocationOrdinalValue": 1,
                    "LocationType": "Bay",
                    "ServiceLabel": "Slot=12:Port=1I:Box=1:Bay=1"
                }
            },
            "Protocol": "SAS",
            "RedfishURI": "/redfish/v1/Systems/1/Storage/DE07A000/Drives/16",
            "Revision": "HPD4",
            "RotationSpeedRPM": 7200,
            "SerialNumber": "34K0A0LCFU8H",
            "Status": {
                "Health": "OK",
                "State": "Enabled"
            },
            "Volumes": [
                "/redfish/v1/Systems/1/Storage/DE07A000/Volumes/129"
            ]
        }
    ],
    "StorageID": "DE07A000"
}

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request module_utils module_utils plugins plugin (any type) labels Sep 26, 2024
@@ -911,6 +913,7 @@ def get_disk_inventory(self, systems_uri):
drive_result[property] = data[property]
drive_results.append(drive_result)
drives = {'Controller': controller_name,
'StorageId': storage_id,
Copy link
Contributor

@mraineri mraineri Sep 26, 2024

Choose a reason for hiding this comment

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

It might be worth adding "RedfishURI" for the Storage resource in addition to "StorageId" since there can be different storage subsystems across multiple systems with the same "Id" property. For example, these URIs are all unique Storage resources, but they all have "1" for the Id property...

/redfish/v1/Systems/RackMount1/Storage/1
/redfish/v1/Systems/RackMount2/Storage/1
/redfish/v1/Systems/RackMount3/Storage/1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mraineri , Thank you for your review, I understand, but i have a question:
My will here is to have a storage_subsystem_id to give to the CreateVolume command of community.general.redfish_config.
In your case what storage_subsystem_id would you provide to the command ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, specifically with CreateVolume, "resource_id" is already used to point to a specific system if needed. So, at least for your usage, it's not needed today; the combination of "resource_id" and "storage_subsystem_id" will guarantee uniqueness.

Okay, so this isn't necessary at this time based on the current interfaces.

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-9 Automatically create a backport for the stable-9 branch labels Sep 26, 2024
@mraineri
Copy link
Contributor

shipit

@felixfontein felixfontein merged commit 84e0190 into ansible-collections:main Sep 27, 2024
141 checks passed
@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 27, 2024
Copy link

patchback bot commented Sep 27, 2024

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/84e0190eee332ffba6e79132c2a78483f283b659/pr-8937

Backported as #8942

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Sep 27, 2024
* Disks controller is now uniquely identified by the controller ID

* Fix typo `StorageID` to `StorageId`

* Add changelog fragments

---------

Co-authored-by: Pierre-yves FONTANIERE <[email protected]>
(cherry picked from commit 84e0190)
@felixfontein
Copy link
Collaborator

@pyfontan thanks for your contribution!
@mraineri thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Sep 28, 2024
…a StorageId and a RedfishURI (#8942)

Disk description now contains a StorageId and a RedfishURI (#8937)

* Disks controller is now uniquely identified by the controller ID

* Fix typo `StorageID` to `StorageId`

* Add changelog fragments

---------

Co-authored-by: Pierre-yves FONTANIERE <[email protected]>
(cherry picked from commit 84e0190)

Co-authored-by: Pierre-yves Fontaniere <[email protected]>
aioue pushed a commit to aioue/community.general that referenced this pull request Oct 1, 2024
…ollections#8937)

* Disks controller is now uniquely identified by the controller ID

* Fix typo `StorageID` to `StorageId`

* Add changelog fragments

---------

Co-authored-by: Pierre-yves FONTANIERE <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-9 Automatically create a backport for the stable-9 branch feature This issue/PR relates to a feature request has_issue module_utils module_utils plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redfish facts about storage devices should contain unique identifiers
4 participants