fix(dpf): detect outdated DPUs by BlueFieldSoftware, not just flavor - #4763
Conversation
A DPUDeployment provisions its DPUs from one of two sources: a BFB, or a BlueFieldSoftware CR for BF4-class hardware. Only the BFB case was compared against the DPU. When a deployment provisioned from BlueFieldSoftware there was no BFB to compare, so the staleness check fell back to the DPU flavor alone. A BlueFieldSoftware change therefore never registered as drift: the DPU was reported up to date and never queued for reprovisioning, so it kept running superseded software indefinitely. Only a flavor change could dislodge it. Compare whichever provisioning source the owning deployment declares, so a BlueFieldSoftware change marks its DPUs outdated the same way a BFB change already did. A deployment declaring neither source or both violates the DPU CRD, which requires exactly one. Skip and log those rather than reading them as drift, so a malformed deployment cannot queue every DPU under it for reprovisioning. Signed-off-by: abhi <abvarshney@nvidia.com>
|
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 (4)
Summary by CodeRabbit
WalkthroughThe SDK now detects outdated DPUs provisioned through BFB or BlueFieldSoftware sources. It compares the DPU flavor and active provisioning target, reports the expected target, and skips deployments with invalid source configurations. ChangesDPU mismatch detection
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DpuScan
participant DeploymentSource
participant DPU
DpuScan->>DeploymentSource: Read owning deployment source configuration
DeploymentSource->>DPU: Compare flavor and BFB or BlueFieldSoftware source
DPU-->>DpuScan: Return mismatch or skip malformed configuration
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/dpf/src/types.rs`:
- Around line 120-124: Introduce and re-export the tagged DpuProvisioningTarget
enum with BfbFilename(String) and BlueFieldSoftware(String) variants, and change
DpuMismatch::target_bfb to use it while updating OutdatedDpfDpu documentation.
In crates/dpf/src/sdk.rs:1754-1770 and 1787-1791, update both dpu_mismatch
branches to construct the appropriate variant; at
crates/dpf/src/sdk.rs:4042-4081, explicitly convert each variant to the trace
string at the machine-controller boundary and update tests to cover both
variants.
🪄 Autofix
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: 52249675-8eba-4a98-a0d2-e23308586312
📒 Files selected for processing (2)
crates/dpf/src/sdk.rscrates/dpf/src/types.rs
The field now carries either a BFB filename or a BlueFieldSoftware CR name, depending on which source the owning DPUDeployment declares, so a name that says "bfb" misreads for half its values. Addresses review feedback asking for a tagged enum here. A tagged enum would not survive the trip: api-model does not depend on carbide-dpf, so the variant would be flattened back to a String at the machine-controller boundary and stay one through every consumer, none of which branch on the source kind. Renaming fixes the misleading part without adding a type nothing reads. Signed-off-by: abhi <abvarshney@nvidia.com>
…VIDIA#4763) Carbide decides whether a DPU needs reprovisioning by comparing what it is running against what its owning DPUDeployment declares. A deployment provisions its DPUs from one of two sources: a BFB, or a BlueFieldSoftware CR for BF4-class hardware. The DPU CRD requires exactly one of the two to be set. Only the BFB case was ever compared. When a deployment provisioned from BlueFieldSoftware there was no BFB to compare against, so the staleness check fell back to the DPU flavor alone. A BlueFieldSoftware change therefore never registered as drift: the DPU was reported up to date, was never queued for reprovisioning, and kept running superseded software indefinitely. Only a flavor change could dislodge it. This compares whichever provisioning source the owning deployment actually declares, so a BlueFieldSoftware change now marks its DPUs outdated exactly as a BFB change already did. A deployment that declares neither source, or both, violates the CRD's own constraint. Those are skipped and logged rather than read as drift, so a malformed deployment cannot queue every DPU beneath it for reprovisioning. ## Related issues None. ## Type of Change - [ ] **Add** - New feature or capability - [ ] **Change** - Changes in existing functionality - [x] **Fix** - Bug fixes - [ ] **Remove** - Removed features or deprecated functionality - [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.) ## Breaking Changes - [ ] **This PR contains breaking changes** ## Testing - [x] Unit tests added/updated - [ ] Integration tests added/updated - [ ] Manual testing performed - [ ] No testing required (docs, internal refactor, etc.) Covers each provisioning source both matching and drifting, a flavor change on a BlueFieldSoftware-provisioned DPU, and the malformed-deployment guard. ## Additional Notes One asymmetry is worth knowing when reasoning about the behaviour. BFB staleness is judged against the image the DPU actually installed, whereas BlueFieldSoftware is judged against the software the DPU was created from, because DPU status exposes no installed BlueFieldSoftware version. This holds because DPUs are replaced rather than mutated when their deployment changes, so a DPU that predates the current software is still detected. It does mean the BlueFieldSoftware check answers "was this DPU created from the current software?" rather than "did it finish installing it?". If DPF later surfaces an installed version in DPU status, the check can be tightened to match the BFB path. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: abhi <abvarshney@nvidia.com>
Carbide decides whether a DPU needs reprovisioning by comparing what it is running against what its owning DPUDeployment declares. A deployment provisions its DPUs from one of two sources: a BFB, or a BlueFieldSoftware CR for BF4-class hardware. The DPU CRD requires exactly one of the two to be set.
Only the BFB case was ever compared. When a deployment provisioned from BlueFieldSoftware there was no BFB to compare against, so the staleness check fell back to the DPU flavor alone. A BlueFieldSoftware change therefore never registered as drift: the DPU was reported up to date, was never queued for reprovisioning, and kept running superseded software indefinitely. Only a flavor change could dislodge it.
This compares whichever provisioning source the owning deployment actually declares, so a BlueFieldSoftware change now marks its DPUs outdated exactly as a BFB change already did.
A deployment that declares neither source, or both, violates the CRD's own constraint. Those are skipped and logged rather than read as drift, so a malformed deployment cannot queue every DPU beneath it for reprovisioning.
Related issues
None.
Type of Change
Breaking Changes
Testing
Covers each provisioning source both matching and drifting, a flavor change on a BlueFieldSoftware-provisioned DPU, and the malformed-deployment guard.
Additional Notes
One asymmetry is worth knowing when reasoning about the behaviour. BFB staleness is judged against the image the DPU actually installed, whereas BlueFieldSoftware is judged against the software the DPU was created from, because DPU status exposes no installed BlueFieldSoftware version. This holds because DPUs are replaced rather than mutated when their deployment changes, so a DPU that predates the current software is still detected. It does mean the BlueFieldSoftware check answers "was this DPU created from the current software?" rather than "did it finish installing it?". If DPF later surfaces an installed version in DPU status, the check can be tightened to match the BFB path.
🤖 Generated with Claude Code