Skip to content

Use grubby to update kernel command line arguments #511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mihaelabalutoiu wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use grubby to update kernel command line arguments #511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions coriolis/osmorphing/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ def _ensure_cloud_init_not_disabled(self):
"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"])

@Dany9966 Dany9966 Aug 31, 2026 •
edited
Loading

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.

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:

  1. Use Grub2ConfigEditor whenever possible instead of sed-ing the file directly.
    This implies implementing a new method to remove cmdline entries, call it remove_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=disabled only remove cloud-init from cmdline if it's disabled, but won't
    remove cloud-init=enabled).

  2. 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

  3. (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)


def _reset_cloud_init_run(self):
self._exec_cmd_chroot("cloud-init clean --logs")
Expand Down Expand Up @@ -951,6 +952,36 @@ def _set_grub2_cmdline(self, config_obj, options, clobber=False):
"GRUB_CMDLINE_LINUX", kernel_cmd, config_obj, replace=replace
)

def _update_kernel_cmdline_args(self, args_to_add=None, args_to_remove=None):
if isinstance(args_to_add, str):
args_to_add = [args_to_add]
if isinstance(args_to_remove, str):
args_to_remove = [args_to_remove]
if not args_to_add and not args_to_remove:
return False

try:
for option, args in (
("--remove-args", args_to_remove),
("--args", args_to_add),
):
if not args:
continue
self._exec_cmd_chroot(
"grubby --update-kernel=ALL %s=%s"
% (option, shlex.quote(" ".join(args)))
)
except Exception:
LOG.warning(
"Failed to update the kernel arguments using 'grubby' (it is "
"not available on all distros). Falling back to the GRUB "
"config regeneration. Error was: %s",
utils.get_exception_details(),
)
return False

return True

