Skip to content

Replace generated xname BMC IDs with BMC Address to BMC ID mapping#109

Merged
davidallendj merged 4 commits into
OpenCHAMI:mainfrom
erl-hpe:add-xname-mapping
Aug 1, 2025
Merged

Replace generated xname BMC IDs with BMC Address to BMC ID mapping#109
davidallendj merged 4 commits into
OpenCHAMI:mainfrom
erl-hpe:add-xname-mapping

Conversation

@erl-hpe

@erl-hpe erl-hpe commented Jul 14, 2025

Copy link
Copy Markdown
Contributor

Description

This PR addresses issue #106. For details on the discussion that led to the current implementation, see that issue.

The question it answers is how to produce useful values for the ID field of a BMC structure given to a consumer of magellan collect results. The current implementation conjurs an xname to be used as the ID of each BMC it finds. There are a few problems with this approach:

  • xnames are HPE/Cray vendor specific IDs meaningful only within the SMD framework.
  • on-the-fly generated xnames only serve as IDs but not as locators as xnames are expected to do in the SMD framework
  • on-the-fly generated xnames are not repeatable with subsequent magellan collect runs leading to potentially incorrect data updates.

This PR implements a mechanism that allows a vendor or integrator of magellan to supply a mapping between the BMC (represented currently by its discovered IP address) and an ID value (a valid xname if the consumer is SMD, something else if the consumer is not). This mapping can be supplied as JSON data on the command line or in a YAML or JSON file. If no mapping is provided, the ID value produced is the IP address string used to discover the BMC.

This allows systems that use the SMD framework to map BMCs onto xnames accurately. It allows vendors or integrators of systems with different consumers to map BMCs onto whatever is appropriate to their consumers, and it removes the HPE/Cray specific knowledge of xnames from the magellan collect code. In the default case, the identifier produced by this approach is repeatable and locally unique, and does not require any special handling to achieve those qualities. At present, the attribute used to map BMCs to other IDs, is the IPv4 address used to discover the BMC at scan time.

NOTE: as a side issue, this work called for a way to allow the data format for the data generated by collect to be distinct from the data format used for the BMC ID mapping file (if the BMC ID mappings are in a file). Instead of adding options to the command line for this, I implemented auto-detection of data format based on the file extension (.json or .JSON is a JSON file, .yml, .yaml, .YML or .YAML is a YAML file). The file name extension, if recognized, overrides the default (JSON) format or any format specified by the -F option on the command line. If the filename extension is not recognized, the default or -F format is used.

Testing

This change was tested on an OpenCHAMI on vTDS system using an enhanced version of the OpenCHAMI Quickckstart Deployment Recipe that contains both RIE and a magellan-dicovery container that scans both the RIE network and any specified external networks for RedFish servers and collects / sends to SMD the information it finds. I tested two system configurations:

  • a system with a sushy-emulator from SushyTools running and handling four Virtual Compute nodes and a management node along with two RIE simulated RedFish endpoints,
  • a system with only two RIE simulated RedFish endpoints

The vTDS deployment mechanism now builds the BMC ID Mapping file and places it inside the magellan-discover container before launching the container and letting it run under docker-compose. For those wanting to try this for themselves either with RIE or with the magellan supplied RedFish emulator, there is a new section in the README describing how to set up a BMC ID Mapping and use it.

@davidallendj

Copy link
Copy Markdown
Collaborator

Can you put a couple of steps on how you tested this so it's reproducible by anyone who wants to take a look at this PR?

@davidallendj

Copy link
Copy Markdown
Collaborator

I'm getting an error whenever I try to run a collect.

[allend@re-head magellan-109]$ ./magellan collect -v
{"level":"error","error":"failed to parse URI: parse \"\": empty url","time":"2025-07-15T16:13:30-06:00","message":"failed to sanitize host"}
{"level":"debug","time":"2025-07-15T16:13:30-06:00","message":"one or both of --username and --password NOT passed, attempting to obtain missing credentials from secret store at secrets.json"}
{"level":"error","error":"invalid character 'm' looking for beginning of value","time":"2025-07-15T16:13:30-06:00","message":"failed to collect data"}

I think you meant to use StringVarP instead of StringVar to include a short option for the flag, but it's taking it as the default value.

@erl-hpe
erl-hpe force-pushed the add-xname-mapping branch 3 times, most recently from a04b515 to 86e0624 Compare July 16, 2025 16:25
@erl-hpe

erl-hpe commented Jul 16, 2025

Copy link
Copy Markdown
Contributor Author

I'm getting an error whenever I try to run a collect.

[allend@re-head magellan-109]$ ./magellan collect -v
{"level":"error","error":"failed to parse URI: parse \"\": empty url","time":"2025-07-15T16:13:30-06:00","message":"failed to sanitize host"}
{"level":"debug","time":"2025-07-15T16:13:30-06:00","message":"one or both of --username and --password NOT passed, attempting to obtain missing credentials from secret store at secrets.json"}
{"level":"error","error":"invalid character 'm' looking for beginning of value","time":"2025-07-15T16:13:30-06:00","message":"failed to collect data"}

