Update docs for prior PRs#3512
Conversation
Signed-off-by: Stoo Davies <189670495+stoo-davies@users.noreply.github.com>
|
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. |
Summary by CodeRabbit
WalkthroughUpdated the documented iPXE submodule version and added DHCP troubleshooting guidance covering Kea NAK responses after prior DHCPDECLINE events and the configured quarantine duration. ChangesDocumentation updates
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔍 Container Scan SummaryNo Grype artifacts were found to aggregate. |
|
🌿 Preview your docs: https://nvidia-preview-pull-request-3512.docs.buildwithfern.com/infra-controller |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/development/build-guide.md`:
- Line 12: Update the iPXE version entry in the build-guide table to identify
submodule commit bbd7821bd42da5456ee068a471ef73d525ea26a1, using either the full
SHA or its exact describe string, while leaving the submodule path unchanged.
In `@docs/playbooks/stuck_objects/network_connectivity.md`:
- Around line 65-69: Update the DHCPDECLINE documentation to capitalize “Kea”
consistently and state that nico-dhcp.config.kea.declineProbationPeriod defaults
to 900 seconds only when unset, while preserving the explanation of the
temporary quarantine behavior.
🪄 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: 331d7fa1-0a84-47d7-967e-debdd3328839
📒 Files selected for processing (2)
docs/development/build-guide.mddocs/playbooks/stuck_objects/network_connectivity.md
| |-----------|------|----------------| | ||
| | mkosi | `pxe/mkosi` | `v25` | | ||
| | iPXE (secboot fork) | `pxe/ipxe/upstream` | `secboot-ioactive-20221109-302-gd7e58c5a8` | | ||
| | iPXE (upstream) | `pxe/ipxe/upstream` | `v2.0.0-130-gbbd7821bd8` | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
remote="https://github.com/ipxe/ipxe.git"
ref="v2.0.0-130-gbbd7821bd8"
git ls-remote --exit-code "$remote" "refs/tags/$ref" >/dev/null ||
git ls-remote "$remote" | awk '$1 ~ /^bbd7821bd8/ { found=1 } END { exit !found }'Repository: NVIDIA/infra-controller
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- docs/development/build-guide.md (around line 12) ---\n'
sed -n '1,40p' docs/development/build-guide.md
printf '\n--- submodule metadata ---\n'
if [ -f .gitmodules ]; then
cat -n .gitmodules
fi
printf '\n--- repo references to v2.0.0-130-gbbd7821bd8 ---\n'
rg -n "v2\.0\.0-130-gbbd7821bd8|pxe/ipxe/upstream|ipxe" docs .gitmodules . || trueRepository: NVIDIA/infra-controller
Length of output: 50383
🌐 Web query:
iPXE "v2.0.0-130-gbbd7821bd8" tag or commit
💡 Result:
The string v2.0.0-130-gbbd7821bd8 is not a recognized tag or official release version for iPXE [1][2]. iPXE uses a versioning format for rolling releases generated from the Git repository, typically following the pattern v<last_tag>-<number_of_commits_since_tag>-g<commit_hash>. Based on the official iPXE repository, the version v2.0.0 was released on March 6, 2026, with the commit SHA 12798ec29aa8a64d8675c4378b99f5fe28447afb [1][3]. The specific string v2.0.0-130-gbbd7821bd8 appears to be a misidentified or non-existent build string in the context of the iPXE project. While v2.0.0 is the current stable major release, there is no evidence of a build with that specific commit hash (bbd7821bd8) in the official iPXE codebase [1][3]. Furthermore, searches indicate that the hex code bbd782 corresponds to a color, and similar versioning strings appear in unrelated repositories, such as the AWS CDK, which uses v2.130.0 [4][5].
Citations:
- 1: https://github.com/ipxe/ipxe/releases/tag/v2.0.0
- 2: https://github.com/ipxe/ipxe/releases
- 3: ipxe/ipxe@12798ec
- 4: https://github.com/aws/aws-cdk/releases/tag/v2.130.0
- 5: https://www.color-hex.com/color/bbd782
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- superproject gitlink for pxe/ipxe/upstream ---\n'
git ls-tree HEAD pxe/ipxe/upstream || true
printf '\n--- submodule repository presence ---\n'
if [ -d .git/modules/pxe/ipxe/upstream ]; then
echo ".git/modules/pxe/ipxe/upstream exists"
git --git-dir=.git/modules/pxe/ipxe/upstream rev-parse --verify HEAD
printf '\nrecent refs (if any):\n'
git --git-dir=.git/modules/pxe/ipxe/upstream show-ref --head | tail -n 20
else
echo "submodule repo not present"
fiRepository: NVIDIA/infra-controller
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- gitlink for pxe/ipxe/upstream in HEAD ---'
git ls-tree HEAD pxe/ipxe/upstream || true
echo
echo '--- submodule directory check ---'
if [ -d pxe/ipxe/upstream ]; then
echo 'pxe/ipxe/upstream exists'
if [ -d .git/modules/pxe/ipxe/upstream ]; then
echo 'submodule gitdir exists'
git --git-dir=.git/modules/pxe/ipxe/upstream rev-parse --verify HEAD
echo
echo 'HEAD commit object type:'
git --git-dir=.git/modules/pxe/ipxe/upstream cat-file -t HEAD
else
echo 'submodule gitdir missing'
fi
else
echo 'pxe/ipxe/upstream directory missing'
fi
echo
echo '--- iPXE version string references in repo (limited) ---'
rg -n --max-count 20 'v2\.0\.0-130-gbbd7821bd8|pxe/ipxe/upstream|iPXE \(upstream\)' docs .gitmodules || trueRepository: NVIDIA/infra-controller
Length of output: 773
Update the iPXE pin in docs/development/build-guide.md:12 The table entry should match the submodule gitlink (bbd7821bd42da5456ee068a471ef73d525ea26a1). v2.0.0-130-gbbd7821bd8 does not identify that pinned commit, so use the full SHA or the exact describe string for the commit.
🤖 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 `@docs/development/build-guide.md` at line 12, Update the iPXE version entry in
the build-guide table to identify submodule commit
bbd7821bd42da5456ee068a471ef73d525ea26a1, using either the full SHA or its exact
describe string, while leaving the submodule path unchanged.
Source: Path instructions
| A prior DHCPDECLINE can make Kea reject the correct address with a NAK | ||
| because the lease is temporarily marked unavailable. | ||
| nico-dhcp.config.kea.declineProbationPeriod now defaults to 900 seconds, | ||
| instead of the kea default of 24 hours, limiting how long the address | ||
| remains quarantined. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify the probation period as the chart default.
The Helm configuration allows declineProbationPeriod to be overridden, so this should say “defaults to 900 seconds when unset.” Also capitalize Kea for consistency.
Proposed wording
- nico-dhcp.config.kea.declineProbationPeriod now defaults to 900 seconds,
- instead of the kea default of 24 hours, limiting how long the address
+ When unset, nico-dhcp.config.kea.declineProbationPeriod defaults to 900 seconds,
+ instead of Kea's default of 24 hours, limiting how long the addressAs per path instructions, documentation must accurately reflect current deployment configuration and remain clear for operators.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| A prior DHCPDECLINE can make Kea reject the correct address with a NAK | |
| because the lease is temporarily marked unavailable. | |
| nico-dhcp.config.kea.declineProbationPeriod now defaults to 900 seconds, | |
| instead of the kea default of 24 hours, limiting how long the address | |
| remains quarantined. | |
| A prior DHCPDECLINE can make Kea reject the correct address with a NAK | |
| because the lease is temporarily marked unavailable. | |
| When unset, nico-dhcp.config.kea.declineProbationPeriod defaults to 900 seconds, | |
| instead of Kea's default of 24 hours, limiting how long the address | |
| remains quarantined. |
🤖 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 `@docs/playbooks/stuck_objects/network_connectivity.md` around lines 65 - 69,
Update the DHCPDECLINE documentation to capitalize “Kea” consistently and state
that nico-dhcp.config.kea.declineProbationPeriod defaults to 900 seconds only
when unset, while preserving the explanation of the temporary quarantine
behavior.
Source: Path instructions
1 participant
Minor doc update for prior PRs.
Related issues
Type of Change
Breaking Changes
Testing
Additional Notes