OCPBUGS-43501: Handle kargs.json correctly in minimal ISO#618
OCPBUGS-43501: Handle kargs.json correctly in minimal ISO#618zaneb wants to merge 5 commits intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@zaneb: This pull request references Jira Issue OCPBUGS-43501, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zaneb The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/jira refresh |
|
@zaneb: This pull request references Jira Issue OCPBUGS-43501, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #618 +/- ##
==========================================
+ Coverage 59.02% 60.51% +1.49%
==========================================
Files 27 27
Lines 1674 1755 +81
==========================================
+ Hits 988 1062 +74
- Misses 524 527 +3
- Partials 162 166 +4
🚀 New features to boost your workflow:
|
|
/retitle OCPBUGS-43501,OCPBUGS-64929: Handle kargs.json correctly in minimal ISO |
|
@zaneb: This pull request references Jira Issue OCPBUGS-43501, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. This pull request references Jira Issue OCPBUGS-64929, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@zaneb: This pull request references Jira Issue OCPBUGS-43501, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-64929, which is invalid:
Comment DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@zaneb: This pull request references Jira Issue OCPBUGS-43501, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-64929, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
ef112b8 to
ab25b69
Compare
carbonin
left a comment
There was a problem hiding this comment.
Can we rename some of the functions so it's easier to understand the difference between updateKargs, updateKernelArgs, and updateDefaultKargs?
|
@zaneb: This pull request references Jira Issue OCPBUGS-43501, which is invalid:
Comment This pull request references Jira Issue OCPBUGS-64929, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@zaneb: This pull request references Jira Issue OCPBUGS-43501, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-64929, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retitle OCPBUGS-43501: Handle kargs.json correctly in minimal ISO I will rebase this once #620 merges. |
|
@zaneb: This pull request references Jira Issue OCPBUGS-43501, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Quoting is required in grub.cfg, and was added in MGMT-4693. This solves the problem that the '&' character in a URL is considered a word separator by Grub. However, having different kernel args between isolinux.cfg and grub.cfg is likely problematic for coreos-installer when editing kargs. Making them the same length also avoids the need to adjust padding in order to keep the same embed area length in each file (which is required, as kargs.json does not store per-file embed area sizes). isolinux.cfg does not use quoting, so the parameter will be passed to the kernel quoted. The kernel will only strip double quotes, not single quotes. In grub.cfg, using double quotes allows the expansion of variables with '$' and escaping with '\' (both of which are suppressed when using single quotes). '\' is not a valid character in a URL. '$' technically is, but is vanishingly rare. We will prohibit these characters so that we can use the same kargs in both files. Assisted-by: Cursor
Consolidate the duplicated logic for modifying kernel arguments into a single transformKernelArgs() function. This function: - Validates the rootfs URL - Removes the coreos.liveiso parameter - Adds the coreos.live.rootfs_url parameter at a specified position Both fixGrubConfig and fixIsolinuxConfig use this single implementation instead of each having their own inline logic. This eliminates code duplication and makes the transformation logic easier to understand and maintain. Assisted-by: Cursor Assisted-by: Claude Code
Introduce editString() function that uses named capture groups to precisely target content for replacement. The (?P<replace>) syntax makes it explicit what is being replaced in each regex operation. Convert transformKernelArgs, fixGrubConfig and fixIsolinuxConfig to use editString for their transformations. This provides better control over regex substitutions than full-string replacement. Assisted-by: Cursor Assisted-by: Claude Code
Read and update kargs.json metadata that coreos-installer uses to manage kernel arguments in the ISO. When we modify the default kernel arguments (removing coreos.liveiso and adding coreos.live.rootfs_url), update kargs.json to reflect these changes. This ensures coreos-installer can correctly edit kernel arguments after the ISO is created. Assisted-by: Cursor Assisted-by: Claude Code
When modifying kernel arguments in grub.cfg or isolinux.cfg, track how these changes shift the position of the kargs embed area. Update the offsets in kargs.json so that coreos-installer can find the correct embed area after our modifications. Without this fix, adding initramfs images in isolinux.cfg can shift the kargs embed area, causing coreos-installer kargs edits to fail or corrupt the file. Assisted-by: Cursor Assisted-by: Claude Code
|
ofcir setup failed |
|
@zaneb: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Description
When editing the bootloader files in the minimal ISO, update
kargs.jsonso that it maintains the correct default args, embed area offsets, and embed area sizes. This allows the kernel arguments in the ISO to be manipulated by the user with coreos-installer commands.How was this code tested?
Generated a minimal ISO using this code and verified that coreos-installer can see and edit the kargs.
Assignees
/cc @carbonin
Links
Checklist
docs, README, etc)