Skip to content

Feature/rfd 133 explore - #133

Draft
alexlovelltroy wants to merge 11 commits into
mainfrom
feature/RFD-133-Explore
Draft

Feature/rfd 133 explore#133
alexlovelltroy wants to merge 11 commits into
mainfrom
feature/RFD-133-Explore

Conversation

@alexlovelltroy

@alexlovelltroy alexlovelltroy commented Jun 12, 2026

Copy link
Copy Markdown
Member

See OpenCHAMI/roadmap#133 for discussion.

Draft until #180 is merged

This pull request introduces a new vendor-agnostic BMC (Baseboard Management Controller) client abstraction layer, enabling vendor-specific handling and plugin registration for BMC operations. It provides a unified interface for BMC interactions, a registry for vendor plugin detection and dispatch, and a manager for connection/session management. Comprehensive tests are included for all new components. The most important changes are as follows:

BMC Client Abstraction and Vendor Plugin System:

  • Introduced a new Client interface in pkg/bmc/client.go that abstracts BMC operations and shields callers from vendor-specific details. Added a default GenericClient implementation and a mechanism for vendor plugins to override behavior. Also defined the Vendor type and error handling for unsupported operations.
  • Added a plugin registry system in pkg/bmc/registry.go to allow vendor plugins to register detectors and factories. The clientFor function wraps a gofish client with the most specific registered vendor client or falls back to the generic client.
  • Registered the in-tree BMC vendor plugins in cmd/root.go by blank-importing pkg/bmc/vendors so their init() functions run and register themselves.
  • Added a Cray-specific vendor plugin in pkg/bmc/vendors/cray/cray.go that detects Cray BMCs and provides a placeholder for Cray-specific quirks.

Connection and Session Management:

  • Implemented a ConnConfig struct in pkg/bmc/conn.go for canonical BMC connection configuration, including credential resolution logic.
  • Added a Manager in pkg/bmc/manager.go as the single authority for opening and caching BMC sessions, performing vendor detection, and managing session lifecycles.

Testing and Validation:

  • Added comprehensive unit tests for the new client abstraction (pkg/bmc/client_test.go), connection configuration (pkg/bmc/conn_test.go), manager/session logic (pkg/bmc/manager_test.go), and plugin registry (pkg/bmc/registry_test.go). [1] [2] [3] [4]

For more info, see Contributing Guidelines.


Some testing to verify and convey functionality:

1. Automated baseline

make emulator

Second terminal:

go build -o magellan .
go test ./...
go test tests/api_test.go tests/compatibility_test.go

2. Test the enhanced power CLI directly

This uses the emulator without needing an inventory file:

./magellan power 127.0.0.1:5000 --insecure -u root -p root_password
$ ./magellan power 127.0.0.1:5000 --insecure -u root -p root_password
{"level":"info","time":"2026-08-28T11:55:10-07:00","caller":"/Users/benmcdonald/magellan/cmd/power.go:259","message":"connecting directly to BMC at 127.0.0.1:5000 (no inventory file required)"}
127.0.0.1:5000: On

3. Test the new REST service

Create a throwaway encrypted credential store and start the server:

export MASTER_KEY=$(./magellan secrets generatekey)

./magellan --file /tmp/magellan-emulator-secrets.json \
  secrets store https://127.0.0.1:5000 root:root_password

./magellan serve --host 127.0.0.1 --port 8443 --insecure \
  --auth-token test-token \
  --secrets-file /tmp/magellan-emulator-secrets.json

In another terminal:

curl http://127.0.0.1:8443/healthz

curl -H 'Authorization: Bearer test-token' \
  --get --data-urlencode 'bmc=https://127.0.0.1:5000' \
  --data-urlencode 'system=Node0' \
  http://127.0.0.1:8443/v1/power

curl -H 'Authorization: Bearer test-token' \
  -H 'Content-Type: application/json' \
  --data '{"bmc":"https://127.0.0.1:5000"}' \
  http://127.0.0.1:8443/v1/inventory
$ curl http://127.0.0.1:8443/healthz
{"status":"ok"}

$ curl -H 'Authorization: Bearer test-token' \
  --get --data-urlencode 'bmc=https://127.0.0.1:5000' \
  --data-urlencode 'system=Node0' \
  http://127.0.0.1:8443/v1/power
{"bmc":"https://127.0.0.1:5000","powerState":"On","system":"Node0"}

