Skip to content

fix(pxe): always seed NoCloud userdata during imaging#2814

Merged
ajf merged 2 commits into
NVIDIA:mainfrom
iExalt:codex/pxe-seed-nocloud
Jun 25, 2026
Merged

fix(pxe): always seed NoCloud userdata during imaging#2814
ajf merged 2 commits into
NVIDIA:mainfrom
iExalt:codex/pxe-seed-nocloud

Conversation

@iExalt

@iExalt iExalt commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Description

This PR replaces #2608 with the smaller NoCloud-seeding portion of that change.

It updates pxe/common_files/disk_imaging.sh so host OS imaging:

  • creates /mnt/etc/cloud/cloud.cfg.d when cloud-init is present before writing the datasource list
  • seeds allocation-time cloud-init through the canonical NoCloud user-data and meta-data files
  • creates /mnt/var/lib/cloud/seed/nocloud-net even when the source image does not already contain cloud-init state directories

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Related Issues (Optional)

Part of #2604. Replaces #2608.

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Manual testing:

  • bash -n pxe/common_files/disk_imaging.sh
  • git diff --check

Additional Notes

N/A

@copy-pr-bot

copy-pr-bot Bot commented Jun 23, 2026

Copy link
Copy Markdown

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.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6e0e2819-e34f-41d1-9940-2a8e399e43ca

📥 Commits

Reviewing files that changed from the base of the PR and between 34e1701 and 784b384.

📒 Files selected for processing (1)
  • pxe/common_files/disk_imaging.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • pxe/common_files/disk_imaging.sh

Summary by CodeRabbit

  • Refactor
    • Updated cloud-init seeding during disk imaging to use NoCloud’s seed directory, ensuring user-data and meta-data are seeded reliably.
    • Removed the previous conditional behavior that depended on the presence of cloud.cfg.d, replacing it with more flexible directory handling.
    • Kept datasource list/config generation, while broadening checks to improve compatibility and creating cloud.cfg.d when required.

Walkthrough

The add_cloud_init() function in disk_imaging.sh is modified to seed cloud-init via the NoCloud seed directory (/mnt/var/lib/cloud/seed/nocloud-net), downloading both user-data and meta-data there. The prior conditional download of user-data into cloud.cfg.d is removed, and the directory existence check is relaxed to /mnt/etc/cloud.

Changes

NoCloud Seed Behavior Change

Layer / File(s) Summary
NoCloud seed directory creation and data download
pxe/common_files/disk_imaging.sh
add_cloud_init() now unconditionally creates /mnt/var/lib/cloud/seed/nocloud-net and downloads both user-data and meta-data from cloud_init_url into it. The datasource-list config (98-forge-dslist.cfg) write is relaxed to check only /mnt/etc/cloud, with cloud.cfg.d created on demand. The previous conditional user-data download into cloud.cfg.d is removed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the primary change: updating the disk imaging process to consistently seed NoCloud cloud-init configuration.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the three key modifications to cloud-init seeding behavior in the disk imaging script.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@iExalt iExalt marked this pull request as ready for review June 23, 2026 19:48
@iExalt iExalt requested a review from a team as a code owner June 23, 2026 19:48
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{}

@iExalt

iExalt commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@pxe/common_files/disk_imaging.sh`:
- Around line 150-151: Add the `--fail` flag to both curl commands (the one
downloading to "$seed_dir/user-data" and the one downloading to
"$seed_dir/meta-data") so that HTTP error responses are properly treated as
failures rather than writing error pages into the seed files. Additionally,
properly quote the `$log_output` variable in both `tee $log_output` invocations
by wrapping it as `tee "$log_output"` to resolve shell quoting issues and ensure
path variables with spaces are handled correctly.
🪄 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: f5a8adee-cc9d-4aa0-96e7-2927e9473306

📥 Commits

Reviewing files that changed from the base of the PR and between 40c7ca4 and 34e1701.

📒 Files selected for processing (1)
  • pxe/common_files/disk_imaging.sh

Comment thread pxe/common_files/disk_imaging.sh Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Clement Liaw <cman101202@gmail.com>
@github-actions

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
boot-artifacts-aarch64 3 0 0 3 0 0
boot-artifacts-x86_64 3 0 0 3 0 0
forge-admin-cli-x86_64 264 6 24 98 6 130
machine-validation-runner 712 32 184 267 35 194
machine_validation 712 32 184 267 35 194
nvmetal-carbide 712 32 184 267 35 194
TOTAL 2406 102 576 905 111 712

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

@ajf ajf merged commit a6448eb into NVIDIA:main Jun 25, 2026
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants