grub.cfg: Remove grub.cfg in bootupd favor #3900
Conversation
ravanelli
commented
Oct 8, 2024
ravanelli
commented
- Add a comment explaining that changes to this config file (grub.cfg) do not directly affect the system's GRUB configuration.
- Bootupd uses this file as a reference to generate static GRUB config files, and the actual grub.cfg installed on the system comes from bootupd.
- This clarification helps avoid confusion for those expecting changes here to take immediate effect.
|
When looking at the grub configs to test the VMware issue, I realized that even I needed more clarification on where the GRUB configurations are actually coming from. |
src/grub.cfg
Outdated
| @@ -1,3 +1,6 @@ | |||
| # Changes in this file won't take affect, unless you also | |||
| # update the Static GRUB configuration files in bootup | |||
There was a problem hiding this comment.
LGTM. Just one minor comment, bootup should be bootupd
|
is this file even used anymore? isn't the source of truth the ones from bootupd? |
So, no, that's the reason we need to either clarify it or remove it. I added the comment here, since we have a reference for it in https://github.com/coreos/bootupd/blob/main/src/grub2/grub-static-pre.cfg#L1 To be honestly I don't know each one is better, because for those how don't know about bootupd, is hard to understand where this configs come from. If we decide to remove it, we need a better description in the bootupd file. |
yeah. if this is dead code (essentially) then I would delete this file and update the comment in grub-static-pre.cfg to point to a permalink of the file at a previous point in time.
It's definitely more confusing to have it here and not used than to not have it here at all IMO. |
ravanelli
changed the title
+1 removed it here, and will update the link in bootupd as well |
- Bootupd used this file as a reference to generate static GRUB config files, the actual grub.cfg installed on the system comes from bootupd; - Remove grub.cfg since this file is not used here anymore. Signed-off-by: Renata Ravanelli <[email protected]>
| # Serialize our default GRUB config | ||
| with open("/usr/lib/coreos-assembler/grub.cfg") as f: | ||
| r['grub-script'] = f.read() |
There was a problem hiding this comment.
ok, just to confirm 100% - this definitely isn't used anywhere?
There was a problem hiding this comment.
I could not find any other place using it, looking at c9036fa it was needed for create_disk, what makes sense. So, I will say no
|
/retest-required |
Hit same failure, see openshift/os#1638 (comment) |
|
SSL Certificate failures are fixed for centos.org |
|
/test rhcos |
- grub.cfg was removed from the main branch in coreos/coreos-assembler#3900 - Updated the link to maintain reference to the file’s origin and history. Signed-off-by: Renata Ravanelli <[email protected]>
6 participants
