feat: DHCP every network interface by default when no custom network-config is set#3708
Draft
RajMandaliya wants to merge 10 commits into
Draft
feat: DHCP every network interface by default when no custom network-config is set#3708RajMandaliya wants to merge 10 commits into
RajMandaliya wants to merge 10 commits into
Conversation
Keeping Pace with original repo
…tings NICo seeds NoCloud's user-data and meta-data files, but never seeds the separate network-config file. A top-level 'network:' key inside user-data is not a valid user-data format (confirmed via cloud-init's own schema validator) and is silently ignored, regardless of the change in NVIDIA#2814 that correctly moved user-data to the canonical NoCloud seed location. Adds a new /network-config PXE route that extracts an existing 'network:' key from the tenant's custom_cloud_init (if present) and serves it as its own document, plus one line in disk_imaging.sh to fetch it into the seed directory. No API changes needed; existing 'network:' key placement keeps working as-is. Verified end-to-end against real cloud-init: a populated network-config applies correctly, and an empty one (the common case, no custom network settings) falls back cleanly to default DHCP with no errors. Addresses NVIDIA#3303. Signed-off-by: Raj <rajmandaliya2249@gmail.com>
- Avoid cloning the extracted network YAML value before serializing. - Consolidate the three extract_network_config tests into a single table-driven test, moved into the existing tests module. Signed-off-by: Raj <rajmandaliya2249@gmail.com>
Signed-off-by: RajMandaliya <112234020+RajMandaliya@users.noreply.github.com>
The GitHub web UI merge left an extra closing brace mid-file after combining the extract_network_config test with the two new metrics tests from main. All 22 tests pass. Signed-off-by: RajMandaliya <rajmandaliya2249@gmail.com>
Adds doc comments to get_router and network_config, the functions touched/added in this PR that were missing them. Signed-off-by: RajMandaliya <rajmandaliya2249@gmail.com>
Signed-off-by: RajMandaliya <rajmandaliya2249@gmail.com>
…config is set cloud-init's own default behavior (no network-config at all) only DHCPs the first network interface it finds. For multi-NIC hosts using qcow-imager (or any tenant not providing a custom network: key), this leaves every other interface unconfigured. Adds DEFAULT_NETWORK_CONFIG, served whenever a tenant hasn't supplied their own network: key, that DHCPs every interface matching either the predictable (en*) or legacy (eth*) naming convention. Verified against real cloud-init: confirmed the exact constant string (byte-for-byte, written to a real seed directory) produces the expected netplan config covering both naming conventions. Addresses NVIDIA#3665. Signed-off-by: RajMandaliya <rajmandaliya2249@gmail.com>
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 |
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.
Addresses #3665.
Draft/stacked on #3392 — this branch is built on top of the
network_config/extract_network_configwork in #3392, which hasn't merged yet, so this diff currently includes those commits too. Will rebase ontomainand clean this up once #3392 lands, at which point this should shrink down to just the new default-config logic below.Problem
cloud-init's own default behavior (no network-config at all) only DHCPs the first network interface it finds. For multi-NIC hosts (e.g. via qcow-imager) where a tenant hasn't provided a custom
network:key, every other interface is left unconfigured.Fix
Adds
DEFAULT_NETWORK_CONFIG, served whenever a tenant hasn't supplied their ownnetwork:key, that DHCPs every interface matching either the predictable (en*) or legacy (eth*) naming convention.Testing
Verified against real cloud-init: confirmed the exact constant string (byte-for-byte, written to a real seed directory) produces the
expected netplan config covering both naming conventions. Three new unit tests cover the decision logic directly (default when absent, custom value used when present, default when no cloud-init at all).
Opening as a draft for early visibility/feedback since #3665 was just filed - happy to adjust the interface-matching approach if there's a preference (e.g. whether to include dhcp6, or a different naming convention assumption) before this is ready to merge.