Skip to content

fix: remove appended index to component ID in parseRedfishEndpointV2 - #99

Open
davidallendj wants to merge 2 commits into
mainfrom
fix/remove-component-index
Open

fix: remove appended index to component ID in parseRedfishEndpointV2#99
davidallendj wants to merge 2 commits into
mainfrom
fix/remove-component-index

Conversation

@davidallendj

Copy link
Copy Markdown
Collaborator

…intDataV2

Pull Request Template

Thank you for your contribution! Please ensure the following before submitting:

Checklist

  • My code follows the style guidelines of this project
  • I have added/updated comments where needed
  • I have added tests that prove my fix is effective or my feature works
  • I have run make test (or equivalent) locally and all tests pass
  • DCO Sign-off: All commits are signed off (git commit -s) with my real name and email
  • REUSE Compliance:
    • Each new/modified source file has SPDX copyright and license headers
    • Any non-commentable files include a <filename>.license sidecar
    • All referenced licenses are present in the LICENSES/ directory

Description

This PR removes the append index value from the component ID created when using the parseRedfishEndpointDataV2 function. This should allow for requests to include IDs that are submitted as is without any internal modifications from SMD.

Fixes #94

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

For more info, see Contributing Guidelines.

…intDataV2

Signed-off-by: David Allen <davidallendj@gmail.com>
@davidallendj davidallendj changed the title refactor: removed appended index to component ID in parseRedfishEndpo… Removed appended index to component ID in parseRedfishEndpointV2 Jul 14, 2026
Signed-off-by: David Allen <davidallendj@gmail.com>
@synackd synackd changed the title Removed appended index to component ID in parseRedfishEndpointV2 fix: remove appended index to component ID in parseRedfishEndpointV2 Aug 27, 2026
Comment thread cmd/smd/smd-api.go
enabled := true
component := base.Component{
ID: root.ID + fmt.Sprintf("n%d", ceNum),
ID: root.ID,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't this just gonna create the component with the BMC xname? There won't be a nX at the end?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that's what it looks like.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I presume that we will want to set the xname either via Magellan or ochami static discovery, right? is there a use case where we need to let SMD set it dynamically?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The way SMD is configured you create the BMC component (x1000c1s1b1) and then the attached node(s) are x1000c1s1b1n[0-n]. The problem here is that if I add x1000c1s1b1n1 before I add x1000c1s1b1n0 then they get swapped in the RedfishEndpoints. If we're gonna add via discovery, static or magellan, then the payload is gonna have to specify the xname of the BMC and all attached nodes

@davidallendj davidallendj Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think there's a way to do that already in magellan using the BMC ID mapping feature that was added a while ago. I assumed there's a way to do it with ochami as well.

Edit: Actually I don't think there's a way to specify the nodes at all since there's no way to do that in the parsing code itself. I think we could add a code path to look for all the node names managed by a BMC or something like that. I'd have to look at the code again for the specifics though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The way SMD is configured you create the BMC component (x1000c1s1b1) and then the attached node(s) are x1000c1s1b1n[0-n]. The problem here is that if I add x1000c1s1b1n1 before I add x1000c1s1b1n0 then they get swapped in the RedfishEndpoints. If we're gonna add via discovery, static or magellan, then the payload is gonna have to specify the xname of the BMC and all attached nodes

I'm also pretty sure that ochami does this since xnames are either explicitly defined or inferred from the discovery YAML.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

BMC xname is mandatory in ochami and nodes are assigned BMCs either explicitly or implicitly.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@travisbcotton Can you try using this PR and see if it fixes your issue? If we don't need to change anything, we can go ahead and get this merged.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I mean this is creating a Node type component is it not? So it can't be the BMC xname

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah it's under the Systems loop, which createa the nodes.

Right now, there is no way to get the desired component xname from the RedfishEndpoint requests, since InventoryDetail (the structure each System uses) does not have a field for the desired xname, which is likely why this loop was put here in the first place. We should rethink if we should expose it as a field so tools like ochami and Magellan can set it on request and/or let SMD dynamically it if not provided.

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.

[Bug]: V2 API creates components by order in the array

3 participants