Use grubby to update kernel command line arguments - #511
Conversation
| "sed -i '/cloud-init=disabled/d' %s" % grub_conf_disabler | ||
| ) | ||
| self._schedule_grub2_update() | ||
| self._update_kernel_cmdline_args(args_to_remove=["cloud-init=disabled"]) |
There was a problem hiding this comment.
I don't like that both grub update and grubby are being called. It should be the one or the other, not both.
An ideal implementation would be to call for _update_kernel_cmdline_args indeed, but you only implemented it for redhat based. There should also be a debian implementation which does the former (reads /etc/default/grub, removes/adds/edits the cmdline, schedules grub2 update).
I propose the following:
-
Use Grub2ConfigEditor whenever possible instead of
sed-ing the file directly.
This implies implementing a new method to remove cmdline entries, call itremove_from_option
This method should pretty much emulate what grubby does on redhat, and that is if you pass a single option
(--remove-args=cloud-init), then it will be removed even if it's a key_val, no matter what value cloud-init has
in cmdline. If you pass a key_val, then only remove the key if the value matches
(--remove-args=cloud-init=disabledonly remove cloud-init from cmdline if it's disabled, but won't
removecloud-init=enabled). -
We should somehow abstractize this for redhat. On base, when calling for
_update_kernel_cmdline_args,
it should instantiate a Grub2ConfigEditor, append or remove from GRUB_CMDLINE_LINUX and
GRUB_CMDLINE_LINUX_DEFAULT options (depending on what args_to_add or args_to_remove are being
passed).
If it's redhat, then simply use grubby to handle args_to_add/args_to_remove -
(only if grubby commands take too long, otherwise treat this as optional) I think the final grubby command should also be run once at the end, so add some
schedule_grubby
methods as well when adding args to remove/add. (similar to_schedule_grub2_update)
There was a problem hiding this comment.
Thank you for that! I updated the code, please take a look now.
Removes cmdline entries with `grubby --remove-args` semantics: a bare name drops the argument whichever value it holds, a name/value pair drops it only on an exact match. Signed-off-by: Mihaela Balutoiu <mbalutoiu@cloudbasesolutions.com>
501c1ee to
17e0300
Compare
Signed-off-by: Mihaela Balutoiu <mbalutoiu@cloudbasesolutions.com>
Use `grubby --update-kernel=ALL` to update kernel arguments on BLS-based systems such as RHEL 10, where `grub2-mkconfig` does not propagate them to `/boot/loader/entries`. Signed-off-by: Mihaela Balutoiu <mbalutoiu@cloudbasesolutions.com>
17e0300 to
f6177de
Compare
| @@ -69,33 +69,7 @@ def check_os_supported(cls, detected_os_info): | |||
| return False | |||
|
|
|||
| def disable_predictable_nic_names(self): | |||
There was a problem hiding this comment.
Worth taking into account to modify custom implementations of disable_predictable_nic_names in the other providers as well.
Besides that LGTM
3 participants
Failing to update the kernel console options breaks text console functionality on affected platforms.
The issue is caused by BLS being enabled by default in
/etc/default/grub. OnRHEL 10, kernel arguments are managed through/boot/loader/entries, andgrub2-mkconfigdoes not propagate the updated options there.This PR implements the following:
Grub2ConfigEditor.remove_from_option, withgrubby --remove-argssemantics: a bare name drops the argument whichever value it holds, a name/value pair drops it only on an exact match._update_kernel_cmdline_argsto the baseOSMorphingtools, editing bothGRUB2cmdline options throughGrub2ConfigEditor. Debian and SUSE each had their own copy of these edits and now share this one.grubby --update-kernel=ALL, which reaches the BLS boot entries.