I think you meant to use StringVarP instead of StringVar to include a short option for the flag, but it's taking it as the default value.

Good catch, and thanks for the analysis. I had made that as a minor change and had not had a chance to test it properly yet.

@erl-hpe

erl-hpe commented Jul 16, 2025

Copy link
Copy Markdown
Contributor Author

Can you put a couple of steps on how you tested this so it's reproducible by anyone who wants to take a look at this PR?

I will add a testing procedure to the description that shows how to generate and add a mapping file that maps from RIE information in the quickstart with RIE and magellan scenario I currently use and what I did to test that.

@erl-hpe
erl-hpe force-pushed the add-xname-mapping branch from 86e0624 to b79366d Compare July 30, 2025 15:22
@alexlovelltroy

Copy link
Copy Markdown
Member

Looks like there's some cool stuff here. We're going to need some docs in the README or man page before we can merge.

@davidallendj

Copy link
Copy Markdown
Collaborator

I agree. I think @erl-hpe is going to update the README detailing how this all works. As for man pages, I'm starting to think it might be worth having something like the ochami's man pages in magellan as well.

@erl-hpe

erl-hpe commented Jul 31, 2025

Copy link
Copy Markdown
Contributor Author

I have updated the README with information on ID Mapping and made sure that the examples for collect work correctly both with RIE and with the magellan supplied RedFish emulator. I have also fixed the help output from collect to reflect the current behavior. I think this is ready for actual review, so I am moving it from draft to proposed.

@erl-hpe
erl-hpe marked this pull request as ready for review July 31, 2025 18:09
@davidallendj
davidallendj self-requested a review July 31, 2025 18:11
Comment thread README.md Outdated
@erl-hpe
erl-hpe force-pushed the add-xname-mapping branch from fb3c300 to 3b1fec4 Compare July 31, 2025 18:20
Comment thread README.md Outdated
Comment thread README.md
Comment thread cmd/collect.go Outdated
Comment thread cmd/list.go Outdated
Comment thread internal/util/path.go
Comment thread internal/util/path.go Outdated
Comment thread pkg/collect.go Outdated

@davidallendj davidallendj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Did you have a strategy for generating IDs for BMCs without a mapping using the mappings file? I think we want to be able to include these BMCs in the collect output as well and not strictly require a mapping to each one. In other words, I want to be able to do:

magellan collect -v -o nodes.yaml

And have all the BMCs including in the output before but with non-xname ID values. Maybe it would make sense to have these IDs generated when --bmc-id-map isn't specified or also include another key in the mappings file that specifies the strategy when it is used (i.e. include BMCs with generated IDs, skip BMCs with no mappings).

I think this would make sense if SMD does not require the xname validation introduced in this PR.

@erl-hpe

erl-hpe commented Jul 31, 2025

Copy link
Copy Markdown
Contributor Author

| Did you have a strategy for generating IDs for BMCs without a mapping using the mappings file?

If you don't specify a mapping or mapping file, you get results that use the IP address for the ID field. If you specify a mapping but it is missing some of the BMCs, those BMCs are suppresed on the assumption that they are not part of the configured inventory and, therefore, not trusted as hosts on the network. Since there is no meaningful ID field for those, it makes sense to suppress them in the case where there is a mapping provided and they don't match (that way we don't have a mismatched collection of IDs in the results.

I am adding a note explaining that to the README section.

@erl-hpe
erl-hpe force-pushed the add-xname-mapping branch from 3b1fec4 to ce57d11 Compare July 31, 2025 20:08
@erl-hpe

erl-hpe commented Jul 31, 2025

Copy link
Copy Markdown
Contributor Author

I believe I have updated everything to respond to the comments I have seen so far. If there are other comments, I will respond to them as they come in.

@davidallendj davidallendj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. I tried this out following the instructions in the README both with the --bmc-id-map flag and without it and everything seemed to work like expected. If we decide we want to add more to this feature later, we can do so in another PR.

@davidallendj

davidallendj commented Jul 31, 2025

Copy link
Copy Markdown
Collaborator

@erl-hpe Can you verify your commits and then force-push?
https://docs.github.com/en/authentication/managing-commit-signature-verification

@erl-hpe
erl-hpe force-pushed the add-xname-mapping branch 3 times, most recently from 1b8ad8d to 238f1b5 Compare August 1, 2025 13:18
erl-hpe added 4 commits August 1, 2025 09:24
Signed-off-by: Eric Lund <77127214+erl-hpe@users.noreply.github.com>
Signed-off-by: Eric Lund <77127214+erl-hpe@users.noreply.github.com>
Signed-off-by: Eric Lund <77127214+erl-hpe@users.noreply.github.com>
Signed-off-by: Eric Lund <77127214+erl-hpe@users.noreply.github.com>
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.

[FEATURE]Add Some Mechanism for Deriving Real BMC XNAMEs Instead of Synthesizing Them

3 participants