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

Add ContainedByChassis to inventory catalog #85

Closed
wants to merge 1 commit into from

Conversation

dchanman
Copy link

@dchanman dchanman commented Aug 2, 2022

The motivation for this is part of DMTF/Redfish-Use-Case-Checkers#62, where we'd like to create a usecase checker for assigning location-based identifiers for replaceable components in a system.

The motivation for that Usecase Checker is to help codify the Google repair identifier requirements documented in https://github.com/google/ecclesia-machine-management/blob/master/ecclesia/lib/redfish/g3doc/topology.md

The Google usage requirement depends on using label to provide an identifier relative to a less ambiguous FRU in the system. The RelativeRepairableUnitUri identifies such a less ambiguous FRU.

The current standard does not narrowly define ServiceLabel, leading to usage proposals where physical hierarchy is embedded into the ServiceLabel itself: e.g. https://github.com/DMTF/Redfish/issues/5172. The overall motivation for https://github.com/google/ecclesia-machine-management/blob/master/ecclesia/lib/redfish/g3doc/topology.md is to provide an interoperable proposal to represent such desired hierarchies.

Signed-off-by: Derek Chan [email protected]

@mraineri
Copy link
Contributor

mraineri commented Aug 5, 2022

One thing that occurred to me is that the structure of the inventory is all in the context of an array of objects, where each object represents a chassis instance, which contains an array of objects per component class. I think we already have the mapping you're suggesting as part of the general structure of the inventory.

So, if we use the simple rack mounted server mockup as an example, it should produce a structure like this:

[
    {
        "ChassisName": "1U",    <--- This equates to /redfish/v1/Chassis/1U
        "Chassis": [],
        "Processors": [
            { <CPU1 info> },
            { <CPU2 info> }
            { <FPGA1 info> }
        ],
        "Memory": [
            { <DIMM1 info> },
            { <DIMM2 info> },
            { <DIMM3 info> },
            { <DIMM4 info> }
        ],
        <Other device classes>
    }
]

@dchanman dchanman force-pushed the relative-repair-only branch 2 times, most recently from 3aafd8e to 27ac15d Compare August 5, 2022 18:17
@dchanman
Copy link
Author

dchanman commented Aug 5, 2022

Thanks Mike,

I'm wondering if the structure you are describing would extend to Chassis hierarchies to simpli

For example, for a hypothetical system with multiple identical compute risers:

[
    {
        "ChassisName": "Tray",
        "Chassis": ["Backplane 1", "Backplane 2"],  <- will we have these populated?
    },
    {
        "ChassisName": "Backplane 1",
        "Chassis": ["Module 1", "Module 3"],
    },
    {
        "ChassisName": "Backplane 2",
        "Chassis": ["Module 2", "Module 4"],
    },
    {
        "ChassisName": "Module 1",
        "Processors": [...],
        "Memory": [...],
    },
    {
        "ChassisName": "Module 2",
        "Processors": [...],
        "Memory": [...],
    },
    {
        "ChassisName": "Module 3",
        "Processors": [...],
        "Memory": [...],
    },
    {
        "ChassisName": "Module 4",
        "Processors": [...],
        "Memory": [...],
    }
]

Assuming we have the above, part of the motivation for this CL is also to make it easier to look up the relationship in the opposite direction. We seem to be able to look up child Chassis, but aren't easily looking up parent Chassis.

@mraineri
Copy link
Contributor

mraineri commented Aug 5, 2022

Discussion 8/5: From the proposed usage, since all of the device objects are already assigned in a chassis object, all we need is to add the contains/containedby relationship in each of the chassis objects.

@dchanman dchanman force-pushed the relative-repair-only branch from 27ac15d to a6d3210 Compare August 16, 2022 21:40
@dchanman dchanman changed the title Add RelativeRepairableUnitUri to inventory catalog Add ContainedByChassisUri and ContainsChassisUris to inventory catalog Aug 22, 2022
@@ -207,7 +207,9 @@ def catalog_resource( context, resource, inventory, chassis_id ):
"AssetTag": resource.get( "AssetTag", None ),
"Label": resource.get( location_prop, {} ).get( "PartLocation", {} ).get( "ServiceLabel", None ),
"State": resource.get( "Status", {} ).get( "State", None ),
"Description": None
"Description": None,
"ContainedByChassisUri": None,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can move these new items up to line 60, and simply look at Contains/ContainedBy when building the outer chassis object. A component within a chassis should be placed in the right chassis object, which means you shouldn't need to track the containment path at the component level.

@dchanman dchanman force-pushed the relative-repair-only branch from a6d3210 to 8a6a764 Compare September 9, 2022 19:00
@dchanman dchanman changed the title Add ContainedByChassisUri and ContainsChassisUris to inventory catalog Add ContainedByChassis to inventory catalog Sep 9, 2022
@dchanman
Copy link
Author

dchanman commented Sep 9, 2022

Need to work on this some more.

@mraineri
Copy link
Contributor

11/18: Derek still needs to get back to testing this; will follow-up later.

@dchanman
Copy link
Author

Internally at Google we've been investigating a different way of fetching the equivalent ContainedByChassis information using other properties within Location; I don't think this pull is relevant any longer.

@dchanman dchanman closed this Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants