Skip to content
This repository was archived by the owner on Jun 2, 2026. It is now read-only.

Commit 1f318d4

Browse files
committed
feat: Add tray-level firmware sub-target selection
Lets callers restrict a firmware update to a subset of sub-parts within each targeted tray (e.g. just BMC + NVOS for switch trays, just PSU for powershelf trays) instead of always installing the whole bundle. REST surface ------------ - OpenAPI: new optional `targets` array on `FirmwareUpdateRequest` and `BatchTrayFirmwareUpdateRequest`. Lowercase names; empty/omitted keeps the historical "update everything in the bundle" behavior; unknown names are rejected. Requires `version` to be set when non-empty. Field is named `targets` (not `components`) to avoid colliding with carbide's tray-level "Component" resource vocabulary. - API model: `APIUpdateFirmwareRequest.Targets`, `APIBatchTrayFirmwareUpdateRequest.Targets`, `validateFirmwareTargets`. - Handlers: `tray.go` (single + batch) plumbs `apiRequest.Targets` through `common.ExecuteFirmwareUpdateWorkflow`. `rack.go` continues to pass nil (no rack-level sub-target selection - rack requests are fanned out to individual tray operations downstream). - SDK regenerated. Flow gRPC + Go internals ------------------------ - `flow/proto/v1/flow.proto`: new `repeated string sub_targets = 8;` on `UpgradeFirmwareRequest`. Named `sub_targets` (not `components`) because `OperationTargetSpec.components` already exists on the same request and means "tray INSTANCES to hit"; keeping both as `components` would have been ambiguous. - Regenerated `flow/pkg/proto/v1/flow.pb.go` and `workflow-schema/flow/protobuf/v1/flow.pb.go`. - `operations.FirmwareControlTaskInfo` gains `SubTargets []string` (JSON tag `sub_targets`). Populated from `req.GetSubTargets()` in `service/server_impl.go` and `converter/protobuf/converter.go`. - All 5 component-manager FirmwareControl implementations consume `info.SubTargets`: - nvlswitch/nsm -> NSM QueueUpdates - nvlswitch/nico -> UpdateSwitchFirmwareTarget.Components - powershelf/psm -> UpdatePowershelfFirmwareRequest.Components - powershelf/nico -> UpdatePowerShelfFirmwareTarget.Components (defaults to PMC when empty, preserving prior behavior) - compute/nico -> currently logs a warning and applies the full bundle; the SetFirmwareUpdateTimeWindow + SetMachineAutoUpdate path has no per-sub-target selection. Will be honored once compute moves to UpdateComponentFirmware. Centralized name -> enum mapping -------------------------------- - New package `flow/pkg/common/firmwarecomponents` owns the string -> tray-type-specific component-manager-enum mapping for all three tray types (NSM NVSwitch, PSM PowerShelf, and the three NICo *Component enums). Mappings are explicit hand-written maps rather than auto-derived from proto, so adding a new enum value in Core requires a deliberate decision about its REST-facing name. - Completeness-guard unit tests (TestNICo*MapIsComplete) fail if Core adds a non-UNKNOWN enum value that the mapping forgot to cover. - Package name keeps "Component" because it is the semantic destination of the parse - the enums in Core are named NvSwitchComponent / PowerShelfComponent / ComputeTrayComponent. Compatibility ------------- - Proto field number 8 is new (added in this commit), so there are no in-flight Temporal workflows or existing gRPC consumers depending on any prior name; the JSON-tag/field-name choices are risk-free. - gRPC wire format only depends on field numbers; the name `sub_targets` changes nothing on the wire. - Empty/omitted `targets` is the legacy behavior, so existing callers are unaffected. Example ------- PATCH /v2/tray/{id}/firmware { "version": "24.11.0", "targets": ["bmc", "nvos"] } POST /v2/tray/firmware/batch { "siteId": "...", "filter": {"type": "switch"}, "version": "24.11.0", "targets": ["bmc", "nvos"] }
1 parent 538260d commit 1f318d4

24 files changed

Lines changed: 794 additions & 27 deletions

File tree