$ curl -H 'Authorization: Bearer test-token' \
  -H 'Content-Type: application/json' \
  --data '{"bmc":"https://127.0.0.1:5000"}' \
  http://127.0.0.1:8443/v1/inventory
{"bmc":"https://127.0.0.1:5000","managers":[{"uri":"https://127.0.0.1:5000/redfish/v1/Managers/MCU0","name":"MCU0","description":"Microcontroller Unit for Mezz0","type":"AuxiliaryController","serial_console":null,"command_shell":null},{"uri":"https://127.0.0.1:5000/redfish/v1/Managers/MCU1","name":"MCU1","description":"Microcontroller Unit for Mezz1","type":"AuxiliaryController","serial_console":null,"command_shell":null},{"uri":"https://127.0.0.1:5000/redfish/v1/Managers/BMC","name":"BMC","description":"Shasta Manager","type":"EnclosureManager","serial_console":null,"command_shell":null}],"systems":[{"uri":"https://127.0.0.1:5000/redfish/v1/Systems/Node0","manufacturer":"HPE","system_type":"Physical","name":"Node0","model":"HPE CRAY EX235a","serial":"SEH0221913701","serial_console":{"impi":{},"telnet":{},"ssh":{}},"bios_version":"ex235a.bios-1.3.6","ethernet_interfaces":[{"uri":"https://127.0.0.1:5000/redfish/v1/Systems/Node0/EthernetInterfaces/HPCNet1","mac":"Not Available","description":"SS11 200Gb 2P NIC Mezz REV02 (HSN)"},{"uri":"https://127.0.0.1:5000/redfish/v1/Systems/Node0/EthernetInterfaces/ManagementEthernet","mac":"02:0b:b8:00:30:00","description":"Node Maintenance Network"},{"uri":"https://127.0.0.1:5000/redfish/v1/Systems/Node0/EthernetInterfaces/HPCNet2","mac":"Not Available","description":"SS11 200Gb 2P NIC Mezz REV02 (HSN)"},{"uri":"https://127.0.0.1:5000/redfish/v1/Systems/Node0/EthernetInterfaces/HPCNet3","mac":"Not Available","description":"SS11 200Gb 2P NIC Mezz REV02 (HSN)"},{"uri":"https://127.0.0.1:5000/redfish/v1/Systems/Node0/EthernetInterfaces/HPCNet0","mac":"Not Available","description":"SS11 200Gb 2P NIC Mezz REV02 (HSN)"}],"actions":["On","ForceOff","Off"],"power":{"state":"On"},"processor_count":9,"processor_type":"AMD INSTINCT MI200 (MCM) OAM LC","memory_total":512,"links":{},"node_id":"Node0"}]}

@alexlovelltroy
alexlovelltroy marked this pull request as draft June 12, 2026 14:56
@davidallendj
davidallendj self-requested a review July 28, 2026 22:27
@davidallendj

Copy link
Copy Markdown
Collaborator

This PR looks good especially with some of the smaller improvements, but I may make a few changes based on the incoming PRs being merged. Overall though, I think we could probably merged this after #132 gets merged after #142 .

Comment thread pkg/bmc/bmc.go
ClusterID string `yaml:"cluster_id"`
BmcIP string `yaml:"bmc_ip"`
NodeID string `yaml:"node_id"`
UUID string `yaml:"uuid,omitempty"`

@davidallendj davidallendj Aug 13, 2026

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.

I believe there's redundancy here with pkg/crawler/identify.go which is also true for SerialNumber too.
https://github.com/OpenCHAMI/magellan/blob/main/pkg/crawler/identify.go#L17

I think it makes sense to define the BMC in one spot and use it where we need it. In fact, after reviewing pkg/crawler/identify.go more, it looks like everything there would make sense to move to the bmc package.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That makes sense. I agree the Redfish manager-identification logic belongs in bmc.go, and I can move it there.

I might keep BMCInfo separate from bmc.Node though, since Node represents the managed system while BMCInfo represents the BMC/Redfish Manager itself. Even though both have UUID and serial fields, they may refer to different hardware. Does that distinction sound right to you?

@davidallendj

davidallendj commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Note to future me: I'm going to need pkg/bmc/client.go in this PR to define how to configure BMC settings for #129.

@bmcdonald3 bmcdonald3 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks good to me. To make sure I've got it right, someone would interact with this just by adding something like this in the HPE pkg/bmc/vendors/hpe/hpe.go file:

func (c *Client) GatherFru(ctx context.Context) (FruData, error) {
    // Add custom HPE-specific FRU gathering functionality
    ...
    // Call through to standard Redfish when no special handling is required
    return c.GenericClient.GatherFRU(ctx, systemID)
}

And then add it to the base:

type Client interface {
    // ... existing methods ...
    GatherFru(ctx context.Context) (FruData, error)
}

Add a default:

func (g *GenericClient) GatherFru(ctx context.Context) (FruData, error) {
    // Implement standard Redfish FRU collection here,
    // or return bmc.ErrUnsupportedQuirk if it cannot be safely done generally.
}

And then interact with that in other services like this:

Import:

import (
    "context"

    "github.com/OpenCHAMI/magellan/pkg/bmc"
    "github.com/OpenCHAMI/magellan/pkg/bmc/vendors/hpe"
)

And usage:

