fix(dpf): start without the site-wide BMC root credential#4167
Draft
wminckler wants to merge 2 commits into
Draft
fix(dpf): start without the site-wide BMC root credential#4167wminckler wants to merge 2 commits into
wminckler wants to merge 2 commits into
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Operators set the site-wide BMC root credential through the API, so on a fresh site it does not exist when nico-api first boots. DPF SDK init read it unconditionally and propagated the failure out of setup, aborting the boot before the listener started -- leaving no way to set the credential that would let the next boot succeed. Make the initial read best-effort when a BMC password refresh interval is configured: initialization continues without the Secret and the existing 60s refresh task writes it as soon as the credential appears, so no restart is needed. With no refresh task nothing would ever retry, so there the failure stays fatal. The refresh task could not have recovered as written: create_secret returned early when the Secret already existed, so a rotated password was never propagated to the cluster. Rename it to apply_secret and implement it as a server-side apply, matching apply_configmap. Document the credential as a DPF setup step, note the Site Explorer precondition on the two getting-started pages that previously implied "after deploy" was the only option, and add a commented-out kvSeeds entry so a fresh cluster has an obvious place to seed it. Fixes NVIDIA#4130 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mirrors the cluster-side steps of the `dpf` recipe in the forged repo: namespace, hbn-user-password and dpf-pull-secret Secrets labelled for DPF mirroring, the dpf-operator chart, and cluster-wide access for the nico-api ServiceAccount. Off by default -- it needs an NGC API key and pulls the operator images. Stops short of DPFOperatorConfig, DPUCluster, and the keepalived VIP LoadBalancer that the forged recipe applies. Those describe DPU-bearing hosts and a routable management network with nothing to bind to locally. What is installed exercises nico-api's DPF initialization path against a real operator; no DPU is provisioned. Enabling DPF also requires `[dpf].enabled = true` in the site config, which lived inline in values.base.yaml. Helm values are strings, so a second values file could only replace that config, not extend it. Move the TOML to site-config.toml and render it into values.generated.yaml, appending site-config.dpf.toml when DPF is on. LOCAL_DEV_SEED_BMC_ROOT=0 skips seeding the site-wide BMC root credential into Vault, which reproduces the fresh-site startup path from NVIDIA#4130. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
wminckler
force-pushed
the
fix/dpf-init-without-bmc-root-cred
branch
from
July 25, 2026 12:43
a838452 to
708d0bd
Compare
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
1 participant
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.
Operators set the site-wide BMC root credential through the API, so on a fresh
site it does not exist when nico-api first boots. DPF SDK initialization read
that credential unconditionally and propagated the failure out of setup,
aborting the boot before the listener started — which left no way to set the
credential that would have let the next boot succeed. A fresh site with DPF
enabled could not be brought up at all.
This makes the initial read best-effort when a BMC password refresh interval is
configured: initialization continues without the Secret, and the existing 60s
refresh task writes it as soon as the credential appears, so no restart is
needed. With no refresh task configured nothing would ever retry, so there the
failure stays fatal.
The refresh task could not have recovered as written:
create_secretreturnedearly when the Secret already existed, so a rotated password was never
propagated to the cluster. It is renamed to
apply_secretand implemented as aserver-side apply, matching
apply_configmap.The second commit adds optional DPF operator installation to the devspace
environment, mirroring the cluster-side steps of the
dpfrecipe in the forgedrepo (namespace,
hbn-user-passwordanddpf-pull-secretSecrets labelled forDPF mirroring, the
dpf-operatorchart, and cluster-wide access for thenico-apiServiceAccount). It is off by default because it needs an NGC APIkey and pulls the operator images. It stops short of
DPFOperatorConfig,DPUCluster, and the keepalived VIP LoadBalancer, which describe DPU-bearinghosts and a routable management network with nothing to bind to locally. What
is installed exercises nico-api's DPF initialization path against a real
operator; no DPU is provisioned.
LOCAL_DEV_SEED_BMC_ROOT=0skips seeding thesite-wide BMC root credential into Vault, which reproduces the fresh-site
startup path this PR fixes.
Enabling DPF also requires
[dpf].enabled = truein the site config, whichlived inline in
values.base.yaml. Helm values are strings, so a second valuesfile could only replace that config, not extend it — the TOML moves to
site-config.tomland is rendered intovalues.generated.yaml, appendingsite-config.dpf.tomlwhen DPF is on.Related issues
Fixes #4130
Type of Change
Breaking Changes
Testing
Four new unit tests cover the refresh task writing the Secret once the password
appears, staying unwritten while it is unavailable, and build succeeding vs.
failing when the password is unset with and without a refresh interval
configured.
Additional Notes
Docs updates ride along: the credential is documented as a DPF setup step, the
Site Explorer precondition is noted on the two getting-started pages that
previously implied "after deploy" was the only option, and a commented-out
kvSeedsentry gives a fresh cluster an obvious place to seed it.🤖 Generated with Claude Code