api/pkg/api/handler/rack.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,7 +1150,7 @@ func (furh UpdateRackFirmwareHandler) Handle(c echo.Context) error {
11501150
}
11511151

11521152
flowResp, err := common.ExecuteFirmwareUpdateWorkflow(ctx, c, logger, stc, targetSpec, apiRequest.Version,
1153-
fmt.Sprintf("rack-firmware-update-%s", rackStrID), "Rack")
1153+
nil, fmt.Sprintf("rack-firmware-update-%s", rackStrID), "Rack")
11541154
if err != nil {
11551155
return err
11561156
}
@@ -1268,7 +1268,7 @@ func (furbh BatchUpdateRackFirmwareHandler) Handle(c echo.Context) error {
12681268
targetSpec := request.Filter.ToTargetSpec()
12691269

12701270
flowResp, err := common.ExecuteFirmwareUpdateWorkflow(ctx, c, logger, stc, targetSpec, request.Version,
1271-
fmt.Sprintf("rack-firmware-batch-update-%s", common.RequestHash(request.Filter)), "Rack")
1271+
nil, fmt.Sprintf("rack-firmware-batch-update-%s", common.RequestHash(request.Filter)), "Rack")
12721272
if err != nil {
12731273
return err
12741274
}

api/pkg/api/handler/tray.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,7 +1144,7 @@ func (futh UpdateTrayFirmwareHandler) Handle(c echo.Context) error {
11441144
}
11451145

11461146
flowResp, err := common.ExecuteFirmwareUpdateWorkflow(ctx, c, logger, stc, targetSpec, apiRequest.Version,
1147-
fmt.Sprintf("tray-firmware-update-%s", trayStrID), "Tray")
1147+
apiRequest.Targets, fmt.Sprintf("tray-firmware-update-%s", trayStrID), "Tray")
11481148
if err != nil {
11491149
return err
11501150
}
@@ -1259,7 +1259,7 @@ func (futbh BatchUpdateTrayFirmwareHandler) Handle(c echo.Context) error {
12591259
targetSpec := request.Filter.ToTargetSpec()
12601260

12611261
flowResp, err := common.ExecuteFirmwareUpdateWorkflow(ctx, c, logger, stc, targetSpec, request.Version,
1262-
fmt.Sprintf("tray-firmware-batch-update-%s", common.RequestHash(request.Filter)), "Tray")
1262+
request.Targets, fmt.Sprintf("tray-firmware-batch-update-%s", common.RequestHash(request.Filter)), "Tray")
12631263
if err != nil {
12641264
return err
12651265
}