client, err := bmc.DefaultManager.Client(ctx, config)
    if err != nil {
        return err
    }
    defer client.Logout()

    // You can call GatherFru on any client. The generic client will handle it
    // standardly, and the HPE client will automatically use your custom override.
    fruData, err := client.GatherFru(ctx)
    if err != nil {
        return err
    }

I like this idea generally, I do think that it adds a layer of complexity when developing services with AI-assisted development, if you have an agent and need to instruct it how to use this Magellan-specific workflow, that requires some context that I think will be easy for an agent to get wrong. That's not to say that it's not the right architectural decision, but it doesn't feel like AI-centric development to me.

I guess what I'd do if this were me is go forward with this, but make sure there is clear documentation for an agent to understand this in Fabrica. I think this is a broader dream list ask for me is coming up with like an "agent directives" file in Fabrica that would explain all the oddities about developing with it (when to update, what things like this need to be known, etc.).

When I think about some of the original vision (that at least I had) of Fabrica it was basically "make it so that developers don't have to memorize and be up to date with all the standards of the project that are constantly evolving and changing". This seems to me like that, but at the usage-level, rather than being at the TSC level.

Like, in other words, this is a standard we are making up about Fabrica usage. We don't want Fabrica users to have to be up to date about standards. Let's figure out a way that they don't have to be.

Curious if others have thoughts on this or if I'm off base here.

alexlovelltroy and others added 8 commits August 28, 2026 11:40
Signed-off-by: Alex Lovell-Troy <alex@lovelltroy.org>

Signed-off-by: Ben McDonald <ben.mcdonald@hpe.com>
Signed-off-by: Alex Lovell-Troy <alex@lovelltroy.org>

Signed-off-by: Ben McDonald <ben.mcdonald@hpe.com>
- Updated imports from redfish to schemas in various files to align with the new package structure.
- Modified Client interface methods to return types from schemas instead of redfish.
- Adjusted implementations in GenericClient to accommodate the new schemas types.
- Updated tests to reflect changes in the types used for power states and reset types.
- Refactored functions in collect, crawler, power, service, and update packages to utilize schemas types.
- Introduced helper functions to dereference optional numeric fields for better handling of nil values.

Signed-off-by: Alex Lovell-Troy <alex@lovelltroy.org>

Signed-off-by: Ben McDonald <ben.mcdonald@hpe.com>
…BMC layer

Signed-off-by: Alex Lovell-Troy <alex@lovelltroy.org>

Signed-off-by: Ben McDonald <ben.mcdonald@hpe.com>
- Add middleware for request logging and bearer token authentication in the server.
- Create response handling functions for JSON responses and error messages.
- Develop the main server structure to handle API routes for inventory and power operations.
- Implement health check endpoints for liveness and readiness.
- Introduce a mock Redfish service for testing power state transitions.
- Add power transition logic with confirmation and escalation handling.
- Create tests for various power operations, including success, timeout, and escalation scenarios.
- Enhance service layer to support power transition operations.

Signed-off-by: Alex Lovell-Troy <alex@lovelltroy.org>

Signed-off-by: Ben McDonald <ben.mcdonald@hpe.com>
* Refactor to upgrade to gofish 0.22.0 with new schemas package

- Updated imports from redfish to schemas in various files to align with the new package structure.
- Modified Client interface methods to return types from schemas instead of redfish.
- Adjusted implementations in GenericClient to accommodate the new schemas types.
- Updated tests to reflect changes in the types used for power states and reset types.
- Refactored functions in collect, crawler, power, service, and update packages to utilize schemas types.
- Introduced helper functions to dereference optional numeric fields for better handling of nil values.

Signed-off-by: Alex Lovell-Troy <alex@lovelltroy.org>

* feat: implement context-aware power operations and reset handling in BMC layer

Signed-off-by: Alex Lovell-Troy <alex@lovelltroy.org>

* feat: enhance power command to support flexible node identifiers and improve inventory handling

Signed-off-by: Alex Lovell-Troy <alovelltroy@lanl.gov>

---------

Signed-off-by: Alex Lovell-Troy <alex@lovelltroy.org>
Signed-off-by: Alex Lovell-Troy <alovelltroy@lanl.gov>

Signed-off-by: Ben McDonald <ben.mcdonald@hpe.com>
Signed-off-by: Ben McDonald <ben.mcdonald@hpe.com>
Signed-off-by: Ben McDonald <ben.mcdonald@hpe.com>
Signed-off-by: Ben McDonald <ben.mcdonald@hpe.com>
Signed-off-by: Ben McDonald <ben.mcdonald@hpe.com>
@bmcdonald3
bmcdonald3 force-pushed the feature/RFD-133-Explore branch from bfa293f to df1b04c Compare August 28, 2026 19:34
Signed-off-by: Ben McDonald <ben.mcdonald@hpe.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.

3 participants