def _get_grub_default_conf(self):
grub_conf = "/etc/default/grub"
if self._test_path_chroot(grub_conf):
Expand Down Expand Up @@ -1057,6 +1088,7 @@ def _set_grub2_console_settings(

self._set_grub2_cmdline(config_obj, options)
self._apply_grub2_config(config_obj, execute_update_grub)
self._update_kernel_cmdline_args(args_to_add=options)

def _add_net_udev_rules(self, net_ifaces_info):
coriolis_udev_rules_file = "etc/udev/rules.d/99-coriolis-net.rules"
Expand Down
3 changes: 1 addition & 2 deletions coriolis/osmorphing/redhat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ def __init__(
)

def disable_predictable_nic_names(self):
cmd = 'grubby --update-kernel=ALL --args="%s"'
self._exec_cmd_chroot(cmd % "net.ifnames=0 biosdevname=0")
self._update_kernel_cmdline_args(args_to_add=["net.ifnames=0", "biosdevname=0"])

def get_update_grub2_command(self):
location = self._get_grub2_cfg_location()
Expand Down
57 changes: 57 additions & 0 deletions coriolis/tests/osmorphing/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,7 @@ def test__disable_installer_cloud_config_no_file(
((False, False, True), 'GRUB_CMDLINE_LINUX="console=ttyS0"', [], False),
)
@ddt.unpack
@mock.patch.object(base.BaseLinuxOSMorphingTools, "_update_kernel_cmdline_args")
@mock.patch.object(base.BaseLinuxOSMorphingTools, "_read_file_sudo")
@mock.patch.object(base.BaseLinuxOSMorphingTools, "_schedule_grub2_update")
@mock.patch.object(base.BaseLinuxOSMorphingTools, "_exec_cmd_chroot")
Expand All @@ -1026,6 +1027,7 @@ def test__ensure_cloud_init_not_disabled(
mock__exec_cmd_chroot,
mock__schedule_grub2_update,
mock__read_file_sudo,
mock__update_kernel_cmdline_args,
):
mock__test_path.side_effect = test_path_results
mock__read_file_sudo.return_value = grub_defaults_contents
Expand All @@ -1036,8 +1038,12 @@ def test__ensure_cloud_init_not_disabled(
self.assertEqual(called_cmds, expected_cmds)
if updates_grub:
mock__schedule_grub2_update.assert_called_once()
mock__update_kernel_cmdline_args.assert_called_once_with(
args_to_remove=["cloud-init=disabled"]
)
else:
mock__schedule_grub2_update.assert_not_called()
mock__update_kernel_cmdline_args.assert_not_called()

@mock.patch.object(base.BaseLinuxOSMorphingTools, "_exec_cmd_chroot")
def test__reset_cloud_init_run(self, mock__exec_cmd_chroot):
Expand Down Expand Up @@ -1534,6 +1540,47 @@ def test__set_grub2_cmdline_no_options_to_add(self, mock_set_grub_value):
self.os_morphing_tools._set_grub2_cmdline(config_obj, options, clobber=False)
mock_set_grub_value.assert_not_called()

@mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot')
def test__update_kernel_cmdline_args_remove(self, mock_exec_cmd_chroot):
result = self.os_morphing_tools._update_kernel_cmdline_args(
args_to_remove=['cloud-init=disabled']
)

self.assertTrue(result)
mock_exec_cmd_chroot.assert_called_once_with(
'grubby --update-kernel=ALL --remove-args=cloud-init=disabled'
)

@mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot')
def test__update_kernel_cmdline_args_add_and_remove(self, mock_exec_cmd_chroot):
result = self.os_morphing_tools._update_kernel_cmdline_args(
args_to_add=['console=ttyS0'], args_to_remove=['console=ttyS1']
)

self.assertTrue(result)
self.assertEqual(
[
mock.call('grubby --update-kernel=ALL --remove-args=console=ttyS1'),
mock.call('grubby --update-kernel=ALL --args=console=ttyS0'),
],
mock_exec_cmd_chroot.call_args_list,
)

@mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot')
def test__update_kernel_cmdline_args_no_grubby(self, mock_exec_cmd_chroot):
mock_exec_cmd_chroot.side_effect = CoriolisTestException(
'grubby: command not found'
)

result = self.os_morphing_tools._update_kernel_cmdline_args(
args_to_add=['console=ttyS0']
)

self.assertFalse(result)
mock_exec_cmd_chroot.assert_called_once_with(
'grubby --update-kernel=ALL --args=console=ttyS0'
)

@mock.patch.object(
base.BaseLinuxOSMorphingTools,
'_get_grub_default_conf',
Expand Down Expand Up @@ -1792,6 +1839,7 @@ def test__set_grub2_console_settings_invalid_consoles(self):
consoles='invalid_consoles',
)

@mock.patch.object(base.BaseLinuxOSMorphingTools, '_update_kernel_cmdline_args')
@mock.patch.object(base.BaseLinuxOSMorphingTools, '_apply_grub2_config')
@mock.patch.object(base.BaseLinuxOSMorphingTools, '_set_grub2_cmdline')
@mock.patch.object(base.BaseLinuxOSMorphingTools, 'set_grub_value')
Expand All @@ -1802,6 +1850,7 @@ def test__set_grub2_console_settings_all_params(
mock_set_grub_value,
mock_set_grub2_cmdline,
mock_apply_grub2_config,
mock_update_kernel_cmdline_args,
):
consoles = ['tty0', 'ttyS0']
speed = 9600
Expand All @@ -1825,7 +1874,11 @@ def test__set_grub2_console_settings_all_params(
config_obj, ['console=tty0', 'console=ttyS0']
)
mock_apply_grub2_config.assert_called_once_with(config_obj, False)
mock_update_kernel_cmdline_args.assert_called_once_with(
args_to_add=['console=tty0', 'console=ttyS0']
)

@mock.patch.object(base.BaseLinuxOSMorphingTools, '_update_kernel_cmdline_args')
@mock.patch.object(base.BaseLinuxOSMorphingTools, '_apply_grub2_config')
@mock.patch.object(base.BaseLinuxOSMorphingTools, '_set_grub2_cmdline')
@mock.patch.object(base.BaseLinuxOSMorphingTools, 'set_grub_value')
Expand All @@ -1836,6 +1889,7 @@ def test__set_grub2_console_settings_default_params(
mock_set_grub_value,
mock_set_grub2_cmdline,
mock_apply_grub2_config,
mock_update_kernel_cmdline_args,
):
grub_conf = '/etc/default/grub'

Expand All @@ -1856,6 +1910,9 @@ def test__set_grub2_console_settings_default_params(
config_obj, ['console=tty0', 'console=ttyS0']
)
mock_apply_grub2_config.assert_called_once_with(config_obj, True)
mock_update_kernel_cmdline_args.assert_called_once_with(
args_to_add=['console=tty0', 'console=ttyS0']
)

@mock.patch.object(base.BaseLinuxOSMorphingTools, '_test_path')
@mock.patch.object(base.BaseLinuxOSMorphingTools, '_write_file_sudo')
Expand Down
13 changes: 11 additions & 2 deletions coriolis/tests/osmorphing/test_redhat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,20 @@ def test_check_os_not_supported(self):

self.assertFalse(result)

@mock.patch.object(base.BaseLinuxOSMorphingTools, '_update_kernel_cmdline_args')
def test_disable_predictable_nic_names(self, mock_update_kernel_cmdline_args):
self.morphing_tools.disable_predictable_nic_names()
mock_update_kernel_cmdline_args.assert_called_once_with(
args_to_add=['net.ifnames=0', 'biosdevname=0']
)

@mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot')
def test_disable_predictable_nic_names(self, mock_exec_cmd_chroot):
def test_disable_predictable_nic_names_updates_all_kernels(
self, mock_exec_cmd_chroot
):
self.morphing_tools.disable_predictable_nic_names()
mock_exec_cmd_chroot.assert_called_once_with(
'grubby --update-kernel=ALL --args="net.ifnames=0 biosdevname=0"'
"grubby --update-kernel=ALL --args='net.ifnames=0 biosdevname=0'"
)

@mock.patch.object(redhat.BaseRedHatMorphingTools, '_get_grub2_cfg_location')
Expand Down
Loading