api/pkg/api/handler/util/common/common.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,19 +1762,28 @@ func ExecuteBringUpRackWorkflow(
17621762

17631763
// ExecuteFirmwareUpdateWorkflow builds an UpgradeFirmwareRequest, executes the UpgradeFirmware
17641764
// workflow via Temporal, and returns the raw SubmitTaskResponse.
1765+
//
1766+
// targets, when non-empty, restricts the upgrade to the listed firmware
1767+
// sub-parts within each targeted tray (e.g. ["bmc", "nvos"] for switch
1768+
// trays). An empty/nil slice keeps the historical "update everything in
1769+
// the bundle" behavior. Names are passed through verbatim to Flow as
1770+
// `sub_targets`, which resolves them against the tray-type-specific
1771+
// component-manager enums (see flow/pkg/common/firmwarecomponents).
17651772
func ExecuteFirmwareUpdateWorkflow(
17661773
ctx context.Context,
17671774
c echo.Context,
17681775
logger zerolog.Logger,
17691776
stc tclient.Client,
17701777
targetSpec *flowv1.OperationTargetSpec,
17711778
version *string,
1779+
targets []string,
17721780
workflowID string,
17731781
entityName string,
17741782
) (*flowv1.SubmitTaskResponse, error) {
17751783
flowRequest := &flowv1.UpgradeFirmwareRequest{
17761784
TargetSpec: targetSpec,
17771785
TargetVersion: version,
1786+
SubTargets: targets,
17781787
Description: fmt.Sprintf("API firmware update %s", entityName),
17791788
}
17801789

api/pkg/api/model/firmware.go

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,29 @@ import (
2929
type APIUpdateFirmwareRequest struct {
3030
SiteID string `json:"siteId"`
3131
Version *string `json:"version,omitempty"`
32+
// Targets, when non-empty, restricts the update to a subset of
33+
// firmware sub-parts within the targeted tray (e.g. ["bmc", "nvos"]
34+
// for switch trays). Names are lowercase. The authoritative supported
35+
// set per tray type is derived from the Flow service's NICo proto
36+
// bindings (mirroring Core's per-tray-type enums in
37+
// carbide-core/crates/rpc/proto/forge.proto); see
38+
// flow/pkg/common/firmwarecomponents for the resolution logic and
39+
// helpers like SupportedNICoNVSwitchNames.
40+
// Empty/nil means "update everything in the bundle". Requires Version.
41+
//
42+
// REST surface intentionally calls these "targets" to avoid confusion
43+
// with carbide's tray-level "Component" vocabulary; the downstream
44+
// Flow proto field is still named `components` and represents the
45+
// same enum subset.
46+
Targets []string `json:"targets,omitempty"`
3247
}
3348

3449
// Validate validates the firmware update request
3550
func (r *APIUpdateFirmwareRequest) Validate() error {
3651
if r.SiteID == "" {
3752
return fmt.Errorf("siteId is required")
3853
}
39-
return nil
54+
return validateFirmwareTargets(r.Targets, r.Version)
4055
}
4156

4257
// ========== Firmware Update Response ==========
@@ -89,12 +104,38 @@ type APIBatchTrayFirmwareUpdateRequest struct {
89104
SiteID string `json:"siteId"`
90105
Filter *TrayFilter `json:"filter,omitempty"`
91106
Version *string `json:"version,omitempty"`
107+
// Targets, when non-empty, restricts the update to a subset of
108+
// firmware sub-parts within each matched tray. Same semantics as the
109+
// single-tray variant. Requires Version.
110+
Targets []string `json:"targets,omitempty"`
92111
}
93112

94113
// Validate checks required fields and filter constraints.
95114
func (r *APIBatchTrayFirmwareUpdateRequest) Validate() error {
96115
if r.SiteID == "" {
97116
return fmt.Errorf("siteId is required")
98117
}
99-
return r.Filter.Validate()
118+
if err := r.Filter.Validate(); err != nil {
119+
return err
120+
}
121+
return validateFirmwareTargets(r.Targets, r.Version)
122+
}
123+
124+
// validateFirmwareTargets enforces the cross-field constraint that a
125+
// firmware-target subset selection is only meaningful when a target version
126+
// is also supplied. Per-tray-type name validation is delegated to Flow,
127+
// where the mapping from string to component-manager enum lives.
128+
func validateFirmwareTargets(targets []string, version *string) error {
129+
if len(targets) == 0 {
130+
return nil
131+
}
132+
for _, t := range targets {
133+
if t == "" {
134+
return fmt.Errorf("targets must not contain empty strings")
135+
}
136+
}
137+
if version == nil || *version == "" {
138+
return fmt.Errorf("targets requires version to be set")
139+
}
140+
return nil
100141
}

api/pkg/api/model/firmware_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,88 @@ func TestAPIUpdateFirmwareRequest_Validate(t *testing.T) {
4545
request: APIUpdateFirmwareRequest{Version: strPtr("24.11.0")},
4646
wantErr: true,
4747
},
48+
{
49+
name: "valid - targets with version",
50+
request: APIUpdateFirmwareRequest{
51+
SiteID: "site-1",
52+
Version: strPtr("24.11.0"),
53+
Targets: []string{"bmc", "nvos"},
54+
},
55+
wantErr: false,
56+
},
57+
{
58+
name: "invalid - targets without version",
59+
request: APIUpdateFirmwareRequest{
60+
SiteID: "site-1",
61+
Targets: []string{"bmc"},
62+
},
63+
wantErr: true,
64+
},
65+
{
66+
name: "invalid - targets with empty version string",
67+
request: APIUpdateFirmwareRequest{
68+
SiteID: "site-1",
69+
Version: strPtr(""),
70+
Targets: []string{"bmc"},
71+
},
72+
wantErr: true,
73+
},
74+
{
75+
name: "invalid - targets contains empty string",
76+
request: APIUpdateFirmwareRequest{
77+
SiteID: "site-1",
78+
Version: strPtr("24.11.0"),
79+
Targets: []string{"bmc", ""},
80+
},
81+
wantErr: true,
82+
},
83+
}
84+
85+
for _, tt := range tests {
86+
t.Run(tt.name, func(t *testing.T) {
87+
err := tt.request.Validate()
88+
if tt.wantErr {
89+
assert.Error(t, err)
90+
} else {
91+
assert.NoError(t, err)
92+
}
93+
})
94+
}
95+
}
96+
97+
func TestAPIBatchTrayFirmwareUpdateRequest_Validate(t *testing.T) {
98+
tests := []struct {
99+
name string
100+
request APIBatchTrayFirmwareUpdateRequest
101+
wantErr bool
102+
}{
103+
{
104+
name: "valid - siteId only",
105+
request: APIBatchTrayFirmwareUpdateRequest{SiteID: "site-1"},
106+
wantErr: false,
107+
},
108+
{
109+
name: "valid - targets with version",
110+
request: APIBatchTrayFirmwareUpdateRequest{
111+
SiteID: "site-1",
112+
Version: strPtr("24.11.0"),
113+
Targets: []string{"bmc"},
114+
},
115+
wantErr: false,
116+
},
117+
{
118+
name: "invalid - targets without version",
119+
request: APIBatchTrayFirmwareUpdateRequest{
120+
SiteID: "site-1",
121+
Targets: []string{"bmc"},
122+
},
123+
wantErr: true,
124+
},
125+
{
126+
name: "invalid - missing siteId",
127+
request: APIBatchTrayFirmwareUpdateRequest{},
128+
wantErr: true,
129+
},
48130
}
49131

50132
for _, tt := range tests {

flow/internal/converter/protobuf/converter.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,7 @@ func ScheduledOperationFrom(
12391239
info := &operations.FirmwareControlTaskInfo{
12401240
Operation: operations.FirmwareOperationUpgrade,
12411241
TargetVersion: r.UpgradeFirmware.GetTargetVersion(),
1242+
SubTargets: r.UpgradeFirmware.GetSubTargets(),
12421243
}
12431244

12441245
if r.UpgradeFirmware.GetStartTime() != nil {

flow/internal/service/server_impl.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1241,6 +1241,7 @@ func (rs *FlowServerImpl) UpgradeFirmware(
12411241
Operation: operations.FirmwareOperationUpgrade,
12421242
TargetVersion: req.GetTargetVersion(),
12431243
RuleID: protobuf.UUIDStringFrom(req.GetRuleId()),
1244+
SubTargets: req.GetSubTargets(),
12441245
}
12451246

12461247
// Parse optional time parameters for scheduled upgrade

flow/internal/task/componentmanager/compute/nico/nico.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,7 @@ func (m *Manager) FirmwareControl(ctx context.Context, target common.Target, inf
315315
log.Debug().
316316
Str("components", target.String()).
317317
Str("target_version", info.TargetVersion).
318+
Strs("sub_targets", info.SubTargets).
318319
Msg("Scheduling firmware update for compute via NICo")
319320

320321
if m.nicoClient == nil {
@@ -325,6 +326,19 @@ func (m *Manager) FirmwareControl(ctx context.Context, target common.Target, inf
325326
return fmt.Errorf("target is invalid: %w", err)
326327
}
327328

329+
if len(info.SubTargets) > 0 {
330+
// SetFirmwareUpdateTimeWindow + SetMachineAutoUpdate (the path used
331+
// here for compute trays) do not expose per-sub-target selection;
332+
// auto-update will install whatever is in the desired bundle. Log
333+
// the requested subset so the limitation is visible until we can
334+
// route this through UpdateComponentFirmware (planned alongside the
335+
// version → FW Object identifier migration).
336+
log.Warn().
337+
Str("components", target.String()).
338+
Strs("sub_targets", info.SubTargets).
339+
Msg("compute firmware sub-target selection is not yet honored; whole bundle will be applied")
340+
}
341+
328342
desiredEntries, err := m.nicoClient.GetDesiredFirmwareVersions(ctx)
329343
if err != nil {
330344
return fmt.Errorf("failed to query desired firmware versions: %w", err)

flow/internal/task/componentmanager/compute/nico/nico_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,39 @@ func mustMarshal(t *testing.T, v any) json.RawMessage {
102102
return data
103103
}
104104

105+
// TestFirmwareControl_SubTargetsAccepted verifies that the compute/nico
106+
// FirmwareControl path tolerates info.SubTargets without erroring. This
107+
// path goes through SetMachineAutoUpdate + SetFirmwareUpdateTimeWindow,
108+
// which has no per-sub-target selection in NICo, so the manager only logs
109+
// a warning and proceeds; we exercise that branch here. The actual
110+
// per-sub-target dispatch will be added when compute moves to NICo's
111+
// UpdateComponentFirmware (see comment in nico.go).
112+
func TestFirmwareControl_SubTargetsAccepted(t *testing.T) {
113+
tests := map[string]struct {
114+
subTargets []string
115+
}{
116+
"nil sub_targets (legacy path)": {subTargets: nil},
117+
"empty sub_targets (legacy path)": {subTargets: []string{}},
118+
"non-empty sub_targets (warn path)": {subTargets: []string{"bmc", "bios"}},
119+
}
120+
121+
for name, tc := range tests {
122+
t.Run(name, func(t *testing.T) {
123+
m := New(nicoapi.NewMockClient(), 0)
124+
target := common.Target{
125+
Type: devicetypes.ComponentTypeCompute,
126+
ComponentIDs: []string{"machine-1"},
127+
}
128+
129+
err := m.FirmwareControl(context.Background(), target, operations.FirmwareControlTaskInfo{
130+
Operation: operations.FirmwareOperationUpgrade,
131+
SubTargets: tc.subTargets,
132+
})
133+
require.NoError(t, err)
134+
})
135+
}
136+
}
137+
105138
// --- Tests for firmware version helper functions ---
106139

107140
func desiredEntry(versions map[string]string) *pb.DesiredFirmwareVersionEntry {

flow/internal/task/componentmanager/nvlswitch/nico/nico.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"github.com/NVIDIA/infra-controller-rest/flow/internal/task/executor/temporalworkflow/common"
3535
"github.com/NVIDIA/infra-controller-rest/flow/internal/task/operations"
3636
"github.com/NVIDIA/infra-controller-rest/flow/pkg/common/devicetypes"
37+
"github.com/NVIDIA/infra-controller-rest/flow/pkg/common/firmwarecomponents"
3738
)
3839

3940
const (
@@ -242,12 +243,18 @@ func (m *Manager) FirmwareControl(ctx context.Context, target common.Target, inf
242243
log.Debug().
243244
Str("components", target.String()).
244245
Str("target_version", info.TargetVersion).
246+
Strs("sub_targets", info.SubTargets).
245247
Msg("Starting firmware update for NVLSwitch via NICo")
246248

247249
if err := target.Validate(); err != nil {
248250
return fmt.Errorf("target is invalid: %w", err)
249251
}
250252

253+
subComponents, err := firmwarecomponents.ParseNICoNVSwitch(info.SubTargets)
254+
if err != nil {
255+
return err
256+
}
257+
251258
if info.TargetVersion == "" {
252259
upToDate, err := m.checkFirmwareUpToDate(ctx, target)
253260
if err != nil {
@@ -263,7 +270,8 @@ func (m *Manager) FirmwareControl(ctx context.Context, target common.Target, inf
263270
req := &pb.UpdateComponentFirmwareRequest{
264271
Target: &pb.UpdateComponentFirmwareRequest_Switches{
265272
Switches: &pb.UpdateSwitchFirmwareTarget{
266-
SwitchIds: switchIDsProto(target.ComponentIDs),
273+
SwitchIds: switchIDsProto(target.ComponentIDs),
274+
Components: subComponents,
267275
},
268276
},
269277
TargetVersion: info.TargetVersion,

0 commit comments

Comments
 (0)