Include dracut module & systemd files for ignition-integration subpackage - #2250
Include dracut module & systemd files for ignition-integration subpackage#2250PeaceRebel wants to merge 7 commits into
Conversation
This is controlled by a flag and only included if set, intended for ignition-integration subpackage. We can remove this module once these imports are included in fedora-bootc. Related issue: coreos/fedora-coreos-tracker#1582
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (39)
🚧 Files skipped from review as they are similar to previous changes (38)
📜 Recent review details⏰ Context from checks skipped due to timeout. (6)
|
| Layer / File(s) | Summary |
|---|---|
Installation and initramfs packaging Makefile, dracut/dracut.conf.d/*, dracut/modules.d/01ignition-scsi-rules/*, dracut/modules.d/30ignition/* |
WITH_INTEGRATION selects core or full installation. Dracut setup installs Ignition binaries, services, targets, utilities, architecture-specific modules, SCSI rules, and encryption configuration. |
Ignition boot orchestration dracut/modules.d/30ignition/ignition-generator, dracut/modules.d/30ignition/*.target, dracut/modules.d/30ignition/*.service |
The generator selects boot paths. New services execute Ignition stages with explicit ordering and failure handling. |
OSTree filesystem handling dracut/modules.d/40ignition-ostree/* |
The OSTree module mounts and populates /var, relabels paths, and manages filesystem and partition transposition. |
Initrd network activation dracut/modules.d/41ignition-network/* |
The network module enables networking when /run/ignition/neednet exists and attaches the service to ignition-complete.target. |
Logging and provisioning status dracut/modules.d/99ignition-log-kmsg/*, scripts/libexec/ignition-write-issues, systemd/ignition-write-issues.service, systemd/system-preset/40-ignition.preset |
Ignition output is routed to kmsg. A systemd service records provisioning status, warnings, and configuration state under /run/issue.d. |
Estimated code review effort: 5 (Critical) | ~120 minutes
Sequence Diagram(s)
sequenceDiagram
participant ignition-generator
participant ignition-fetch-offline.service
participant ignition-fetch.service
participant ignition-disks.service
participant ignition-complete.target
ignition-generator->>ignition-fetch-offline.service: select initrd fetch stages
ignition-fetch-offline.service->>ignition-fetch.service: complete offline fetch
ignition-fetch.service->>ignition-disks.service: provide fetched configuration
ignition-disks.service->>ignition-complete.target: complete disk stage
🚥 Pre-merge checks | ✅ 3 | ❌ 3
❌ Failed checks (3 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | The title describes the change but does not follow the required subsystem: lowercase description format or imperative mood. |
Rewrite the title with a subsystem prefix, a lowercase imperative description, and no trailing period. | |
| Docstring Coverage | Docstring coverage is 34.29% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Commit Message Convention | Three non-merge commits violate the lowercase-description rule: dracut: Include..., dracut: Add..., and Makefile: Rename.... |
Rewrite those commit subjects with lowercase imperative verbs: include, add, and rename; keep the existing subsystem prefixes and omit periods. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description check | ✅ Passed | The description clearly covers the added Ignition integration files, conditional installation, and dracut directory reorganization. |
| 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. |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Fix failing CI checks
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs.sh (1)
244-244: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid parsing
lsoutput for the deployment paths.Line 244 word-splits the output of
ls -d. Deployment paths are normally safe, but the command also fails with a shell error message if the glob matches nothing.findhandles both cases directly.♻️ Proposed refactor
- chattr +i $(ls -d /sysroot/ostree/deploy/*/deploy/*/) + find /sysroot/ostree/deploy -mindepth 3 -maxdepth 3 -type d -path '*/deploy/*' \ + -exec chattr +i {} +🤖 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 `@dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs.sh` at line 244, Replace the chattr command’s ls-based path expansion with find-based path discovery in the ignition-ostree transposefs flow. Ensure matching deployment directories are passed safely without parsing ls output, and allow the no-match case to complete without emitting a shell glob error.
🤖 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 `@dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs.sh`:
- Around line 27-30: Update is_rhcos9 to safely handle missing os-release
variables under set -u by using default expansions for ID and RHEL_VERSION, and
replace the arithmetic version comparison with a plain string comparison so
empty or non-numeric values cannot abort cleanup. Keep sourcing /etc/os-release
behavior unchanged.
In `@dracut/modules.d/40ignition-ostree/ignition-relabel`:
- Around line 13-16: Update the no-argument branch in ignition-relabel to use
fatal for the usage messages instead of err, ensuring the script exits before
reaching the setfiles invocation when $# is zero.
In `@dracut/modules.d/41ignition-network/module-setup.sh`:
- Around line 19-26: Update install_and_enable_unit to invoke the configured
$SYSTEMCTL wrapper instead of hardcoding systemctl, while preserving the
existing --root="$initdir", add-requires arguments, and failure exit behavior.
In `@dracut/modules.d/99ignition-log-kmsg/00-journal-log-level-kmsg.conf`:
- Around line 1-4: Update the [Journal] configuration in
00-journal-log-level-kmsg.conf to add ForwardToKMsg=yes alongside
MaxLevelKMsg=info, ensuring journald forwards Ignition logs to /dev/kmsg for
kernel-log visibility.
In `@Makefile`:
- Around line 40-52: Merge the duplicate install target declarations into one
explicit .PHONY install declaration and a single install: all target containing
the existing recipe. Remove the empty install: entry, preserving the conditional
installation commands under install: all.
In `@scripts/libexec/ignition-write-issues`:
- Around line 19-29: Update the issue-file redirections in the blocks around the
previous Ignition report warning and the other warning block to write under
/run/issue.d instead of /etc/issue.d. Preserve the existing filenames and
message content, and keep the directory consistent with the directory created
earlier in the script.
---
Nitpick comments:
In `@dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs.sh`:
- Line 244: Replace the chattr command’s ls-based path expansion with find-based
path discovery in the ignition-ostree transposefs flow. Ensure matching
deployment directories are passed safely without parsing ls output, and allow
the no-match case to complete without emitting a shell glob error.
🪄 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: Pro Plus
Run ID: 343001d5-def5-46b4-b299-3761879f0e66
📒 Files selected for processing (39)
Makefiledracut/dracut.conf.d/60-omit-nfs.confdracut/modules.d/01ignition-scsi-rules/module-setup.shdracut/modules.d/30ignition/99-xx-ignition-systemd-cryptsetup.rulesdracut/modules.d/30ignition/ignition-cexdracut/modules.d/30ignition/ignition-complete.targetdracut/modules.d/30ignition/ignition-diskful-subsequent.targetdracut/modules.d/30ignition/ignition-diskful.targetdracut/modules.d/30ignition/ignition-disks.servicedracut/modules.d/30ignition/ignition-fetch-offline.servicedracut/modules.d/30ignition/ignition-fetch.servicedracut/modules.d/30ignition/ignition-files.servicedracut/modules.d/30ignition/ignition-generatordracut/modules.d/30ignition/ignition-kargs-helper.shdracut/modules.d/30ignition/ignition-kargs.servicedracut/modules.d/30ignition/ignition-luks.confdracut/modules.d/30ignition/ignition-mount.servicedracut/modules.d/30ignition/ignition-remount-sysroot.servicedracut/modules.d/30ignition/ignition-subsequent.targetdracut/modules.d/30ignition/module-setup.shdracut/modules.d/40ignition-ostree/ignition-ostree-mount-var.servicedracut/modules.d/40ignition-ostree/ignition-ostree-mount-var.shdracut/modules.d/40ignition-ostree/ignition-ostree-populate-var.servicedracut/modules.d/40ignition-ostree/ignition-ostree-populate-var.shdracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-detect.servicedracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-restore.servicedracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-save.servicedracut/modules.d/40ignition-ostree/ignition-ostree-transposefs.shdracut/modules.d/40ignition-ostree/ignition-relabeldracut/modules.d/40ignition-ostree/module-setup.shdracut/modules.d/41ignition-network/ignition-enable-network.servicedracut/modules.d/41ignition-network/ignition-enable-network.shdracut/modules.d/41ignition-network/module-setup.shdracut/modules.d/99ignition-log-kmsg/00-journal-log-level-kmsg.confdracut/modules.d/99ignition-log-kmsg/10-stdout-kmsg.confdracut/modules.d/99ignition-log-kmsg/module-setup.shscripts/libexec/ignition-write-issuessystemd/ignition-write-issues.servicesystemd/system-preset/40-ignition.preset
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: Test ignition-validate (1.26.x, windows-latest)
- GitHub Check: Shellcheck
- GitHub Check: Test (1.25.x)
- GitHub Check: Test (1.26.x)
- GitHub Check: tmt-tests
⚠️ CI failures not shown inline (4)
GitHub Actions: Release notes / 0_Require release note.txt: Include dracut module & systemd files for ignition-integration subpackage
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -n "skip-notes" ]; then�[0m
�[36;1m # Don't trust the label list in the event metadata, since runs�[0m
�[36;1m # can be scheduled out of order and the list might be stale.�[0m
�[36;1m label=$(curl --no-progress-meter \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m -H "Authorization: token ***" \�[0m
�[36;1m "https://api.github.com/repos/coreos/ignition/pulls/2250" |�[0m
�[36;1m jq '.labels[] | select(.name == "skip-notes")')�[0m
�[36;1m if [ -n "${label}" ]; then�[0m
�[36;1m echo "PR has skip-notes label; skipping"�[0m
�[36;1m exit 0�[0m
�[36;1m fi�[0m
�[36;1mfi�[0m
�[36;1mdiffinfo=$(curl --no-progress-meter \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m -H "Authorization: token ***" \�[0m
�[36;1m "https://api.github.com/repos/coreos/ignition/compare/a8fa306b0baa9169dcf25300d48979587304926e...5f3183fc9eb6e0edc291ffb9d28c7eaeb1032b5f" |�[0m
�[36;1m jq '.files[] | select(.filename == "docs/release-notes.md")')�[0m
�[36;1mif [ -z "${diffinfo}" ]; then�[0m
�[36;1m echo "Found no changes to docs/release-notes.md."�[0m
�[36;1m if [ -n "skip-notes" ]; then�[0m
�[36;1m echo "To ignore, add skip-notes label to PR."�[0m
�[36;1m fi�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1mecho "Found change to docs/release-notes.md."�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
Found no changes to docs/release-notes.md.
To ignore, add skip-notes label to PR.
##[error]Process completed with exit code 1.
GitHub Actions: Release notes / Require release note: Include dracut module & systemd files for ignition-integration subpackage
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -n "skip-notes" ]; then�[0m
�[36;1m # Don't trust the label list in the event metadata, since runs�[0m
�[36;1m # can be scheduled out of order and the list might be stale.�[0m
�[36;1m label=$(curl --no-progress-meter \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m -H "Authorization: token ***" \�[0m
�[36;1m "https://api.github.com/repos/coreos/ignition/pulls/2250" |�[0m
�[36;1m jq '.labels[] | select(.name == "skip-notes")')�[0m
�[36;1m if [ -n "${label}" ]; then�[0m
�[36;1m echo "PR has skip-notes label; skipping"�[0m
�[36;1m exit 0�[0m
�[36;1m fi�[0m
�[36;1mfi�[0m
�[36;1mdiffinfo=$(curl --no-progress-meter \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m -H "Authorization: token ***" \�[0m
�[36;1m "https://api.github.com/repos/coreos/ignition/compare/a8fa306b0baa9169dcf25300d48979587304926e...5f3183fc9eb6e0edc291ffb9d28c7eaeb1032b5f" |�[0m
�[36;1m jq '.files[] | select(.filename == "docs/release-notes.md")')�[0m
�[36;1mif [ -z "${diffinfo}" ]; then�[0m
�[36;1m echo "Found no changes to docs/release-notes.md."�[0m
�[36;1m if [ -n "skip-notes" ]; then�[0m
�[36;1m echo "To ignore, add skip-notes label to PR."�[0m
�[36;1m fi�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1mecho "Found change to docs/release-notes.md."�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
Found no changes to docs/release-notes.md.
To ignore, add skip-notes label to PR.
##[error]Process completed with exit code 1.
GitHub Actions: Binary Size / 0_Check binary size.txt: Include dracut module & systemd files for ignition-integration subpackage
Conclusion: failure
##[group]Run ./build
�[36;1m./build�[0m
shell: /usr/bin/bash -e {0}
env:
SIZE_THRESHOLD_PCT: 5
##[endgroup]
Usage: ./build [ignition|ignition-validate|ignition-validate-cross]
##[error]Process completed with exit code 1.
GitHub Actions: Binary Size / Check binary size: Include dracut module & systemd files for ignition-integration subpackage
Conclusion: failure
##[group]Run ./build
�[36;1m./build�[0m
shell: /usr/bin/bash -e {0}
env:
SIZE_THRESHOLD_PCT: 5
##[endgroup]
Usage: ./build [ignition|ignition-validate|ignition-validate-cross]
##[error]Process completed with exit code 1.
🧰 Additional context used
🪛 ast-grep (0.45.0)
dracut/modules.d/41ignition-network/ignition-enable-network.sh
[warning] 3-3: set +e (or set +o errexit) disables the shell's errexit option, so the script keeps running after a command fails. This masks failures of security-critical operations (downloads, signature/checksum verification, permission changes, cleanup of secrets), letting the script proceed with a bad or insecure state. Leave errexit enabled (set -e / set -euo pipefail), or handle failures explicitly with if/|| and an explicit exit instead of globally turning off failure detection.
Context: set +euo pipefail
Note: [CWE-754] Improper Check for Unusual or Exceptional Conditions.
(set-plus-e-error-masking-bash)
[warning] 9-9: set +e (or set +o errexit) disables the shell's errexit option, so the script keeps running after a command fails. This masks failures of security-critical operations (downloads, signature/checksum verification, permission changes, cleanup of secrets), letting the script proceed with a bad or insecure state. Leave errexit enabled (set -e / set -euo pipefail), or handle failures explicitly with if/|| and an explicit exit instead of globally turning off failure detection.
Context: set +euo pipefail
Note: [CWE-754] Improper Check for Unusual or Exceptional Conditions.
(set-plus-e-error-masking-bash)
[warning] 27-27: set +e (or set +o errexit) disables the shell's errexit option, so the script keeps running after a command fails. This masks failures of security-critical operations (downloads, signature/checksum verification, permission changes, cleanup of secrets), letting the script proceed with a bad or insecure state. Leave errexit enabled (set -e / set -euo pipefail), or handle failures explicitly with if/|| and an explicit exit instead of globally turning off failure detection.
Context: set +euo pipefail
Note: [CWE-754] Improper Check for Unusual or Exceptional Conditions.
(set-plus-e-error-masking-bash)
🪛 checkmake (0.3.2)
Makefile
[warning] 40-40: Target body for "install" exceeds allowed length of 5 lines (12).
(maxbodylength)
[warning] 40-40: Target "install" defined multiple times (lines 39 and 40).
(uniquetargets)
🔇 Additional comments (41)
dracut/modules.d/41ignition-network/ignition-enable-network.service (1)
1-24: LGTM!dracut/modules.d/41ignition-network/ignition-enable-network.sh (1)
1-31: LGTM!dracut/modules.d/41ignition-network/module-setup.sh (1)
1-17: LGTM!Also applies to: 28-33
dracut/modules.d/40ignition-ostree/ignition-ostree-mount-var.service (1)
1-26: LGTM!dracut/modules.d/40ignition-ostree/ignition-ostree-mount-var.sh (1)
9-54: LGTM!dracut/modules.d/40ignition-ostree/ignition-ostree-populate-var.service (1)
1-16: LGTM!dracut/modules.d/40ignition-ostree/ignition-ostree-populate-var.sh (1)
17-39: LGTM!dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-detect.service (1)
1-20: LGTM!dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-restore.service (1)
1-23: LGTM!dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-save.service (1)
1-19: LGTM!dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs.sh (4)
166-204: LGTM!
131-164: LGTM!
93-129: LGTM!Also applies to: 205-237
290-322: LGTM!dracut/modules.d/40ignition-ostree/module-setup.sh (2)
5-22: LGTM!Also applies to: 64-67, 75-75
24-27: 🩺 Stability & AvailabilityNo change needed for the initramfs tools.
40ignition-ostreedepends onignition, and30ignitionalready installs bothmkfs.xfsandlsblkinto the initramfs.> Likely an incorrect or invalid review comment.Makefile (1)
3-4: LGTM!dracut/dracut.conf.d/60-omit-nfs.conf (1)
1-7: LGTM!dracut/modules.d/01ignition-scsi-rules/module-setup.sh (1)
1-19: LGTM!dracut/modules.d/30ignition/99-xx-ignition-systemd-cryptsetup.rules (1)
1-8: LGTM!dracut/modules.d/30ignition/ignition-cex (1)
1-15: LGTM!dracut/modules.d/30ignition/ignition-luks.conf (1)
1-3: LGTM!dracut/modules.d/30ignition/module-setup.sh (1)
1-135: LGTM!dracut/modules.d/30ignition/ignition-generator (1)
1-62: LGTM!dracut/modules.d/30ignition/ignition-complete.target (1)
1-14: LGTM!dracut/modules.d/30ignition/ignition-kargs-helper.sh (1)
1-8: LGTM!dracut/modules.d/30ignition/ignition-diskful-subsequent.target (1)
1-13: LGTM!dracut/modules.d/30ignition/ignition-diskful.target (1)
1-13: LGTM!dracut/modules.d/30ignition/ignition-subsequent.target (1)
1-13: LGTM!dracut/modules.d/30ignition/ignition-fetch-offline.service (1)
1-24: LGTM!dracut/modules.d/30ignition/ignition-fetch.service (1)
1-28: LGTM!dracut/modules.d/30ignition/ignition-disks.service (1)
1-35: LGTM!dracut/modules.d/30ignition/ignition-mount.service (1)
1-39: LGTM!dracut/modules.d/30ignition/ignition-files.service (1)
1-21: LGTM!dracut/modules.d/30ignition/ignition-kargs.service (1)
1-22: LGTM!dracut/modules.d/30ignition/ignition-remount-sysroot.service (1)
1-20: 🗄️ Data Integrity & IntegrationNo change needed.
The target files are installed, and
ignition-remount-sysroot.serviceis wired toignition-diskful.target.dracut/modules.d/99ignition-log-kmsg/10-stdout-kmsg.conf (1)
1-4: LGTM!dracut/modules.d/99ignition-log-kmsg/module-setup.sh (1)
4-28: LGTM!systemd/ignition-write-issues.service (1)
1-16: LGTM!systemd/system-preset/40-ignition.preset (1)
5-6: LGTM!scripts/libexec/ignition-write-issues (1)
51-53: 🗄️ Data Integrity & IntegrationNo change needed for the kmsg journal path.
journald preserves
SYSLOG_IDENTIFIER=ignitionfor kmsg imports, and initrd logs are accessible throughjournalctlafter switch-root, so the current filter path matches Ignition warnings.
| is_rhcos9() { | ||
| source /etc/os-release | ||
| [ "${ID}" == "rhcos" ] && [ "${RHEL_VERSION%%.*}" -eq 9 ] | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard the os-release variables in is_rhcos9.
Line 29 expands ${ID} and ${RHEL_VERSION} without defaults. set -u is active. If an image ships /etc/os-release without RHEL_VERSION, the script aborts during cleanup, so the zram device stays attached and the unit stop fails. source also leaks os-release variables into the script scope.
🛠️ Proposed fix
is_rhcos9() {
- source /etc/os-release
- [ "${ID}" == "rhcos" ] && [ "${RHEL_VERSION%%.*}" -eq 9 ]
+ (
+ source /etc/os-release
+ [ "${ID:-}" == "rhcos" ] && [ "${RHEL_VERSION%%.*:-}" = "9" ]
+ )
}Use a plain string comparison so an empty or non-numeric value cannot fail the arithmetic test.
🤖 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 `@dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs.sh` around
lines 27 - 30, Update is_rhcos9 to safely handle missing os-release variables
under set -u by using default expansions for ID and RHEL_VERSION, and replace
the arithmetic version comparison with a plain string comparison so empty or
non-numeric values cannot abort cleanup. Keep sourcing /etc/os-release behavior
unchanged.
| if [ $# -eq 0 ]; then | ||
| err "Usage: $0 [PATTERN...]" | ||
| err " e.g.: $0 /etc/passwd '/etc/group*'" | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The usage branch does not exit.
Lines 13-16 print usage but continue execution. With no arguments and an SELinux config present, the script reaches line 35 and runs setfiles with an empty pattern list. Use fatal so the script stops.
🐛 Proposed fix
if [ $# -eq 0 ]; then
err "Usage: $0 [PATTERN...]"
- err " e.g.: $0 /etc/passwd '/etc/group*'"
+ fatal " e.g.: $0 /etc/passwd '/etc/group*'"
fi📝 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.
| if [ $# -eq 0 ]; then | |
| err "Usage: $0 [PATTERN...]" | |
| err " e.g.: $0 /etc/passwd '/etc/group*'" | |
| fi | |
| if [ $# -eq 0 ]; then | |
| err "Usage: $0 [PATTERN...]" | |
| fatal " e.g.: $0 /etc/passwd '/etc/group*'" | |
| fi |
🤖 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 `@dracut/modules.d/40ignition-ostree/ignition-relabel` around lines 13 - 16,
Update the no-argument branch in ignition-relabel to use fatal for the usage
messages instead of err, ensuring the script exits before reaching the setfiles
invocation when $# is zero.
| if ! test -f /run/ostree-live && jq -e .previousReport.provisioningDate "${IGNITION_RESULT}" &>/dev/null; then | ||
| prevdate=$(date --date "$(jq -r .previousReport.provisioningDate "${IGNITION_RESULT}")" +"%Y/%m/%d %H:%M:%S %Z") | ||
| cat << EOF > /etc/issue.d/30_ignition_run_more_than_once.issue | ||
| ${WARN} | ||
| ############################################################################ | ||
| WARNING: Ignition previously ran on ${prevdate}. Unexpected | ||
| behavior may occur. Ignition is not designed to run more than once per system. | ||
| ############################################################################ | ||
| ${RESET} | ||
| EOF | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the inconsistent issue-file directory.
Line 9 creates /run/issue.d only. Line 21 and line 52 write to /etc/issue.d instead. /etc/issue.d is not created anywhere in this script and is not guaranteed to exist on the host. If it does not exist, the redirection fails and set -euo pipefail at line 2 aborts the script.
Use /run/issue.d consistently at lines 21 and 52, matching the rest of the script.
🐛 Proposed fix for the directory mismatch
- cat << EOF > /etc/issue.d/30_ignition_run_more_than_once.issue
+ cat << EOF > /run/issue.d/30_ignition_run_more_than_once.issue journalctl -t ignition -o cat -p warning | sed -r 's/, line [0-9]+ col [0-9]+//g' | sort -u | while read line; do
- echo -e "${WARN}Ignition: $line${RESET}" >> /etc/issue.d/30_ignition_warnings.issue
+ echo -e "${WARN}Ignition: $line${RESET}" >> /run/issue.d/30_ignition_warnings.issue
doneAlso applies to: 51-53
🤖 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 `@scripts/libexec/ignition-write-issues` around lines 19 - 29, Update the
issue-file redirections in the blocks around the previous Ignition report
warning and the other warning block to write under /run/issue.d instead of
/etc/issue.d. Preserve the existing filenames and message content, and keep the
directory consistent with the directory created earlier in the script.
5f3183f to
efdc88f
Compare
PeaceRebel
added
the
override-size-check
efdc88f to
c7c2258
Compare
PeaceRebel
removed
the
override-size-check
There was a problem hiding this comment.
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 `@dracut/modules.d/41ignition-network/ignition-enable-network.sh`:
- Around line 4-6: Update the sourcing logic in ignition-enable-network.sh to
capture the exit status immediately after sourcing /usr/lib/dracut-lib.sh and
/usr/lib/dracut/hooks/cmdline/99-nm-config.sh while errexit is disabled, then
exit with that status when non-zero before continuing to the service success
path.
🪄 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: Pro Plus
Run ID: ec56e575-7e6b-444d-920c-9f525f3f9a95
📒 Files selected for processing (39)
Makefiledracut/dracut.conf.d/60-omit-nfs.confdracut/modules.d/01ignition-scsi-rules/module-setup.shdracut/modules.d/30ignition/99-xx-ignition-systemd-cryptsetup.rulesdracut/modules.d/30ignition/ignition-cexdracut/modules.d/30ignition/ignition-complete.targetdracut/modules.d/30ignition/ignition-diskful-subsequent.targetdracut/modules.d/30ignition/ignition-diskful.targetdracut/modules.d/30ignition/ignition-disks.servicedracut/modules.d/30ignition/ignition-fetch-offline.servicedracut/modules.d/30ignition/ignition-fetch.servicedracut/modules.d/30ignition/ignition-files.servicedracut/modules.d/30ignition/ignition-generatordracut/modules.d/30ignition/ignition-kargs-helper.shdracut/modules.d/30ignition/ignition-kargs.servicedracut/modules.d/30ignition/ignition-luks.confdracut/modules.d/30ignition/ignition-mount.servicedracut/modules.d/30ignition/ignition-remount-sysroot.servicedracut/modules.d/30ignition/ignition-subsequent.targetdracut/modules.d/30ignition/module-setup.shdracut/modules.d/40ignition-ostree/ignition-ostree-mount-var.servicedracut/modules.d/40ignition-ostree/ignition-ostree-mount-var.shdracut/modules.d/40ignition-ostree/ignition-ostree-populate-var.servicedracut/modules.d/40ignition-ostree/ignition-ostree-populate-var.shdracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-detect.servicedracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-restore.servicedracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-save.servicedracut/modules.d/40ignition-ostree/ignition-ostree-transposefs.shdracut/modules.d/40ignition-ostree/ignition-relabeldracut/modules.d/40ignition-ostree/module-setup.shdracut/modules.d/41ignition-network/ignition-enable-network.servicedracut/modules.d/41ignition-network/ignition-enable-network.shdracut/modules.d/41ignition-network/module-setup.shdracut/modules.d/99ignition-log-kmsg/00-journal-log-level-kmsg.confdracut/modules.d/99ignition-log-kmsg/10-stdout-kmsg.confdracut/modules.d/99ignition-log-kmsg/module-setup.shscripts/libexec/ignition-write-issuessystemd/ignition-write-issues.servicesystemd/system-preset/40-ignition.preset
🚧 Files skipped from review as they are similar to previous changes (38)
- systemd/system-preset/40-ignition.preset
- dracut/modules.d/99ignition-log-kmsg/00-journal-log-level-kmsg.conf
- dracut/modules.d/30ignition/ignition-luks.conf
- dracut/modules.d/41ignition-network/ignition-enable-network.service
- dracut/modules.d/30ignition/99-xx-ignition-systemd-cryptsetup.rules
- dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-detect.service
- systemd/ignition-write-issues.service
- dracut/modules.d/99ignition-log-kmsg/10-stdout-kmsg.conf
- dracut/modules.d/30ignition/ignition-diskful-subsequent.target
- dracut/modules.d/40ignition-ostree/ignition-ostree-populate-var.service
- dracut/modules.d/40ignition-ostree/ignition-relabel
- dracut/modules.d/30ignition/ignition-complete.target
- dracut/modules.d/40ignition-ostree/ignition-ostree-mount-var.service
- dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-restore.service
- dracut/modules.d/30ignition/ignition-fetch-offline.service
- dracut/dracut.conf.d/60-omit-nfs.conf
- dracut/modules.d/30ignition/ignition-kargs-helper.sh
- dracut/modules.d/40ignition-ostree/ignition-ostree-mount-var.sh
- dracut/modules.d/30ignition/ignition-subsequent.target
- Makefile
- dracut/modules.d/30ignition/ignition-mount.service
- dracut/modules.d/30ignition/ignition-cex
- scripts/libexec/ignition-write-issues
- dracut/modules.d/01ignition-scsi-rules/module-setup.sh
- dracut/modules.d/99ignition-log-kmsg/module-setup.sh
- dracut/modules.d/30ignition/ignition-remount-sysroot.service
- dracut/modules.d/30ignition/ignition-files.service
- dracut/modules.d/40ignition-ostree/module-setup.sh
- dracut/modules.d/30ignition/ignition-diskful.target
- dracut/modules.d/41ignition-network/module-setup.sh
- dracut/modules.d/30ignition/ignition-kargs.service
- dracut/modules.d/30ignition/ignition-disks.service
- dracut/modules.d/30ignition/ignition-generator
- dracut/modules.d/30ignition/ignition-fetch.service
- dracut/modules.d/30ignition/module-setup.sh
- dracut/modules.d/40ignition-ostree/ignition-ostree-populate-var.sh
- dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs.sh
- dracut/modules.d/40ignition-ostree/ignition-ostree-transposefs-save.service
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Test (1.26.x)
- GitHub Check: Test ignition-validate (1.26.x, windows-latest)
- GitHub Check: Test (1.25.x)
- GitHub Check: tmt-tests
⚠️ CI failures not shown inline (4)
GitHub Actions: Release notes / Require release note: Include dracut module & systemd files for ignition-integration subpackage
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -n "skip-notes" ]; then�[0m
�[36;1m # Don't trust the label list in the event metadata, since runs�[0m
�[36;1m # can be scheduled out of order and the list might be stale.�[0m
�[36;1m label=$(curl --no-progress-meter \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m -H "Authorization: token ***" \�[0m
�[36;1m "https://api.github.com/repos/coreos/ignition/pulls/2250" |�[0m
�[36;1m jq '.labels[] | select(.name == "skip-notes")')�[0m
�[36;1m if [ -n "${label}" ]; then�[0m
�[36;1m echo "PR has skip-notes label; skipping"�[0m
�[36;1m exit 0�[0m
�[36;1m fi�[0m
�[36;1mfi�[0m
�[36;1mdiffinfo=$(curl --no-progress-meter \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m -H "Authorization: token ***" \�[0m
�[36;1m "https://api.github.com/repos/coreos/ignition/compare/a8fa306b0baa9169dcf25300d48979587304926e...c7c225885b614aa121269ac08d99a0d4aa2dcbc4" |�[0m
�[36;1m jq '.files[] | select(.filename == "docs/release-notes.md")')�[0m
�[36;1mif [ -z "${diffinfo}" ]; then�[0m
�[36;1m echo "Found no changes to docs/release-notes.md."�[0m
�[36;1m if [ -n "skip-notes" ]; then�[0m
�[36;1m echo "To ignore, add skip-notes label to PR."�[0m
�[36;1m fi�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1mecho "Found change to docs/release-notes.md."�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
Found no changes to docs/release-notes.md.
To ignore, add skip-notes label to PR.
##[error]Process completed with exit code 1.
GitHub Actions: Release notes / 0_Require release note.txt: Include dracut module & systemd files for ignition-integration subpackage
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [ -n "skip-notes" ]; then�[0m
�[36;1m # Don't trust the label list in the event metadata, since runs�[0m
�[36;1m # can be scheduled out of order and the list might be stale.�[0m
�[36;1m label=$(curl --no-progress-meter \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m -H "Authorization: token ***" \�[0m
�[36;1m "https://api.github.com/repos/coreos/ignition/pulls/2250" |�[0m
�[36;1m jq '.labels[] | select(.name == "skip-notes")')�[0m
�[36;1m if [ -n "${label}" ]; then�[0m
�[36;1m echo "PR has skip-notes label; skipping"�[0m
�[36;1m exit 0�[0m
�[36;1m fi�[0m
�[36;1mfi�[0m
�[36;1mdiffinfo=$(curl --no-progress-meter \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m -H "Authorization: token ***" \�[0m
�[36;1m "https://api.github.com/repos/coreos/ignition/compare/a8fa306b0baa9169dcf25300d48979587304926e...c7c225885b614aa121269ac08d99a0d4aa2dcbc4" |�[0m
�[36;1m jq '.files[] | select(.filename == "docs/release-notes.md")')�[0m
�[36;1mif [ -z "${diffinfo}" ]; then�[0m
�[36;1m echo "Found no changes to docs/release-notes.md."�[0m
�[36;1m if [ -n "skip-notes" ]; then�[0m
�[36;1m echo "To ignore, add skip-notes label to PR."�[0m
�[36;1m fi�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1mecho "Found change to docs/release-notes.md."�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
Found no changes to docs/release-notes.md.
To ignore, add skip-notes label to PR.
##[error]Process completed with exit code 1.
GitHub Actions: Binary Size / Check binary size: Include dracut module & systemd files for ignition-integration subpackage
Conclusion: failure
##[group]Run ./build
�[36;1m./build�[0m
shell: /usr/bin/bash -e {0}
env:
SIZE_THRESHOLD_PCT: 5
##[endgroup]
Usage: ./build [ignition|ignition-validate|ignition-validate-cross]
##[error]Process completed with exit code 1.
GitHub Actions: Binary Size / 0_Check binary size.txt: Include dracut module & systemd files for ignition-integration subpackage
Conclusion: failure
##[group]Run ./build
�[36;1m./build�[0m
shell: /usr/bin/bash -e {0}
env:
SIZE_THRESHOLD_PCT: 5
##[endgroup]
Usage: ./build [ignition|ignition-validate|ignition-validate-cross]
##[error]Process completed with exit code 1.
🧰 Additional context used
🪛 ast-grep (0.45.0)
dracut/modules.d/41ignition-network/ignition-enable-network.sh
[warning] 3-3: set +e (or set +o errexit) disables the shell's errexit option, so the script keeps running after a command fails. This masks failures of security-critical operations (downloads, signature/checksum verification, permission changes, cleanup of secrets), letting the script proceed with a bad or insecure state. Leave errexit enabled (set -e / set -euo pipefail), or handle failures explicitly with if/|| and an explicit exit instead of globally turning off failure detection.
Context: set +euo pipefail
Note: [CWE-754] Improper Check for Unusual or Exceptional Conditions.
(set-plus-e-error-masking-bash)
[warning] 9-9: set +e (or set +o errexit) disables the shell's errexit option, so the script keeps running after a command fails. This masks failures of security-critical operations (downloads, signature/checksum verification, permission changes, cleanup of secrets), letting the script proceed with a bad or insecure state. Leave errexit enabled (set -e / set -euo pipefail), or handle failures explicitly with if/|| and an explicit exit instead of globally turning off failure detection.
Context: set +euo pipefail
Note: [CWE-754] Improper Check for Unusual or Exceptional Conditions.
(set-plus-e-error-masking-bash)
[warning] 27-27: set +e (or set +o errexit) disables the shell's errexit option, so the script keeps running after a command fails. This masks failures of security-critical operations (downloads, signature/checksum verification, permission changes, cleanup of secrets), letting the script proceed with a bad or insecure state. Leave errexit enabled (set -e / set -euo pipefail), or handle failures explicitly with if/|| and an explicit exit instead of globally turning off failure detection.
Context: set +euo pipefail
Note: [CWE-754] Improper Check for Unusual or Exceptional Conditions.
(set-plus-e-error-masking-bash)
| set +euo pipefail | ||
| . /usr/lib/dracut-lib.sh | ||
| set -euo pipefail |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Dracut path references ---'
rg -n -C 5 \
'dracut-lib\.sh|99-nm-config\.sh|hookdir' \
--glob '*.sh' --glob 'module-setup.sh' . || true
printf '%s\n' '--- Ignition network service references ---'
rg -n -C 6 \
'ignition-enable-network|rd\.neednet|NetworkManager' \
--glob '*.sh' --glob '*.service' . || trueRepository: coreos/ignition
Length of output: 8841
🌐 Web query:
dracut module-setup.sh install_hook install_dirhook dracut version hookdir /usr/lib/dracut/hooks/cmdline default hooks path documentation
💡 Result:
In Dracut, module-setup.sh is the central file for defining how a module is installed into the initramfs [1][2]. The functions inst_hook and inst_dir are utility functions used within the install or installkernel functions of module-setup.sh to place scripts and files into the initramfs [3][4]. Hook Management The inst_hook function is used to install an executable or script into a specified Dracut hook directory [4][5]. Its usage syntax is inst_hook [3][4]: - : The target hook stage (e.g., cmdline, pre-udev, pre-trigger, initqueue, pre-mount, mount, pre-pivot, or cleanup) [6][7]. - : A two-digit numeric priority that determines the execution order within that hook stage (lower numbers execute first) [8][7]. - : The source path of the script to install [4]. The script must end in.sh [8][9]. Dracut looks for these hook scripts in specific directories within the initramfs, typically rooted at /usr/lib/dracut/hooks, /etc/dracut/hooks, and /var/lib/dracut/hooks [6][10]. The /usr/lib/dracut/hooks path is primarily reserved for standard, distribution-shipped hook scripts, while /etc/dracut/hooks allows for local overrides, and /var/lib/dracut/hooks is used for runtime modifications by Dracut modules [6][11]. Directory Management The inst_dir function is used to create a directory in the initramfs [4][8]. Unlike other installation functions, it creates the directory itself but does not install its contents [4]. Example Usage To install a script named parse-insmodpost.sh into the cmdline hook with a priority of 20, you would use [1][5]: inst_hook cmdline 20 "$moddir/parse-insmodpost.sh" This ensures that the script is included in the initramfs and executed during the appropriate boot process stage [8][9].
Citations:
- 1: https://manpages.ubuntu.com/manpages/jammy/man7/dracut.modules.7.html
- 2: https://man.adelielinux.org/html/packages/user/x86_64/dracut-050-r3/man-7-dracut.modules.html
- 3: https://manpages.ubuntu.com/manpages/focal/en/man7/dracut.modules.7.html
- 4: https://man7.org/linux/man-pages/man7/dracut.modules.7.html
- 5: https://github.com/dracutdevs/dracut/blob/master/man/dracut.modules.7.asc
- 6: https://manpages.debian.org/unstable/dracut-core/dracut.modules.7.en.html
- 7: https://github.com/barbecued/dracut/blob/6ed73e37f225ece526caefe539e6365779f50f81/dracut-init.sh
- 8: http://wwoods.fedorapeople.org/doc/dracut-notes.html
- 9: https://manpages.debian.org/trixie/dracut-core/dracut.modules.7.en.html
- 10: https://manpages.opensuse.org/Tumbleweed/dracut/dracut.modules.7.en.html
- 11: https://man.archlinux.org/man/dracut.modules.7
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- module-setup.sh ---'
cat -n dracut/modules.d/41ignition-network/module-setup.sh
printf '%s\n' '--- ignition Enable Network script ---'
cat -n dracut/modules.d/41ignition-network/ignition-enable-network.sh
printf '%s\n' '--- installed service ---'
cat -n dracut/modules.d/41ignition-network/ignition-enable-network.service
printf '%s\n' '--- broader ignore references ---'
rg -n -C 3 'ignition-enable-network|NetworkManager-config-initrd|rd\.neednet' . || true
printf '\n%s\n' '--- git status/stat ---'
git status --short
git diff --statRepository: coreos/ignition
Length of output: 7497
Propagate failures from sourced Dracut scripts.
Both sourced hooks disable errexit, so a failure in /usr/lib/dracut-lib.sh or /usr/lib/dracut/hooks/cmdline/99-nm-config.sh is discarded. Capture $? after each source and exit on non-zero before the service can succeed without requiring network setup or configuring NetworkManager.
Proposed fix
set +euo pipefail
. /usr/lib/dracut-lib.sh
+rc=$?
set -euo pipefail
+if (( rc != 0 )); then
+ printf '%s\n' "failed to source /usr/lib/dracut-lib.sh" >&2
+ exit "$rc"
+fi
...
set +euo pipefail
. /usr/lib/dracut/hooks/cmdline/99-nm-config.sh
+ rc=$?
set -euo pipefail
+ if (( rc != 0 )); then
+ printf '%s\n' "failed to configure NetworkManager in the initrd" >&2
+ exit "$rc"
+ fi🤖 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 `@dracut/modules.d/41ignition-network/ignition-enable-network.sh` around lines
4 - 6, Update the sourcing logic in ignition-enable-network.sh to capture the
exit status immediately after sourcing /usr/lib/dracut-lib.sh and
/usr/lib/dracut/hooks/cmdline/99-nm-config.sh while errexit is disabled, then
exit with that status when non-zero before continuing to the service success
path.
Source: Linters/SAST tools
This module is only included only if network-manager is available. Ordering is important here as `network-manager`(or others) is included by `network` dracut module with prefix 40. We need this module to have higher numbered prefix. Contains scripts and services to enable networking if ignition requires it. Removed all parts that uses afterburn. We can switch to using `NetworkManager-config-initrd.service` once it's available. Related to: coreos/fedora-coreos-tracker#1582
This module contains scripts and services to reprovision rootfs, mount and populate /var, etc. Related to: coreos/fedora-coreos-tracker#1582
1. ignition-write-issues & ignition-write-issue.service To write issues files with info about ignition runwq Include systemd presets to enable the above services (and a few others too). Related to: coreos/fedora-coreos-tracker#1582
Omitting `nfs` is important here as it prevents `/var` from being mounted and eventually causing boot failure. Moved module to subdir `modules.d`. Related to: coreos/fedora-coreos-tracker#1582
This module replaces global log forwarding from https://github.com/coreos/fedora-coreos-config/tree/0a9be179334d809ba034468e238d0e7f6d1cac0a/overlay.d/05core/usr/lib/dracut/modules.d/99journal-conf and enable logging to kmsg to unit level configuration. Related to: coreos/fedora-coreos-tracker#1582
c7c2258 to
b5306f9
Compare
1 participant
WITH_INTEGRATION=1so default installs keep only the existing30ignitionmodule.See: coreos/fedora-coreos-tracker#1582