feat(dpf): support BlueFieldSoftware (BF4) provisioning with per-PSID.#3361
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (11)
Summary by CodeRabbit
WalkthroughChangesThe PR adds BF4 BF4 provisioning and source-aware deployment
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Config as initialize_dpf_sdk
participant SDK as DpfSdk
participant Repository as BlueFieldSoftwareRepository
participant Deployment as DPUDeployment
Config->>Config: validate_provisioning_sources
Config->>SDK: create_initialization_objects
SDK->>Repository: create or reuse BlueFieldSoftware
SDK->>Deployment: set blue_field_software reference
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
crates/api-core/src/cfg/file.rs (1)
4953-5029: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpress the provisioning-source matrix as table-driven cases.
Use
check_casesfor the accepted/rejected combinations, including BF4 with BFB only. This will make the deployment/source compatibility matrix explicit and prevent omitted combinations.As per coding guidelines, use
check_casesdirectly when several inputs or expected fields make a macro table clearer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/api-core/src/cfg/file.rs` around lines 4953 - 5029, Refactor the provisioning-source tests around validate_provisioning_sources into table-driven cases using check_cases. Cover accepted and rejected combinations for BF3 and BF4, including BF4 with BFB only, BF4 with bluefield_software only, both sources, neither source, an empty PSID map, and BF3 bluefield_software; encode each case’s expected result in the table and remove the separate repetitive test setup.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/api-core/src/cfg/file.rs`:
- Around line 1219-1244: Validate every effective per-PSID identifier before
initialization, including values generated by per_psid_deployment_name and
per_psid_node_label_key: enforce Kubernetes syntax, separator rules, and length
limits, and detect collisions both among sanitized PSIDs and with existing
BF3/base identifiers. Reject invalid or ambiguous configurations with clear
errors rather than relying on lossy sanitize_psid normalization; update the
relevant configuration validation path and tests.
- Around line 1355-1367: Update the validation loop in the configuration
validation method containing the match on (&cfg.bfb_url,
&cfg.bluefield_software) to reject the (Some(_), None) case when the deployment
is bf4_generic, requiring bluefield_software and reporting a clear
deployment-specific error. Preserve the existing acceptance of BFB-only
configurations for other deployment types and keep the exactly-one validation
behavior intact.
In `@crates/dpf/src/sdk.rs`:
- Around line 465-521: Update create_bluefield_software to mirror the
terminating-object guard used by the analogous creation helper: after an
AlreadyExists/conflict error, re-fetch the BlueFieldSoftware by namespace and
name, and return an error if it is missing or has metadata.deletion_timestamp
set; only reuse the name when the fetched object exists and is not terminating.
In `@crates/dpf/src/types.rs`:
- Around line 54-62: Enforce exactly one provisioning source in
InitDpfResourcesConfig at the SDK boundary: replace the independently
optional/defaultable bfb_url and bluefield_software fields with a
compiler-enforced provisioning-source enum or otherwise require validation
before resource creation. Update all consumers and Default implementations so
neither both sources nor no source can reach resource creation, preserving the
existing BF3/BF4 behavior without silently prioritizing or accepting an empty
BFB URL.
In `@crates/machine-controller/src/handler/dpf.rs`:
- Around line 532-541: The deployment-type lookup currently waits for a PSID
before creation-capable states can register the DPUDevice. In the state dispatch
logic around the provisioning and reprovisioning handlers, create or ensure the
DPUDevice first, then perform deployment resolution and label validation; retain
the wait outcome only after registration has been attempted. Add regression
coverage proving lookup returns None before registration, registration occurs,
and lookup returns Some afterward, including safe retry behavior for partial
failures.
---
Nitpick comments:
In `@crates/api-core/src/cfg/file.rs`:
- Around line 4953-5029: Refactor the provisioning-source tests around
validate_provisioning_sources into table-driven cases using check_cases. Cover
accepted and rejected combinations for BF3 and BF4, including BF4 with BFB only,
BF4 with bluefield_software only, both sources, neither source, an empty PSID
map, and BF3 bluefield_software; encode each case’s expected result in the table
and remove the separate repetitive test setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4317e2bb-61e8-440c-a6b3-37f81430280b
📒 Files selected for processing (19)
crates/api-core/src/cfg/file.rscrates/api-core/src/setup.rscrates/api-core/src/tests/dpf/duplicate_events.rscrates/api-core/src/tests/dpf/happy_path.rscrates/api-core/src/tests/dpf/reprovisioning.rscrates/api-core/src/tests/dpf/stale_labels.rscrates/api-core/src/tests/dpf/waiting_for_ready.rscrates/api-core/src/tests/machine_admin_force_delete.rscrates/dpf/src/bin/api_harness.rscrates/dpf/src/flavor.rscrates/dpf/src/lib.rscrates/dpf/src/repository/kube.rscrates/dpf/src/repository/traits.rscrates/dpf/src/sdk.rscrates/dpf/src/test/sdk_device_registration.rscrates/dpf/src/test/sdk_initialization.rscrates/dpf/src/types.rscrates/machine-controller/src/dpf.rscrates/machine-controller/src/handler/dpf.rs
💤 Files with no reviewable changes (2)
- crates/dpf/src/test/sdk_device_registration.rs
- crates/dpf/src/bin/api_harness.rs
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
wminckler
approved these changes
Jul 10, 2026
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/api-core/src/cfg/file.rs (2)
1181-1182: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPreserve the BF3 default for partial blocks.
#[serde(default)]onOption<String>makes an omittedbfb_urldeserialize toNonein an explicit[dpf.deployments.bf3]block, so the new validation rejects a config that previously inherited the default BFB URL. Either keep the BF3 default during deserialization or make this a documented breaking change and add a regression test for the partial-block case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/api-core/src/cfg/file.rs` around lines 1181 - 1182, Preserve the BF3 default BFB URL when an explicit partial deployment is deserialized: update the BF3 configuration deserialization/defaulting logic around the bfb_url field instead of allowing #[serde(default)] to produce None, and add a regression test covering a partial [dpf.deployments.bf3] block that omits bfb_url and still inherits the default.
1215-1222: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the broken intra-doc links and state the one-entry constraint.
DpfDeploymentConfig::per_psid_deployment_nameandDpfDeploymentConfig::per_psid_node_label_keyno longer exist, and this section should explicitly saypldm_fw_bundleis expected to contain exactly one PSID→bundle entry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/api-core/src/cfg/file.rs` around lines 1215 - 1222, Update the BlueFieldSoftware specification documentation comment by removing the broken links to DpfDeploymentConfig::per_psid_deployment_name and DpfDeploymentConfig::per_psid_node_label_key, and explicitly state that pldm_fw_bundle must contain exactly one PSID-to-bundle entry.crates/dpf/src/sdk.rs (1)
510-531: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle the missing-CR race in
crates/dpf/src/sdk.rs:510-528. Ifget()returnsNoneafter anAlreadyExists/conflict, this still returnsOk(name), andcreate_initialization_objectswill emit aDPUDeploymentthat points at aBlueFieldSoftwareCR that no longer exists. Mirror thecreate_dpu_flavorguard and returnInvalidStatehere as well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/dpf/src/sdk.rs` around lines 510 - 531, Update the existing-resource branch in the BlueFieldSoftware creation flow to handle `BlueFieldSoftwareRepository::get` returning `None`: return `DpfError::InvalidState` with a descriptive missing-resource message instead of `Ok(name)`. Mirror the guard and error style used by `create_dpu_flavor`, while preserving the deletion-timestamp check and reuse behavior.
🧹 Nitpick comments (1)
crates/api-core/src/cfg/file.rs (1)
5019-5027: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the validation tests table-driven and assert the failure rule.
The new cases repeat setup and only assert
is_err(). Use a table of cases with targeted error-substring assertions so the tests prove the BF4-only and exactly-one-PSID rules rather than merely detecting any failure.Also applies to: 5029-5057
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/api-core/src/cfg/file.rs` around lines 5019 - 5027, The validation tests around bf4_with_psids should be refactored into a table-driven set of cases, each defining its PSID input and expected error substring. Iterate through the cases, build the configuration with bf4_with_psids, run validation, and assert the error both occurs and contains the targeted text proving the BF4-only and exactly-one-PSID rules.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/api-core/src/cfg/file.rs`:
- Around line 1181-1182: Preserve the BF3 default BFB URL when an explicit
partial deployment is deserialized: update the BF3 configuration
deserialization/defaulting logic around the bfb_url field instead of allowing
#[serde(default)] to produce None, and add a regression test covering a partial
[dpf.deployments.bf3] block that omits bfb_url and still inherits the default.
- Around line 1215-1222: Update the BlueFieldSoftware specification
documentation comment by removing the broken links to
DpfDeploymentConfig::per_psid_deployment_name and
DpfDeploymentConfig::per_psid_node_label_key, and explicitly state that
pldm_fw_bundle must contain exactly one PSID-to-bundle entry.
In `@crates/dpf/src/sdk.rs`:
- Around line 510-531: Update the existing-resource branch in the
BlueFieldSoftware creation flow to handle `BlueFieldSoftwareRepository::get`
returning `None`: return `DpfError::InvalidState` with a descriptive
missing-resource message instead of `Ok(name)`. Mirror the guard and error style
used by `create_dpu_flavor`, while preserving the deletion-timestamp check and
reuse behavior.
---
Nitpick comments:
In `@crates/api-core/src/cfg/file.rs`:
- Around line 5019-5027: The validation tests around bf4_with_psids should be
refactored into a table-driven set of cases, each defining its PSID input and
expected error substring. Iterate through the cases, build the configuration
with bf4_with_psids, run validation, and assert the error both occurs and
contains the targeted text proving the BF4-only and exactly-one-PSID rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ca3af771-08a2-42e0-949f-2d8ee5d9355d
📒 Files selected for processing (13)
crates/api-core/src/cfg/file.rscrates/api-core/src/setup.rscrates/api-core/src/tests/dpf/duplicate_events.rscrates/api-core/src/tests/dpf/happy_path.rscrates/api-core/src/tests/dpf/reprovisioning.rscrates/api-core/src/tests/dpf/stale_labels.rscrates/api-core/src/tests/dpf/waiting_for_ready.rscrates/api-core/src/tests/machine_admin_force_delete.rscrates/dpf/src/sdk.rscrates/dpf/src/test/sdk_initialization.rscrates/dpf/src/types.rscrates/machine-controller/src/dpf.rscrates/machine-controller/src/handler/dpf.rs
✅ Files skipped from review due to trivial changes (3)
- crates/machine-controller/src/dpf.rs
- crates/api-core/src/tests/dpf/reprovisioning.rs
- crates/api-core/src/tests/dpf/stale_labels.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/dpf/src/test/sdk_initialization.rs
abvarshney-nv
force-pushed
the
dpf_bluefield_software
branch
from
20d9eff to
d84733a
Compare
wminckler
approved these changes
Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add BF4-class DPU provisioning from a BlueFieldSoftware CR (OS ISO + PLDM firmware bundle) alongside the existing BFB (BF3) path. bfb_url is now optional. A deployment uses either bfb_url (BF3) or bluefield_software (BF4).
With this change, NICO will create a separate DPUDeployment per PSID. To provision a DPU, user has to configure bf4 in following way:
Please note only one PSID is allowed as of now in plum_fw_bundle due to DPF limitation.
Related issues
Type of Change
Breaking Changes
Testing
Additional Notes