feat(helm): machine-a-tron mac address allocation#3700
Open
akorobkov-nvda wants to merge 2 commits into
Open
Conversation
akorobkov-nvda
requested review from
a team,
polarweasel and
shayan1995
as code owners
July 17, 2026 22:23
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 (5)
🚧 Files skipped from review as they are similar to previous changes (5)
Summary by CodeRabbit
WalkthroughThe Helm chart adds deterministic, configurable per-pod MAC pools and hardware MAC ranges. Values, templates, multi-pod examples, and deployment documentation now describe automatic allocation and explicit override paths. ChangesPer-pod MAC pool allocation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HelmValues
participant ConfigMapTemplate
participant MatToml
HelmValues->>ConfigMapTemplate: Supply MAC defaults and pod overrides
ConfigMapTemplate->>ConfigMapTemplate: Sort active pods and calculate podIndex
ConfigMapTemplate->>MatToml: Render per-pod MAC pool sections
🚥 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
🤖 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 `@helm/charts/nico-machine-a-tron/README.md`:
- Around line 47-51: Restore the TOML code-fence boundaries in
helm/charts/nico-machine-a-tron/README.md: at lines 47-51, open a toml fence
before [site_explorer] and close it after override_target_port; at lines
235-236, open a toml fence before [networks.pod-0-oob] and close it after the
final gateway. Ensure subsequent Markdown sections render normally.
- Around line 367-375: Update the troubleshooting section in the README by
changing the “Check service selector” and “Check pod labels” headings to
lower-level headings that remain nested under the parent section. Wrap each
kubectl command in its own closed bash code fence, keeping the existing commands
and making the Markdown structure render correctly.
In `@helm/charts/nico-machine-a-tron/templates/configmap.yaml`:
- Line 36: Update the $hwMacBasePrefix fallback in the ConfigMap template from
the single-octet "06" value to the documented two-octet default "02:01",
matching values.yaml and producing valid hardware address prefixes when
basePrefix is absent.
- Around line 54-60: Validate the computed $podIndex after the podList lookup
and fail template rendering when it is greater than or equal to 256, before the
mac_address_pool and hw_mac_address_ranges values format it with %02x. Preserve
the existing index calculation for valid indices and use the chart’s established
template failure mechanism.
- Around line 29-35: Update the macAddressPool and hwMacAddressRanges generation
toggle assignments, including $macPoolEnabled and $hwMacRangesEnabled, so
explicit false values are preserved instead of being replaced by default true.
Use dig with an enabled key default or an explicit key-presence check, while
retaining true as the fallback when the key is absent.
🪄 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: 40c58f70-2d80-4a00-a75e-ab3bba7f42e5
📒 Files selected for processing (5)
docs/development/machine-a-tron-deployment.mdhelm-prereqs/values/machine-a-tron-multipod.yamlhelm/charts/nico-machine-a-tron/README.mdhelm/charts/nico-machine-a-tron/templates/configmap.yamlhelm/charts/nico-machine-a-tron/values.yaml
|
🌿 Preview your docs: https://nvidia-preview-pull-request-3700.docs.buildwithfern.com/infra-controller |
poroh
approved these changes
Jul 17, 2026
poroh
approved these changes
Jul 20, 2026
akorobkov-nvda
force-pushed
the
feat/machine-a-tron-mac-assign
branch
from
July 20, 2026 18:24
94eb19c to
ce37eff
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
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.
Adds auto-generated per-pod MAC address pools to prevent collisions in multi-pod machine-a-tron deployments.
mac_address_pool- Standalone MACs (BMC, DPU interfaces)hw_mac_address_ranges- Contiguous NIC blocks (ConnectX, storage)Related issues
Closes #3698
Type of Change
Breaking Changes
Testing
Test Results